Skip to content

Added transaction commits to multi sql functions in session_manager#5693

Merged
DOsinga merged 2 commits intoblock:mainfrom
tiensi:tiensi/wrap-sql-transactions
Nov 12, 2025
Merged

Added transaction commits to multi sql functions in session_manager#5693
DOsinga merged 2 commits intoblock:mainfrom
tiensi:tiensi/wrap-sql-transactions

Conversation

@tiensi
Copy link
Contributor

@tiensi tiensi commented Nov 12, 2025 •
edited
Loading

Summary

Added transaction commits to add_message and delete_session in session_manager

Type of Change

  • Feature
  • Bug fix
  • Refactor / Code quality
  • Performance improvement
  • Documentation
  • Tests
  • Security fix
  • Build / Release
  • Other (specify below)

AI Assistance

  • This PR was created or reviewed with AI assistance

Testing

Unit Tests added

Related Issues

Discussion: Discord

@tiensi tiensi marked this pull request as ready for review November 12, 2025 06:39
Copilot AI review requested due to automatic review settings November 12, 2025 06:39
Copilot AI reviewed Nov 12, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds explicit transaction management to add_message and delete_session methods in SessionStorage to ensure atomicity when multiple SQL statements are executed together.

  • Wraps multi-statement database operations in transactions with explicit commits
  • Adds comprehensive unit tests for both modified functions
  • Follows the existing transaction pattern already used in replace_conversation

katzdave
katzdave reviewed Nov 12, 2025
View reviewed changes
Copy link
Collaborator

@katzdave katzdave left a comment

Choose a reason for hiding this comment

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

Nice work, definitely agree transactions make sense here.

crates/goose/src/session/session_manager.rs Outdated
}

