Skip to content

Keep audit overflow observation tests correct across minute boundaries - #391

Merged
samovers merged 2 commits into
mainfrom
delivery/audit-overflow-clock-tests
Sep 14, 2026
Merged

samovers merged 2 commits into
mainfrom
delivery/audit-overflow-clock-tests

Conversation

@samovers

@samovers samovers commented Sep 14, 2026

Copy link
Copy Markdown
Owner

Merged and verified: PR #391 merged at eea160152077523c97d877af851b1a2d511c075a on 2026-09-14T17:01:08Z through GitHub's expected-head merge after the original final packet and later exact same-task authorization. Delivery #390 is closed. The fresh pre-merge receipt recheck passed; new exact-head review 5200536975 reported zero findings. No runtime or scope change was added. Next: resume PR #388 with its own fresh applicable evidence.

The accepted final packet below is preserved as the pre-merge record.


Closes #390. Separate test-maintenance prerequisite for #387 / PR #388; part of #180.

The audit overflow test assumed its setup bucket stayed current while two older buckets were closed. The final observation now uses PostgreSQL time immediately before and after the call: require no rows before expiry, the exact remaining row after expiry, and only those two exact outcomes if the interval spans expiry. Stable active-bucket exclusion stays strict.

Implemented head: c430cf8140b43d36f041bb92382c7cfdd1481eeb. All applicable technical and evidence gates passed. Final task-user acceptance is pending; this PR remains unmerged.

Final scope and authority

Primary boundary: test evidence for database-owned security-audit overflow observation and closure. The only changed file is kernel/tests/test_postgresql_audit_operations.py, 22 additions / 2 deletions. There is no Phase A deviation. The implementation exactly matches the approved proposed patch.

PostgreSQL retains time, selection, ordering, closure, attribution and role authority. The existing fixture owns fictional setup and cleanup in an isolated disposable database; observation and closure use the existing audit-control role. The test owns result assertions only. The task user retains semantic and final merge authority.

  • T01: all four fixture rows, both exact oldest-first observations, both closures and the existing wrong-role refusal remain.
  • T02: two database clock reads bracket only the final observation. The three time cases above require exact lists; reversed samples, wrong identities, duplicates and extra rows fail with timing diagnostics.
  • T03: only the existing final expectation and its clock reads change. No test, helper, fixture, inventory entry, SQL mutation, deadline, repeated probe, retry or skip is added.

Runtime, roles, migrations, clocks, quotas, closure and publication policy are unchanged. No new privilege, audit restart, deployment, release or production-readiness claim is introduced. The removed two-probe design has not returned. PR #388 stays unchanged at 524c31d295094c54f648e6d72941e6ff3709c9c2; its failed source run 34828888000 stays failed and supplies no evidence here.

The risk is weakening the expectation; exact results for each measured interval contain it. A crossing or already-expired execution does not independently establish active exclusion. The local focused run measured stable-current behavior; it did not force the other branches. This remains provisional development maintenance. A changed boundary, authority, effect, invariant, named PR or production posture requires a new decision.

