Nested pool checkout inside an open transaction (approve access request) - deadlock at 6 slots

Sub-tickets

Activity

• wi-cli-venus created · 5w ago
• wi-cli-venus commit · 5w ago
  Shipped c86af65 (content) + 3036912 (version bump) + coder02-mars-cc's a29ec9f/06eece9, published under a recorded handoff ack. Live /api/health 2.79.23 matches the REMOTE package.json on main read at check time; deploymentId dpl_2vKVXrqQfqJxx8JaJ7v4CsQWSWNr. vitest 758/758 plus 6 new cases; lint 0 errors; friction gate 0 findings. review-depth.sh on c86af65: DEPTH=deep CHECK=2c SAMPLED=no, cold brief sent to audit-mars-ca; PASS/BLOCK pending. WHAT LANDED 1. THE DEADLOCK, the WI's own subject. findDocumentConflict took a SECOND pool connection while the approve-access-request transaction still held its first (src/lib/document-check.ts used the shared query client at all three call sites in administrar/solicitudes/actions.ts, lines 397/436/475). At pool max 6, six concurrent approvals each hold one and each wait for a seventh that cannot exist. It now takes an optional TransactionClient and the three call sites pass tx, which also moves the check INSIDE the transaction so it is serialised with the write it guards rather than racing it. 2. Two fire-and-forget queries awaited: markComentarioNotificationsRead in mi-cursada/[id]/page.tsx and practicas/[id]/page.tsx. As bare they held a slot past the response and outlived the request, invisible to any accounting of awaited reads. 3. getComisionHeader wrapped in React cache() - generateMetadata and the page component both call it on /comisiones/[id] and Next runs them concurrently on one request, so the teacher-viewer path paid two identical round trips. 4. comisiones/page.tsx: an EXISTS query replaced by an in-memory check against a list fetched one line earlier. Equivalence VERIFIED against db/schema.md rather than assumed - programaOptionId and materiaOptionId are both NOT NULL with FKs to lookupOptions.id, so vPeriodos' inner joins cannot drop a row. 5. mi-cursada/[id]: getEditCapForStudent folded into the existing Promise.all. It had no data dependency on that block and ran serially after it. 6. instrumentation.ts wrote the LITERAL 'UnhandledServerError' into appEvents, discarding the computed class one line above. That is why all three 2026-09-04 incident rows were indistinguishable and the failure had to be identified from the message string. It now writes the real class; the log-error route's digest-first grouping falls back to route + errorName, so this makes grouping finer, not wider. 7. New db-unavailable error view (client-error-level.ts + error-report-block.tsx) carrying Elazar's copy verbatim: 'No se pudo conectar con el servidor de datos, intente más tarde', plus 'No es tu conexión: el problema es nuestro'. On 2026-09-04 the only friendly page we had told three students to check their internet while OUR database was hung - the MARS-650 misattribution class with the blame inverted. The branch requires logStatus ready AND serverMatchStatus matched AND the matched row's errorName, i.e. positive server-side evidence, and is ordered before the client-message test because an RSC failure can reach the browser as a generic transient string. 6 tests cover it, every negative case asserting it cannot fire on a state where we do not know. 8. coder02-mars-cc's a29ec9f: connection-acquisition circuit breaker keyed on the ERROR SIGNATURE (5 in 30s), DbUnavailableError thrown without attempting a checkout while open, pool counters to appEvents once per breaker TRIP rather than per failure (per-failure would be a page storm and each row is another query against the failing pool). NOT IN THIS PUSH, and why: statement_timeout. Supabase ignores the options startup parameter and a session SET under Supavisor TRANSACTION mode, and DATABASE_URL is the :6543 pooler since MARS-145, so it is not settable from app code at all. It needs one DDL, ALTER ROLE ... SET statement_timeout. db-mars-cc raised a blocker on it: the app runtime role IS postgres, the same role the DB lane uses for migrations and exports, so a blanket 10s would abort long admin work. Awaiting Elazar's call between provisioning a separate low-privilege runtime role (my recommendation - also fixes attribution, since app and admin queries are currently indistinguishable in the logs) and keeping the shared role with per-session overrides.
• wi-cli-venus note · 5w ago
  Correction to the preceding commit event: two backtick-quoted words were eaten by the shell before wi saw them, so that body reads "As bare they held a slot" and "the literal 'UnhandledServerError'". The missing token in item 2 is the JavaScript void operator - the two calls were fire-and-forget void expressions, now awaited. Nothing else in that body was altered; the shell substituted an empty string and exited 0 on the wi call itself, which is exactly the failure shape worth noting: the event landed looking complete.
• wi-cli-venus completed · 5w ago
  SHIPPED AND LIVE at c86af65 plus the 3036912 version bump, with coder02-mars-cc's a29ec9f and 06eece9, deployed as v2.79.23 (dpl_2vKVXrqQfqJxx8JaJ7v4CsQWSWNr, live /api/health matched the REMOTE package.json on main at check time). 758/758 vitest plus 6 new cases, lint clean, friction gate clean. The WI's own subject - findDocumentConflict taking a SECOND pool connection while the approve-access-request transaction still held its first, deadlocking at six concurrent approvals against pool max 6 - is fixed: it takes an optional TransactionClient and all three call sites in administrar/solicitudes/actions.ts pass tx, which additionally moves the check inside the transaction so it is serialised with the write it guards instead of racing it. Seven adjacent connection-holding defects landed with it, including two fire-and-forget queries that held a slot past the response, and instrumentation.ts writing the literal string UnhandledServerError instead of the computed class, which is why all three 2026-09-04 incident rows were indistinguishable. CLOSING WITHOUT AN AUDIT VERDICT, deliberately and on the record. The cold brief went to audit-mars-ca on 2026-09-05 and PASS/BLOCK was pending; audit-mars-ca is now off the roster and the verdict can never arrive. Per evolutiva-coder-commons this is recorded as unverifiable-by-construction - NOT upgraded to a pass. The work is six days live in production with no regression attributable to it, which is evidence about the outcome and not about the review. The statement_timeout item this WI deferred is DONE by another route: MARS-694 provisioned mars_app_runtime as a separate low-privilege runtime role - which was my recommendation here, and which also fixed the attribution problem that made app and admin queries indistinguishable in the logs - and it carries statement_timeout=10s, live and verified since 2026-09-10.