Skip to content

fix: preserve SQLite commit outcomes during cancellation - #696

Merged
pedronauck merged 1 commit into
mainfrom
fix/tool-event-persistence-ci
Oct 6, 2026
Merged

fix: preserve SQLite commit outcomes during cancellation#696
pedronauck merged 1 commit into
mainfrom
fix/tool-event-persistence-ci

Conversation

@pedronauck

@pedronauck pedronauck commented Oct 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Corrige uma ambiguidade na confirmação de transações SQLite: quando o contexto expirava durante um COMMIT já concluído, a gravação retornava erro e o retry podia duplicar um evento de ferramenta. A fronteira compartilhada agora verifica o cancelamento após a autorização e conclui o COMMIT preservando seu resultado real. Cancelamento observado antes dessa etapa continua causando rollback.

A falha surgiu no shard Go 2 da release #688 (112218483055). Reproduções com SQLite real e race detector em macOS e Linux expuseram dois eventos onde deveria existir um. A regressão determinística na suíte existente TestExecuteWrite falha antes da correção; o teste original do daemon mantém suas asserções e seus prazos.

O runtime (112218483056) também revelou que a jornada de histórico reutilizava um timeout de dois segundos destinado ao cenário de lifecycle. Ela agora tem uma fixture própria, bloqueada até o cancelamento real e recuperada no retry, com orçamento de dez segundos para a preparação da sessão. Permanecem as duas tentativas obrigatórias e o limite total de 45 segundos. O cenário específico de timeout continua com seus dois segundos e sua fixture original.

Validação já concluída:

  • Suíte TestExecuteWrite com race: passou (16,293 s).
  • Dois casos novos de cancelamento: dez repetições cada, passaram.
  • Suíte original de eventos: 30 repetições em macOS e 30 em Linux com duas CPUs, passaram.
  • Jornada real de roster: três repetições com daemon de produção e race, passaram (50,927 s).
  • Integração de skill gerenciada com subprocesso ACP e contenção SQLite: passou (5,869 s).
  • make test-e2e-runtime: 313 testes passaram localmente (daemon, HTTP, UDS, harness e CLI).
  • CI do head 486ea616: E2E runtime aprovado, os mesmos 313 testes passaram em Linux.
  • CI do head 486ea616: shard Go 2 aprovado, incluindo daemon com race, banco global e integração real de Goal. Todos os oito shards Go passaram.
  • Lint Go: zero problemas.

make gate final passou: lint Go sem problemas e todas as suítes Go afetadas com race. CI final do head 486ea616568c4585b6d265e938a8de4632c87b4b aprovado: 33 checks passaram, 17 skips esperados, sem falhas ou pendências. Inclui todos os shards Go/Web e E2E Desktop. CodeRabbit aprovou esse mesmo head; nenhuma discussão de revisão permanece aberta.

Impacto registrado em docs/_memory/change-impact.md; evidência em docs/qa/reports/2026-10-05-dependency-upgrades.md. Sem alterações de schema, contratos públicos, configuração, hooks, IDs de tools ou isolamento de workspace/perfil. Inclui nota de release.

Revisão de duração do COMMIT (Greptile): a confirmação permanece síncrona, com conexão e admissão pertencentes à chamada até seu retorno; não cria goroutine ou tarefa destacada. A configuração compartilhada usa busy_timeout(5000) para espera por locks. Isso não constitui um limite total de I/O. O próprio modernc.org/[email protected] executa tx.Commit() com context.Background(). Em stmt.exec, um cancelamento concorrente substitui o resultado real por ctx.Err(), inclusive após sucesso. Um novo deadline nessa etapa recriaria o defeito em outra fronteira temporal. A interrupção SQLite é cooperativa e explicitamente permite que uma operação quase concluída termine; não fornece um limite rígido de I/O. Mantemos o cancelamento antes do commit e preservamos o resultado da confirmação iniciada, sem introduzir um novo sistema de execução assíncrona.

@vercel

vercel Bot commented Oct 6, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
compozy-site Ready Ready Preview Oct 6, 2026 10:53am UTC

Request Review

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: compozy/compozy/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 24496fc5-c819-4d50-b203-4ebd8a395477
📥 Commits

Reviewing files that changed from the base of the PR and between 3dd014b and 486ea61.

⛔ Files ignored due to path filters (6)
  • .release-notes/sqlite-commit-cancellation.md is excluded by !**/*.md
  • docs/_memory/change-impact.md is excluded by !**/*.md
  • docs/qa/reports/2026-10-05-dependency-upgrades.md is excluded by !**/*.md
  • docs/qa/scenarios/ET-skill-view-actionable-errors.md is excluded by !**/*.md
  • docs/qa/scenarios/LP-run-read-agent-journey.md is excluded by !**/*.md
  • internal/testutil/acpmock/testdata/loop_node_lifecycle_fixture.json is excluded by !**/*.json
📒 Files selected for processing (3)
  • internal/daemon/loop_read_cli_e2e_integration_test.go
  • internal/store/write.go
  • internal/store/write_test.go

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

The write path now checks cancellation before commit and runs an accepted commit without the caller's cancellation. Tests cover cancellation at the commit fence and after SQLite commits. The daemon roster integration test uses a dedicated retry-loop definition with a longer timeout.

Changes

Write commit cancellation

Layer / File(s) Summary
Commit handling and cancellation tests
internal/store/write.go, internal/store/write_test.go		 The write path returns an error if the context is canceled before commit. It runs COMMIT with cancellation removed from the context. Tests cover cancellation at the commit fence and cancellation after SQLite commits. A test connector wraps SQLite connections to simulate post-commit cancellation.

Daemon roster retry fixture

Layer / File(s) Summary
Retry fixture and roster test
internal/daemon/loop_read_cli_e2e_integration_test.go		 The test setup seeds and waits for a dedicated retry-loop definition. The settled-attempts roster test uses that definition. Its YAML is derived from the lifecycle retry loop, with a different name, timeout, and prompt.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 486ea

No outstanding issue is established by this review. Merge after the pending CI checks pass.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving SQLite commit outcomes during cancellation.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Oct 6, 2026 •
edited
Loading

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Critical risk] Changes how database writes handle cancellation during commit.

The COMMIT lifetime should be bounded before merging because it violates the repository’s runtime-lifetime requirement.

Findings

  1. P2 COMMIT has no lifetime bound ▶
Summary

The PR changes the shared SQLite write boundary to preserve the result of a COMMIT when the caller is canceled during it. It also adds transaction regressions and gives the roster retry journey a separate cancellation-driven fixture and longer session-preparation budget.

  • The new COMMIT lifetime needs an explicit bound without reintroducing an ambiguous success result.
Diagram 
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Write callback] --> B[Mutation commit fence]
  B -->|Rejected| R[Rollback]
  B -->|Authorized| C[Check caller cancellation]
  C -->|Canceled| R
  C -->|Active| D[COMMIT without caller cancellation]
  D -->|Succeeded| S[Return success]
  D -->|Failed| E[Return commit error]
Loading

Reviews (1) · Last reviewed commit: "fix: preserve SQLite commit outcomes dur..."

greptile-apps[bot]
greptile-apps Bot reviewed Oct 6, 2026
View reviewed changes
Comment thread internal/store/write.go
@pedronauck
pedronauck merged commit d4fb84d into main Oct 6, 2026
50 checks passed
@pedronauck
pedronauck deleted the fix/tool-event-persistence-ci branch October 6, 2026 12:18
This was referenced Oct 6, 2026
Merged
Merged
Merged
Merged
Open

This branch was successfully deployed

1 active deployment
Preview — 486ea616 Deployed Oct 6, 2026 by vercel[bot]
View deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments
@coderabbitai coderabbitai[bot] coderabbitai[bot] approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@pedronauck