Completed checks and evidence

  • Existing audit operations module: 9 passed in 0.98s, no failures or skips; CPython 3.12.13, psycopg 3.3.4, PostgreSQL 17.10. The measured stable-current bracket was 0.7ms, result []. A temporary external return-only profiler observed existing locals, replaced no calls or SQL and added profiling overhead. Expired/crossing cases were source-reviewed; no separate executed-branch claim is made.
  • Mandatory package, architecture and temporal checks: PASS, zero failures, 8.79s. Ruff and whitespace passed. Source comparison confirms only one existing test function changed. Inventory remains 4,483 entries, raw SHA256 c7595e8f475ae25c5e7268a45344e31787b01c7d108244df27a0455c3942e157.
  • Owned local database fixture, roles, service, container and temporary data were removed; unrelated resources were preserved.
  • Lightweight run 34837355600: PASS.
  • Exact-head implementation review: zero Blockers, Follow-ups or Preferences. Sealed report below, SHA256 65d63702c99daafe5f8254478cb6f9b46c0a99862fcd11daf32de2c9bcd61c2c.
  • Immutable admission 5663115510 remains bound to this head. Source 34837594371, attempt 1: PASS. Both baselines passed 4,483 / 4,483, zero failures/skips, one exact inventoried Starlette deprecation warning each. Baseline durations were 15m20s and 15m21s. Clean-run equivalence and both AMD64/ARM64 native jobs passed.
  • Publisher 34840469949, attempt 1: PASS. The existing read-only consumer verified exact source/publisher identities, live admission, all four source and five published artifact bindings, pinned environment, baseline inventories and recomputed equivalence. Execution merge 0f41a4b89eb6bc90c3eeac426811ca37a58de4f5 has tree a1555242ae2e9f1f90bfbf17a52022a1667def77, equal to the reviewed head tree. Trusted base/policy remains 9d7541d96bc708e9270b986927d7f4b8a035454f.
  • Final receipt artifact 10346046242, ZIP SHA256 2d79bbec31170105644d39c625ee97cb9d1f0fc76ac998290fc99f4acdb186b4; canonical receipt JSON SHA256 628aea6269deb819b0ae50267b21c1021b5dcd8e70514e7062108139331bff0c. Verification report SHA256 bf0bf97359e6f244352a62d47956737439a91a3b17b573a9daff2a3ff10796ff, capture 2026-09-14 12:09:00–12:09:32 UTC.

Native archive IDs/digests were verified through the live API and sealed receipt; those archives were not independently downloaded or re-executed locally. Live state is observed at capture time. No workflow was rerun. The watcher lost an API connection; a fresh read confirmed the original source's successful completion.

Review, simplicity and approval provenance

The automatic ready-for-review workflow 34842078120 succeeded on this head and posted no new findings. Its full model output is hidden; this is workflow status and feedback evidence, not a substitute for the inspected content review.

No unresolved finding is carried by this PR. Historical Phase A comment 5663248770 reviews the empty design head 4e5a53c; it is not implementation clearance. The implementation review below is the applicable content review.

EXC-001: one database-owned eligibility path. EXC-002: no duplicate authority, state or validation helper. EXC-003: T01–T03 trace directly to preserved controls and the final expectation. EXC-004: the stale unconditional empty-result assertion is removed. EXC-005: no abstraction added. EXC-006: the direct clock bracket is the smallest credible correction; accepting either outcome unconditionally weakens stable-case evidence, while scheduling or extra probes add unnecessary machinery.

Decision OFARM2-AUDIT-OVERFLOW-CLOCK-TEST-001 version 1 names PR #391. Original same-task card msg_0d813f8071772f9a016aa7d0abd05887d29fdfbe7302ea5bd9 (session line 14914, 2026-09-14T10:47:17.317Z) precedes the exact task-user approval msg_01a09f9d-e8b3-7e12-b1e7-7c22b931fd47 (line 14921, 2026-09-14T11:12:01.203Z), in task 01a07cc8-4157-7b33-a0ca-becb772e0e8b. Both original messages were directly retrieved by root and reviewer; hashes and order were rechecked after publication. No later cancellation or merge authorization was present. The original Phase A body is retained in local evidence; these references are navigation, not substitute authority.

Next: present the complete final packet in the same task and yield unmerged. Only a later exact-head task-user authorization permits the normal expected-head merge after fresh live checks.

Sealed exact-head implementation review (historical pre-admission snapshot, preserved verbatim)

PR #391 — exact-head implementation review

Reviewer: /root/audit_clock_publication_preflight, 2026-09-14.
Delivery: #390.
Pull request: #391.

Verdict: zero Blockers, zero new Follow-ups, zero Preferences. The approved
one-test correction is implemented without scope expansion. This is the one
exact-head content review of this implementation; it does not restart the
security audit or supply hosted evidence, publication custody or human merge
authority.

Identity and same-task authority

  • Reviewed head: c430cf8140b43d36f041bb92382c7cfdd1481eeb.
  • Reviewed tree: a1555242ae2e9f1f90bfbf17a52022a1667def77.
  • Base: 9d7541d96bc708e9270b986927d7f4b8a035454f.
  • Changed path: kernel/tests/test_postgresql_audit_operations.py only,
    22 additions / 2 deletions.
  • Source SHA-256:
    1f987e5975ebd6ab1f96f318fd0e0bf116a12301336c334736d2424d9c6c2cc3.
  • Approved proposed diff SHA-256:
    2889d6a895e33f1a45a1947d39dfd10ba57d825cf3c659faaec819dcd316ff20.

Direct live GitHub metadata and independent local Git inspection agree on the
full head, base and one-file delta. The pull request was OPEN/DRAFT at review;
the worktree was clean. Applying the proposed patch to the base in memory
produces byte-identical implemented source. AST comparison confirms only the
existing final-observation test function changes; all other top-level nodes
and test names are identical.

I directly read the original same-task session messages, then compared their
IDs, roles, bodies and hashes with authority-retrieval.json:

  • Card: line 14914,
    msg_0d813f8071772f9a016aa7d0abd05887d29fdfbe7302ea5bd9,
    body SHA-256
    9801560a914c7381e81a9bdde47d0d04983264ecbfa0ae939b9d80a8d459d12d.
  • Later exact task-user approval: line 14921,
    msg_01a09f9d-e8b3-7e12-b1e7-7c22b931fd47,
    body SHA-256
    8e218b9d40b16a14450a9efe39dbea3728c21adcdb6e465a21f31c2df2ddbcc7.

The card names existing PR #391 and decision
OFARM2-AUDIT-OVERFLOW-CLOCK-TEST-001 version 1. The later user's entire visible
message is its exact approval form, with a terminal newline. There were no
later original user messages in the session at retrieval. This authorizes
in-boundary implementation and checks; it is not final merge authorization.

Approved invariant review

The primary boundary remains test evidence for database-owned audit overflow
observation and closure
. PostgreSQL retains selection, time, ordering,
closure and role authority. No runtime, migration, fixture, grant, clock,
publication-policy or PR #388 source change is included.

  • T01: All four seeded rows, both exact oldest-first observations and both
    closures are unchanged. The separate wrong-role refusal test is unchanged.
    These controls passed in the recorded focused module execution.
  • T02: The implemented final call is bracketed by PostgreSQL clock reads.
    The ordered-endpoint assertion rejects reversed samples. Before expiry it
    requires exactly no rows; after expiry it requires exactly the remaining
    seeded identity; across expiry it permits only those two exact results.
    Wrong identities, duplicates and extra rows fail every branch. The bucket
    and both clock samples appear in assertion diagnostics. This matches the
    existing function's minute-bucket eligibility comparison without replacing
    database time or selection authority.
  • T03: Exactly the approved final expectation and its two clock reads
    change. There is no added test, helper, fixture, inventory entry, retry, skip,
    deadline, scheduled transition, repeated probe or SQL mutation. The removed
    two-probe design has not returned. No unrelated H1 or audit-runtime work is
    included.

No new ambiguity requiring a decision amendment was found. In particular,
crossing and already-expired executions do not independently establish active
exclusion; that stated limit is preserved. The local run did measure the
stable-current branch. Arbitrary backward clock manipulation is not a claimed
capability of this test correction.

Evidence checked and limits

I inspected the focused runner, test log, recorded values, package log,
environment, scope and cleanup records. The temporary runner only observes the
target function's return locals through Python profiling and restores the
original profile function. It replaces no production/test call, SQL or clock;
profiling adds some execution overhead. It invokes the existing full module
once, with no retry or conditional skip.

  • Focused PostgreSQL module: 9 passed in 0.98 seconds, comprising seven
    test functions, one parameterized into three cases. No failed or skipped
    case is reported.
  • Measured branch: stable current. The final call was bracketed from
    2026-09-14 11:15:24.339673+00:00 to
    2026-09-14 11:15:24.340373+00:00 (0.7 ms), before the seeded bucket's
    11:16:00+00:00 expiry, and returned [].
  • The expiry-crossing and already-expired branches were reviewed against
    source semantics; this local run did not execute them. No forced crossing
    or faulty SQL was introduced to claim otherwise.
  • Mandatory package/architecture/temporal check: recorded PASS, zero
    failures, 8.79 seconds
    . Its detailed log agrees with the result record.
  • Root reported Ruff and whitespace PASS. This reviewer did not repeat those
    checks or the PostgreSQL execution.
  • Independent source/AST/proposal comparison: PASS. The inventory is
    byte-identical to the base, with 4,483 entries, SHA-256
    c7595e8f475ae25c5e7268a45344e31787b01c7d108244df27a0455c3942e157.
    AGENTS.md is also byte-identical to the base.
  • Recorded local environment: CPython 3.12.13 and owned disposable PostgreSQL
    17.10. The cleanup record reports the fixture service, fixture roles, owned
    container and temporary data removed; unrelated resources untouched. I did
    not perform another database operation to recheck removed resources.

This is local evidence, not proof of the hosted Linux hash-locked environment.
No expensive workflow, baseline admission or publication check was performed
by this reviewer. PR #388's failed hosted run remains failed and cannot clear
this head.

Code excellence and next step

EXC-001 retains one database-owned eligibility path. EXC-002 adds no duplicate
authority, helper or fixture state. EXC-003 directly connects T01–T03 to the
unchanged controls and three-case expectation. EXC-004 removes the stale
unconditional empty-result assertion. EXC-005 adds no abstraction. EXC-006 uses
the approved direct classification; accepting either result unconditionally
would weaken stable-case evidence, while the discarded extra apparatus would
add an unapproved every-run coverage requirement.

Next: root may perform existing exact-head admission coordination for this
reviewed head, obtain the required fresh hosted evidence and publication
receipt, then prepare the unmerged final packet. Any new commit requires a
new exact-head review under the existing workflow.

Delivery #390: publish the proposed one-test clock assertion correction in the draft PR description. This commit intentionally changes no files and does not implement the proposal.
Keep the existing ordering and closure controls while comparing the final result with PostgreSQL time before and after observation. Permit only the exact result justified by the measured interval, preserving active-bucket exclusion when observed. Delivery #390; approved decision OFARM2-AUDIT-OVERFLOW-CLOCK-TEST-001 version 1.
@samovers

Copy link
Copy Markdown
Owner Author

PR #391 implementation review is complete at head c430cf8 with zero Blockers, Follow-ups or Preferences. The one-test +22/-2 change exactly matches approved decision OFARM2-AUDIT-OVERFLOW-CLOCK-TEST-001 version1. Sealed review SHA25665d63702c99daafe5f8254478cb6f9b46c0a99862fcd11daf32de2c9bcd61c2c is in the PR description.

Focused module9 passed; mandatory package/architecture/temporal, Ruff/whitespace and lightweight34837355600 passed. Inventory4483 is unchanged. Admit fresh required hosted evidence for this head only. This technical trigger supplies no human approval, final acceptance, deployment or merge authority. PR388's failed evidence is not reused.

OFARM2_BASELINE_ADMISSION
head=c430cf8140b43d36f041bb92382c7cfdd1481eeb
blockers=0

Copy link
Copy Markdown
Owner Author

Proposal review — no blockers in the proposed fix

Reviewed draft head: 4e5a53cb752a8d36cbdab5a88c75191a8f4fae3a
Reviewed base: 9d7541d96bc708e9270b986927d7f4b8a035454f
Proposed diff SHA-256: 2889d6a895e33f1a45a1947d39dfd10ba57d825cf3c659faaec819dcd316ff20

Posting-time scope notice: This is the previously completed review of the Phase A proposal. At that reviewed head, the draft had zero changed files and the proposed patch was in its description. The PR has since advanced to implementation head c430cf8140b43d36f041bb92382c7cfdd1481eeb. This comment does not transfer proposal-review clearance to that implementation head, replace its implementation review, provide baseline admission, or authorize merge.

Verdict

No blockers in the proposed fix. At the reviewed proposal stage, the recommendation was to proceed to the required implementation decision—not merge.

Technical assessment

The correction addresses a defect in the test's expectation. The existing test samples current_bucket, closes two older buckets, then unconditionally expects no remaining closeable bucket. The database observer instead evaluates eligibility using pg_catalog.clock_timestamp() during observation. Once the minute advances, returning the originally current bucket is legitimate.

Reviewed source:

The proposed assertions correctly distinguish the three cases:

Measured observation interval Required result
Finishes before bucket expiry Exactly no rows
Starts at or after expiry Exactly the remaining seeded row
Straddles expiry Either of those two exact results

The comparison operators handle equality correctly: a call starting exactly at expiry must return the seeded row; a surrounding interval ending exactly at expiry may legitimately contain an observation from either side of the boundary. This matches the SQL's strict comparison against the current 60-second bucket, under ordinary non-reversing clock progression.

The test is not being broadly relaxed. Stable before-expiry and after-expiry executions still demand one exact answer. All branches reject wrong identities, duplicates, and additional rows. The four-row setup, both oldest-first observations, both closure calls, and the separate wrong-role 42501 test remain untouched. No database function, privilege, fixture, or runtime behavior changes.

The coverage limitation is accurately disclosed: a crossing or already-expired execution does not independently prove active-bucket exclusion. I would not block this bounded correction by demanding a new clock-control mechanism or an every-run active-bucket guarantee.

Verification limits

This proposal review checked the assertion logic against the existing test and database source. It did not execute the PostgreSQL audit-operations module, run hosted baselines, or independently reproduce the original CI failure. Any assertion-only logic checks are not PostgreSQL integration evidence. Minute rollover is a supported explanation for the original failure, not an instrumented diagnosis.

Recommendation at the reviewed proposal stage

Proceed with the required implementation decision, apply the proposed one-test change, and verify the resulting implementation head with the existing isolated PostgreSQL fixture and applicable checks. Preserve PR #388's failed evidence as failed; this proposal review clears none of its gates. No redesign or additional scope was warranted by this review.

Implementation and subsequent evidence reported after 4e5a53cb752a8d36cbdab5a88c75191a8f4fae3a remain outside this historical proposal-review verdict.

@samovers
samovers marked this pull request as ready for review September 14, 2026 12:11

@samovers samovers left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #391 — independent implementation review at head c430cf8

Reviewed head: c430cf8140b43d36f041bb92382c7cfdd1481eeb
Reviewed base / trusted policy: 9d7541d96bc708e9270b986927d7f4b8a035454f (refs/heads/main at review time)
Delivery: #390 · Decision: OFARM2-AUDIT-OVERFLOW-CLOCK-TEST-001 version 1
Changed files: kernel/tests/test_postgresql_audit_operations.py, +22 / −2, nothing else.

Position of this review

This is an independent third pass at the same head. Two reviews already exist on this PR:

  1. the Phase A proposal review of the empty design head 4e5a53c (comment 5663248770), which explicitly
    disclaims implementation clearance; and
  2. the sealed exact-head implementation review carried in the PR description
    (SHA256 65d63702c99daafe5f8254478cb6f9b46c0a99862fcd11daf32de2c9bcd61c2c), zero findings.

I read both after measuring, so the results below are not downstream of them. What this pass adds over
them is execution: the prior reviews state that the expired and crossing branches were source-reviewed
only
and that minute rollover is "a supported explanation for the original failure, not an instrumented
diagnosis". I reproduced the defect, executed all three branches, and mutation-tested the surviving
assertions against a live PostgreSQL 17.10. What it merely confirms is the scope, patch-identity and
gate results.

Governance note, stated plainly: every review on this PR so far, including this one, is authored under
samovers. GitHub refuses APPROVE/REQUEST_CHANGES from the PR author (422), so all of these are COMMENT
reviews by the same account. If any gate anywhere requires independent review, this PR does not meet it,
regardless of how many passes are recorded.

Verdict

Blockers: 0. Follow-ups: 0. Preferences: 0.

The change is exactly the approved patch, it is logically correct against the database's real eligibility
rule, it fixes a defect I reproduced, and it does not blunt the assertion in the case that actually runs in
CI. The strictness it gives up is real, bounded, disclosed by the PR, and measured below at
≈0.05 % of runs.


1. Environment (so the numbers are checkable)

Everything below was measured from an independent anonymous clone, not from the workspace tree.

git clone https://github.com/samovers/OFARM2.git
git fetch origin refs/pull/391/head:pr391
git merge-base origin/main pr391  -> 9d7541d96bc708e9270b986927d7f4b8a035454f
git diff --numstat 9d7541d pr391  -> 22  2  kernel/tests/test_postgresql_audit_operations.py
git ls-remote origin refs/pull/388/head -> 524c31d295094c54f648e6d72941e6ff3709c9c2   (unchanged, as claimed)
  • PostgreSQL 17.10, built from source at tag REL_17_10; server_version = 17.10,
    server_version_num = 170010.
  • CPython 3.12.13, built from tag v3.12.13; pinned wheels installed with --require-hashes from
    requirements-review-baseline.lock and requirements-review-tools.lock; ruff 0.15.5.
  • One unavoidable deviation: deployment/postgresql/version_policy.py pins the exact string
    "17.10 (Debian 17.10-1.pgdg13+1)"; a source build reports "17.10". I patched that one string in a
    throwaway copy of the tree
    and nothing else (diff -r confirms a single differing file).
    SUPPORTED_POSTGRESQL_SERVER_VERSION_NUM = 170010 matched unmodified. The review clone used for the
    conformance gates below is unpatched.

2. The implementation is byte-identical to the approved patch

The PR claims "The implementation exactly matches the approved proposed patch." Verified, not taken on trust:

sha256(plans/audit-overflow-clock-evidence/minimal-final-observation.diff)
  = 2889d6a895e33f1a45a1947d39dfd10ba57d825cf3c659faaec819dcd316ff20   (matches the Phase A review's stated digest)

git show 9d7541d:kernel/tests/test_postgresql_audit_operations.py > base.py
patch base.py < minimal-final-observation.diff                      -> PATCH_OK
cmp base.py  <(git show pr391:kernel/tests/test_postgresql_audit_operations.py)
  -> identical

Applying the approved Phase A patch to the base file reproduces the head file exactly. The only difference
between the approved diff and the PR diff is the @@ hunk-header function-context suffix that git adds.
No Phase A deviation.

3. What the database actually decides (measured, not read)

ofarm_security.observe_next_closeable_overflow_bucket() (migration 0002_hmac_v2_operations.sql:133)
selects on:

bucket.overflow_started_at IS NOT NULL
AND bucket.bucket_start < date_bin('60 seconds', clock_timestamp(), '2000-01-01 00:00:00+00')

The test reasons in terms of expires_at = current_bucket + 1 minute and uses < / >= against it. Those
are two different spellings, so I measured the equivalence rather than asserting it. Reproducing the exact
predicate with an explicit observation time T against a bucket-aligned bucket_start:

T eligible?
bucket_start f
bucket_start + 59.999999 s f
bucket_start + 60 s (exactly expires_at) t
bucket_start + 60.000001 s t

So eligibility is exactly T >= bucket_start + 60 s, and the test's boundary operators are right in both
directions:

  • observed_after < expires_at ⟹ the observation ran strictly before expiry ⟹ must be [];
  • observed_before >= expires_at ⟹ the observation ran at or after expiry ⟹ must be the seeded row;
  • otherwise the interval straddles expiry and both exact results are legitimate.

close_overflow_bucket (0001_initial.sql:1318) deletes the bucket row, and the never_overflowed
row has overflow_started_at IS NULL, so after the two closures the seeded current-bucket row is the only
possible result. expected_remaining is therefore complete, not merely plausible, and LIMIT 1 makes
"extra rows" impossible without a mutation.

4. The defect is real — reproduced, not inferred

The PR says minute rollover is a supported explanation, not an instrumented diagnosis. I instrumented it.

I re-ran the reviewed test's own sequence against the live fixture with the whole four-row seed backdated
by one minute
, which puts the database in exactly the state the real test reaches when the minute rolls
over between setup and the final observation. Same SQL, same closures, same role.

[STABLE]
  current_bucket  = 2026-09-14 15:49:00+00:00
  expires_at      = 2026-09-14 15:50:00+00:00
  observed_before = 2026-09-14 15:49:18.025400+00:00
  observed_after  = 2026-09-14 15:49:18.025678+00:00   (bracket 0.278 ms)
  remaining       = []
  head branch     = before-expiry
  deleted assert  = PASSES

[ROLLED_OVER]
  current_bucket  = 2026-09-14 15:48:00+00:00
  expires_at      = 2026-09-14 15:49:00+00:00
  observed_before = 2026-09-14 15:49:18.046203+00:00
  observed_after  = 2026-09-14 15:49:18.046481+00:00   (bracket 0.278 ms)
  remaining       = [('REQUEST_ROUTER_BOUNDARY_V1', 'REQUEST_ROUTER', 2026-09-14 15:48:00+00:00)]
  head branch     = after-expiry
  deleted assert  = FAILS

The assertion this PR deletes fails, under a legitimate database result, with no mutation anywhere. The
assertion this PR adds passes on the same data via its second branch. That is the whole case for the change,
executed.

5. All three branches executed

The prior reviews executed only the stable branch. All three now have a run behind them:

  • before-expiry — the ordinary run, above and in the 5 focused-module runs below.
  • after-expiry — the backdated seed, above.
  • crossing — forced by inserting pg_sleep(0.6) inside the observer while leaving the eligibility
    predicate untouched, then firing the bracketed observation 0.30 s before a real minute boundary. Caught on
    the first attempt; the bracket genuinely spanned expiry and the observed result was the post-expiry row,
    which the third branch accepts:
[CROSSING PROBE] [('crossing', [('REQUEST_ROUTER_BOUNDARY_V1', 'REQUEST_ROUTER', 2026-09-14 15:49:00+00:00)])]

The pg_sleep is a deliberate deviation and I am naming it: it widens the call, it does not touch what the
call decides.

6. Does the corrected test still catch what it exists for?

A relaxed assertion is worth nothing if it stops failing on real defects. I mutated the observer through
CREATE OR REPLACE FUNCTION after migration (function body only; role check, grants, ordering tie-breakers
and return shape preserved) and ran the head assertion block against each mutant, in the stable timing
case that CI actually hits:

Mutant Head assertions
active-bucket exclusion removed entirely CAUGHT
bucket_start <bucket_start <= (closes one bucket early) CAUGHT
ORDER BY bucket_start DESC (wrong oldest-first) CAUGHT (first observation)

The failure output carries both the timing tuple and the actual rows, so T02's "fail with timing
diagnostics" is true as written:

(datetime.datetime(2026, 9, 14, 16, 0, ...), ..., datetime.datetime(2026, 9, 14, 16, 0, 49, 78868, ...))
assert [('REQUEST_RO...='Etc/UTC')))] == []
  Left contains one more item: ('REQUEST_ROUTER_BOUNDARY_V1', 'REQUEST_ROUTER', 2026-09-14 16:00:00+00:00)

7. The cost of the relaxation, quantified

The PR discloses that "a crossing or already-expired execution does not independently establish active
exclusion". That is correct, and here is what it costs, measured rather than described.

Both eligibility mutants above survive when the run lands after expiry:

[MUTANTS, run landing after expiry]
  {'no_active_exclusion': 'SURVIVES (branch=after-expiry)', 'off_by_one_<=': 'SURVIVES (branch=after-expiry)'}

How often does a run land there? The exposure window is the elapsed time between the date_bin sample that
defines current_bucket and the final bracketed observation. Five measurements:

[EXPOSURE] sample->final-observation ms: ['26.5', '23.8', '21.8', '29.7', '25.5']  => 0.0495 % of a 60 s minute (max)

So roughly 1 run in 2,000 loses the active-exclusion check, and in that run the test still demands one
exact result — it does not pass vacuously. Before the fix, that same 1-in-2,000 was a false failure.
Trading a 0.05 % false failure for a 0.05 % blind spot on one sub-assertion is a clear improvement, and the
strictness-preserving alternatives (a second probe, a controlled clock, a re-seeded active bucket) are
exactly what T03 and EXC-006 excluded. Reviewers do not own scope expansion, so this is not a finding —
it is the disclosed limit, now with a number on it.

8. The repository's own gates, re-run here

Run on the unpatched review clone at c430cf8 with the exact interpreter:

$ /tmp/venv13/bin/python conformance/ofarm_pkg_contract_check.py
TEMPORAL CANDIDATE PASS: CONFORMANT_CLASSIFIED
TEMPORAL DECISION LOG PASS
rewrite architecture constraints: PASS
RESULT: PASS (0 failures)

$ ruff 0.15.5 check kernel/tests/test_postgresql_audit_operations.py
All checks passed!

Focused module, five consecutive runs against live PostgreSQL 17.10:

9 passed in 1.19s / 0.96s / 0.93s / 0.94s / 1.03s

Inventory claims, checked independently:

sha256(conformance/review_baseline_test_inventory.json) = c7595e8f475ae25c5e7268a45344e31787b01c7d108244df27a0455c3942e157
entryCount = 4483
pytest --collect-only kernel/tests/  -> 4483 tests collected

The digest and count match the PR body exactly, and collection equals entryCount, so the inventory is not
stale at this head.

9. Checked and decided were not findings

So the next reviewer does not re-spend this:

  • The earlier two observations were not bracketed. Correct as-is. oldest and newer are 3 and 2
    minutes old and ORDER BY bucket_start LIMIT 1 returns them first regardless of whether the seeded
    current bucket has become eligible. Only the final observation was ever exposed. Verified by running the
    backdated case with the first two assertions left strict — they still hold.
  • Other tests in the module sharing the same assumption. There are none.
    date_bin/current_bucket appear only in this test; the other clock_timestamp() uses
    (_insert_historical_v1, the access-clock mutex test) carry no minute-boundary expectation.
  • assert observed_before <= observed_after adds a new failure mode. True in principle —
    clock_timestamp() is wall clock and an NTP step backwards would trip it. The approved decision
    explicitly requires rejecting reversed samples, and a backwards step would also invalidate the branch
    logic, so failing loudly is the right behaviour, not a defect.
  • remaining in ([], expected_remaining) is a tuple membership test. Intentional and correct; in
    compares by equality, so it accepts exactly the two lists and nothing else.
  • timing omits remaining. Not a gap — pytest's rewritten assertion prints the actual rows alongside
    the message (§6).
  • ruff format --check would reformat this file. It would also reformat the base version of the file
    and 163 other files in kernel/; the repository gates on ruff check, not ruff format. Pre-existing,
    not introduced here.
  • EXC-004. The stale unconditional assertion is deleted, not left behind next to the new one.
  • Scope. One file, one existing test function, no fixture/helper/SQL/grant/runtime change; the
    withdrawn two-probe design has not returned. AGENTS.md trust-boundary discipline holds.

10. What I could not check

  • Hosted evidence. The source run 34837594371, publisher 34840469949, receipt artifact 10346046242
    and their digests are stated in the PR body; I did not download or re-execute any of them, and a local
    source build says nothing about publication custody or native release identity. Those claims stand on the
    PR's own capture, not on this review.
  • The original CI failure. I reproduced the mechanism on demand; I did not obtain the original failing
    run and confirm that this mechanism, rather than something else, produced it.
  • Merge authority. Nothing here supplies it. This review is a technical pass at one head.

11. What my method made easier than production

  • PostgreSQL 17.10 from source reports server_version "17.10", so I patched that one pinned string in a
    throwaway tree. The hosted baselines run the real Debian build; my run therefore exercised the version gate
    in a weakened form.
  • My container is fast (bracket 0.278 ms, exposure ~25 ms). A slower or loaded CI runner widens the exposure
    window and therefore the blind spot in §7 — the 0.05 % figure is a floor for my machine, not a bound for CI.
  • The backdated seed and the pg_sleep observer are simulations of timing, not real timing. They put the
    database in the right state and force the right branches, but they prove the assertions are correct for
    those states — not that CI will ever reach them.
  • Mutants were installed with CREATE OR REPLACE after migration, bypassing the migration digest and
    structural-verifier machinery. That tests the observer's logic, not the repository's ability to prevent
    such a change reaching a real database.
  • All probe code lived in a throwaway copy of the tree (kernel/tests/test_zz_review_probes.py); the
    conformance, ruff and inventory numbers in §8 come from the clean unmodified clone.

@samovers
samovers merged commit eea1601 into main Sep 14, 2026
4 checks passed
samovers pushed a commit that referenced this pull request Sep 14, 2026
Integrate merged PR #391 from main for Delivery #387 / PR #388.
The observation diff against the new base is byte-identical to the
previously reviewed implementation; preserve its historical replay base.

Verify 42 observation cases, mandatory package/architecture/temporal
checks, Ruff and whitespace. Fresh exact-head review and admitted hosted
evidence follow this new merge head; the earlier failed source stays failed.
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.

Keep audit overflow observation tests correct across minute boundaries

1 participant