From 6adaf7bf258527b56745af8e0747355d099eeb1d Mon Sep 17 00:00:00 2001 From: Spencer McGuire Date: Tue, 3 Nov 2026 15:52:09 -0700 Subject: [PATCH] Archive copy must succeed before a packet counts as filed (Gitea issue 57) FILE checks the archive folder and copies the packet inside the filing transaction. A failed check or copy rolls the filing back, so the packet stays in its previous stage; the person filing gets 503 archive_failed, and in its own transaction the server writes an archive.filing_blocked audit row and raises the critical archive_failed condition to Admin for that packet (who tried, folder, error). The next successful filing clears it. The archive-copy job now only catches up filings made while the archive was off. PLAN question 53, CHANGELOG, FEED and SECURITY updated. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 + docs/FEED.md | 4 +- docs/PLAN.md | 4 +- docs/SECURITY.md | 6 +- server/packetstation/api/packets.py | 57 ++++---- server/packetstation/domain/archive.py | 129 ++++++++++++++++-- server/packetstation/domain/pipeline.py | 11 +- server/packetstation/notify/catalog.py | 4 +- .../packetstation/worker/jobs/archive_copy.py | 14 +- tests/e2e/tests/archive.spec.ts | 2 +- tests/server/test_archive.py | 119 ++++++++++------ 11 files changed, 254 insertions(+), 98 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13c64f5..8b308f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to this project are recorded here. The format follows [Keep ### Decided +- Gitea issue 57: Spencer chose the stricter archive rule (2026-11-03, open question 53). The archive copy must succeed before a packet counts as filed. A failed copy keeps the packet where it was and tells Admin. See Changed. - Gitea issue 9: SethD kept the drawing-origin change order for a same-rev re-release on an in-shop packet (2026-10-29), so the shop confirms the new pages. A byte-identical file is still ignored. Nothing changes in the build. - Gitea issue 4: SethD wants the order a packet shows to follow the shop schedule, which is Presley's Production Planner (2026-10-29). Asked on the issue on 2026-10-30: what it is, where it lives, what a row holds and how often it changes. Packet Station would only read it. The order rule stays as built until then. - Gitea issue 31: two follow-up questions asked on 2026-10-30. A work order deleted from VISUAL while open blocks filing for good: count a leg gone for 2 runs as closed, or let an Admin close it with a reason? And should Shipping packets also wait for the VISUAL close? Both stay as built until answered. @@ -38,6 +39,7 @@ All notable changes to this project are recorded here. The format follows [Keep ### Changed +- Filing copies the packet to the archive inside the filing transaction (Gitea issue 57). If the folder check or the copy fails, nothing is filed: the packet stays in its previous stage, the person filing sees why (503 `archive_failed`), and Admin gets the critical `archive_failed` alert for that packet with who tried, the folder and the error, plus an `archive.filing_blocked` audit row. Filing it again once the archive works files it and clears the alert. The `archive-copy` worker job now only catches up filings made while the archive was off. With the archive off or no folder saved, filing is unchanged. - Phase 10: the content security policy is final. Styles no longer need `'unsafe-inline'`, `data:` images and `blob:` workers are gone, and `frame-src` and `object-src` are `'none'` (a PDF opened in its own tab gets `object-src 'self'` so the browser can show it). The print area's rules moved into the theme stylesheet. - Phase 10: backups and the restore run on the VM with `pg_dump` and `pg_restore` as the postgres user, with no `docker exec`. Same markers, 7 dumps and the 02:00 timer. - Phase 10: security fixes in the pinned dependencies found by the CI scan: `pypdf` 6.21.2 to 6.21.4, `pyasn1` 0.6.4 to 0.6.5, and `certifi` to 2026.10.23. diff --git a/docs/FEED.md b/docs/FEED.md index 8e70156..108962c 100644 --- a/docs/FEED.md +++ b/docs/FEED.md @@ -144,9 +144,9 @@ For each rule whose digest time (site time) has passed today and that has outbox ## archive-copy (every 15 minutes, phase 10) -Gitea issue 7. Not pausable. Off when `ARCHIVE_ROOT` is unset or blank: the run does nothing (counts `enabled: false`) and FILE asks for nothing. With the root set but no folder saved on Admin, File locations, the run idles (`skipped: no archive folder saved`). +Gitea issue 7. Not pausable. Off when `ARCHIVE_ROOT` is unset or blank: the run does nothing (counts `enabled: false`). With the root set but no folder saved on Admin, File locations, the run idles (`skipped: no archive folder saved`). -FILE inserts a `job_requests` row for `archive-copy` and NOTIFYs `ps_job_request` inside its own transaction when the archive is on and a folder is saved, so the copy starts right after the filing commits. The request is made in a savepoint: if it fails, the filing still commits and the next scheduled run copies the packet. A failed copy never blocks filing and never touches the packet. Saving a folder and "Retry now" also request a run. +FILE makes its own copy inside the filing transaction and refuses to file when it fails (Gitea issue 57, open question 53), so this job does not copy new filings. It catches up the filings made while the archive was off. Those packets are already filed, so a failed copy here never touches the packet. Saving a folder and "Retry now" request a run. Each run: diff --git a/docs/PLAN.md b/docs/PLAN.md index eb89a6f..43db13b 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -121,7 +121,7 @@ Each phase ends with passing tests, docs updated (README, CHANGELOG, SECURITY.md - Rewrite the 38 scenarios against the new UI with synthetic fixtures; run in Gitea Actions against compose.ci.yml. Policy matrix, merge rules and packet-rule tests already run from phase 2 and 3. - CSP final pass, dependency scan, build-version banner and reload, DEPLOY.md with the NPM proxy host settings, SECURITY.md rewritten with the ID table, GO-LIVE.md (date, boundary, groups, tablets, URL, M: retirement as read-only archive), PAPER-FALLBACK.md, incident notes. - Production and test each on their own Linux VM, natively, with no Docker (Spencer, 2026-10-27, Gitea issue 37; see "Host layout"). The test VM is built first from the same steps, so the production install, backups, restore and doctor are rehearsed before go-live. The app and the worker run as systemd services from a venv built from the same `server/requirements.txt` and `server/requirements-pesco.txt` pins as the image, with PostgreSQL 16, ODBC Driver 18 and the PESCO CA installed on the VM. Settings from an env file, secrets as root-owned files read through the same `*_PASSWORD_FILE` settings, VISUAL_SERVER and VISUAL_DATABASE set explicitly; first start is Spencer's approval. NPM on docker-1 proxies `packetstation.pescoinc.biz` and `packetstation-test.pescoinc.biz` to their VMs over the LAN. Backups, the doctor and the restore are rewritten for the VMs (no `docker exec`). DEPLOY.md gets a VM section; the compose files stay for CI. The docker-1 test stack keeps running until the test VM is ready, then it is stopped. -- Archive location (Gitea issue 7, Spencer 2026-10-27). Spencer creates an NFS share on the QNAP and mounts it on docker-1 (root, steps in DEPLOY.md), with write access for uid 10001. Compose bind-mounts it into the app and the worker at a fixed root, `ARCHIVE_ROOT` (unset turns the feature off). The Admin page gets a File locations section with one setting for now, the archive folder, as a path under `ARCHIVE_ROOT`. The server refuses a path that leaves the root (`..`, absolute paths elsewhere, symlinks out). "Check" writes a probe file, reads it back, removes it and shows the result, and saving runs the same check first. The setting change and every check are audited. `doctor` runs the check too. At filing, the sealed data book, each of the packet's documents and a manifest are written to the archive folder (SethD, 2026-10-28, issue 7). Whether a failed copy blocks the filing or is retried and raised to Admin is settled in the phase 10 design. +- Archive location (Gitea issue 7, Spencer 2026-10-27). Spencer creates an NFS share on the QNAP and mounts it on docker-1 (root, steps in DEPLOY.md), with write access for uid 10001. Compose bind-mounts it into the app and the worker at a fixed root, `ARCHIVE_ROOT` (unset turns the feature off). The Admin page gets a File locations section with one setting for now, the archive folder, as a path under `ARCHIVE_ROOT`. The server refuses a path that leaves the root (`..`, absolute paths elsewhere, symlinks out). "Check" writes a probe file, reads it back, removes it and shows the result, and saving runs the same check first. The setting change and every check are audited. `doctor` runs the check too. At filing, the sealed data book, each of the packet's documents and a manifest are written to the archive folder (SethD, 2026-10-28, issue 7). A failed copy blocks the filing and is raised to Admin (Spencer, 2026-11-03, Gitea issue 57; open question 53). - Closes: O3, O4; S4 is IT's pentest, deferred. - Done 2026-10-30. The bench maps all 38 old scenarios (tests/e2e/BENCH.md; trel is open question 54) and runs 111 Playwright tests, each failing on a CSP violation. The CSP dropped `'unsafe-inline'`. pip-audit found pypdf, pyasn1 and certifi to bump. The archive copy, the version banner, the VM kit in `deploy/vm/` with VM backups and restore, and the go-live, paper-fallback and security docs are built. The VMs, the NFS share, the timers and the go-live date are Spencer's and Seth's (open question 53 records the archive copy policy). @@ -292,7 +292,7 @@ From phase 9 (2026-10-21), for Seth or Spencer to veto: 50. The profile CSV `stations` cell: default the cell is the person's whole list of station limits across departments and replaces every limit they had; `-` clears them all; an empty cell leaves them. Phase 9 (2026-10-21) applied the default. 51. A direct welder assign: default it does not check `profile_active`, so an Admin's inactive switch hides a person from the picker but a typed assign still works. Phase 9 (2026-10-21) applied the default; say if an inactive profile should be refused there too. 52. A checklist item added while a stage is open: default it applies to that open stage too, because completeness reads the live template, so the item must be ticked before submit. Phase 9 (2026-10-21) applied the default. Replaced by change C8: a template change applies to new packets only. Built on 2026-10-30: a stage freezes its list at START, so a change reaches only stages not yet started. -53. A failed archive copy at filing (phase 10, Gitea issue 7): default it never blocks the filing. The FILE transaction asks the worker for an `archive-copy` run; the job retries a failed copy after 5, 15 and 60 minutes, then hourly, and raises the `archive_failed` condition to Admin after 3 failed tries or a failed folder check. Admin, Storage, File locations shows the failures and has "Retry now". Turning the archive on later also copies the earlier filings. Seth or Spencer may ask for the stricter rule (the copy must succeed before the packet counts as filed) instead. +53. A failed archive copy at filing (phase 10, Gitea issues 7 and 57). Answered by Spencer on 2026-11-03 (issue 57): the copy must succeed before the packet counts as filed. FILE checks the folder and copies the packet inside the filing transaction. If the check or the copy fails, the whole filing rolls back, so the packet stays in its previous stage and can be filed again. The person filing gets 503 `archive_failed` with the reason. In a transaction of its own the server writes an `archive.filing_blocked` audit row and raises the `archive_failed` condition to Admin (subject `filing:`, critical) with the packet, who tried, the archive folder and the error. A retry that fails again only counts on the open condition, and the next successful filing of that packet clears it. With `ARCHIVE_ROOT` unset or no folder saved, the archive is not in use and filing goes ahead. The worker job still copies the filings made while the archive was off, with its retries and its own `archive_failed` condition (subject `archive`). The earlier default (never block, retry in the worker) is replaced. 54. The old Clerical "Released" section (every released packet by place, with overdue and 7+ day counts; old bench scenario trel) is not in the new build and not on the Drop list (phase 10, tests/e2e/BENCH.md). Default: not built; the boards and the find bar's Mine, Board and List views show where each packet is. Seth decides whether it is needed for parity. Changes decided by SethD on 2026-10-26. Each replaces a default above. All five were built on 2026-10-30 (branch `seth-changes`): diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 7fe6198..660783b 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -195,10 +195,10 @@ The archive closes D6 (Gitea issue 7). It is a copy of every filed packet on the - An Admin picks the archive folder in the File locations section of the Admin page (setting `files.archive_dir`, versioned with compare and set like the security settings). The folder is a path under `ARCHIVE_ROOT`. The server refuses an empty path, `..`, an absolute path outside the root, a NUL, a path whose real location is outside the real root (a symlink out), and a folder that does not exist. It never creates folders. - "Check" writes a probe file in the folder, syncs it to disk, reads it back, compares it and deletes it. It shows the result, the resolved path and the free space. A save runs the same check first and is refused when it fails. `doctor` runs the check on the saved folder. - Every setting change (old and new value), every check (passed or not) and every copy (done or failed) writes an audit row. Admin writes need a password session and pass the Origin check, as every Admin write does. -- At filing, the worker copies the sealed data book, every document of the packet and a `manifest.json` into `//-seal/`. The manifest names the packet, project, unit, seal id, seal hash, key id, who sealed and filed it and when, the build, and for every file its name, kind, sha256, size and source asset. Anyone can check the copy against the manifest without Packet Station. +- At filing, FILE copies the sealed data book, every document of the packet and a `manifest.json` into `//-seal/`. The manifest names the packet, project, unit, seal id, seal hash, key id, who sealed and filed it and when, the build, and for every file its name, kind, sha256, size and source asset. Anyone can check the copy against the manifest without Packet Station. - Each file is written under a temporary name, synced and renamed. A different file is never overwritten; an identical file counts as done. Nothing in the archive is ever deleted by Packet Station. A refile after a reopen gets a new seal and a new folder, so both copies stay. -- A failed copy never blocks filing (decided in the phase 10 design; recorded as an open question in [PLAN.md](PLAN.md)). The worker job `archive-copy` runs every 15 minutes and right after each filing. It retries a failed copy after 5, 15 and 60 minutes, then hourly. When a copy has failed 3 times, or the folder check fails, it raises the Admin alert `archive_failed`, cleared when nothing is failed. "Retry now" on the File locations section asks for a run at once. -- When the archive is turned on later, the job also copies every earlier filing that has no copy yet. +- The copy must succeed before the packet counts as filed (Spencer, 2026-11-03, Gitea issue 57; open question 53 in [PLAN.md](PLAN.md)). FILE checks the folder and copies inside the filing transaction. A failed check or copy rolls the filing back, so the packet stays in its previous stage. The server then writes an `archive.filing_blocked` audit row and raises the critical Admin alert `archive_failed` for that packet, with who tried, the folder and the error. Filing it again once the archive works clears the alert. +- When the archive is turned on later, the worker job `archive-copy` copies every earlier filing that has no copy yet. It runs every 15 minutes, retries a failed copy after 5, 15 and 60 minutes, then hourly, and raises `archive_failed` when a copy has failed 3 times or the folder check fails. "Retry now" on the File locations section asks for a run at once. - The archive is a plain copy outside the app's access control. The files are not encrypted by the app (D1 is deferred). Who can read or change the share, the NFS export rule (the production VM only), snapshots and the share's own backup are IT's. ### Warehouse rules (phase 4) diff --git a/server/packetstation/api/packets.py b/server/packetstation/api/packets.py index 2615e89..a0cbc55 100644 --- a/server/packetstation/api/packets.py +++ b/server/packetstation/api/packets.py @@ -18,9 +18,9 @@ from psycopg.types.json import Jsonb from pydantic import BaseModel, Field from .. import audit, events, policy +from ..domain import archive, packets, search, shop from ..domain import assets as asset_store from ..domain import ncr as ncrs -from ..domain import packets, search, shop from ..domain import qc as qc_dom from ..domain.pipeline import ( Expect, @@ -543,6 +543,10 @@ TRANSITION_FIELDS: dict[policy.Action, tuple[str, ...]] = { A.FILE: ("note",), A.ADMIN_REOPEN: ("reason",), } +MSG_ARCHIVE = ( + "The packet could not be copied to the archive: {reason}. Nothing was filed, and Admin has been told." + " File it again once the archive works." +) DEPT_ACTIONS = (A.RELEASE_TO_SHOP, A.ROUTE_FROM_REVIEW, A.QC_ACCEPT_AND_ROUTE) @@ -591,29 +595,34 @@ def transition( else: del extra["stamp"] content = {"action": str(action), "packet_id": packet_id, "expect": body.expect.model_dump(), **extra} - with packet_write( - d, - p, - packet_id, - action, - request=request, - extra=extra, - it_reason=body.it_reason, - sign=body.sign, - expect=body.expect, - sign_content=content, - policy_principal=pp, - ) as w: - if action is A.ASSIGN: - ok = conn.execute( - "SELECT 1 FROM stations WHERE id = %s AND dept_code = %s AND active", - (extra["station_id"], w.state.dept), - ).fetchone() - if ok is None: - raise ApiError(422, "invalid_station", "Choose a station in this department.") - active_user(conn, extra["welder_id"], "welder") - shop.welder_check(conn, w.state.dept, str(extra["welder_id"]), int(extra["station_id"])) - apply_transition(w) + try: + with packet_write( + d, + p, + packet_id, + action, + request=request, + extra=extra, + it_reason=body.it_reason, + sign=body.sign, + expect=body.expect, + sign_content=content, + policy_principal=pp, + ) as w: + if action is A.ASSIGN: + ok = conn.execute( + "SELECT 1 FROM stations WHERE id = %s AND dept_code = %s AND active", + (extra["station_id"], w.state.dept), + ).fetchone() + if ok is None: + raise ApiError(422, "invalid_station", "Choose a station in this department.") + active_user(conn, extra["welder_id"], "welder") + shop.welder_check(conn, w.state.dept, str(extra["welder_id"]), int(extra["station_id"])) + apply_transition(w) + except archive.FilingCopyFailed as e: + # The filing rolled back (Gitea issue 57); record it and tell Admin in a transaction of its own. + archive.record_filing_failure(conn, p, packet_id, e) + raise ApiError(503, "archive_failed", MSG_ARCHIVE.format(reason=e.message.rstrip("."))) from None return _read(d, pp, packet_id) diff --git a/server/packetstation/domain/archive.py b/server/packetstation/domain/archive.py index 61571e0..0e344a4 100644 --- a/server/packetstation/domain/archive.py +++ b/server/packetstation/domain/archive.py @@ -6,9 +6,14 @@ A path that leaves the root is refused: empty, `..`, an absolute path elsewhere, the root, or a folder that does not exist. The check writes a probe file, reads it back and removes it. Nothing here creates folders or deletes anything in the archive. -FILE inserts a job request for `archive-copy` in its own transaction. The worker job (worker/jobs/archive_copy.py) -queues every unbroken seal with a data book that has no `archive_copies` row, then copies the pending and due failed -rows. A failed copy never blocks filing: it is retried with a backoff and raised to Admin as `archive_failed`. +FILE copies the packet into the archive inside its own transaction (Gitea issue 57, Spencer 2026-11-03): the copy +must succeed before the packet counts as filed. A failed folder check or copy rolls the whole filing back, so the +packet stays where it was and can be filed again. The failure is then recorded in its own transaction and raised to +Admin as the `archive_failed` condition with subject `filing:`, cleared when that packet files. + +The worker job (worker/jobs/archive_copy.py) copies the seals filed while the archive was off: it queues every +unbroken seal with a data book that has no `archive_copies` row, then copies the pending and due failed rows with a +backoff, and raises `archive_failed` (subject `archive`) to Admin. Folder layout under the archive folder: `//-seal/` with the data book PDF, the packet's files (drawing page files, packet documents, the Q.C. folder documents of its unit and project, MTRs) @@ -45,6 +50,8 @@ JOB = "archive-copy" JOB_CHANNEL = "ps_job_request" EVENT = "archive_failed" EVENT_SUBJECT = "archive" +FILING_SUBJECT = "filing:{packet_id}" +FILING_TITLE = "Filing blocked: archive copy failed" PROBE_PREFIX = ".packetstation-probe-" MANIFEST = "manifest.json" DIR_MAX = 400 @@ -72,6 +79,16 @@ class CopyError(Exception): """One copy failed. The message goes into archive_copies.last_error and the audit row.""" +class FilingCopyFailed(Exception): + """FILE could not copy the packet into the archive. The filing rolls back; the message is for the person filing + and for Admin.""" + + def __init__(self, message: str, archive_dir: str | None): + super().__init__(message) + self.message = message + self.archive_dir = archive_dir + + @dataclass(frozen=True) class CheckResult: ok: bool @@ -320,20 +337,104 @@ def request_copy(conn: Any, user_id: Any = None, note: str | None = None) -> Non conn.execute("SELECT ps_notify(%s, %s)", (JOB_CHANNEL, json.dumps({"job": JOB}, separators=(",", ":")))) -def on_filed(conn: Any, settings: Settings, user_id: Any, packet_id: int) -> bool: - """FILE's hook: request a prompt copy when the archive is on and a folder is saved. Never raises; a failure - here only means the copy waits for the next scheduled run. Returns whether a request went in.""" - if root_of(settings) is None: - return False +def copy_at_filing(conn: Any, settings: Settings, packet_id: int, seal_id: int) -> dict[str, Any] | None: + """FILE's copy, in the filing transaction: check the folder, copy the seal, record the copy as done. + Returns {folder, files}, or None when the archive is off (ARCHIVE_ROOT unset or no folder saved). + Raises FilingCopyFailed, which rolls the filing back.""" + root = root_of(settings) + if root is None: + return None + archive_dir = load(conn)[0] + if archive_dir is None: + return None + rel, result = check(root, archive_dir) + if not result.ok or rel is None: + raise FilingCopyFailed(f"The archive folder check failed: {result.message}", archive_dir) + base = root.joinpath(*rel.split("/")) + try: + folder, sha, files = copy_seal(conn, settings, base, rel, seal_id) + except CopyError as e: + raise FilingCopyFailed(str(e), archive_dir) from None + except Exception as e: # anything unexpected blocks the filing the same way + log.exception("archive copy of packet %s at filing failed", packet_id) + raise FilingCopyFailed(f"{type(e).__name__}: {e}", archive_dir) from None + with conn.transaction(): + conn.execute( + "INSERT INTO archive_copies (seal_id, packet_id, status, attempts, folder, manifest_sha256, done_at)" + " VALUES (%s, %s, 'done', 1, %s, %s, now())", + (seal_id, packet_id, folder, sha), + ) + audit.append( + conn, + action="archive.copied", + subject_type="seal", + subject_id=str(seal_id), + packet_id=int(packet_id), + detail={"seal_id": seal_id, "folder": folder, "files": files, "manifest_sha256": sha, "source": "filing"}, + ) + from ..notify import resolve_event + + resolve_event( + conn, EVENT, subject=FILING_SUBJECT.format(packet_id=packet_id), body="The packet was filed and copied." + ) + return {"folder": folder, "files": files} + + +def record_filing_failure(conn: Any, who: Any, packet_id: int, failure: FilingCopyFailed) -> None: + """After the filing rolled back: one `archive.filing_blocked` audit row and the Admin alert, in their own + transaction. Never raises; the person filing gets the refusal either way.""" + from ..notify import raise_event + try: with conn.transaction(): - if load(conn)[0] is None: - return False - request_copy(conn, user_id, f"filed packet {packet_id}") - return True + row = conn.execute( + "SELECT pr.number, u.lot, p.drawing, p.status, p.dept_code FROM packets p" + " JOIN units u ON u.id = p.unit_id JOIN projects pr ON pr.id = u.project_id WHERE p.id = %s", + (packet_id,), + ).fetchone() + label = packet_label(*row[:3]) if row else f"packet {packet_id}" + user_id = getattr(who, "user_id", None) + name = conn.execute("SELECT display_name FROM users WHERE id = %s", (user_id,)).fetchone() + by = name[0] if name else None + status = str(row[3]) if row else None + dept = row[4] if row else None + detail = { + "packet_id": packet_id, + "packet_label": label, + "status": status, + "dept": dept, + "archive_dir": failure.archive_dir, + "error": failure.message, + "by": by, + } + audit.append( + conn, + action="archive.filing_blocked", + user_id=user_id, + session_id=getattr(who, "session_id", None), + subject_type="packet", + subject_id=str(packet_id), + packet_id=int(packet_id), + detail=detail, + ip=getattr(who, "ip", None), + ) + where = f"{status or 'where it was'}{' in ' + dept if dept else ''}" + raise_event( + conn, + EVENT, + subject=FILING_SUBJECT.format(packet_id=packet_id), + title=f"{FILING_TITLE}: {label}", + body=( + f"{by or 'Someone'} tried to file {label}, but the packet could not be copied to the archive," + f" so it was not filed. It stays {where} and can be filed again once the archive works.\n\n" + f"Archive folder: {failure.archive_dir or '(none)'}\nError: {failure.message}" + ), + detail=detail, + severity="critical", + condition=True, + ) except Exception: - log.exception("archive copy request for packet %s not made; the scheduled run picks it up", packet_id) - return False + log.exception("the blocked filing of packet %s was not recorded", packet_id) def reset_backoff(conn: Any) -> int: diff --git a/server/packetstation/domain/pipeline.py b/server/packetstation/domain/pipeline.py index 3e32f35..48039da 100644 --- a/server/packetstation/domain/pipeline.py +++ b/server/packetstation/domain/pipeline.py @@ -833,7 +833,8 @@ def apply_transition(w: Write) -> None: def _file(w: Write) -> None: """FILE: the data book date, the seal, and the data book built and stored on the seal, all in this transaction. - A data book that cannot be built answers 500 databook_failed and the whole filing rolls back.""" + A data book that cannot be built answers 500 databook_failed and the whole filing rolls back. So does an archive + copy that fails (archive.FilingCopyFailed, answered 503 archive_failed by the transition endpoint).""" key = _seal_key(w) _set_packet(w, "databook_at = now()") row = packets.load_row(w.conn, w.id) @@ -850,11 +851,13 @@ def _file(w: Write) -> None: w.log_also("Data book filed", f"{pages} pages") w.detail["seal_id"] = int(seal_id) w.detail["databook_pages"] = pages - # The archive copy runs in the worker after this commits. A failed copy never blocks filing. + # The archive copy must succeed before the packet counts as filed (Gitea issue 57). A failure raises + # archive.FilingCopyFailed and the whole filing rolls back; the transition endpoint records it for Admin. from . import archive - if archive.on_filed(w.conn, _settings(w), w.user_id, w.id): - w.detail["archive_requested"] = True + copied = archive.copy_at_filing(w.conn, _settings(w), w.id, int(seal_id)) + if copied is not None: + w.detail["archive_folder"] = copied["folder"] def _file_databook(w: Write, row: Mapping[str, Any], seal: dict[str, Any], seal_id: int) -> int: diff --git a/server/packetstation/notify/catalog.py b/server/packetstation/notify/catalog.py index 764d467..32eadf9 100644 --- a/server/packetstation/notify/catalog.py +++ b/server/packetstation/notify/catalog.py @@ -75,8 +75,8 @@ _TYPES = ( EventType( "archive_failed", "Archive copy failing", - "A filed packet could not be copied to the archive folder after three tries, or the archive folder check" - " failed.", + "A packet could not be filed because its archive copy failed, an earlier filing could not be copied after" + " three tries, or the archive folder check failed.", ), ) diff --git a/server/packetstation/worker/jobs/archive_copy.py b/server/packetstation/worker/jobs/archive_copy.py index c3d13a2..16b45ef 100644 --- a/server/packetstation/worker/jobs/archive_copy.py +++ b/server/packetstation/worker/jobs/archive_copy.py @@ -1,10 +1,12 @@ -"""The archive copy (Gitea issue 7), every 15 minutes and on request (FILE asks for a run when it commits). +"""The archive copy (Gitea issue 7) for seals filed while the archive was off, every 15 minutes and on request. -Off when ARCHIVE_ROOT is unset, and idle until the Admin saves a folder. Each run checks the folder (the probe), queues -every unbroken seal with a data book that has no `archive_copies` row (so turning the archive on later copies earlier -filings too), then copies the pending rows and the failed rows that are due. A failed copy waits 5, 15, 60 minutes, -then an hour between tries. The `archive_failed` condition (subject `archive`) is raised when a copy has failed three -times or the folder check fails, and cleared when no copy is failed. A failed copy never touches the packet. +FILE copies its own packet in the filing transaction and refuses to file when the copy fails (Gitea issue 57; see +domain/archive.py). This job catches up the rest: off when ARCHIVE_ROOT is unset, and idle until the Admin saves a +folder. Each run checks the folder (the probe), queues every unbroken seal with a data book that has no +`archive_copies` row (so turning the archive on later copies earlier filings too), then copies the pending rows and +the failed rows that are due. A failed copy waits 5, 15, 60 minutes, then an hour between tries. The `archive_failed` +condition (subject `archive`) is raised when a copy has failed three times or the folder check fails, and cleared when +no copy is failed. These packets are already filed, so a failed copy here never touches the packet. """ from __future__ import annotations diff --git a/tests/e2e/tests/archive.spec.ts b/tests/e2e/tests/archive.spec.ts index a9630ca..7768ecd 100644 --- a/tests/e2e/tests/archive.spec.ts +++ b/tests/e2e/tests/archive.spec.ts @@ -72,7 +72,7 @@ test.describe.serial("archive", () => { expect((await packet(clara, p.id)).seal).toBeTruthy(); await closeAll(clara, wade, wes, andy, quinn); - // FILE asks the worker for a copy at once. The Admin section shows it done. + // FILE made the copy in the filing transaction (Gitea issue 57). The Admin section shows it done. const ann = await admin(browser); await expect .poll( diff --git a/tests/server/test_archive.py b/tests/server/test_archive.py index a6e765c..e33877f 100644 --- a/tests/server/test_archive.py +++ b/tests/server/test_archive.py @@ -1,6 +1,7 @@ -"""The archive location (Gitea issue 7): the path rules, the check, the File locations API, the archive-copy job -(copy, idempotence, failure and backoff, the Admin condition, old seals queued) and filing with the archive off or -broken. Synthetic data only; the archive root is a temp folder.""" +"""The archive location (Gitea issue 7): the path rules, the check, the File locations API, the copy FILE makes in +its own transaction, filing refused when that copy fails (Gitea issue 57), and the archive-copy job (idempotence, +failure and backoff, the Admin condition, old seals queued). Synthetic data only; the archive root is a temp +folder.""" from __future__ import annotations @@ -13,7 +14,7 @@ from pathlib import Path from typing import Any import pytest -from closehelpers import file_packet, filed, q1, qall, rich_complete, vclose +from closehelpers import at_complete, file_packet, filed, q1, qall, rich_complete, vclose from fakedir import FakeDirectory from pshelpers import make_settings, needs_pg from shophelpers import Shop, ok, open_shop, refused, run @@ -283,12 +284,9 @@ def test_copy_job(shop: Shop, monkeypatch: pytest.MonkeyPatch) -> None: run(s, "UPDATE bom_shorts SET closed_at = now() WHERE packet_id = %s", (pid,)) out = ok(file_packet(shop, vclose(shop, pkt))) assert out["status"] == "filed" - assert q1(s, "SELECT count(*) FROM job_requests WHERE job = 'archive-copy' AND claimed_at IS NULL")[0] == 1 + # FILE made the copy itself, in the filing transaction: nothing waits for the worker + assert q1(s, "SELECT count(*) FROM job_requests WHERE job = 'archive-copy'")[0] == 0 seal_id, book_id = q1(s, "SELECT id, databook_asset_id FROM packet_seals WHERE packet_id = %s", (pid,)) - - row = run_copy(s) - assert row["status"] == "ok", row - assert row["trigger"] == "request" and row["counts"]["queued"] == 1 and row["counts"]["copied"] == 1 ((sid, status, attempts, err, nxt, folder),) = copies(s) assert (sid, status, attempts, err, nxt) == (seal_id, "done", 1, None, None) project, unit = q1( @@ -315,11 +313,13 @@ def test_copy_job(shop: Shop, monkeypatch: pytest.MonkeyPatch) -> None: assert q1(s, "SELECT manifest_sha256 FROM archive_copies")[0] == sha(where / "manifest.json") (done,) = audits(s, "archive.copied") assert done["seal_id"] == seal_id and done["folder"] == folder and done["files"] == len(names) - assert events == [("resolve", "archive_failed", {"subject": "archive", "title": "Archive copy failing"})] + assert done["source"] == "filing" - # idempotent: a second run copies nothing; a row set back to pending finds the identical files and is done + # idempotent: a run copies nothing; a row set back to pending finds the identical files and is done + row = run_copy(s) + assert row["status"] == "ok" and row["counts"]["queued"] == 0 and row["counts"]["copied"] == 0, row + assert events == [("resolve", "archive_failed", {"subject": "archive", "title": "Archive copy failing"})] events.clear() - assert run_copy(s)["counts"]["copied"] == 0 run(s, "UPDATE archive_copies SET status = 'pending'") before = {p.name: p.stat().st_mtime_ns for p in where.iterdir()} assert run_copy(s)["counts"]["copied"] == 1 @@ -390,50 +390,89 @@ def test_old_seals_are_queued_when_the_archive_is_turned_on(off_shop: Shop, tmp_ assert copies(s)[0][:2] == (seal, "done") +def open_filing_events(settings: Settings, pid: int) -> list[Any]: + return qall( + settings, + "SELECT title, body, detail, severity, raised_count FROM notification_events" + " WHERE event_type = 'archive_failed' AND subject = %s AND resolved_at IS NULL", + (f"filing:{pid}",), + ) + + @needs_pg -def test_filing_succeeds_with_the_archive_broken(shop: Shop, monkeypatch: pytest.MonkeyPatch) -> None: +def test_filing_is_refused_when_the_archive_folder_is_gone(shop: Shop) -> None: s = shop.settings root = Path(str(s.archive_root)) - events: list[tuple[str, str, dict[str, Any]]] = [] - monkeypatch.setattr(jobs, "raise_event", lambda conn, t, **kw: events.append(("raise", t, kw))) - monkeypatch.setattr(jobs, "resolve_event", lambda conn, t, **kw: events.append(("resolve", t, kw))) save_dir(s, "gone") # saved, then the folder went away - pkt = filed(shop) - assert pkt["status"] == "filed" - assert q1(s, "SELECT count(*) FROM job_requests WHERE job = 'archive-copy'")[0] == 1 + pkt = vclose(shop, at_complete(shop)) + pid = pkt["id"] - row = run_copy(s) - assert row["status"] == "degraded" and "does not exist" in row["counts"]["check"] + body = refused(file_packet(shop, pkt), 503, "archive_failed") + assert body["detail"].startswith("The packet could not be copied to the archive: The archive folder check failed:") + assert "does not exist" in body["detail"] and "Nothing was filed, and Admin has been told." in body["detail"] + # the whole filing rolled back: still complete, no seal, no data book, no copy row + view = ok(shop("clerk.clara").get(f"/api/packets/{pid}")) + assert view["status"] == "complete" and view["seal"] is None + assert q1(s, "SELECT count(*) FROM packet_seals WHERE packet_id = %s", (pid,))[0] == 0 + assert q1(s, "SELECT databook_at FROM packets WHERE id = %s", (pid,))[0] is None assert copies(s) == [] - ((kind, etype, kw),) = events - assert (kind, etype, kw["subject"], kw["condition"]) == ("raise", "archive_failed", "archive", True) - checks = audits(s, "archive.checked") - assert [(c["source"], c["ok"]) for c in checks] == [("job", False)] - run_copy(s) - assert len(audits(s, "archive.checked")) == 1 # the same result again is not audited again + # recorded on its own: the audit row and one open Admin condition with the details + (blocked,) = audits(s, "archive.filing_blocked") + assert blocked["packet_id"] == pid and blocked["archive_dir"] == "gone" and blocked["by"] == "Clara Clerk" + assert blocked["status"] == "complete" and "does not exist" in blocked["error"] + ((title, text, detail, severity, raised),) = open_filing_events(s, pid) + assert title.startswith("Filing blocked: archive copy failed: ") and blocked["packet_label"] in title + assert "Clara Clerk tried to file" in text and "Archive folder: gone" in text and "does not exist" in text + assert severity == "critical" and raised == 1 and detail["packet_id"] == pid - # the folder comes back: the copy is made and the condition cleared + # a second try fails the same way: the open condition only counts it + refused(file_packet(shop, pkt), 503, "archive_failed") + assert open_filing_events(s, pid)[0][4] == 2 and len(audits(s, "archive.filing_blocked")) == 2 + + # the folder comes back: the filing goes through, the copy is done and the condition is cleared (root / "gone").mkdir() - events.clear() - row = run_copy(s) - assert row["status"] == "ok" and row["counts"]["copied"] == 1 - assert events == [("resolve", "archive_failed", {"subject": "archive", "title": "Archive copy failing"})] - assert [(c["source"], c["ok"]) for c in audits(s, "archive.checked")] == [("job", False), ("job", True)] + out = ok(file_packet(shop, pkt)) + assert out["status"] == "filed" + ((_, status, attempts, err, _, folder),) = copies(s) + assert (status, attempts, err) == ("done", 1, None) and folder.startswith("gone/") + assert open_filing_events(s, pid) == [] + cleared = q1( + s, + "SELECT title, body FROM notification_events WHERE event_type = 'archive_failed' AND subject = %s" + " AND recovery_of IS NOT NULL", + (f"filing:{pid}",), + ) + assert cleared[0].startswith("Cleared: Filing blocked") and cleared[1] == "The packet was filed and copied." @needs_pg -def test_filing_succeeds_when_the_request_fails(shop: Shop, monkeypatch: pytest.MonkeyPatch) -> None: +def test_filing_is_refused_when_the_copy_fails(shop: Shop, monkeypatch: pytest.MonkeyPatch) -> None: s = shop.settings save_dir(s) - def boom(*a: Any, **k: Any) -> None: - raise RuntimeError("no requests today") + def boom(*a: Any, **k: Any) -> Any: + raise archive.CopyError("A different file is already at Data_Book.pdf in the archive; it was not overwritten.") - monkeypatch.setattr(archive, "request_copy", boom) + monkeypatch.setattr(archive, "copy_seal", boom) + pkt = vclose(shop, at_complete(shop)) + body = refused(file_packet(shop, pkt), 503, "archive_failed") + assert "it was not overwritten. Nothing was filed" in body["detail"] + assert q1(s, "SELECT status FROM packets WHERE id = %s", (pkt["id"],))[0] == "complete" + assert q1(s, "SELECT count(*) FROM packet_seals WHERE packet_id = %s", (pkt["id"],))[0] == 0 + ((_, text, detail, _, _),) = open_filing_events(s, pkt["id"]) + assert "not overwritten" in text and detail["archive_dir"] == "packets" + + # anything unexpected blocks the filing the same way + monkeypatch.setattr(archive, "copy_seal", lambda *a, **k: 1 / 0) + body = refused(file_packet(shop, pkt), 503, "archive_failed") + assert "ZeroDivisionError" in body["detail"] + + +@needs_pg +def test_filing_with_no_folder_saved_copies_nothing(shop: Shop) -> None: + # ARCHIVE_ROOT is set but the Admin has not saved a folder: the archive is not in use yet, so filing goes ahead pkt = filed(shop) - assert pkt["status"] == "filed" - assert q1(s, "SELECT count(*) FROM job_requests WHERE job = 'archive-copy'")[0] == 0 - assert run_copy(s)["counts"]["copied"] == 1 # the scheduled run picks it up + assert pkt["status"] == "filed" and copies(shop.settings) == [] @needs_pg