[freeqaz]

Problem

Two postgres-platform files still carried private copies of patterns that have shared helpers:

• postgres-standing-approval-api.test.ts polled pg_blocking_pids in a hand-rolled deadline loop and wrapped app.inject by hand (host header, session headers, JSON body) for its login and its request helper.
• postgres-prepared-standing-expiry.test.ts polled pg_blocking_pids in a hand-rolled loop until two backends waited on the withdrawal transaction.

Both files were held by open PRs when the earlier helper batches ran.

Change

• Both lock-wait loops now call waitForLockWaiter (tests/helpers/postgres-lock-waits.mjs), keeping their timeouts (10 s and 900 ms), the statement filter (queryLike: "%FROM occ.namespaces%FOR UPDATE%") and the waiter count (2).
• The approval test's login and request wrapper use injectJson (tests/helpers/inject-json.mjs). The wrapper stays as a closure for the default session.
• waitForLockWaiter and injectJson get JSDoc @param types. These are the first TypeScript callers, and without the types tsc inferred the destructured options from their defaults alone and refused timeoutMs, queryLike and body. Nothing changes at runtime.

Two small behaviour differences, both the ones earlier migrations accepted: the helper polls every 20 ms instead of spinning, and it waits for at least the expected number of waiters instead of exactly that many. The helper also drops the wait_event_type='Lock' filter, which is redundant because pg_blocking_pids is non-empty only for heavyweight lock waits.

Left as is: postgres-prepared-standing-removal's blocked() (it takes an array of blockers, matches an exact count and an optional specific waiter, and returns every pid), and standing-git-conversation.ts's fixture server (it runs only with OCC_TEST_STANDING_GIT_CODEX, which CI does not set).

Verification

• tsc --build tsconfig.json tests/tsconfig.json: clean. eslint and prettier --check on the four files: clean.
• Mutants run through the postgres-platform lane restricted to the two files, on main 4936991 and on this branch. Each was killed by the same test cases on both sides:
  • lockNamespace without FOR UPDATE: killed the approval test on both sides (the lock wait times out).
  • lockNamespace with FOR NO KEY UPDATE: killed the approval test on both sides (the statement filter no longer matches).
  • lockLiveSession without FOR SHARE OF n: killed both "protected successor ... draft custody" expiry cases on both sides.
  • Registry standing-approval policy check disabled: killed the approval test on both sides, at the same 404 assertion.
  • Standing-approval removal fence generation=$4 changed to >=: survived on both sides.
• Both files passed 10/10 at 2 CPUs through the local postgres-platform lane runner on the branch, and 10/10 on main. Median wall was 13.4 s on the branch and 12.8 s on main; the spread comes from host load in branch runs 5-8.
• Internal review: approve, no must-fix or should-fix. Two of its nits are applied in b825923 (the waitForLockWaiter description moved into its JSDoc block, and injectJson's session typed as string | { cookie, origin? }). After that commit, tsc --build --force is clean and both files pass 2/2 again.

Risks

Test-only. The JSDoc additions affect type checking only.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs maintainer review before merge.

What this changes

This PR replaces duplicated request and lock-wait plumbing in two standing-approval integration tests with existing shared helpers.

Example: Two PostgreSQL operations wait behind an uncommitted withdrawal transaction.

• Before: The test repeatedly queries PostgreSQL without a polling delay until exactly two waiters appear, with a 900 ms deadline.
• After: The shared helper polls every 20 ms until at least two waiters appear, retaining the 900 ms timeout and subsequent withdrawal-denial assertions.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The patch is focused and source-consistent; supporting evidence consists of reported integration and mutation runs plus exact-head CI, whose PostgreSQL Platform lane is still running.
Proof confidence	🦐 gold shrimp (3/6)	This collaborator-authored test-maintenance PR changes no shipped behavior; its reported PostgreSQL and Fastify integration runs support helper equivalence, with no claim of live provider or deployed-runtime proof.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: Contributors must maintain duplicate request construction and lock-wait loops in two integration tests.
Reason: The collaborator-authored direction continues existing helper migrations, reduces duplication, and preserves the meaningful integration assertions.

Merge readiness

✅ Ready for maintainer review

Keep this PR open: it completes a useful test-helper migration, and the introduced diff contains no actionable defect.

Priority: P3
Reviewed head: b825923a2d809479b5c6e8cb9ddeef54cef0fd8d

Before merge

None.

Findings

None.

Agent review details

How this fits together

The PostgreSQL integration suite sends authenticated requests through the controller's Fastify app and exercises credential owners against database fixtures; shared helpers construct requests and observe PostgreSQL lock waits before assertions.

flowchart TB
  Tests[Standing approval tests] --> Inject[injectJson]
  Inject --> API[Fastify controller API]
  API --> Database[PostgreSQL]
  Tests --> Owners[Prepared credential owners]
  Owners --> Database
  Tests --> Waiter[waitForLockWaiter]
  Waiter --> Database

Loading

Technical review

Best possible solution:

Retain the focused migration to existing helpers with the current race and authorization assertions intact.

Do we have a high-confidence way to reproduce the issue?

This PR addresses duplicated test plumbing rather than a reported product failure; source inspection confirms both copies remain on fetched main.

Is this the best way to solve the issue?

Using the existing helpers is the smallest complete cleanup, and the retained callers preserve their authenticated requests and consequential race assertions.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against f8002a432b2c.

Provenance checked

• Standing-approval HTTP request construction keeps the original intent (3115679: The Credential Proxy implementation added real HTTP/IAM/state coverage for exact standing scope, withdrawal, and authorization races.)
• Standing-approval and protected-successor lock waits keeps the original intent (3115679: The tests observe actual database contention before releasing policy or withdrawal transactions so their denial assertions exercise the intended race.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

• Introduced scope: The pinned introduction changes only two test helpers and two integration tests: helper implementations receive JSDoc types, while callers replace duplicated plumbing without removing outcome assertions. (b825923a2d80)
• Preserved race assertions: The approval test retains the Namespace statement filter, 10-second timeout, concurrent account-lock check, forbidden response, and persisted-state assertions; the expiry test retains two contenders, its 900 ms timeout, withdrawal results, and absence of a recorded use. (tests/integration/postgres-standing-approval-api.test.ts:342, b825923a2d80)
• Existing helper ownership and intent: Merged test(helpers): share one bounded waiter and Postgres lock-wait probe across tests #3165 established bounded PostgreSQL lock-wait helpers and deferred files held by the Credential Proxy work; merged test(helpers): share one injectJson for session and API-key inject requests #3196 established the authenticated inject helper. (tests/helpers/postgres-lock-waits.mjs:10, 7c7ed1f86a3b)
• Current main still needs this migration: The four touched files are unchanged between the pinned introduction base and fetched main, so the shared helpers exist on main but these two callers still retain their local implementations. (f8002a432b2c)
• Verification and discussion: The full PR body reports repeated PostgreSQL lane runs, matching mutation kill sets on base and branch, and final-head reruns; discussion contains only bot status comments, with no submitted GitHub review. Exact-head checks show Static Checks passing and PostgreSQL Platform and First Agent Smoke still running. (b825923a2d80)
• Policy and inspection boundary: Read the complete root AGENTS.md; no nested test AGENTS.md, maintainer-notes directory, or pr-simplify skill was found. Applied test-audit and enterprise-testing guidance without running tests or another reviewer; final git status remained clean. Rename-following history reached an unavailable historical object, while scoped commit history and relevant blame remained available. (AGENTS.md, b825923a2d80)

Review metrics

Metric	Value	Why it matters
Change footprint	Production +0/-0; test helpers and integration tests +39/-57	The net reduction removes duplicated plumbing without adding production machinery or test cases.

Labels

Label changes:

• add P3: This is a focused test-maintenance improvement with no production behavior change.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

• P3: This is a focused test-maintenance improvement with no production behavior change.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 23, 2026, 1:39 AM ET / 05:39 UTC.