Skip to content

[bug] archive cascade and scope-invalidation publish are not transactionally paired #554

Closed as not planned

Description

@Sinity

Actual behavior

In crate/core/sinex-gateway/src/replay_control.rs:1779-1790, the archive-cascade flow is:

  1. SQL transaction archives matching events into audit.archived_events and removes them from core.events. Transaction commits.
  2. NATS sinex.derived.invalidation publish is issued so downstream automata know to re-derive.

If step 2 fails (NATS unavailable, transient publish error, sigterm between commit and publish), the DB transaction has already committed. Automata never receive the invalidation signal and continue serving stale derived state. The user-facing replay operation reports failure ("publish failed"), but the archival side effect persists.

The compensating logic at replay_control.rs:1839-1852 only fires on the abort path; in the normal success-then-publish-fail path there is no rollback.

Expected behavior

Archive and scope-invalidation must be atomic from the operator's perspective: either both succeed, or the system can deterministically recover and re-publish without leaving automata stranded.

Acceptable shapes:

  • Two-phase: durable outbox row inside the same transaction as the archive; a publisher loop drains the outbox to NATS with retry. NATS failure no longer corrupts state.
  • Inline retry with bounded backoff and a final failure path that re-archives or marks the operation for replay-republish.
  • Server-side trigger that emits via pg_notify plus a daemon that bridges to NATS.

The right shape is a design decision, but the current "publish best effort after commit" is a silent split-brain.

Reproduction or trigger

  1. Begin a replay operation with a non-empty cascade.
  2. After the archive transaction commits, before the publish completes, kill the gateway (or block NATS publish via firewall rule).
  3. Observe: events are in audit.archived_events, removed from core.events, but downstream automata never see the invalidation. Their derived outputs continue to reference archived parents.

Suspected scope

  • crate/core/sinex-gateway/src/replay_control.rs (archive_cascade, scope-invalidation publish)
  • crate/lib/sinex-db/src/replay/
  • possibly a new outbox table in core schema
  • VM / fault-injection test surface

Severity

p1 — silent automaton stale-state on transient NATS issue, no operator signal, no recovery path.

Evidence

  • replay_control.rs:1779-1790 — archive then publish, no transactional pairing.
  • replay_control.rs:1839-1852 — abort-path compensation only.
  • Matches CLAUDE.md event-lifecycle.md step 18 caveat: "Per-event NATS confirmation publish failure ... event persisted but node doesn't know."

Fix acceptance criteria

  • Archive completion guarantees that the corresponding invalidation signal is durable before the user is told the operation succeeded.
  • NATS publish failure does not leave automata permanently stale; a retry path drains pending invalidations.
  • Fault-injection test that kills the gateway between archive commit and publish; on restart, automata observe the invalidation.
  • Operations log records archive commit and publish ack independently so operators can audit the gap.

Activity

  1. added
    area:coresinex-core: event model, schema, replay
    area:gatewaysinex-gateway: JSON-RPC query/control
    on Apr 26, 2026
  2. Sinity commented on Apr 26, 2026

    @Sinity
    OwnerAuthor

    Determined: this cannot be transactional without 2PC between DB and NATS. The operation spans DB (archive) + NATS (invalidation publish, scan command dispatch) — holding a DB transaction open across NATS request-reply would block the connection pool and risk indefinite locks. The compensation saga (abort_before_scan_ack) and cascade analyzer integrity checks handle the practical consequences. Architecture note added to replay_control.rs in #594.

  3. Sinity commented on Apr 27, 2026

    @Sinity
    OwnerAuthor

    2026-04-27 audit note: the JetStream invalidation fix was attempted in #594 and reverted. The outbox-then-publish pattern remains in replay_control.rs:1779-1790. Reopening consideration tracked in #640.

  4. Sinity commented on Apr 28, 2026

    @Sinity
    OwnerAuthor

    Reopening per #640 audit. PR #594 was reverted/deferred for the JetStream consumer bootstrapping side, but the underlying atomicity gap (archive DB commit → NATS invalidation publish, no transactional pairing) was not addressed in code. This branch (feature/fix/audit-closure-integrity) adds a TODO(#554) comment at the exact gap in replay_control.rs explaining the transactional-outbox path needed to fully close it, and promotes the publish-failure log to an explicit error!() at the call site for operator visibility. The crash-recovery gap (process dies between archive commit and publish) remains open pending a proper outbox implementation. Tracking: #640.

  5. reopened this on Apr 28, 2026
  6. Sinity commented on Apr 28, 2026

    @Sinity
    OwnerAuthor

    Closing as wontfix in current scope.

    The two failure modes:

    1. Process alive, NATS publish fails — already handled. replay_control.rs calls abort_before_scan_ack which (a) restores the archived cascade, (b) publishes compensating invalidations, (c) returns the error to the operator. The replay operation fails cleanly; no silent split-brain.

    2. Process crashes between archive-commit and publish — only closable by a transactional outbox (or 2PC).

    PR #667 (commit 186a0e7, 2026-04-28) deliberately punted (2) by promoting the publish-failure log to explicit error!() rather than introducing outbox machinery. The outbox concept itself was tried and retired during the sensd→JetStream refactor (PRs #76 / #78 / #94 / #138, late 2025): bringing it back for one publish path would re-introduce machinery the codebase deliberately walked away from.

    For the pre-release single-user reality, the residual crash window — and the operator-visible ERROR that flags it — is acceptable. If/when this becomes a real-traffic concern, reopen with a fresh design call rather than implementing the path retired in #94.

    Closes via design decision, not via code change.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:coresinex-core: event model, schema, replayarea:gatewaysinex-gateway: JSON-RPC query/controltype:fixBug fix

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions