Skip to content

fix(core): tolerate orphan part projection from cascade-delete race - #33134

Closed
randomvariable wants to merge 3 commits into
anomalyco:devfrom
randomvariable:fix-orphan-part-projection
Closed

fix(core): tolerate orphan part projection from cascade-delete race#33134
randomvariable wants to merge 3 commits into
anomalyco:devfrom
randomvariable:fix-orphan-part-projection

Conversation

@randomvariable

@randomvariablerandomvariable commented Jun 20, 2026

Copy link
Copy Markdown

Issue for this PR

Closes#31990

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Occasionally the app crashes while projecting session events into SQLite, dumping a raw stack to the TUI with no log line. The failing query is an INSERT/UPSERT into a session-scoped table (part, message, session_message, session_input, ...) during a turn, failing with a foreign-key ConstraintError.

Root cause — a cascade-delete commit race. Session events are each committed in their own BEGIN IMMEDIATE transaction (event.ts), so they are not atomic with each other, and SQLite checks FKs at COMMIT. Writes are serialized (single-writer), so this is purely commit-ordering between concurrently-scheduled fibers: a session/message removal commits and cascade-deletes a parent row, then a later child-write event from a still-running turn commits and fails its FK. Effect.orDie turns the ConstraintError into a defect and the turn fiber crashes. The window stays open because session removal does not interrupt/await the in-flight turn.

This PR closes the whole class by skipping (and warning) an orphan child write when its parent row is already gone — mirroring the FK's own cascade intent. It fabricates nothing, skips the associated usage accounting (correct, since nothing is persisted), and is replay-safe (events apply in seq order, so the removal -> late-write sequence deterministically hits the skip).

Every child table with a cascade FK to a concurrently-removable parent:

Child -> parent FKWrite siteStatus
part.message_id -> messagePartUpdated projectionguarded
message.session_id -> sessionMessageUpdated projectionguarded
session_message.session_id -> sessioninsertMessage (chokepoint for all append events + Promoted)guarded
session_input.session_id -> sessionPrompted + Admitted projectionsguarded
todo.session_id -> sessionSessionTodo.update (command path)guarded
session_context_epoch.session_id -> sessionrunner initializealready safe (SELECTs session in-txn, dies before insert)
event.aggregate_id -> event_sequencecommit funnelsafe (parent inserted in same txn)
session.project_id -> projectCreated projectiononConflictDoNothing; cross-aggregate, project delete cascades sessions anyway

Changes:

  • event.ts: log eventID/eventType/aggregateID via Effect.tapError before Effect.orDie at the durable-commit funnel, so DB commit failures are visible in logs instead of only as a raw TUI stack. Crash behaviour is unchanged (typed SqlError in the error channel, not a defect).
  • session/projector.ts: add a shared sessionPresent helper; guard the PartUpdated, MessageUpdated, insertMessage (covers every append event), Prompted, and Admitted paths.
  • session/todo.ts: parity guard for the command-path todo insert.

This makes the orphan benign and logged. It does not stop the source from emitting the stray child event; quiescing the turn on session removal (interrupt + drain before publishing Deleted) is a sensible follow-up that would shrink the window at the source — but it can't replace these guards (replay and crash-recovery re-drive events independent of the live runner).

How did you verify your code works?

  • Added tests in packages/core/test/session-projector.test.ts covering each guarded edge: a late PartUpdated after MessageRemoved; a MessageUpdated for a removed session; a Step.Started append (session_message) for a removed session; and an admit (session_input) for a removed session. Each asserts no defect and no row written.
  • bun typecheck clean in packages/core.
  • bun test test/session-projector.test.ts -> 14 pass / 0 fail; todo tests 3 pass / 0 fail.

Screenshots / recordings

Not a UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

A concurrent MessageRemoved cascade-delete can remove an assistant
message (and its parts) while an aborting fiber still flushes a trailing
snapshot patch part. The PartUpdated projection then upserts a part whose
message_id FK has no parent row; SQLite enforces the FK at COMMIT, the
transaction re-fails as a SqlError, and Effect.orDie turns it into a
defect that crashes the prompt fiber and prints a raw stack to the TUI
with no log line.
- event.ts: log (eventID/eventType/aggregateID) before orDie at the
durable-commit funnel so DB commit failures are observable instead of
surfacing only as a raw TUI stack.
- projector.ts: in the PartUpdated projection, skip-and-warn when the
parent message row is absent, mirroring the FK's onDelete: cascade
intent. Replay-safe: fabricates nothing, deterministic under seq order.
Fixesanomalyco#31990
@github-actionsgithub-actionsBot added needs:compliance This means the issue will auto-close after 2 hours. and removed needs:compliance This means the issue will auto-close after 2 hours. labels Jun 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for updating your PR! It now meets our contributing guidelines. 👍

Sibling of the orphan-part guard. A session removal can cascade-delete the
parent session row while a late MessageUpdated from an in-flight turn is still
in the durable-commit funnel. message.session_id is a FK to session.id, checked
at COMMIT, so the trailing upsert fails with a ConstraintError, Effect.orDie
turns it into a defect, and the prompt fiber crashes (leaking a raw stack to
the TUI). Observed in practice: "removing share" + session removal at T, then a
trailing message.updated for the now-gone session ~10s later -> FK fail.
- session/projector.ts: in the MessageUpdated projection, skip and warn when
the parent session row is gone, mirroring the existing PartUpdated guard.
Replay-safe: events apply in seq order, so the removal -> late MessageUpdated
sequence deterministically hits the skip.
- session-projector.test.ts: add a test projecting a MessageUpdated for a
deleted session and asserting no defect and no message row.
… race
Extends the orphan-skip guards to every other child table that has a cascade
FK to a parent that a concurrently-committed removal event can delete, closing
the rest of this FK-crash class (each durable event commits in its own
transaction, so a child write can land after its parent was cascade-deleted and
fail the FK at COMMIT -> orDie -> fiber crash).
- projector.ts: add a shared `sessionPresent` helper and:
- guard `insertMessage` (session_message.session_id -> session). This is the
chokepoint for every append event (Step.Started, Shell.Started, Synthetic,
ContextUpdated, Agent/ModelSwitched, Compaction.Ended, Prompted user msg,
and Promoted), so one guard covers them all. Step.Started fires every step
of every turn, the dominant trigger.
- guard the Prompted and Admitted handlers (session_input.session_id ->
session) before their inbox writes.
- refactor the existing MessageUpdated guard onto the shared helper.
(Update paths in message-updater no-op on a missing row, so only the append
inserts needed guarding; context-epoch already SELECTs the session in-txn.)
- todo.ts: parity guard for the command-path TodoTable insert
(todo.session_id -> session), which has the same race outside the projector.
Tests: add orphan-skip cases for a session_message append (Step.Started) and a
session_input admit against a removed session, asserting no defect and no row.
marcusrbrown added a commit to fro-bot/agent that referenced this pull request Jun 22, 2026
* docs(plan): OpenCode 1.17.9 upgrade + SQLite-reliability carries
* feat(opencode): upgrade to 1.17.9 with SQLite-reliability carries
Bump the action/runtime OpenCode SDK and the @fro.bot/harness base version
from 1.17.6 to 1.17.9, and grow the carry set from 3 to 5 with two paired
upstream reliability fixes for the SQLite session-durability path.
- @opencode-ai/sdk -> 1.17.9 (root + runtime); FALLBACK_VERSION -> 1.17.9;
Renovate cap -> <=1.17.9; clonedeps + AGENTS.md annotations -> v1.17.9.
- harness base_version -> 1.17.9; add carries anomalyco/opencode#33134
(tolerate orphan part projection from a cascade-delete race) and #33159
(retry transient SQLite lock-timeouts on durable event commits), the latter
stacked on the former.
- DEFAULT_OPENCODE_VERSION and the workspace Dockerfile ARG are intentionally
left at the current harness build; the harness release's sync-default-version
job bumps them once the real 1.17.9+harness.<short8> publishes.
Bun stays 1.3.14 (matches upstream packageManager at 1.17.9). The 1.17.9 SDK
event-typing rename (catalog.model.updated -> catalog.updated) is not consumed
by Fro Bot, so the bump is type-safe; typecheck, tests, lint, and build pass.
* test(opencode): add env-gated live 1.17.9 SDK integration probe
Mirrors the prior 1.17.x upgrade-cycle probe: drives the real harness consumer
path (createOpencode -> event.subscribe -> promptAsync -> runPromptAttempt)
against a stock isolated 1.17.9 server. Gated behind OPENCODE_LIVE_PROBE=1 so it
is skipped in normal CI.
Live run proved the 1.17.9 streaming path: a bash tool rendered via
message.part.updated, message.part.delta flowed, session.idle fired with no
session.error, and v2 session.wait still returns the structured
ServiceUnavailableError that drives the poll-watchdog fallback.
* build(dist): refresh third-party notices for SDK 1.17.9 dependency tree
* test(opencode): clear CodeQL dead-store + clarify live-probe gating/coverage
- Drop the always-overwritten initializer on waitResult (CodeQL dead-store).
- Document why the probe suite must stay gated behind OPENCODE_LIVE_PROBE: it
mutates global process.env and would bleed into sibling files if run
concurrently.
- Correct the docstring to describe the actual exercised path
(createOpencode -> event.subscribe -> promptAsync -> runPromptAttempt); the
arming/startPrompt branch is not driven.
@github-actions

Copy link
Copy Markdown
Contributor

Automated PR Cleanup

Thank you for contributing to opencode.

Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions.

This PR was closed because it matched the following cleanup criteria:

  • The PR was created more than 1 month ago
  • The PR had fewer than 2 positive reactions
  • Positive reactions are counted as thumbs-up, heart, celebration, or rocket reactions on the PR

PRs created within the last month are not affected by this cleanup.

If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate.

Thanks again for taking the time to contribute.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] SQLite UPSERT into part table fails during step-finish event projection

1 participant

@randomvariable