Compare commits

...

4 Commits

Author SHA1 Message Date
6034c08bad Alembic runs one transaction PER MIGRATION, not one for the whole chain
The 2026-08-21 crash at material_items printed 'Running upgrade' lines for
location_nodes and wp_files and then rolled all three back together - env.py
wrapped the entire run in a single transaction. The repair that followed
trusted those lines: material_items was hand-created, the version stamped to
head, and production ran for two days missing two tables it claimed to have.
Found 2026-08-23 when the locations import 500'd on UndefinedTable.

transaction_per_migration=True makes the log truthful: a crash keeps every
step that completed, and a stamp-to-head repair after a crash repairs ONE
migration, not an unknowable prefix of the chain. Verified: the full chain
still applies on a fresh scratch SQLite; the offline --sql render is
unchanged.

The production surgery (creating the two rolled-back tables from the offline
postgres render) is recorded in the session; no version stamp is needed
there - it is already, now truthfully, at head.

Items: BL-027's class, third finding; env.py infrastructure.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-09-01 15:35:36 -07:00
17cabbd032 Merge branch 'fix/import-row-hazards': imports reject rows, never 500
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-09-01 15:26:38 -07:00
222c0b1c29 CR-005/D6 fix - a bad CSV row rejects by line number instead of 500ing Postgres
Nick's real location list hit the production import and got 'Internal Server
Error' with no line number - BL-027's class again, three days after the
migration outage: Postgres enforces VARCHAR lengths and refuses control
bytes, SQLite shrugs at both, and the importers were only ever rehearsed on
SQLite. Reproduced both hazards locally (an over-long value and a NUL byte
import cleanly on SQLite; either 500s Postgres wholesale).

Both importers now validate per row, before any INSERT, so every dialect
answers the same way - with the line number and a reason:
- locations: control characters; names over 200; codes over 60; combined
  paths over 200 (checked where the path exists, with read-counts taken
  before the loop so a mid-loop rejection is not counted twice)
- materials: control characters; description/unit/code over 300/20/80

And the client stops lying about it: wp-list-import.js read every response
with r.json(), so a plain-text 500 threw mid-parse and surfaced as 'Could not
reach the server' while the server was answering fine. One tolerant reader
(text -> parse if it parses -> keep status) now serves import, add and patch;
a real error reads 'Import refused - HTTP 500'.

Pins: materials_check +2 (over-long and control-byte rows reject at line,
20/20), locations_check +1 (over-long name rejects at line, 59/59).

Items: CR-005, D6, BL-027 (second instance of its class; the probe-side
dialect guard it proposes is still open).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-09-01 15:26:37 -07:00
e31234beef Merge branch 'docs/bl-026-027': the outage's two lessons, backlogged
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-21 12:20:32 -07:00
6 changed files with 80 additions and 5 deletions

View File

@@ -292,7 +292,7 @@ Wave 8 adds these:
```bash ```bash
python tests/kitting_check.py # CR-009/010/012 - statuses, owner, delivery 26 checks python tests/kitting_check.py # CR-009/010/012 - statuses, owner, delivery 26 checks
python tests/kitting_notify_check.py # CR-011 - kitting mail, coalesced, gated 17 checks python tests/kitting_notify_check.py # CR-011 - kitting mail, coalesced, gated 17 checks
python tests/materials_check.py # D6 - material list, the CR-005 pattern 17 checks python tests/materials_check.py # D6 - material list, the CR-005 pattern 20 checks
python tests/mreq_check.py # CR-013 - lightweight request, end to end 19 checks python tests/mreq_check.py # CR-013 - lightweight request, end to end 19 checks
``` ```

View File

@@ -105,6 +105,18 @@
say(bits.join(''), problem); say(bits.join(''), problem);
} }
// A 500 answers plain text ("Internal Server Error"), and r.json() on that
// throws - which used to land in catch() and read as "could not reach the
// server" while the server was answering fine (found 2026-08-23, the
// production locations import). Read text, parse if it parses, keep status.
function readJson(r) {
return r.text().then(function (t) {
var j = null;
try { j = t ? JSON.parse(t) : null; } catch (e) { /* not JSON: a proxy or 500 page */ }
return { ok: r.ok, status: r.status, body: j };
});
}
function importText(dryRun) { function importText(dryRun) {
var text = (el(p + '-paste') || {}).value || ''; var text = (el(p + '-paste') || {}).value || '';
if (!text.trim()) { say('Paste some rows or choose a CSV file first.', true); return; } if (!text.trim()) { say('Paste some rows or choose a CSV file first.', true); return; }
@@ -114,7 +126,7 @@
method: 'POST', headers: { 'Content-Type': 'application/json', 'Accept': 'application/json' }, method: 'POST', headers: { 'Content-Type': 'application/json', 'Accept': 'application/json' },
body: JSON.stringify({ text: text, dry_run: !!dryRun }), body: JSON.stringify({ text: text, dry_run: !!dryRun }),
}) })
.then(function (r) { return r.json().then(function (j) { return { ok: r.ok, status: r.status, body: j }; }); }) .then(readJson)
.then(function (res) { .then(function (res) {
if (!res.ok) { if (!res.ok) {
say('⚠ Import refused — ' + esc((res.body && res.body.detail) || ('HTTP ' + res.status)), true); say('⚠ Import refused — ' + esc((res.body && res.body.detail) || ('HTTP ' + res.status)), true);
@@ -142,7 +154,7 @@
method: 'POST', headers: { 'Content-Type': 'application/json', 'Accept': 'application/json' }, method: 'POST', headers: { 'Content-Type': 'application/json', 'Accept': 'application/json' },
body: JSON.stringify(read.payload), body: JSON.stringify(read.payload),
}) })
.then(function (r) { return r.json().then(function (j) { return { ok: r.ok, status: r.status, body: j }; }); }) .then(readJson)
.then(function (res) { .then(function (res) {
if (!res.ok) { if (!res.ok) {
setAddError((res.body && res.body.detail) || ('Could not add it (HTTP ' + res.status + ')')); setAddError((res.body && res.body.detail) || ('Could not add it (HTTP ' + res.status + ')'));
@@ -161,7 +173,7 @@
method: 'PATCH', headers: { 'Content-Type': 'application/json', 'Accept': 'application/json' }, method: 'PATCH', headers: { 'Content-Type': 'application/json', 'Accept': 'application/json' },
body: JSON.stringify(patchBody), body: JSON.stringify(patchBody),
}) })
.then(function (r) { return r.json().then(function (j) { return { ok: r.ok, status: r.status, body: j }; }); }) .then(readJson)
.then(function (res) { .then(function (res) {
if (!res.ok) { if (!res.ok) {
say('⚠ ' + esc((res.body && res.body.detail) || ('HTTP ' + res.status)), true); say('⚠ ' + esc((res.body && res.body.detail) || ('HTTP ' + res.status)), true);

View File

@@ -52,6 +52,13 @@ def run_migrations_online() -> None:
connection=connection, connection=connection,
target_metadata=target_metadata, target_metadata=target_metadata,
compare_type=True, compare_type=True,
# Each migration commits on its own. One transaction for the WHOLE
# run meant a crash at step N rolled back steps 1..N-1 while their
# "Running upgrade" lines stayed on screen claiming they ran - the
# 2026-08-21 outage's stamp-to-head repair trusted those lines and
# left production missing two tables (found 2026-08-23 when the
# locations import 500'd on a table that "had been created").
transaction_per_migration=True,
) )
with context.begin_transaction(): with context.begin_transaction():
context.run_migrations() context.run_migrations()

View File

@@ -2388,6 +2388,24 @@ def parse_location_rows(text: str) -> tuple[list[tuple[int, list[str]]], list[di
rejected.append({"line": i, "text": line, rejected.append({"line": i, "text": line,
"reason": "no letters or digits to make a code from"}) "reason": "no letters or digits to make a code from"})
continue continue
# Postgres enforces VARCHAR lengths and refuses NUL/control bytes;
# SQLite shrugs at both - which is how ONE bad CSV line 500'd the whole
# production import (2026-08-23, BL-027's class again) instead of coming
# back as a rejection with its line number. Validate per row, here,
# so every dialect answers the same way: with a reason.
if any(any(ord(ch) < 32 for ch in p) for p in parts):
rejected.append({"line": i, "text": line[:120],
"reason": "contains control characters — re-save the file as plain CSV (UTF-8)"})
continue
long_p = next((p for p in parts if len(p) > 200), None)
if long_p is not None:
rejected.append({"line": i, "text": line[:120],
"reason": "a name is longer than 200 characters (%d)" % len(long_p)})
continue
if any(len(location_slug(p)) > 60 for p in parts):
rejected.append({"line": i, "text": line[:120],
"reason": "a code would be longer than 60 characters"})
continue
rows.append((i, parts)) rows.append((i, parts))
return rows, rejected return rows, rejected
@@ -2454,6 +2472,7 @@ def import_locations(project_id: str, body: LocationImportIn,
require_project_writable(db, user, project_id, "The location list cannot be changed") require_project_writable(db, user, project_id, "The location list cannot be changed")
rows, rejected = parse_location_rows(body.text) rows, rejected = parse_location_rows(body.text)
read_total = len(rows) + len(rejected)
existing = {n.path: n for n in db.scalars( existing = {n.path: n for n in db.scalars(
select(models.LocationNode).where(models.LocationNode.project_id == project_id) select(models.LocationNode).where(models.LocationNode.project_id == project_id)
@@ -2468,6 +2487,10 @@ def import_locations(project_id: str, body: LocationImportIn,
for line_no, parts in rows: for line_no, parts in rows:
segs = [location_slug(p) for p in parts] segs = [location_slug(p) for p in parts]
full = "/".join(segs) full = "/".join(segs)
if len(full) > 200:
rejected.append({"line": line_no, "text": "/".join(parts)[:120],
"reason": "the combined path is longer than 200 characters"})
continue
if full in seen_in_file: if full in seen_in_file:
duplicates.append({"line": line_no, "path": full, "names": parts, duplicates.append({"line": line_no, "path": full, "names": parts,
"reason": "already on line %d of this import" % seen_in_file[full]}) "reason": "already on line %d of this import" % seen_in_file[full]})
@@ -2510,7 +2533,7 @@ def import_locations(project_id: str, body: LocationImportIn,
result = { result = {
"project_id": project_id, "dry_run": bool(body.dry_run), "project_id": project_id, "dry_run": bool(body.dry_run),
"read": len(rows) + len(rejected), "read": read_total,
"created": created, "duplicates": duplicates, "created": created, "duplicates": duplicates,
"reactivated": reactivated, "rejected": rejected, "reactivated": reactivated, "rejected": rejected,
} }
@@ -3007,6 +3030,18 @@ def parse_material_rows(text: str):
rejected.append({"line": i, "text": raw.strip()[:120], rejected.append({"line": i, "text": raw.strip()[:120],
"reason": "more than three columns - description, unit, code is the whole shape"}) "reason": "more than three columns - description, unit, code is the whole shape"})
continue continue
# Same guard as parse_location_rows: reject what Postgres would refuse
# (VARCHAR limits, control bytes) with the line number, never a 500.
if any(any(ord(ch) < 32 for ch in p) for p in parts):
rejected.append({"line": i, "text": raw.strip()[:120],
"reason": "contains control characters - re-save the file as plain CSV (UTF-8)"})
continue
caps = ((300, "description"), (20, "unit"), (80, "code"))
long_col = next((("%s is longer than %d characters (%d)" % (label, cap, len(p)))
for (cap, label), p in zip(caps, parts) if len(p) > cap), None)
if long_col:
rejected.append({"line": i, "text": raw.strip()[:120], "reason": long_col})
continue
rows.append((i, parts)) rows.append((i, parts))
return rows, rejected return rows, rejected

View File

@@ -323,6 +323,13 @@ def run(page, base, tok):
{"text": "Probe Building One,Probe Level 1,Probe Sector B"}) {"text": "Probe Building One,Probe Level 1,Probe Sector B"})
chk("the import reports it as reactivated, not created or duplicate", chk("the import reports it as reactivated, not created or duplicate",
len(again["body"]["reactivated"]) == 1 and not again["body"]["created"], again["body"]) len(again["body"]["reactivated"]) == 1 and not again["body"]["created"], again["body"])
# The 2026-08-23 production 500, pinned (locations side): Postgres-refused
# values reject by line, on every dialect, never crash the request.
hz = api(page, "POST", "/api/projects/projA/locations/import",
{"text": "Probe Building One," + "Y" * 220 + ",S1", "dry_run": True})
chk("an over-long name is a line rejection, not a 500",
hz["status"] == 200 and hz["body"]["rejected"]
and "200 characters" in hz["body"]["rejected"][0]["reason"], hz["body"])
same = [n for n in api(page, "GET", same = [n for n in api(page, "GET",
"/api/projects/projA/locations?include_inactive=true")["body"]["nodes"] "/api/projects/projA/locations?include_inactive=true")["body"]["nodes"]
if n["path"] == sec["path"]] if n["path"] == sec["path"]]

View File

@@ -105,6 +105,20 @@ def main():
_, listing = api(base, "/api/projects/projA/materials", root) _, listing = api(base, "/api/projects/projA/materials", root)
chk("...and nothing was written", listing["items"] == []) chk("...and nothing was written", listing["items"] == [])
# The 2026-08-23 production 500, pinned: what Postgres refuses (VARCHAR
# overflow, control bytes) must come back as a per-line rejection - on
# EVERY dialect - never crash the request.
code, rep = api(base, "/api/projects/projA/materials/import", root, "POST",
{"text": "Sample " + "x" * 300 + ",EA", "dry_run": True})
chk("an over-long description is a line rejection, not a 500",
code == 200 and rep["rejected"] and "300 characters" in rep["rejected"][0]["reason"]
and not rep["created"], ascii_(rep))
code, rep = api(base, "/api/projects/projA/materials/import", root, "POST",
{"text": "Sample widget\u0000,EA", "dry_run": True})
chk("a control byte is a line rejection, not a 500",
code == 200 and rep["rejected"]
and "control characters" in rep["rejected"][0]["reason"], ascii_(rep))
code, rep = api(base, "/api/projects/projA/materials/import", root, "POST", code, rep = api(base, "/api/projects/projA/materials/import", root, "POST",
{"text": text, "dry_run": False}) {"text": text, "dry_run": False})
_, listing = api(base, "/api/projects/projA/materials", root) _, listing = api(base, "/api/projects/projA/materials", root)