Skip to content

avoid duplicate compaction system prompt#2647

Merged
aheritier merged 1 commit intodocker:mainfrom
rumpl:fix-compaction-duplicate-system-prompt
May 6, 2026
Merged

avoid duplicate compaction system prompt#2647
aheritier merged 1 commit intodocker:mainfrom
rumpl:fix-compaction-duplicate-system-prompt

Conversation

@rumpl
Copy link
Copy Markdown
Member

@rumpl rumpl commented May 5, 2026

Create the synthetic compaction agent without an instruction because the compaction system prompt is already inserted into the child session. Otherwise session.GetMessages prepends the agent instruction and the sub-run sends the same system prompt twice.

Create the synthetic compaction agent without an instruction because the compaction system prompt is already inserted into the child session. Otherwise session.GetMessages prepends the agent instruction and the sub-run sends the same system prompt twice.

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
@rumpl rumpl requested a review from a team as a code owner May 5, 2026 22:03
Copy link
Copy Markdown

@docker-agent docker-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

The fix is correct and well-motivated. The root cause is clear: explicitly prepends as a system message into the session items it builds, so when is later called inside the sub-run, would prepend the agent's own instruction a second time — producing a duplicate. Passing as the instruction causes to skip the instruction block entirely (the guard is ), so the system prompt arrives exactly once via the session items.

The new test accurately validates the regression by counting system messages matching inside the closure and asserting the count is exactly 1. The correctly implements the interface.

No high- or medium-severity issues found.

Copy link
Copy Markdown

@docker-agent docker-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

The fix is correct and well-motivated.

Root cause: extractMessages() explicitly prepends compaction.SystemPrompt as a system-role message into the session items it builds. When compactionSession.GetMessages(compactionAgent) is later called inside the sub-run, buildInvariantSystemMessages would also prepend the agent's own Instruction() — causing the compaction system prompt to appear twice. Passing "" as the instruction causes buildInvariantSystemMessages to skip the instruction block entirely (guarded by if instructions := a.Instruction(); instructions != "") so the system prompt arrives exactly once via the session items.

Test: TestRunLLM_DoesNotDuplicateSystemPrompt accurately validates the fix. It counts system messages matching compaction.SystemPrompt inside the RunAgent closure and asserts the count is exactly 1. The fakeProvider correctly satisfies the provider.Provider interface.

No high- or medium-severity issues found.

@aheritier aheritier merged commit 19742e0 into docker:main May 6, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants