Skip to content

P1: Post-commit audit sink failure leaves save reported failed after Outbox commit #225

Open

Description

@philipgreat

Reproduced on committed-mutation PR #216 exact head 54d0f11 with the existing GraphTraceSqliteTest fixture.

Setup: install SqliteCommittedMutationRecorder and an AppAuditEventSink that throws IllegalStateException. Save a new CustomerOrder through the normal audited graph API. The save call throws the sink exception, but direct SQLite read finds the business row and outboxAfter(source, 0, 10) contains one durable event. Replace the sink with a working one and retry the same entity: save throws again; Outbox still contains one event.

Focused diagnostic command: mvn -q -pl teaql-sqlite -am -Dtest=GraphTraceSqliteTest#diagnosticPostCommitAuditFailureLeavesCommittedGraphAndOutbox -Dsurefire.failIfNoSpecifiedTests=false test. It passed twice in an isolated exact-head worktree. The diagnostic method was not committed to the PR.

Source path: TeaQLRuntime.saveGraph calls completeLedgerPlan after executeInTransaction commits; completeLedgerPlan applies each persisted entity, calls emitAuditEvent, then clears the current ledger only after the loop. An application sink exception therefore escapes after commit and before ledger clear. The post-commit callback behavior predates PR #216; the durable Outbox makes the ambiguous outcome directly observable.

Expected policy needs confirmation: either report success while preserving a safe observable audit-delivery failure, or return an explicit committed-but-delivery-failed outcome that cannot be mistaken for a retryable transaction failure. In both cases, finish post-commit state reconciliation for every graph member and do not duplicate the committed mutation on retry. Do not move an external callback into the database transaction or claim it is atomic with the Outbox.

Activity

  1. philipgreat commented on Oct 18, 2026

    @philipgreat
    ContributorAuthor

    The exact diagnostic added temporarily to GraphTraceSqliteTest was:

    @Test public void diagnosticPostCommitAuditFailureLeavesCommittedGraphAndOutbox() throws Exception {
        var fixture = new Fixture(true);
        fixture.context.putAttribute(AppAuditEventSink.class.getName(),
                (AppAuditEventSink) (caller, event) -> {
                    throw new IllegalStateException("diagnostic audit sink failure");
                });
        var order = fixture.create("CustomerOrder", 199, "committed before audit delivery");
        assertThrows(IllegalStateException.class,
                () -> order.auditAs("diagnose post-commit audit failure").save(fixture.context));
        assertEquals(1, fixture.driver.queryForList(
                "SELECT id FROM customer_order_data WHERE id = ?", new Object[] {199L}).size());
        assertEquals(1, fixture.committedRecorder.eventStore()
                .outboxAfter("java-graph-source", 0, 10).size());
        fixture.context.putAttribute(AppAuditEventSink.class.getName(),
                (AppAuditEventSink) (caller, event) -> fixture.audit.add(event));
        assertThrows(RuntimeException.class,
                () -> order.auditAs("retry after audit delivery failure").save(fixture.context));
        assertEquals(1, fixture.committedRecorder.eventStore()
                .outboxAfter("java-graph-source", 0, 10).size());
    }

    This passed twice at exact PR head 54d0f11. The test is diagnostic, not a proposed acceptance assertion.

  2. philipgreat commented on Oct 18, 2026

    @philipgreat
    ContributorAuthor

    Draft fix proposal: #226 at 9389407. It aligns with Rust PR #255 behavior: clear the committed ledger before callbacks, attempt every graph member and both audit sinks, and surface a sanitized AUDIT_DELIVERY_AFTER_COMMIT outcome rather than a generic retryable failure. The focused Java graph/dynamic SQLite suites pass 57/57 locally. This draft targets the feature branch and does not alter or approve PR #216 until reviewed.

  3. philipgreat commented on Oct 18, 2026

    @philipgreat
    ContributorAuthor

    For exact combined-head CI only, draft PR #227 targets main from the same fix commit 9389407. It must not be merged directly. The review path remains #226 into #216, followed by fresh exact-head and post-merge checks.

  4. philipgreat commented on Oct 18, 2026

    @philipgreat
    ContributorAuthor

    Combined candidate #227 reached 5/5 exact-head CI checks at 9389407: full Build and Test, examples, PostgreSQL/MySQL, SQL Server, and Business ID SQL Server. Draft fix #226 remains the review path into #216; this green CI is not merge approval.

  5. philipgreat commented on Oct 18, 2026

    @philipgreat
    ContributorAuthor

    Post-commit entity-reconciliation fault injection now has a separate stacked draft fix: #228 at 628991a. It marks only the committed save graph reload-required, blocks root/child re-save and ledger rebinding/merge, and leaves an unrelated graph in the same UserContext saveable. The affected Maven reactor passes locally. CI-only combined candidate #229 targets main for exact-head checks and must not be merged. This extends, but does not replace, the unresolved audit-delivery API decision in #226.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions