Skip to content

chore: clear the safe audit-Low cluster (7 findings)#82

Merged
lesnik512 merged 1 commit into
mainfrom
chore/audit-low-safe-cluster
Jun 14, 2026
Merged

chore: clear the safe audit-Low cluster (7 findings)#82
lesnik512 merged 1 commit into
mainfrom
chore/audit-low-safe-cluster

Conversation

@lesnik512

Copy link
Copy Markdown
Member

Summary

Batches the low-risk, no-design-surface Low findings from the 2026-06-14 deep audit (planning/audits/2026-06-14-deep-audit-findings.md). No behavior change to shipped runtime paths. The 3 behavior-change Lows — graceful_timeout=None drain hang, the validate_schema _lease_ck probe, and OutboxResponse eager validation — are intentionally left for a separate focused PR (they each change observable behavior / add a failure mode and want their own review).

Code changes (cosmetic / mechanical)

  • Recorder dedup: OutboxProducer._emit_metric now delegates to metrics._safe_emit; removed the now-dead module logger + logging import. (The subscriber's _emit_metric stays separate by design — it routes through self._log for handler-scoped context.)
  • Type-hint parity: OutboxPublisher.publish body: SendableMessagetyping.Any, matching broker.publish / OutboxResponse (all converge on OutboxPublishCommand(body: Any)).
  • OutboxBroker.request: replaced (*args, **kwargs) with upstream's typed signature (message, queue, /, timeout=0.5); still raises NotImplementedError (outbox is fire-and-forget).
  • client.py docs: fixed the stale _build_dlq_cte_build_dlq_cte_stmt comment, and corrected the false _drift_entry_to_error docstring (the table does declare a CHECK + a partial unique index; Alembic simply has no check-constraint comparator).

Test additions

  • test_dlq_cte_insert_columns_match_make_dlq_table — guards the hardcoded DLQ CTE column list against drift from make_dlq_table (chose the guard-test option over a CTE rewrite).
  • test_worker_rebuilds_writer_connection_after_flush_failure — a real terminal delete_with_lease raises once against live Postgres, forcing _run_with_reconnect to rebuild the AUTOCOMMIT writer connection and drain the redelivered row.
  • test_listen_failure_falls_back_to_polling_against_real_postgres — LISTEN unavailable still delivers via polling against live Postgres.

Verification

  • Full suite: 487 passed, 100% coverage held.
  • ruff format, ruff check, ty check all clean.

Audit record updated with RESOLVED notes (the validate_schema finding is marked PARTIALLY RESOLVED — docstring fixed here, probe deferred).

🤖 Generated with Claude Code

Addresses the low-risk, no-design-surface findings from the 2026-06-14 deep
audit in one batch. No behavior change to shipped paths; the 3 behavior-change
Lows (graceful_timeout=None, validate_schema _lease_ck probe, OutboxResponse
eager validation) are intentionally left for a separate focused PR.

Code:
- OutboxProducer._emit_metric delegates to metrics._safe_emit (dedup); removed
  the now-dead module logger/import.
- OutboxPublisher.publish body type SendableMessage -> typing.Any (parity with
  broker.publish / OutboxResponse).
- OutboxBroker.request mirrors upstream BrokerUsecase.request's typed signature
  (message, queue, /, timeout) instead of (*args, **kwargs); still raises.
- client.py: fix stale `_build_dlq_cte` comment -> `_build_dlq_cte_stmt`; correct
  the false `_drift_entry_to_error` docstring (the table DOES declare a CHECK +
  unique index; alembic just has no check-constraint comparator).

Tests:
- test_dlq_cte_insert_columns_match_make_dlq_table: guards the hardcoded DLQ CTE
  column list against drift from make_dlq_table.
- test_worker_rebuilds_writer_connection_after_flush_failure: real flush failure
  forces _run_with_reconnect to rebuild the AUTOCOMMIT writer conn and recover.
- test_listen_failure_falls_back_to_polling_against_real_postgres: LISTEN
  unavailable still delivers via polling against live Postgres.

Full suite 487 passed, 100% coverage held; ruff + ty clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lesnik512 lesnik512 merged commit 1e508e0 into main Jun 14, 2026
3 checks passed
@lesnik512 lesnik512 deleted the chore/audit-low-safe-cluster branch June 14, 2026 08:12
lesnik512 added a commit that referenced this pull request Jun 14, 2026
Completes the 2026-06-14 deep-audit Low cluster (the safe 7 shipped in #82).
Each changes observable behavior, hence a separate reviewed PR.

- graceful_timeout=None no longer hangs drain: subscriber stop() clamps None to
  a finite fallback (_DEFAULT_DRAIN_TIMEOUT_SECONDS=15.0) so move_on_after always
  has a real deadline; None stays unbounded for ping(). One wedged handler can no
  longer make stop() (pod shutdown) hang forever.
- validate_schema() now probes pg_constraint for <table>_lease_ck via
  _validate_check_constraints_sync (alembic has no check-constraint comparator),
  flagging a missing or drifted lease-pairing CHECK. NOTE: new failure mode — a DB
  lacking the CHECK that passed before will now fail validation (intended).
- OutboxResponse.__init__ runs the activate-mutex + tz-aware checks eagerly, so a
  misconfigured response raises at the return site instead of masquerading as a
  handler failure and exhausting the inbound row's retry budget. OutboxPublishCommand
  still re-checks on as_publish_command() (authoritative source).

Docs: CLAUDE.md (validate_schema CHECK probe; graceful_timeout None clamp),
architecture/drain.md, audit record marked resolved.

Tests: +5 (2 OutboxResponse unit, graceful_timeout clamp, 2 lease_ck validate).
Full suite 492 passed, 100% coverage held; ruff + ty clean.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant