feat(gateway): add compaction checkpoints#62146
Conversation
Greptile SummaryAdds persisted compaction checkpoints, four new
Confidence Score: 4/5Not safe to merge until the snapshot path bug is fixed; branch and restore will always reconstruct from post-compaction content. One P1 defect in captureCompactionCheckpointSnapshot makes the branch and restore operations functionally broken in production. All other surfaces — protocol schemas, scope assignments (read/write/admin), UI controller, the maxLines compact path, and checkpoint persistence logic — look correct. src/gateway/session-compaction-checkpoints.ts lines 72-104: the return statement must use snapshotFile instead of sessionFile. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/gateway/session-compaction-checkpoints.ts
Line: 72-104
Comment:
**Snapshot path is lost — branch and restore silently use post-compaction content**
`captureCompactionCheckpointSnapshot` copies the live session file to `snapshotFile` (a unique `*.checkpoint.<uuid>.jsonl`), but returns `sessionFile` (the original path) in the result. Every checkpoint persisted by `persistSessionCompactionCheckpoint` therefore stores the live transcript path in `preCompaction.sessionFile`. By the time a user calls `sessions.compaction.branch` or `sessions.compaction.restore`, compaction has already rewritten that file, so both handlers call `SessionManager.forkFrom` on the **post**-compaction transcript — the pre-compaction state is never used.
Secondary effect: `cleanupCompactionCheckpointSnapshot` calls `fs.unlink(snapshot.sessionFile)` targeting the live transcript instead of the orphaned snapshot copy, which leaks on disk indefinitely.
The gateway test at line 1291 manually seeds `preCompaction.sessionFile` with `fixture.preCompactionSessionFile` (the actual copy path), so it does not catch this.
```suggestion
return {
sessionId: getSessionId(),
sessionFile: snapshotFile,
leafId,
};
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Gateway: add compaction checkpoints" | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d057fc1527
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d61ac5fa3
ℹ️ 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".
| store[target.canonicalKey] = { | ||
| ...existing, | ||
| updatedAt: Math.max(existing.updatedAt ?? 0, createdAt), | ||
| compactionCheckpoints: trimSessionCheckpoints(checkpoints), |
There was a problem hiding this comment.
Remove orphaned snapshot files when trimming checkpoints
Each successful compaction creates a copied transcript snapshot, but this update only trims the in-store compactionCheckpoints array and never deletes snapshot files for checkpoints that fall out of retention. After many compactions, sessions will accumulate unreachable .checkpoint.*.jsonl files even though metadata is capped, so disk usage can grow without bound and eventually impact gateway stability on smaller disks.
Useful? React with 👍 / 👎.
| if (state.sessionsCheckpointItemsByKey[trimmedKey]) { | ||
| return; |
There was a problem hiding this comment.
Re-fetch checkpoints when reopening expanded session rows
This early return makes checkpoint details a one-time fetch per session key, so later compactions are not reflected in the expanded checkpoint cards even after loadSessions refreshes the row-level counters. In practice, users can see updated checkpoint counts in the table but stale checkpoint actions/details underneath until they hard-reload the page, which is a correctness issue for recovery UX.
Useful? React with 👍 / 👎.
3d5ab81 to
7e69cac
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e69cace5a
ℹ️ 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".
| checkpointSnapshot = engineOwnsCompaction | ||
| ? captureCompactionCheckpointSnapshot({ | ||
| sessionManager: SessionManager.open(params.sessionFile), | ||
| sessionFile: params.sessionFile, |
There was a problem hiding this comment.
Keep engine-owned compaction alive when snapshot open fails
When contextEngine.info.ownsCompaction === true, checkpoint capture now calls SessionManager.open(params.sessionFile) inline before captureCompactionCheckpointSnapshot. If that open throws (for example on a partially repaired/corrupt transcript), the whole compaction request fails even though checkpointing is intended to be best-effort. This creates a hard failure path that did not exist before and blocks compaction in cases where skipping checkpoint capture would have been acceptable.
Useful? React with 👍 / 👎.
6fe1453 to
83dca67
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83dca67013
ℹ️ 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".
| preserveCompactionCheckpoints?: boolean; | ||
| }): SessionEntry { | ||
| return { | ||
| ...params.currentEntry, |
There was a problem hiding this comment.
Reset compaction counters when branching from checkpoints
cloneCheckpointSessionEntry starts from ...params.currentEntry, and the branch path (sessions.compaction.branch) uses it with preserveCompactionCheckpoints unset, so the new child session drops checkpoint history but still inherits compactionCount (and related compaction-derived fields). This makes the branched session look like it has already been compacted before any child compaction runs, and later sessions.compact increments from that stale baseline, which can skew compaction-triggered behavior for the child session.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a83a77b28f
ℹ️ 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".
| ok: true, | ||
| key: canonicalKey, | ||
| checkpoints: listSessionCompactionCheckpoints(entry), | ||
| }, |
There was a problem hiding this comment.
Redact checkpoint transcript paths from read-scope responses
sessions.compaction.list returns raw SessionCompactionCheckpoint objects, which include preCompaction.sessionFile/postCompaction.sessionFile absolute paths captured on the gateway host. Because these methods were added to operator.read scope, any read-scoped client can enumerate local filesystem layout and checkpoint file locations, which is unnecessary for list/get UI workflows (they only need IDs/metadata) and expands sensitive host metadata exposure.
Useful? React with 👍 / 👎.
a83a77b to
e375425
Compare
|
Merged via squash.
Thanks @scoootscooob! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e37542554a
ℹ️ 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".
| await updateSessionStore(storePath, (store) => { | ||
| store[canonicalKey] = nextEntry; | ||
| }); |
There was a problem hiding this comment.
Archive superseded transcript when restoring checkpoints
This replaces the active session entry with a freshly forked transcript but never archives or deletes the previously active entry.sessionFile. After the first restore, repeated sessions.compaction.restore calls from the same checkpoint keep creating new transcript files while dropping references to the prior restored file, so disk usage grows with unreachable transcripts over time.
Useful? React with 👍 / 👎.
| compactionCheckpointCount: entry?.compactionCheckpoints?.length, | ||
| latestCompactionCheckpoint, |
There was a problem hiding this comment.
Strip checkpoint internals from session list rows
Fresh evidence of the path-exposure issue exists in the session row builder: it attaches the full checkpoint object (including preCompaction.sessionFile / postCompaction.sessionFile) to latestCompactionCheckpoint. Because sessions.list and sessions.changed return this row to read-scoped clients, they can still enumerate gateway host filesystem paths even without calling sessions.compaction.list/get.
Useful? React with 👍 / 👎.
…e, activeForm, hydration
Ports three Hermes todo-tool features that OpenClaw's update_plan
was missing, plus adds plan hydration for post-compaction recovery.
## cancelled status (Hermes parity)
Add "cancelled" as a fourth plan step status. Hermes uses this to
mark steps that were started but abandoned due to failure, keeping
them visible in history instead of silently dropping them.
Ref: Hermes tools/todo_tool.py Status enum.
## merge mode (Hermes parity)
Add optional merge: boolean parameter (default false). When true,
incoming steps update existing ones by matching step text and new
steps are appended. When false, the entire plan is replaced.
Ref: Hermes tools/todo_tool.py write(todos, merge=False).
## activeForm field (Claude Code TodoWrite parity)
Add optional activeForm: string field on plan steps. Shows the
present-continuous form while in_progress (e.g. "Running tests"
instead of "Run tests"). Small schema addition with disproportionate
UX impact for plan rendering.
## Post-compaction plan hydration (Hermes parity)
New src/agents/plan-hydration.ts helper that formats active
(pending/in_progress) plan items for injection after context
compression. The injected text uses factual phrasing ("Your active
plan was preserved...") to avoid triggering the planning-only retry
guard's promise-language detection.
Ref: Hermes run_agent.py:6754-6756 + tools/todo_tool.py:format_for_injection().
Note: the compaction hook point in compact.ts is not wired in this
PR — that requires a follow-up to integrate with the compaction
checkpoint system (openclaw#62146). This PR provides the formatter; the
hook wiring is the next step.
Part of GPT 5.4 Enhancement v3 sprint. Tracking: openclaw#66345.
…e, activeForm, hydration
Ports three Hermes todo-tool features that OpenClaw's update_plan
was missing, plus adds plan hydration for post-compaction recovery.
## cancelled status (Hermes parity)
Add "cancelled" as a fourth plan step status. Hermes uses this to
mark steps that were started but abandoned due to failure, keeping
them visible in history instead of silently dropping them.
Ref: Hermes tools/todo_tool.py Status enum.
## merge mode (Hermes parity)
Add optional merge: boolean parameter (default false). When true,
incoming steps update existing ones by matching step text and new
steps are appended. When false, the entire plan is replaced.
Ref: Hermes tools/todo_tool.py write(todos, merge=False).
## activeForm field (Claude Code TodoWrite parity)
Add optional activeForm: string field on plan steps. Shows the
present-continuous form while in_progress (e.g. "Running tests"
instead of "Run tests"). Small schema addition with disproportionate
UX impact for plan rendering.
## Post-compaction plan hydration (Hermes parity)
New src/agents/plan-hydration.ts helper that formats active
(pending/in_progress) plan items for injection after context
compression. The injected text uses factual phrasing ("Your active
plan was preserved...") to avoid triggering the planning-only retry
guard's promise-language detection.
Ref: Hermes run_agent.py:6754-6756 + tools/todo_tool.py:format_for_injection().
Note: the compaction hook point in compact.ts is not wired in this
PR — that requires a follow-up to integrate with the compaction
checkpoint system (openclaw#62146). This PR provides the formatter; the
hook wiring is the next step.
Part of GPT 5.4 Enhancement v3 sprint. Tracking: openclaw#66345.
…e, activeForm, hydration
Ports three Hermes todo-tool features that OpenClaw's update_plan
was missing, plus adds plan hydration for post-compaction recovery.
## cancelled status (Hermes parity)
Add "cancelled" as a fourth plan step status. Hermes uses this to
mark steps that were started but abandoned due to failure, keeping
them visible in history instead of silently dropping them.
Ref: Hermes tools/todo_tool.py Status enum.
## merge mode (Hermes parity)
Add optional merge: boolean parameter (default false). When true,
incoming steps update existing ones by matching step text and new
steps are appended. When false, the entire plan is replaced.
Ref: Hermes tools/todo_tool.py write(todos, merge=False).
## activeForm field (Claude Code TodoWrite parity)
Add optional activeForm: string field on plan steps. Shows the
present-continuous form while in_progress (e.g. "Running tests"
instead of "Run tests"). Small schema addition with disproportionate
UX impact for plan rendering.
## Post-compaction plan hydration (Hermes parity)
New src/agents/plan-hydration.ts helper that formats active
(pending/in_progress) plan items for injection after context
compression. The injected text uses factual phrasing ("Your active
plan was preserved...") to avoid triggering the planning-only retry
guard's promise-language detection.
Ref: Hermes run_agent.py:6754-6756 + tools/todo_tool.py:format_for_injection().
Note: the compaction hook point in compact.ts is not wired in this
PR — that requires a follow-up to integrate with the compaction
checkpoint system (openclaw#62146). This PR provides the formatter; the
hook wiring is the next step.
Part of GPT 5.4 Enhancement v3 sprint. Tracking: openclaw#66345.
Merged via squash. Prepared head SHA: e375425 Co-authored-by: scoootscooob <167050519+scoootscooob@users.noreply.github.com> Co-authored-by: scoootscooob <167050519+scoootscooob@users.noreply.github.com> Reviewed-by: @scoootscooob
Merged via squash. Prepared head SHA: e375425 Co-authored-by: scoootscooob <167050519+scoootscooob@users.noreply.github.com> Co-authored-by: scoootscooob <167050519+scoootscooob@users.noreply.github.com> Reviewed-by: @scoootscooob
Merged via squash. Prepared head SHA: e375425 Co-authored-by: scoootscooob <167050519+scoootscooob@users.noreply.github.com> Co-authored-by: scoootscooob <167050519+scoootscooob@users.noreply.github.com> Reviewed-by: @scoootscooob
Summary
sessions.compaction.*RPCs for list/get/branch/restore, session-row checkpoint metadata, and Sessions UI cards with branch/restore actions.sessions.compactline-trim behavior when callers passmaxLines.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
UX Walkthrough
Before
Manual or automatic compaction could rewrite the active session transcript, but there was no first-class operator surface for understanding what got compacted or recovering the pre-compaction state. In practice that meant compaction worked, but it still felt risky because the user-visible model of the session stayed opaque.
After
This PR makes compaction a visible, inspectable, and recoverable workflow:
Compactioncolumn, so operators can see whether a session has checkpoints without leaving the table.Show checkpointsreveals a card per checkpoint with the saved summary and direct recovery actions.Branch from checkpointis the safe default recovery path because it preserves the current session while reopening the pre-compaction state in a child session.Restorerewinds the active session to the pre-compaction snapshot for operators/admins who want an in-place undo.One important UX detail: Control UI
/compactnow routes through the embedded manual compaction path whenmaxLinesis omitted, so the normal operator flow creates real checkpoints instead of only trimming transcript lines.Screenshots
Sanitized local-dev data from the authenticated Control UI:
Sessions table overview
This is the new at-a-glance signal: operators can see checkpoint count and latest checkpoint metadata directly in the Sessions table.
Expanded checkpoint detail
This is the recovery UX: expanding a row shows the saved checkpoint summaries and exposes
Branch from checkpointplusRestoreinline.Regression Test Plan (if applicable)
src/gateway/server.sessions.gateway-server-sessions-a.test.tssessions.compactwithoutmaxLinesroutes through embedded manual compaction.src/agents/pi-embedded-runner/compact.hooks.test.tscovers the embedded compaction hook/runtime path.User-visible / Behavior Changes
Compactioncolumn with checkpoint count and latest checkpoint metadata.Branch from checkpointorRestorewithout digging through transcript files.sessions.compaction.list,sessions.compaction.get,sessions.compaction.branch, andsessions.compaction.restoreare now available./compactin Control UI now uses real embedded manual compaction when called withoutmaxLines, so the default manual UI flow can create checkpoints.Diagram (if applicable)
Security Impact (required)
Yes, explain risk + mitigation: adds new operator-facing gateway methods for checkpoint inspection and recovery, and routessessions.compactwithoutmaxLinesthrough the existing embedded compaction runner. Risk is limited by existing gateway operator scope enforcement (sessions.compaction.restoreremains admin-only) and by reusing the already-owned compaction/runtime path instead of inventing a broader new execution surface.Repro + Verification
Environment
Steps
/compactin a real chat session or callsessions.compactwithoutmaxLines./sessions, confirm theCompactioncolumn updates, expandShow checkpoints, then branch/restore from the checkpoint.Expected
Actual
Evidence
Attach at least one:
Human Verification (required)
sessions.compactgateway test, and captured authenticated Sessions UI screenshots against the live local gateway.sessions.compactwithmaxLinesremains the old trim path.Branch from checkpointandRestoreagainst a personal live session after the screenshot/PR packaging step.Review Conversations
Compatibility / Migration
Risks and Mitigations
sessions.compactnow has two behaviors depending on whethermaxLinesis supplied.maxLinesbehavior exactly for existing callers and tests; only the default UI/manual path upgrades to the embedded checkpoint-capable flow.pnpm checkis currently blocked by an unrelated existing parse error insrc/gateway/server.talk-config.test.ts:560.pnpm build, and called out the unrelated blocker explicitly here.