From a7e5bc030fa5efab8836410fbdc6243d0dc54da1 Mon Sep 17 00:00:00 2001 From: Knacky Date: Wed, 13 May 2026 09:29:27 +0200 Subject: [PATCH] =?UTF-8?q?fix(m5):=20scenario=20reorder=20500=20=E2=80=94?= =?UTF-8?q?=20wrong=20pg=5Fadvisory=5Fxact=5Flock=20overload?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Editing a scenario and saving (with or without changes) returned 500: function pg_advisory_xact_lock(smallint, bigint) does not exist Postgres only ships (int4, int4) and (bigint) variants. The two-arg call passed `m = hash(uuid) & 0xFFFFFFFF` which can reach 2^32-1, so psycopg promoted it to bigint and no overload matched. Switched to the single-arg bigint form. While there, replaced Python's built-in hash() with hashlib.blake2b(...) — the built-in is randomised per process via PYTHONHASHSEED, so gunicorn workers were computing different lock keys for the same scenario and the lock wasn't actually serialising across workers. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 4 ++++ backend/app/services/scenario_templates.py | 13 ++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 661d8d5..dd72e11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,10 @@ All notable changes to this project will be documented here. Format: [Keep a Cha - **`LogRecord` key collision**: `log.info(..., extra={"name": ...})` raises `KeyError("Attempt to overwrite 'name' in LogRecord")` because `name` is reserved by Python's stdlib logging. Renamed to `template_name`. - **React `currentTarget` null in deferred state updaters**: `onChange={(e) => setX((prev) => ({ ...prev, q: e.currentTarget.value }))}` blanked the page on the first user input because `currentTarget` is cleared after the listener bubble ends, before React invokes the updater. Switched all M5 handlers to `e.target.value`, which persists on the synthetic event. +### Fixed (post-M5 — scenario reorder 500 + cross-worker lock correctness) +- **`PUT /scenario-templates/{id}/tests` returned 500** (`backend/app/services/scenario_templates.py:218`): the two-argument form `pg_advisory_xact_lock(:n, :m)` failed with `function pg_advisory_xact_lock(smallint, bigint) does not exist`. Postgres only provides `(int4, int4)` and `(bigint)` overloads — psycopg promoted `m = hash(uuid) & 0xFFFFFFFF` (up to 2^32-1) to bigint and there's no matching overload. Switched to the single-argument bigint form with `CAST(:key AS bigint)`. +- **Cross-worker lock was a no-op** (same site): Python's built-in `hash()` is randomised per process via `PYTHONHASHSEED`, so each gunicorn worker computed a different key for the same `scenario_id`, and concurrent reorders on different workers acquired independent locks — defeating the serialisation. Replaced with `blake2b(scenario_id.bytes, digest_size=8)` interpreted as a signed int64. Stable, deterministic, fits in `bigint`. + ### Fixed (post-M5 UI — modal layout for the test-template editor) - **Modal box capped its width at `max-w-2xl` and had no vertical scroll** (`frontend/src/components/ui/Modal.tsx`): opening **+ New test** rendered the 15-column MITRE matrix inside a 672 px frame with no height cap, so the matrix spilled to the right and the form bottom dropped below the viewport — buttons unreachable, no scroll. Added a `size` prop (default `2xl` for back-compat), `max-h-[calc(100vh-2rem)]` + `flex flex-col` on the dialog, and an inner `min-w-0 flex-1 overflow-y-auto` body so the header stays pinned while the form scrolls inside the modal. - **MITRE matrix overflow-x failed to scroll inside the modal body** (`frontend/src/components/MitreTagPicker.tsx`): `overflow-x-auto` sat directly on the grid element, but the grid's intrinsic min-width (`15 × minmax(7rem, …)` = 1680 px) prevented it from shrinking below its content, so the grid spilled outside its parent instead of scrolling. Wrapped the grid in a dedicated `overflow-x-auto rounded min-w-0 w-full` scroller and added `min-w-0` to the picker root so the constraint propagates from the modal body. The grid now scrolls horizontally inside the modal. diff --git a/backend/app/services/scenario_templates.py b/backend/app/services/scenario_templates.py index d137545..258a023 100644 --- a/backend/app/services/scenario_templates.py +++ b/backend/app/services/scenario_templates.py @@ -12,6 +12,7 @@ The same test_template may legitimately appear multiple times in a scenario from __future__ import annotations +import hashlib import uuid from dataclasses import dataclass from datetime import datetime, timezone @@ -217,10 +218,16 @@ def set_scenario_tests( """ with session_scope() as s: # Lock keyed on the scenario UUID — different scenarios don't block - # each other. Two-int form: high-32 = constant, low-32 = hash of UUID. + # each other. Single bigint form so we don't have to juggle int32 + # signed ranges. blake2b is used instead of Python's built-in hash() + # because the latter is randomised per-process (PYTHONHASHSEED), so + # two gunicorn workers would compute different keys for the same + # scenario and the lock wouldn't serialise across them. + digest = hashlib.blake2b(scenario_id.bytes, digest_size=8).digest() + lock_key = int.from_bytes(digest, "big", signed=True) s.execute( - text("SELECT pg_advisory_xact_lock(:n, :m)"), - {"n": 0x5C3, "m": hash(scenario_id) & 0xFFFFFFFF}, + text("SELECT pg_advisory_xact_lock(CAST(:key AS bigint))"), + {"key": lock_key}, ) sc = s.get(ScenarioTemplate, scenario_id) if sc is None or sc.deleted_at is not None: