Conversation
Co-commit immutable ledger/readiness events and consumer-aware status-dirty history. Expand trusted subscribers atomically and dispatch encrypted prepared requests through fenced leases and the SDK-owned pinned transport. Keep wiring optional, Core readiness excluded, and status/activity emission deferred to the complete projector. Add migration, restart, adversarial and bounded separate-process PostgreSQL failure coverage. Refs #1168
![aao-secretariat[bot]](https://avatars.githubusercontent.com/u/221690821?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — additive, default-off transactional reporting notification outbox slice.
No blocking findings. The reviewer studied the load-bearing source (outbox pg/routing/worker/memory/_schema, ledger pg.py, store.py, notification_models.py) and verified transactional co-commit/replay idempotency, AES-256-GCM AAD-bound envelope authentication, per-attempt subscription re-resolution, account-scoped isolation, bounded timeouts, and expiry-fenced SKIP LOCKED leases. The feat: semver signal is correct for a purely additive public surface.
No Critical/High/Medium findings were reported. One non-blocking follow-up noted (worker captures failures to durable error_code columns without a log/alert hook) — a Low/nit that does not block.
Decision-table path: no critical/high (row 1 n/a); gated_paths false (row 2 n/a); high_risk false, no deletions (rows 3,5 n/a); no medium findings (rows 4,8 n/a); no author team gate (row 7 n/a); no prior decision (row 6 n/a). Falls through to row 9 → approve.
…urns PostgreSQL dropped a lifecycle-only put_configuration through ON CONFLICT DO NOTHING, so a deactivated feed kept minting obligations and never produced its status-dirty generation, while the memory store applied both. rc.3's reporting-delivery-config-state.json walks one immutable generation from ready to inactive and requires deactivated_at there, which is why activation, deactivation and the recovery/retention windows are excluded from content_sha256. Apply them to the retained generation and co-commit the dirty record on that exact connection under the account lock. An unchanged re-put stays a no-op that enqueues nothing; changed content is still a CONFIGURATION_GENERATION_IMMUTABLE conflict that writes nothing. Exempt exactly those lifecycle columns from the reconciliation reference guard, so a Managed generation bound by reconciliation records keeps its published content immutable and can still be deactivated, and refresh the one schema-contract digest that moves. Restore the parent's set_issue_state return values: retired_at advances on every waive again, since no rc.3 field backs it and default-off Core behavior must not change. Derive the dirty no-op from the resulting record instead of predicting it, so an idempotent re-acknowledge still enqueues nothing while a repeated waive stays reconstructable. Raise the standard actionable [pg] install hint from PgReportingOutbox construction, matching the sibling PostgreSQL stores. Refs #1168 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean follow-up on the reporting ledger correctness delta.
No blocking findings. No Medium findings. Reviewer verified store parity (pg + memory agree via shared configuration_lifecycle() tuple), reconciliation immutability guard correctly exempting the four rc.3 lifecycle columns, set_issue_state restoring parent retired_at behavior, transaction/locking, and both-backend test coverage. The semver signal is noted as a restoration, not a break.
Decision-table walk: no critical/high (row 1 n/a); gated_paths false (row 2 n/a); high_risk false, no deletions (rows 3/5 n/a); no medium findings (rows 4/8 n/a); prior decision was approve so row 6 n/a; no no-auto-approve team match (row 7 n/a). Falls through to row 9 → approve. Prior decision was also approve with no findings; this run confirms continued clean state.
1 participant
Refs #1168
This is the partial #1168A slice: ledger and Managed readiness notifications plus a durable status-dirty handoff. It emits and advertises no status notification or webhook activity support; #1168B owns durable activity, and #1168C owns complete status projection, fingerprint deduplication and clock sweeps.
Reporting mutations can now retain their notification work in the same transaction, so a producer crash after commit cannot lose a ledger/readiness notification. Subscriber expansion and HTTP delivery resume independently after restart with an immutable notification ID, prepared body, and per-subscriber idempotency key.
Stacked base:
conductor/reporting-evidence-currency-integrationat037de4ac822ecefb2f95d32c15c297fb4c45d683(reviewed foundation join, #1177). The existing #1170 plus #1169 → #1171 → #1167A join is preserved. Its combined currency/evidence/retry/restart regression and private harness passed before implementation: 105 tests.Corrected head after independent review:
21bf443e7d850d1800ec8a6f2e4abec1c8f85541(fix(reporting): apply rc.3 configuration lifecycle and keep issue returns).Transaction and replay guarantees
reporting.ledger_changedevents and ordered status-dirty evidence. PostgreSQL uses the exact domain connection and explicit transactions, including autocommit pools. Enqueue failure rolls back the domain write; replay/conflicting identities cannot create ghost events.reporting.delivery_readyrequires the trusted configuration, frozen destination/obligation binding, and verified materialization in the same transaction. Its causal identity includes account and reconciliation consumer. Core subscriber requests and capability/profile strings cannot enable it.Delivery and security guarantees
SKIP LOCKEDleases use random expiring tokens and fence every completion/release/quarantine. HTTP holds no transaction. Claim crashes do not exhaust retries; network/transient signing/DNS and 408/425/429/5xx retry, while permanent/poison records quarantine without starving independent subscribers. Receiver acceptance followed by lost ACK retries the same bytes/key with a freshly verified signature.Migration and compatibility
Adds
reporting_notification_outbox.sqlas the fifth bundled ledger migration (event, expansion/delivery, dirty journal/head, projector checkpoint and typed issue-scope tables). Installation is additive and atomic, including standalone autocommit, concurrent, repeated and interrupted upgrades. Populated historical upgrades preserve ledger row identities/content without backfill. Opted-in startup/capability checks validate the full installed column/constraint/index/trigger/function chain. Default-off Core startup preserves adopter indexes.Existing constructors and protocols remain compatible: concrete stores add only
notifications=False;ReportingProducerhas no required argument/callback andReportingLedgerStoregains no required methods. Base imports work with PG genuinely absent; wheel/sdist include the exact SQL chain, and installed[pg]migration/restart is exercised. Only ledger and structurally verified readiness capability fields are available. Status notification stays absent andsupports_webhook_activity=false; #1168B owns durable activity; #1168C owns complete status projection, fingerprint deduplication and clock sweeps.Independent review fixes
retired_atupdate on a repeated waive. Dirty enqueue follows the actual retained before/after difference, so an unchanged re-acknowledgment enqueues nothing and opting in preserves issue returns.PgReportingOutboxwithout the PG dependencies now raises the existing actionable[pg]installation hint, with installed-base packaging coverage.Validation
PostgreSQL commands below use the disposable PostgreSQL 16 instance with
ADCP_PG_TEST_URL=postgresql://vercel-sandbox@127.0.0.1:55432/postgresandPYTHONUNBUFFERED=1. No essential reporting test uses the default-excludedintegrationmarker. Process/pipe/receiver/barrier waits have hard deadlines and sanitized PID/phase diagnostics; cleanup targets only test-owned processes/groups. The shared receiver self-check passed before the fresh complete eight-case process lane.Final focused recheck on the corrected head: 192 passed.
Source and touched reporting/test Ruff checks pass. Black checks pass on all 28 changed Python files. Full source mypy passes on 1,370 files; strict adopter checks pass on 31 files with zero type-ignore suppressions. The normal commit and commit-message hooks pass, including pinned Ruff, Black, Bandit, type checks and secret/file checks, using provisioned Python 3.11.16.
Open workspace in Conductor