Skip to content

Add transactional conversations - #6

Merged
c0da-ricon merged 1 commit into
mainfrom
c0da/transactional-conversations
Aug 6, 2026
Merged

Add transactional conversations#6
c0da-ricon merged 1 commit into
mainfrom
c0da/transactional-conversations

Conversation

@c0da-ricon

@c0da-ricon c0da-ricon commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • resolve private user/chat identity through a small conversation registry
  • make each Conversation own visible-history continuity, one active turn, and the save-or-rollback decision
  • keep Pi streaming, checkpointing, branch rollback, and disposal with SessionAgent
  • atomically persist private visible history through a focused file store

Consistency boundary

Visible-history save is the commit boundary. Model failure, active abort or stream cancellation, and save failure restore the prior persistent Pi branch and dispose the abandoned session. The registry keeps that conversation reserved until rollback finishes, preventing a retry from racing the same session file.

A real SessionManager reopen test proves the abandoned branch and plain stein.turn_rollback marker stay outside future model context. Registry tests exercise the real Conversation and SessionAgent over one Pi boundary fake rather than reproducing the transaction state machine.

This remains the next incremental slice from #4 after #5. It intentionally excludes HTTP/OpenAI transport, a runnable service, deployment, product policy, cross-process locking, and crash-atomic commit across independent stores. The #4 branch is unchanged.

Validation

  • mise run check (32 tests)
  • git pre-commit
  • intentional one-commit branch replacement guarded by exact-head --force-with-lease; PR had no reviews, requests, or comments

@c0da-ricon
c0da-ricon force-pushed the c0da/transactional-conversations branch from 5e0e3e9 to 50a12c9 Compare August 6, 2026 14:46
@c0da-ricon
c0da-ricon merged commit 31ef0e7 into main Aug 6, 2026
1 check passed
@ikma-ricon

Copy link
Copy Markdown
Contributor

Post-merge adversarial finding:

Abort during visible-history persistence commits instead of rolling back (src/conversation/conversation.ts:143-146, with persistence starting at :163-164). #commitStarted is set before await this.#conversation.commit(...), and abort() becomes a resolved no-op as soon as that flag is set. A client abort while historyStore.save() is pending therefore reports completion while the visible history and Pi turn still commit. That contradicts the active-abort rollback contract and makes the effective commit point earlier than successful visible-history save. The existing abort coverage at test/conversation/conversation-registry.test.ts:200-219 gates model prompting, so it does not exercise this window.

I reproduced this deterministically at exact merge 31ef0e7 with a gated history store through the production ConversationRegistryConversationSessionAgent path: wait for save() to start, await turn.abort(), then release the save. The response/history committed, no Pi branch rollback occurred, and the session remained live. I also ran mise run check (32 tests), reran the real SessionManager rollback/reopen test, and stress-probed stream cancellation. Reopen correctly excludes the abandoned branch and rollback marker.

Surviving concerns: stream cancellation currently reaches Pi abort twice through nested cleanup/rollback, relying on Pi abort idempotency; and the registry has no idle eviction/shutdown policy, so successful conversation sessions remain cached for the registry lifetime. Neither displaced the concrete abort/save race above. The private file store's temp-create/rename/cleanup and permission boundary look proportionate to the explicitly single-process, non-crash-atomic scope.

Disposition: corrective follow-up needed for the abort/save race, with a regression test at the production conversation boundary. Verification was not degraded.

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.

2 participants