#[tokio::test]
async fn test_delete_session() {
Copy link
Collaborator

@katzdave katzdave Nov 12, 2025

Choose a reason for hiding this comment

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

Can we get rid of these tests.

Copy link
Collaborator

@DOsinga DOsinga Nov 12, 2025

Choose a reason for hiding this comment

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

yes please

Copy link
Contributor Author

@tiensi tiensi Nov 12, 2025

Choose a reason for hiding this comment

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

done

@tiensi
Removed tests
3622c97 
Signed-off-by: Vincent Huang <vhuang@squareup.com>
@tiensi tiensi requested review from DOsinga and katzdave November 12, 2025 19:05
@DOsinga DOsinga merged commit e1b5af5 into block:main Nov 12, 2025
14 checks passed
michaelneale added a commit that referenced this pull request Nov 13, 2025
@michaelneale
Merge branch 'main' into micn/goose-mobile-access
04cd39b 
* main: (27 commits)
  hackathon banner  (#5710)
  Fix documentation-only change detection for push events (#5712)
  Added transaction commits to multi sql functions in session_manager (#5693)
  fix: improve and simplify tool call chain rendering (#5704)
  Fix: Always show autocompact threshold ui (#5701)
  chore: Update governance to include Discord (#5690)
  Ollama improvements (#5609)
  feat: add Supabase MCP server to registry (#5629)
  Unlist VS Code extension tutorials from MCP and experimental sections (#5677)
  fix: make image processing work in github copilot provider (#5687)
  fix: do not take into account gitignore in developer mcp (#5688)
  docs: session storage migration (#5682)
  New maintainers (#5685)
  chore: Update governance (#5660)
  chore(release): release version 1.14.0 (minor) (#5676)
  fix : action icons overlap session title in chat history (#5684)
  Document recent goose PRs (#5683)
  docs: add GOOSE_PATH_ROOT environment variable documentation (#5678)
  feat: SessionManager integration for acp sessions (#5657)
  teach copilot our CI (#5672)
  ...
zanesq added a commit that referenced this pull request Nov 13, 2025
@zanesq
Merge branch 'main' of github.com:block/goose into zane/nextcamp-live
80fb2e4 
* 'main' of github.com:block/goose:
  docs: removing double announcements (#5714)
  docs: mcp sampling support (#5708)
  hackathon banner  (#5710)
  Fix documentation-only change detection for push events (#5712)
  Added transaction commits to multi sql functions in session_manager (#5693)
  fix: improve and simplify tool call chain rendering (#5704)
katzdave added a commit that referenced this pull request Nov 13, 2025
@katzdave
Merge branch 'main' of github.com:block/goose into dkatz/tool-loop-cl…
620df9f 
…eanup

* 'main' of github.com:block/goose: (46 commits)
  Fix context progress bar not resetting after /clear command (#5652)
  docs: removing double announcements (#5714)
  docs: mcp sampling support (#5708)
  hackathon banner  (#5710)
  Fix documentation-only change detection for push events (#5712)
  Added transaction commits to multi sql functions in session_manager (#5693)
  fix: improve and simplify tool call chain rendering (#5704)
  Fix: Always show autocompact threshold ui (#5701)
  chore: Update governance to include Discord (#5690)
  Ollama improvements (#5609)
  feat: add Supabase MCP server to registry (#5629)
  Unlist VS Code extension tutorials from MCP and experimental sections (#5677)
  fix: make image processing work in github copilot provider (#5687)
  fix: do not take into account gitignore in developer mcp (#5688)
  docs: session storage migration (#5682)
  New maintainers (#5685)
  chore: Update governance (#5660)
  chore(release): release version 1.14.0 (minor) (#5676)
  fix : action icons overlap session title in chat history (#5684)
  Document recent goose PRs (#5683)
  ...
tlongwell-block added a commit that referenced this pull request Nov 14, 2025
@tlongwell-block
Merge remote-tracking branch 'origin/main' into moim2
2eb9c81 
* origin/main: (51 commits)
  Compaction resiliency improvements (#5618)
  docs: ask goose button (#5730)
  Update prompt injection detection metrics (due to errors exporting to datadog) (#5692)
  Spence/icon2 tokyo drift (#5728)
  docs: logs rotation and misc updates (#5727)
  docs: automatic ollama model detection (#5725)
  Fix context progress bar not resetting after /clear command (#5652)
  docs: removing double announcements (#5714)
  docs: mcp sampling support (#5708)
  hackathon banner  (#5710)
  Fix documentation-only change detection for push events (#5712)
  Added transaction commits to multi sql functions in session_manager (#5693)
  fix: improve and simplify tool call chain rendering (#5704)
  Fix: Always show autocompact threshold ui (#5701)
  chore: Update governance to include Discord (#5690)
  Ollama improvements (#5609)
  feat: add Supabase MCP server to registry (#5629)
  Unlist VS Code extension tutorials from MCP and experimental sections (#5677)
  fix: make image processing work in github copilot provider (#5687)
  fix: do not take into account gitignore in developer mcp (#5688)
  ...
wpfleger96 added a commit that referenced this pull request Nov 14, 2025
@wpfleger96
Merge branch 'main' into wpfleger/persist-provider-config
a5abb97 
* main: (65 commits)
  Fix: Recipes respect the quiet flag (#5743)
  docs: update cli commands (#5744)
  Run smoke tests on a free runner (#5745)
  faster, cheaper (pick two): improve CI workflow and switch to free github runner (#5702)
  Compaction resiliency improvements (#5618)
  docs: ask goose button (#5730)
  Update prompt injection detection metrics (due to errors exporting to datadog) (#5692)
  Spence/icon2 tokyo drift (#5728)
  docs: logs rotation and misc updates (#5727)
  docs: automatic ollama model detection (#5725)
  Fix context progress bar not resetting after /clear command (#5652)
  docs: removing double announcements (#5714)
  docs: mcp sampling support (#5708)
  hackathon banner  (#5710)
  Fix documentation-only change detection for push events (#5712)
  Added transaction commits to multi sql functions in session_manager (#5693)
  fix: improve and simplify tool call chain rendering (#5704)
  Fix: Always show autocompact threshold ui (#5701)
  chore: Update governance to include Discord (#5690)
  Ollama improvements (#5609)
  ...
Surendhar-N-D pushed a commit to Surendhar-N-D/goose that referenced this pull request Nov 17, 2025
@tiensi @Surendhar-N-D
Added transaction commits to multi sql functions in session_manager (b…
b1961ea 
…lock#5693)

Signed-off-by: Vincent Huang <vhuang@squareup.com>
arul-cc pushed a commit to arul-cc/goose that referenced this pull request Nov 17, 2025
@tiensi @arul-g
Added transaction commits to multi sql functions in session_manager (b…
d49066f 
…lock#5693)

Signed-off-by: Vincent Huang <vhuang@squareup.com>
@github-actions github-actions bot mentioned this pull request Nov 18, 2025
Merged
BlairAllan pushed a commit to BlairAllan/goose that referenced this pull request Nov 29, 2025
@tiensi @BlairAllan
Added transaction commits to multi sql functions in session_manager (b…
5479544 
…lock#5693)

Signed-off-by: Vincent Huang <vhuang@squareup.com>
Signed-off-by: Blair Allan <Blairallan@icloud.com>
@RamXX RamXX mentioned this pull request Jan 22, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@katzdave katzdave katzdave approved these changes

@DOsinga DOsinga Awaiting requested review from DOsinga

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tiensi @DOsinga @katzdave