fix(session): dedupe compaction execution#3
Conversation
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc39bc60a1
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tx.insert(MessageTable).values({ id, session_id: sessionID, time_created: now, data: info }).run() | ||
| tx.insert(PartTable) | ||
| .values({ id: partID, message_id: messageID, session_id: partSessionID, time_created: now, data: partData }) | ||
| .run() |
There was a problem hiding this comment.
Publish compaction marker through sync events
This direct insert bypasses session.updateMessage/session.updatePart, which are the only paths here that call sync.run(MessageV2.Event.Updated/PartUpdated). When a compaction marker is created, sync consumers such as CLI/share watchers and remote clients that learn messages from those events will not receive or replay the new user message/part until they reload from SQLite, so the pending compaction can be invisible in live sessions.
Useful? React with 👍 / 👎.
| msg.info.parentID === input.parentID && | ||
| !msg.info.error, | ||
| ) | ||
| if (existing?.info.role === "assistant") return existing.info.finish ? "continue" : "stop" |
There was a problem hiding this comment.
Retry unfinished compaction assistants
If the process is interrupted or crashes after the compaction assistant message is written but before processor.process records a finish/error, this new dedupe branch returns "stop" forever for that marker. The prompt loop will keep seeing the same active compaction task, but process never retries it or marks it failed, leaving the session unable to finish compaction without manual cleanup.
Useful? React with 👍 / 👎.
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed6b00ef80
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Database.transaction( | ||
| (tx) => { | ||
| const messages = Array.from(MessageV2.stream(input.sessionID)).reverse() | ||
| if (activeCompactionMarker(messages)) return undefined |
There was a problem hiding this comment.
Skip stale compaction markers when deduping
If a session already contains an unfinished compaction marker that is older than the latest finished assistant (for example from a crash or from the previous duplicate-compaction behavior), MessageV2.latest() in prompt.ts will no longer surface that marker as a task because it drops task parts on messages at or before the latest finished assistant. This new check still treats that old marker as active and returns before creating a fresh marker, so future auto or manual compaction for that session can be permanently blocked. Consider limiting the dedupe check to markers that are actually pending for the current prompt loop, or otherwise ignoring/closing markers that are older than the latest finished assistant.
Useful? React with 👍 / 👎.
| OPENCODE_CONFIG_CONTENT: JSON.stringify(providerConfig(baseURL)), | ||
| PATH: `${path.dirname(BUN_BIN)}:${process.env.PATH ?? ""}`, | ||
| } | ||
| const proc = Bun.spawn([BUN_BIN, "run", "--conditions=browser", "./src/index.ts", "serve", "--port", "0", "--hostname", "127.0.0.1"], { |
There was a problem hiding this comment.
Use the current Bun executable in the e2e test
In CI or developer environments where Bun is installed by a package manager, asdf/mise, Nix, or another non-default location, ~/.bun/bin/bun may not exist even though the test runner is Bun. This Bun.spawn then fails before the server can start, making the new e2e test environment-dependent; use process.execPath (as other tests in this repo do) or resolve bun from PATH instead of the hard-coded home path.
Useful? React with 👍 / 👎.
| ) | ||
| let inputMessages = input.messages | ||
| if (existing?.info.role === "assistant") { | ||
| if (existing.info.finish) return "continue" |
There was a problem hiding this comment.
Finish marker bookkeeping before returning existing summaries
When a compaction retry sees a finished summary but the previous run crashed after the assistant was finalized and before the marker bookkeeping below ran, this early return prevents the retry from setting tail_start_id or creating the auto-continue follow-up. In that recovery window the compacted session can drop the preserved tail from future context, and automatic compaction can stop without resuming the user's request; handle the already-finished case by completing the missing marker/follow-up work before returning.
Useful? React with 👍 / 👎.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Issue for this PR
Closes anomalyco#27037
Type of change
What does this PR do?
This PR fixes duplicate compaction execution when more than one compaction trigger overlaps in the same session.
Technical spec
Goal. OpenCode should treat compaction as a per-session singleton while a compaction marker is still active. Duplicate manual, automatic, or overflow-triggered compaction requests should merge into the same pending marker instead of creating multiple summary tasks.
Problem scene. In the failing runtime shape, one session can contain several
type=compactionuser markers created close together, while later prompt-loop execution may attach aassistant(agent=compaction)summary to a newer normal/synthetic user message instead of the marker that requested compaction. That makes the UI and sync clients observe duplicate compactions, orphan compaction markers, and summary assistants with the wrongparentID.Root cause. There were three related gaps:
SessionCompaction.create()inserted marker message/part rows directly, so overlapping callers could leave more than one active marker and live sync consumers did not receivemessage.updated/message.part.updatedevents for the marker.SessionCompaction.process()treated an unfinished compaction assistant for the same marker as a permanent"stop". If the process crashed after writing the assistant message but before settingfinishorerror, future loops kept seeing the same active task but never retried or finalized it.Fix strategy. This PR enforces these invariants:
create()checks and inserts inside one immediate DB transaction, so a session has at most one active compaction marker;parentID, not the latest user message;session.updateMessage/session.updatePartsync event path.This fixes the bug because duplicate compaction requests become mergeable events rather than queued work. The marker remains the durable owner of the compaction task, so prompt-loop retries cannot accidentally bind a summary to a later user message. Publishing through the normal sync event path also keeps CLI/share/remote clients in sync without waiting for a SQLite reload.
How did you verify your code works?
Ran locally from
packages/opencode:Results:
113 pass, 0 failacrosscompaction.test.tsandprompt.test.ts1 pass, 0 failforcompaction-runtime-e2e.test.ts4 pass, 0 failforsession.test.tsbun typecheckcompleted successfullyScreenshots / recordings
Not applicable; this is session runtime behavior with automated test coverage.
Checklist
If you do not follow this template your PR will be automatically rejected.