You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Three correctness bugs found by auditing the session runtime, each confirmed by reading the code paths end to end.
1. Compaction deleted history before the summary existed (data loss). In session/compaction.ts, the block that removes summarized head/hidden messages ran beforeprocessor.process(...) generated the summary. So when summarization failed (rate limit, network error, or the "Session too large to compact" branch that returns "stop"), the original messages were already hard-deleted by the projector and unrecoverable. Moved the removal to after the summary is produced and the result === "compact" early-return has been passed.
Added a regression test (keeps summarized history when compaction fails) that seeds head turns, forces a "compact" failure, and asserts the history survives. Verified it fails on the pre-fix ordering and passes after.
Follow-up from review: a "stop" result with an errored assistant message also means no valid summary exists, so the removal block is additionally gated on !processor.message.error, with a matching regression test (keeps summarized history when processing stops with an error).
2. Plan->build switch reminder never fired. The default agent was renamed (build: { name: "code", ... } in agent/agent.ts), but session/reminders.ts still gated on input.agent.name === "build", which is now unreachable. So after working in the plan agent and switching back to implement, the BUILD_SWITCH reminder was never appended and the model often kept refusing edits. Changed the comparison to "code".
3. bolt logs --tail 0 dumped the entire log.lines.slice(-count) with count === 0 is slice(0), which returns everything. Guarded the zero case to print nothing.
Verified: bun run typecheck clean; full compaction suite 53 pass / 1 skip.
Note: the audit also flagged a server-side model-resolution change (runner/model.ts uses catalog.model.cheapest() where upstream used default(), so a configured default model is ignored for model-less v2 sessions). That one may be intentional fork behavior, so I left it out pending a product call.
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is enabled.
Summary by CodeRabbit
New Features
Added support for logs --tail 0 to hide existing log entries while continuing to stream new entries with --follow.
Bug Fixes
Prevented failed or interrupted compaction from removing conversation history.
Preserved summarized history until compaction completes successfully.
Fixed the build-switch reminder when transitioning from plan mode to code mode.
We reviewed changes in 3944bef...7d0d172 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
The reason will be displayed to describe this comment to others. Learn more.
Avoid using console in code that runs on the browser
It is considered a best practice to avoid the use of any console methods in JavaScript code that will run on the browser.
NOTE: If your repository contains a server side project, you can add "nodejs" to the environment property of analyzer meta in .deepsource.toml.
This will prevent this issue from getting raised.
Documentation for the analyzer meta can be found here.
Alternatively, you can silence this issue for your repository as shown here.
If a specific console call is meant to stay for other reasons, you can add a skipcq comment to that line.
This will inform other developers about the reason behind the log's presence, and prevent DeepSource from flagging it.
The reason will be displayed to describe this comment to others. Learn more.
False positive: this is a CLI command (bolt logs) that intentionally writes log lines to the terminal via console.log; it never runs in a browser. The proper fix is adding "nodejs" to the analyzer environment in .deepsource.toml, not a code change.
The reason will be displayed to describe this comment to others. Learn more.
Forbidden non-null assertion
Using non-null assertions cancels out the benefits of strict null-checking, and introduces the possibility of runtime errors. Avoid non-null assertions unless absolutely necessary. If you still need to use one, write a skipcq comment to explain why it is safe.
Lines 423-435 use for loops for collection transformation and Effect sequencing. messageIDsToPreserve stores only input.parentID. Build the IDs with collection operations, filter the parent ID, and use sequential Effect.forEach.
Suggested refactor
- const messageIDsToRemove = new Set<MessageID>()- const messageIDsToPreserve = new Set<MessageID>()- // Add head messages (summarized in this compaction)- for (const msg of selected.head) {- messageIDsToRemove.add(msg.info.id)- }- // Add previously summarized messages (hidden)- for (const index of hidden) {- messageIDsToRemove.add(history[index].info.id)- }- // Preserve the parent message (needed as parent of the new compaction message)- messageIDsToPreserve.add(input.parentID)- for (const msgID of messageIDsToRemove) {- if (!messageIDsToPreserve.has(msgID)) {- yield* session.removeMessage({ sessionID: input.sessionID, messageID: msgID })- }- }+ const messageIDsToRemove = new Set([+ ...selected.head.map((message) => message.info.id),+ ...Array.from(hidden, (index) => history[index].info.id),+ ])+ yield* Effect.forEach(+ Array.from(messageIDsToRemove).filter((messageID) => messageID !== input.parentID),+ (messageID) => session.removeMessage({ sessionID: input.sessionID, messageID }),+ { concurrency: 1 },+ )
As per coding guidelines, “Prefer functional array methods such as flatMap, filter, and map over for loops” and “Reduce variable count by inlining values used only once.”
🤖 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 `@packages/opencode/src/session/compaction.ts` around lines 420 - 436, Refactor
the cleanup block around messageIDsToRemove and messageIDsToPreserve to build
removable IDs with collection operations, combining selected.head IDs and hidden
history IDs, then filter out input.parentID directly instead of maintaining a
preservation set. Replace the removal loop with sequential Effect.forEach over
the filtered IDs, preserving the existing session.removeMessage call and
ordering.
Source: Coding guidelines
🤖 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 `@packages/opencode/src/session/compaction.ts`:
- Around line 419-436: Update the cleanup guard surrounding message removal in
SessionProcessor.process() to require !processor.message.error in addition to
the existing conditions, preventing summarized history deletion when processing
stops with an error. Add a regression test covering the error-stop path and
verifying cleanup is skipped.
---
Nitpick comments:
In `@packages/opencode/src/session/compaction.ts`:
- Around line 420-436: Refactor the cleanup block around messageIDsToRemove and
messageIDsToPreserve to build removable IDs with collection operations,
combining selected.head IDs and hidden history IDs, then filter out
input.parentID directly instead of maintaining a preservation set. Replace the
removal loop with sequential Effect.forEach over the filtered IDs, preserving
the existing session.removeMessage call and ordering.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f584fcf2-4a4c-40b2-b579-52d1fab1e114
📥 Commits
Reviewing files that changed from the base of the PR and between 3944bef and bfbfcf0.
#!/usr/bin/env bashset -euo pipefail
printf'%s\n''--- compaction process ---'
sed -n '340,540p' packages/opencode/src/session/compaction.ts
printf'%s\n''--- processor process implementation ---'
sed -n '600,700p' packages/opencode/src/session/processor.ts
printf'%s\n''--- compaction tests around failure and cleanup ---'
sed -n '820,940p' packages/opencode/test/session/compaction.test.ts
printf'%s\n''--- all compaction test processor fakes and cleanup assertions ---'
rg -n -C 6 'function fake|processorLayer|keeps summarized|removeMessage|message\.error|result\)' packages/opencode/test/session/compaction.test.ts
Repository: bolt-builder/bolt-cli
Length of output: 22569
🏁 Script executed:
#!/usr/bin/env bashset -euo pipefail
python3 - <<'PY'from pathlib import Pathimport recompaction = Path("packages/opencode/src/session/compaction.ts").read_text()processor = Path("packages/opencode/src/session/processor.ts").read_text()tests = Path("packages/opencode/test/session/compaction.test.ts").read_text()cleanup = re.search( r'if \(!input\.overflow\) \{.*?session\.removeMessage\(\{.*?\n\s*\}', compaction, re.S,)error_stop = re.search( r'if \(ctx\.blocked \|\| ctx\.assistantMessage\.error\) return "stop"', processor,)compaction_error_stop = re.search( r'if \(processor\.message\.error\) return "stop"', compaction,)cleanup_pos = compaction.find("session.removeMessage")error_check_pos = compaction.find("if (processor.message.error) return \"stop\"")fake_contract = re.search( r'function fake\(.*?result: "continue" \| "compact".*?process: .*?Effect\.succeed\(result\)', tests, re.S,)assert cleanup, "cleanup block not found"assert error_stop, "processor error-to-stop branch not found"assert compaction_error_stop, "compaction error-to-stop branch not found"assert cleanup_pos >= 0 and error_check_pos > cleanup_pos, "expected cleanup-before-error ordering not found"assert fake_contract, "existing fake processor contract not found"print("processor errors can produce result: stop")print("compaction cleanup precedes its processor.message.error check")print("existing compaction fake covers only continue and compact; no stop-with-error case")PY
Repository: bolt-builder/bolt-cli
Length of output: 346
Gate cleanup on processor.message.error.
When SessionProcessor.process() returns "stop" with an error, cleanup can delete summarized history before the error check. Add !processor.message.error to the guard and cover this path with a regression test.
🤖 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 `@packages/opencode/src/session/compaction.ts` around lines 419 - 436, Update
the cleanup guard surrounding message removal in SessionProcessor.process() to
require !processor.message.error in addition to the existing conditions,
preventing summarized history deletion when processing stops with an error. Add
a regression test covering the error-stop path and verifying cleanup is skipped.
The reason will be displayed to describe this comment to others. Learn more.
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 `@packages/opencode/test/session/compaction.test.ts`:
- Around line 945-968: Update the test around createSummaryCompaction and
SessionCompaction.use.process to first execute a completed summary-generation
call, ensuring the fake processor returns its normal successful result for that
call. Then invoke the stop-path process as the second call and configure the
fake processor to return "stop" only on that second invocation, preserving the
assertions that summarized messages remain available.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 41cf95ab-3ffc-4601-8bf5-15b4392e5a40
📥 Commits
Reviewing files that changed from the base of the PR and between 3e6c5db and 7d0d172.
📒 Files selected for processing (2)
packages/opencode/src/session/compaction.ts
packages/opencode/test/session/compaction.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
Seed a completed summary before testing the stop path.
createSummaryCompaction only creates a compaction marker. It does not create a summary. Add a completed summary message before invoking process with "stop", and make the fake processor return "stop" only for that second call.
🤖 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 `@packages/opencode/test/session/compaction.test.ts` around lines 945 - 968,
Update the test around createSummaryCompaction and SessionCompaction.use.process
to first execute a completed summary-generation call, ensuring the fake
processor returns its normal successful result for that call. Then invoke the
stop-path process as the second call and configure the fake processor to return
"stop" only on that second invocation, preserving the assertions that summarized
messages remain available.
The reason will be displayed to describe this comment to others. Learn more.
The test exercises the cleanup guard within a single process call: the fake sets message.error and returns "stop", and the deletion candidates are the seeded head turns from selected.head, not a prior summary. Seeding a completed summary first and stopping only on a second call would exercise the same guard with extra machinery, mirroring the adjacent pre-verified regression test's seeding pattern.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Three correctness bugs found by auditing the session runtime, each confirmed by reading the code paths end to end.
1. Compaction deleted history before the summary existed (data loss). In
session/compaction.ts, the block that removes summarized head/hidden messages ran beforeprocessor.process(...)generated the summary. So when summarization failed (rate limit, network error, or the "Session too large to compact" branch that returns"stop"), the original messages were already hard-deleted by the projector and unrecoverable. Moved the removal to after the summary is produced and theresult === "compact"early-return has been passed.Added a regression test (
keeps summarized history when compaction fails) that seeds head turns, forces a"compact"failure, and asserts the history survives. Verified it fails on the pre-fix ordering and passes after.Follow-up from review: a
"stop"result with an errored assistant message also means no valid summary exists, so the removal block is additionally gated on!processor.message.error, with a matching regression test (keeps summarized history when processing stops with an error).2. Plan->build switch reminder never fired. The default agent was renamed (
build: { name: "code", ... }inagent/agent.ts), butsession/reminders.tsstill gated oninput.agent.name === "build", which is now unreachable. So after working in theplanagent and switching back to implement, the BUILD_SWITCH reminder was never appended and the model often kept refusing edits. Changed the comparison to"code".3.
bolt logs --tail 0dumped the entire log.lines.slice(-count)withcount === 0isslice(0), which returns everything. Guarded the zero case to print nothing.Verified:
bun run typecheckclean; full compaction suite 53 pass / 1 skip.Note: the audit also flagged a server-side model-resolution change (
runner/model.tsusescatalog.model.cheapest()where upstream useddefault(), so a configured default model is ignored for model-less v2 sessions). That one may be intentional fork behavior, so I left it out pending a product call.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.Summary by CodeRabbit
New Features
logs --tail 0to hide existing log entries while continuing to stream new entries with--follow.Bug Fixes