{name}
" + f"{recorded}
" + f"Open this print in Bambuddy to change it.
", + ) + + +def _render_confirm_prompt_page(request: Request, archive: PrintArchive, verdict: str) -> str: + """The page a one-tap verdict link opens. It has recorded nothing yet. + + GET is where link unfurlers, mail-security scanners and browser prefetchers + arrive, uninvited and within seconds of the message being sent, so GET + writes nothing at all -- the verdict is recorded by the form below, over + POST, which none of them issue. + + The form submits itself only for a page opened from a notification button, + which is what keeps the operator at one tap. The marker for that + (``?tap=1``) is put on the Telegram inline keyboard's URLs and nowhere else + -- never on a URL that travels in message text -- so a mail-security + sandbox that renders HTML and runs JavaScript cannot press the button for + the operator: it only ever sees the unmarked URL out of the body. The + User-Agent heuristic still runs on top of that, but it is no longer the + only thing between a scanner and the write; it cannot be, because such a + sandbox sends an ordinary Chrome string. + + Every other arrival -- an unmarked link somebody typed or mailed, a browser + with JavaScript off -- gets the same page and presses the button. + """ + name = html_escape(archive.print_name or archive.filename or "") + label = _VERDICT_LABELS.get(verdict, verdict) + # No action attribute: the form posts back to the URL the page was loaded + # from, so it works behind a reverse proxy and on a host external_url does + # not name. + form = ( + "" + ) + # The SPA's CSP allows inline scripts only with the per-request nonce the + # security-headers middleware mints (main.py). Without one -- no middleware, + # an unmarked URL, or an unattended-looking caller -- the page simply waits + # for the button. + nonce = getattr(request.state, "csp_nonce", None) + script = "" + if nonce and is_one_tap_request(request.query_params) and not is_unattended_fetch(request.method, request.headers): + script = ( + f"" + ) + return _confirm_page( + "?", + "Confirm this outcome", + f"{name}
" + f"Record this print as {label}?
" + f"{form}{script}", + ) + + +def _confirm_response(html: str): + """The one-tap pages, never cached. + + A proxy holding on to "Saved" or to the prompt would answer a later tap + from its cache, and the prompt page is a capability URL either way. + """ + from fastapi.responses import HTMLResponse + + return HTMLResponse(html, headers={"Cache-Control": "no-store"}) + + +async def _load_confirmable_archive(db: AsyncSession, token: str, verdict: str) -> PrintArchive: + """Resolve a one-tap capability token, or raise the route's 400/404.""" + if verdict not in ("good", "reject"): + raise HTTPException(400, "Verdict must be 'good' or 'reject'") + + result = await db.execute( + select(PrintArchive).where(PrintArchive.confirm_token == token, PrintArchive.confirm_token.isnot(None)) + ) + archive = result.scalar_one_or_none() + if not archive: + raise HTTPException(404, "Confirmation link is invalid or was already used") + return archive + + +@router.get("/confirm/{token}/{verdict}") +async def confirm_outcome_page( + request: Request, + token: str, + verdict: str, + db: AsyncSession = Depends(get_db), +): + """Open a one-tap verdict link. Reads only — the verdict is recorded by POST. + + This used to be the route that recorded, and that was the defect: a GET + that changes state is answered by everything that walks a URL. Telegram + and Slack fetch the links in a message body to build a preview card, mail + gateways detonate them before delivery, browsers prefetch them — any one + of those spent the single-use token and settled the outcome before the + operator had read the question, always in the "good" direction because + good_url came first. Suppressing previews per channel does not cover the + proxies and scanners in between; only removing the write from GET does. + + So GET now hands back the prompt page and nothing else. The page's form + POSTs to this same URL, and :func:`confirm_outcome_by_token` records it. + The human cost is zero for the path the feature is built around: a URL + opened from a notification button carries ``?tap=1`` and the page submits + itself, so that tap is still the only tap. Nothing else gets that script, + including a scanner that runs JavaScript -- the marker is on the buttons, + not in the message text a scanner reads. + """ + archive = await _load_confirmable_archive(db, token, verdict) + + if archive.confirm_token_used_at is not None: + # Answered already — by hand, by the other link, by the plate-clear + # default or by a reaction. Report what is on file and change nothing. + return _confirm_response(await _render_already_answered_page(db, archive)) + + return _confirm_response(_render_confirm_prompt_page(request, archive, verdict)) + + +@router.post("/confirm/{token}/{verdict}") +async def confirm_outcome_by_token( + token: str, + verdict: str, + db: AsyncSession = Depends(get_db), +): + """Record a print-outcome verdict via the capability token from a push notification. + + Deliberately unauthenticated: the token IS the credential. It is a 256-bit + per-archive capability, minted when the confirmation prompt fires, and it + only ever grants writing good/reject on that one archive, exactly once. + "Exactly once" is enforced by ``confirm_token_used_at`` rather than by + dropping the token value, so a link for a print that was already answered + can be recognised and explained instead of looking broken. + + POST is what keeps the capability the operator's: unfurlers, scanners and + prefetchers issue GET, and the GET route above writes nothing. The two + callers that reach here are the prompt page's form and the ntfy action + button, which performs its own request from the phone and is configured + with ``method=POST``. Returns a small HTML page for the phone browser. + """ + archive = await _load_confirmable_archive(db, token, verdict) + + if archive.confirm_token_used_at is not None: + return _confirm_response(await _render_already_answered_page(db, archive)) + + archive.user_verdict = verdict + stamp_verdict(archive, "link") + retire_confirm_token(archive) + + # Same mirror as the PATCH route (#1444): verdict-aware statistics read + # print_log_entries, so the latest run must carry the verdict too. + from backend.app.models.print_log import PrintLogEntry + + latest_entry = await db.scalar( + select(PrintLogEntry).where(PrintLogEntry.archive_id == archive.id).order_by(PrintLogEntry.id.desc()).limit(1) + ) + if latest_entry is not None: + latest_entry.user_verdict = verdict + + await db.commit() + + label = "Good part" if verdict == "good" else "Rejected" + name = archive.print_name or archive.filename + return _confirm_response( + _confirm_page( + "✓" if verdict == "good" else "✗", + label, + f"{html_escape(name)}
" + "Saved — you can close this page.
", + ) + ) + + # ============================================ # QR Code Endpoint # ============================================ diff --git a/backend/app/api/routes/library.py b/backend/app/api/routes/library.py index add65d13e..5f4c098fd 100644 --- a/backend/app/api/routes/library.py +++ b/backend/app/api/routes/library.py @@ -78,6 +78,7 @@ from backend.app.services.design_settings import ( from backend.app.services.filament_requirements import annotate_rack_groups from backend.app.services.pdf_thumbnail import generate_pdf_thumbnail from backend.app.services.plate_thumbnail import inject_plate_thumbnails_if_missing +from backend.app.services.print_confirmation import confirm_outcome_for_new_queue_item from backend.app.services.process_overrides import apply_process_overrides from backend.app.services.slice_output_check import ( missing_start_gcode_message, @@ -2972,6 +2973,10 @@ async def add_files_to_queue( pos_result = await db.execute(select(func.coalesce(func.max(PrintQueueItem.position), 0))) max_position = pos_result.scalar() or 0 + # There is no per-job ask-for-outcome toggle on a bulk add, so the rows take + # the same default the print dialog seeds itself from (#1898). + confirm_outcome = await confirm_outcome_for_new_queue_item(db) + for file_id in request.file_ids: lib_file = files.get(file_id) @@ -3065,6 +3070,7 @@ async def add_files_to_queue( or (folder_projects.get(lib_file.folder_id) if lib_file.folder_id is not None else None), position=max_position, status="pending", + confirm_outcome=confirm_outcome, # Without this the row is ownerless, and `queue:read_own` filters # on `created_by_id` — so the user who queued the file could not # see it in their own queue. diff --git a/backend/app/api/routes/notifications.py b/backend/app/api/routes/notifications.py index dc1f242f5..81bce5d86 100644 --- a/backend/app/api/routes/notifications.py +++ b/backend/app/api/routes/notifications.py @@ -66,6 +66,8 @@ def _provider_to_dict(provider: NotificationProvider) -> dict: # Build plate detection "on_plate_not_empty": provider.on_plate_not_empty, "on_plate_clear_required": provider.on_plate_clear_required, + # Post-print outcome confirmation (#1898) + "on_print_confirm_request": provider.on_print_confirm_request, # Bed cooled "on_bed_cooled": provider.on_bed_cooled, # First layer complete @@ -158,6 +160,8 @@ async def create_notification_provider( # Build plate detection on_plate_not_empty=provider_data.on_plate_not_empty, on_plate_clear_required=provider_data.on_plate_clear_required, + # Post-print outcome confirmation (#1898) + on_print_confirm_request=provider_data.on_print_confirm_request, # Bed cooled on_bed_cooled=provider_data.on_bed_cooled, # First layer complete diff --git a/backend/app/api/routes/pipeline_runs.py b/backend/app/api/routes/pipeline_runs.py index bf649815c..af06ccf29 100644 --- a/backend/app/api/routes/pipeline_runs.py +++ b/backend/app/api/routes/pipeline_runs.py @@ -61,6 +61,7 @@ from backend.app.services.pipeline_eligibility import ( EligibilityReport, check_pipeline_eligibility, ) +from backend.app.services.print_confirmation import confirm_outcome_for_new_queue_item logger = logging.getLogger(__name__) @@ -571,6 +572,9 @@ def _make_orchestration_callable( if len(jobs) != copies: logger.warning("pipeline_run %d expected %d jobs, found %d", run_id, copies, len(jobs)) + # No per-job ask-for-outcome toggle on a pipeline run either (#1898). + confirm_outcome = await confirm_outcome_for_new_queue_item(session) + for job, (printer_id, target_model) in zip(jobs, assignments, strict=False): queue_item = PrintQueueItem( printer_id=printer_id, @@ -578,6 +582,7 @@ def _make_orchestration_callable( library_file_id=slice_response.library_file_id, created_by_id=creator_user_id, status="pending", + confirm_outcome=confirm_outcome, ) session.add(queue_item) await session.flush() diff --git a/backend/app/api/routes/print_queue.py b/backend/app/api/routes/print_queue.py index 6c71c27b6..4c3a9c586 100644 --- a/backend/app/api/routes/print_queue.py +++ b/backend/app/api/routes/print_queue.py @@ -472,6 +472,7 @@ def _enrich_response(item: PrintQueueItem) -> PrintQueueItemResponse: "timelapse": item.timelapse, "use_ams": item.use_ams, "nozzle_offset_cali": item.nozzle_offset_cali, + "confirm_outcome": item.confirm_outcome, "preheat_override": item.preheat_override, "preheat_chamber_target_override": item.preheat_chamber_target_override, "status": item.status, @@ -1193,6 +1194,7 @@ async def add_to_queue( timelapse=data.timelapse, use_ams=data.use_ams, nozzle_offset_cali=data.nozzle_offset_cali, + confirm_outcome=data.confirm_outcome, preheat_override=data.preheat_override, preheat_chamber_target_override=data.preheat_chamber_target_override, gcode_injection=data.gcode_injection, diff --git a/backend/app/api/routes/printers.py b/backend/app/api/routes/printers.py index 56bad42f4..e58bbbffa 100644 --- a/backend/app/api/routes/printers.py +++ b/backend/app/api/routes/printers.py @@ -3537,6 +3537,21 @@ async def clear_plate( printer_manager.set_awaiting_plate_clear(printer_id, False) + # #1898: releasing the plate without answering the outcome prompt can + # count as "good" (opt-in setting) — this is the moment the operator + # moves on, so an unanswered prompt would otherwise linger unconfirmed. + from backend.app.api.routes.settings import get_setting, setting_is_true + + # setting_is_true rather than a comparison of our own: one reader deciding + # for itself what "on" spells is how two parts of the app end up + # disagreeing about the same row. + if setting_is_true(await get_setting(db, "confirm_default_good_on_plate_clear")): + from backend.app.services.print_confirmation import resolve_pending_confirmation_as_good + + resolved = await resolve_pending_confirmation_as_good(db, printer_id) + if resolved is not None: + await db.commit() + return {"success": True, "message": "Plate cleared, next print will start shortly"} diff --git a/backend/app/api/routes/projects.py b/backend/app/api/routes/projects.py index f572699dc..129cd98a6 100644 --- a/backend/app/api/routes/projects.py +++ b/backend/app/api/routes/projects.py @@ -11,7 +11,7 @@ from pathlib import Path from fastapi import APIRouter, Depends, File, HTTPException, UploadFile from fastapi.responses import FileResponse, StreamingResponse -from sqlalchemy import case, func, select, update +from sqlalchemy import and_, case, func, or_, select, update from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload @@ -54,6 +54,12 @@ router = APIRouter(prefix="/projects", tags=["projects"]) _FAILURE_STATUSES = ("failed", "aborted", "cancelled", "stopped") +# A completed run whose user verdict is 'reject' (#1898) finished on the +# machine but produced scrap — everywhere a project counts good parts, that +# run must not contribute. NULL verdict (never asked / not answered) counts +# as good, matching behaviour before the feature existed. +_NOT_REJECTED = or_(PrintLogEntry.user_verdict.is_(None), PrintLogEntry.user_verdict != "reject") + # Soft-deleted archives (#1343) keep their row — and therefore their # ``project_id`` — after their files have been removed from disk, so that global # Quick Stats can still count their filament / time / cost. Nothing in this @@ -140,8 +146,12 @@ async def _load_totals(db: AsyncSession, project_ids: Sequence[int]) -> dict[int func.coalesce(func.sum(PrintLogEntry.energy_kwh), 0).label("total_energy"), func.coalesce(func.sum(PrintLogEntry.energy_cost), 0).label("total_energy_cost"), func.coalesce(func.sum(PrintArchive.quantity), 0).label("total_items"), + # A completed run the user marked as reject (#1898) produced no + # usable parts — keep it out of the good-parts count. func.coalesce( - func.sum(case((PrintLogEntry.status == "completed", PrintArchive.quantity), else_=0)), + func.sum( + case((and_(PrintLogEntry.status == "completed", _NOT_REJECTED), PrintArchive.quantity), else_=0) + ), 0, ).label("completed_items"), func.coalesce( @@ -423,7 +433,9 @@ async def list_projects( func.count(PrintLogEntry.id).label("archive_count"), func.coalesce(func.sum(PrintArchive.quantity), 0).label("total_items"), func.coalesce( - func.sum(case((PrintLogEntry.status == "completed", PrintArchive.quantity), else_=0)), + func.sum( + case((and_(PrintLogEntry.status == "completed", _NOT_REJECTED), PrintArchive.quantity), else_=0) + ), 0, ).label("completed_count"), func.coalesce( @@ -1009,7 +1021,12 @@ async def get_project_file_progress( func.count(PrintLogEntry.id), ) .join(PrintArchive, PrintArchive.id == PrintLogEntry.archive_id) - .where(PrintArchive.project_id == project_id, PrintLogEntry.status == "completed", _LIVE_ARCHIVE) + .where( + PrintArchive.project_id == project_id, + PrintLogEntry.status == "completed", + _NOT_REJECTED, + _LIVE_ARCHIVE, + ) .group_by(PrintArchive.library_file_id, PrintArchive.content_hash, PrintArchive.filename) ) diff --git a/backend/app/api/routes/settings.py b/backend/app/api/routes/settings.py index 2bc9582cb..4ae0d2bd2 100644 --- a/backend/app/api/routes/settings.py +++ b/backend/app/api/routes/settings.py @@ -129,6 +129,22 @@ def normalize_str_setting(key: str, value: object) -> str: raise HTTPException(400, f"{key} must be a string; got {type(value).__name__}") +async def get_external_base_url(db: AsyncSession) -> str: + """Base URL for links Bambuddy hands to the outside world (no trailing slash). + + ``external_url`` is optional and has no default, so anything that must be + absolute — a login link in an e-mail, the one-tap outcome verdict links + (#1898), whose Telegram/ntfy buttons are dropped for a relative URL — falls + back to APP_URL and finally to the dev origin. + """ + import os + + external_url = await get_setting(db, "external_url") + if external_url: + return external_url.rstrip("/") + return os.environ.get("APP_URL", "http://localhost:5173").rstrip("/") + + async def get_external_login_url(db: AsyncSession) -> str: """Get the external URL for the login page. @@ -140,14 +156,7 @@ async def get_external_login_url(db: AsyncSession) -> str: Returns: Full URL to the login page """ - import os - - external_url = await get_setting(db, "external_url") - if external_url: - external_url = external_url.rstrip("/") - else: - external_url = os.environ.get("APP_URL", "http://localhost:5173") - return external_url + "/login" + return await get_external_base_url(db) + "/login" async def set_setting(db: AsyncSession, key: str, value: str) -> None: @@ -199,6 +208,9 @@ async def _build_settings_response(db: AsyncSession, is_api_key: bool = False) - "default_vibration_cali", "default_layer_inspect", "default_timelapse", + "default_confirm_outcome", + "confirm_outcome_external_prints", + "confirm_default_good_on_plate_clear", "billing_enabled", "printer_kill_switch_enabled", "ldap_enabled", diff --git a/backend/app/api/routes/webhook.py b/backend/app/api/routes/webhook.py index da2940843..0a0ca91b5 100644 --- a/backend/app/api/routes/webhook.py +++ b/backend/app/api/routes/webhook.py @@ -11,6 +11,7 @@ from backend.app.models.api_key import APIKey from backend.app.models.archive import PrintArchive from backend.app.models.print_queue import PrintQueueItem from backend.app.models.printer import Printer +from backend.app.services.print_confirmation import confirm_outcome_for_new_queue_item from backend.app.services.printer_manager import printer_manager logger = logging.getLogger(__name__) @@ -115,6 +116,9 @@ async def webhook_add_to_queue( scheduled_time=scheduled_time, require_previous_success=data.require_previous_success, auto_off_after=data.auto_off_after, + # No dialog to pick this per job, so the install-wide default decides + # whether the finished print asks for a verdict (#1898). + confirm_outcome=await confirm_outcome_for_new_queue_item(db), # Attribute to the key's owner so the item shows up under `queue:read_own` # for the person whose key it is. Legacy keys predating per-user ownership # have no `user_id`, and those rows stay ownerless. diff --git a/backend/app/core/database.py b/backend/app/core/database.py index bfa8a4cb7..0075d7f89 100644 --- a/backend/app/core/database.py +++ b/backend/app/core/database.py @@ -5022,6 +5022,45 @@ async def run_migrations(conn): conn, "ALTER TABLE notification_providers ADD COLUMN on_location_ha_sensor_alert BOOLEAN DEFAULT FALSE" ) + # Migration: post-print outcome confirmation (#1898). VARCHAR and the + # BOOLEAN DEFAULT FALSE/TRUE spellings are identical on SQLite and + # Postgres (see the on_ha_sensor_alert note above for why not DEFAULT 0). + await _safe_execute(conn, "ALTER TABLE print_archives ADD COLUMN user_verdict VARCHAR(10)") + await _safe_execute(conn, "ALTER TABLE print_archives ADD COLUMN confirm_requested BOOLEAN DEFAULT FALSE") + await _safe_execute(conn, "ALTER TABLE print_archives ADD COLUMN confirm_token VARCHAR(64)") + await _safe_execute(conn, "ALTER TABLE print_archives ADD COLUMN user_verdict_source VARCHAR(16)") + # ``DATETIME`` is a SQLite-only alias; PostgreSQL rejects it and + # _safe_execute would swallow the error, leaving the column missing. + if is_sqlite(): + await _safe_execute(conn, "ALTER TABLE print_archives ADD COLUMN confirm_token_used_at DATETIME") + else: + await _safe_execute(conn, "ALTER TABLE print_archives ADD COLUMN confirm_token_used_at TIMESTAMP") + # When the verdict on file was recorded (#1898): the "already answered" + # page dates the verdict by this, not by the moment the one-tap token was + # spent, so a verdict changed later in the app reads correctly. + if is_sqlite(): + await _safe_execute(conn, "ALTER TABLE print_archives ADD COLUMN user_verdict_at DATETIME") + else: + await _safe_execute(conn, "ALTER TABLE print_archives ADD COLUMN user_verdict_at TIMESTAMP") + await _safe_execute(conn, "ALTER TABLE print_log_entries ADD COLUMN user_verdict VARCHAR(10)") + await _safe_execute(conn, "ALTER TABLE print_queue ADD COLUMN confirm_outcome BOOLEAN DEFAULT FALSE") + await _safe_execute( + conn, "ALTER TABLE notification_providers ADD COLUMN on_print_confirm_request BOOLEAN DEFAULT TRUE" + ) + # The one-tap verdict route looks archives up by this token and runs with no + # authentication, so an upgraded install needs the index too — without it + # every tap, and every unauthenticated request carrying a bogus token, is a + # sequential scan of print_archives. + await _safe_execute( + conn, + "CREATE UNIQUE INDEX IF NOT EXISTS ix_print_archives_confirm_token ON print_archives (confirm_token)", + ) + # Migration: take the capability URLs out of the outcome prompt's body + # (#1898). Seeding only ever inserts a template that is missing, so an + # install that already ran an earlier build of this feature would keep + # sending the verdict links as body text for every channel. + await _migrate_confirm_prompt_body_template(conn) + # Migration: rename the ha_sensor_alert template (#2824). "Home Assistant # Sensor Alert" was fine as a name while it was the only such template; # next to the new "Storage Location Sensor Alert" it no longer says which @@ -5057,6 +5096,32 @@ async def run_migrations(conn): ) +async def _migrate_confirm_prompt_body_template(conn) -> None: + """Replace the one-tap verdict URLs in the outcome prompt's body (#1898). + + The first shape of this template put ``{good_url}`` and ``{reject_url}`` + into the message body, where a link unfurler, a mail gateway or a proxy + reaches them and spends the single-use token before the operator has read + the question. The body now carries ``{confirm_url}``, which only opens the + archive in Bambuddy; the capability links travel in the ntfy action buttons + and the Telegram inline keyboard instead. + + Rewrites only a body that is still the old default verbatim — an admin who + edited the template keeps their own text. Same shape as the two template + renames below. + """ + from sqlalchemy import text + + await conn.execute( + text("UPDATE notification_templates SET body_template = :new WHERE event_type = :et AND body_template = :old"), + { + "new": "{printer}: {filename}\nConfirm: {confirm_url}", + "et": "print_confirm_request", + "old": "{printer}: {filename}\nGood: {good_url}\nReject: {reject_url}", + }, + ) + + async def _migrate_rename_ha_sensor_alert_template(conn) -> None: """Rename the ha_sensor_alert template to "Printer Sensor Alert" (#2824). diff --git a/backend/app/core/websocket.py b/backend/app/core/websocket.py index 4174894c7..ca2c26014 100644 --- a/backend/app/core/websocket.py +++ b/backend/app/core/websocket.py @@ -109,6 +109,16 @@ class ConnectionManager: } ) + async def send_print_confirm_request(self, printer_id: int, data: dict): + """Ask connected clients for a post-print outcome verdict (#1898).""" + await self.broadcast( + { + "type": "print_confirm_request", + "printer_id": printer_id, + "data": data, + } + ) + async def send_archive_created(self, archive: dict): """Notify clients that a new archive was created.""" await self.broadcast( diff --git a/backend/app/main.py b/backend/app/main.py index c9e6c8982..84a2fb89f 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -3397,6 +3397,190 @@ def _schedule_fallback_3mf_retry( ) +async def _ask_outcome_for_external_print(db, printer_id: int, observed_name: str | None = None) -> bool: + """Whether an archive created here should ask for the print's outcome (#1898). + + Only a print Bambuddy did not dispatch reaches the archive-*creating* + branches below: a queued job already has its archive and takes the + expected-print branch, where the queue item's own ``confirm_outcome`` + decides. The queue is still consulted, because a restart mid-print empties + ``_expected_prints`` — without the check the setting could override a queue + item that deliberately has the flag off. Any failure answers "don't ask": + an unwanted prompt is worse than a missing one, and this must never be the + reason a print goes unarchived. + + A ``printing`` row is not proof on its own, though: Bambuddy deliberately + leaves one behind when a completion cannot be matched to it + (``_completion_belongs_to_queue_item``), and the scheduler's stranded sweep + only takes it back once the printer has sat connected and terminal for + minutes. Until then every screen-started print on that printer would + silently lose its prompt, so the row is held against ``observed_name`` by + the same comparison a completion uses: a positive disagreement means the row + is about some other run and this print is external after all. + """ + logger = logging.getLogger(__name__) + try: + # A savepoint rather than a rollback of the caller's transaction on + # failure: a failed statement leaves the transaction unusable, but a + # full rollback also expires every object the caller has loaded (the + # printer, the archive it just created), and on an async session the + # next attribute read then raises instead of reloading. Everything here + # is a read, so undoing the savepoint loses nothing. + async with db.begin_nested(): + return await _decide_outcome_for_external_print(db, printer_id, observed_name, logger) + except Exception as e: + logger.warning("[CALLBACK] Could not decide the outcome prompt for printer %s: %s", printer_id, e) + return False + + +async def _decide_outcome_for_external_print(db, printer_id: int, observed_name: str | None, logger) -> bool: + """The reads behind ``_ask_outcome_for_external_print``; see there.""" + from backend.app.api.routes.settings import get_setting, setting_is_true + + if not setting_is_true(await get_setting(db, "confirm_outcome_external_prints")): + return False + + from backend.app.models.print_queue import PrintQueueItem + + dispatched_here = await db.scalar( + select(PrintQueueItem) + .where( + PrintQueueItem.printer_id == printer_id, + PrintQueueItem.status == "printing", + ) + .limit(1) + ) + if dispatched_here is None: + return True + + expected = await _queue_item_dispatched_name(db, dispatched_here) + observed = (observed_name or "").strip() + if not expected or not observed or _subtask_names_match(expected, observed): + logger.info( + "[CALLBACK] Not asking for the outcome on printer %s: queue item %s is still printing, so " + "Bambuddy dispatched this run and the item's own ask-for-outcome flag decides.", + printer_id, + dispatched_here.id, + ) + return False + + logger.info( + "[CALLBACK] Queue item %s is still marked printing on printer %s but was dispatched as %r, not " + "%r; treating this as an externally started print.", + dispatched_here.id, + printer_id, + expected, + observed, + ) + return True + + +async def _dispatch_outcome_confirmation_safely( + db, + printer_id: int, + printer_name: str, + data: dict, + archive_id: int, + archive_data: dict | None = None, +) -> None: + """``dispatch_outcome_confirmation`` for a caller that has more to do on ``db``. + + The completion task sends the per-user print email on the same session + right after the prompt. A failed statement in the prompt leaves that + session needing a rollback, and without one the email step fails with + PendingRollbackError; the prompt is the optional part, so it must not be + the reason the email never goes out. + """ + try: + await dispatch_outcome_confirmation(db, printer_id, printer_name, data, archive_id, archive_data) + except Exception as e: + logging.getLogger(__name__).error("[NOTIFY-BG] Outcome-confirmation dispatch failed: %s", e, exc_info=True) + try: + await db.rollback() + except Exception: + pass + + +async def dispatch_outcome_confirmation( + db, + printer_id: int, + printer_name: str, + data: dict, + archive_id: int, + archive_data: dict | None = None, +) -> bool: + """Emit the post-print outcome prompt for a completed archive (#1898). + + Gated on the archive itself: ``confirm_requested`` is the opt-in (from the + queue item, or from ``confirm_outcome_external_prints`` for a print + Bambuddy did not start) and ``user_verdict`` being unset is what makes the + question still open. Mints the per-archive capability token the one-tap + verdict links carry. + + Lives here rather than inline in ``on_print_complete``'s notification task + because that task swallows every exception: extracted, the gate and the two + emissions can be driven by a test, which is the only thing standing between + a regression here and a farm that quietly stops asking. + + Returns whether a prompt was sent. + """ + logger = logging.getLogger(__name__) + from backend.app.models.archive import PrintArchive + + confirm_archive = (await db.execute(select(PrintArchive).where(PrintArchive.id == archive_id))).scalar_one_or_none() + if not (confirm_archive and confirm_archive.confirm_requested and confirm_archive.user_verdict is None): + return False + + import secrets as _secrets + + # A spent token stays on the row so the one-tap route can recognise it, so + # "has a token" no longer means "answerable". A prompt going out now needs a + # live one: mint a new token whenever the stored one is already spent, or + # every button in the new message would land on the already-answered page. + if not confirm_archive.confirm_token or confirm_archive.confirm_token_used_at is not None: + confirm_archive.confirm_token = _secrets.token_urlsafe(32) + confirm_archive.confirm_token_used_at = None + await db.commit() + + from backend.app.api.routes.settings import get_external_base_url, get_setting + + base = await get_external_base_url(db) + if not await get_setting(db, "external_url"): + # Both the Telegram inline keyboard and the ntfy action buttons need an + # absolute URL, so an unconfigured install used to get a message body + # with two unusable relative paths and no buttons at all. The shared + # fallback at least produces tappable links; say which setting makes + # them resolve from a phone. + logger.warning( + "[#1898] No external_url configured — the outcome prompt's Good/Reject links point at %s. " + "Set Settings → External URL so they resolve away from this host.", + base, + ) + token = confirm_archive.confirm_token + good_url = f"{base}/api/v1/archives/confirm/{token}/good" + reject_url = f"{base}/api/v1/archives/confirm/{token}/reject" + confirm_url = f"{base}/archives?confirm={archive_id}" + + await ws_manager.send_print_confirm_request( + printer_id, + { + "archive_id": archive_id, + "print_name": confirm_archive.print_name or confirm_archive.filename, + }, + ) + await notification_service.on_print_confirm_request( + printer_id, + printer_name, + data, + db, + archive_data=archive_data, + good_url=good_url, + reject_url=reject_url, + confirm_url=confirm_url, + ) + return True + + async def on_print_start(printer_id: int, data: dict): """Handle print start - archive the 3MF file immediately.""" logger = logging.getLogger(__name__) @@ -3721,6 +3905,18 @@ async def on_print_start(printer_id: int, data: dict): archive.status = "printing" archive.started_at = datetime.now(timezone.utc) + # The previous run's answer is still on this row and the + # completion prompt is gated on ``user_verdict is None`` (#1898), + # so without a reset the second run inherits the first run's + # verdict: no prompt at all, and a green "good" badge on a run + # nobody ever judged. + if archive.confirm_requested: + archive.user_verdict = None + archive.user_verdict_source = None + archive.user_verdict_at = None + archive.confirm_token = None + archive.confirm_token_used_at = None + # Reprint of an archive reuses the source row. Without resetting # ``timelapse_path`` _scan_for_timelapse_with_retries early-returns # ("already has timelapse") and _capture_finish_photo_from_timelapse @@ -4529,6 +4725,7 @@ async def on_print_start(printer_id: int, data: dict): status="printing", started_at=datetime.now(timezone.utc), subtask_id=subtask_id, + confirm_requested=await _ask_outcome_for_external_print(db, printer_id, subtask_name), filament_type=mqtt_filament_meta.get("filament_type"), filament_color=mqtt_filament_meta.get("filament_color"), extra_data={ @@ -4662,6 +4859,35 @@ async def on_print_start(printer_id: int, data: dict): ) if archive: + # Ask-for-outcome for a print Bambuddy did not dispatch (#1898). + # Set on the row rather than passed to archive_print, which also + # serves the queue dispatcher — there the queue item decides. + # Guarded because this branch has no ``except``: an unhandled + # write error would take the _active_prints registration, the + # start notification, the energy reading and the timelapse + # baseline below it with it, and a missing prompt is by far the + # cheaper failure. + archive_id = archive.id + try: + if await _ask_outcome_for_external_print(db, printer_id, subtask_name): + archive.confirm_requested = True + await db.commit() + except Exception as e: + logger.warning("Could not flag archive %s for the outcome prompt: %s", archive_id, e) + # The rollback expires every loaded object, and on an async + # session reading one afterwards raises instead of + # reloading, so the two this branch goes on to use are + # fetched again. The archive itself was committed by + # archive_print; only the flag is lost. + try: + await db.rollback() + archive = await db.get(PrintArchive, archive_id) + printer = await db.get(Printer, printer_id) + except Exception as reload_error: + logger.warning( + "Could not reload archive %s after the failed flag write: %s", archive_id, reload_error + ) + # Track this active print (use both original filename and downloaded filename) _active_prints[(printer_id, downloaded_filename)] = archive.id if filename and filename != downloaded_filename: @@ -6222,6 +6448,23 @@ def _subtask_names_match(expected: str, observed: str) -> bool: return False +async def _queue_item_dispatched_name(db, item) -> str: + """The subtask name *item* was dispatched under, or "" when unknowable. + + A row with no archive, or an archive with no file name, is unverifiable + rather than wrong; every caller answers that with its permissive branch. + """ + if item.archive_id is None: + return "" + + from backend.app.models.archive import PrintArchive + + archive = await db.get(PrintArchive, item.archive_id) + if archive is None or not archive.filename: + return "" + return _subtask_name_from_filename(archive.filename) + + async def _completion_belongs_to_queue_item(db, item, data: dict) -> bool: """Whether this completion event is plausibly about *item*'s print. @@ -7681,6 +7924,14 @@ async def on_print_complete(printer_id: int, data: dict): else: logger.info("[NOTIFY-BG] Skipped duplicate kill-switch provider notification") + # Post-print outcome confirmation (#1898). Runs in this + # background task so the finish photo fetched above rides + # along with the prompt. + if print_status == "completed" and archive_id: + await _dispatch_outcome_confirmation_safely( + db, printer_id, printer_name, data, archive_id, archive_data + ) + # Send user-specific email notification if archive_data: created_by_id = archive_data.get("created_by_id") @@ -9504,6 +9755,12 @@ PUBLIC_API_PREFIXES = [ "/api/v1/ws", # OIDC authorize redirects — include provider_id in path "/api/v1/auth/oidc/authorize/", + # One-tap outcome-verdict links from push notifications (#1898). Tapped on + # a phone with no session, so no header can carry a JWT — the single-use + # capability token in the path IS the credential (same reasoning as the + # /dl/ slicer downloads below). The route grants nothing beyond writing + # good/reject on the one archive the token was minted for. + "/api/v1/archives/confirm/", ] # Route patterns that are public (read-only display data) @@ -9633,6 +9890,10 @@ async def security_headers_middleware(request, call_next): # script passes the policy without us needing 'unsafe-inline'. See # https://developers.cloudflare.com/cloudflare-challenges/challenge-types/javascript-detections/#if-you-have-a-content-security-policy-csp csp_nonce = secrets.token_urlsafe(16) + # Routes that render their own HTML need it too, or their inline script is + # blocked by the policy below. The outcome-confirmation page (#1898) is the + # one that does: it submits its own form so a verdict still costs one tap. + request.state.csp_nonce = csp_nonce response = await call_next(request) response.headers["X-Content-Type-Options"] = "nosniff" # X-Frame-Options is the legacy cross-origin embedding control. Modern diff --git a/backend/app/models/archive.py b/backend/app/models/archive.py index a185dedf8..f459f289f 100644 --- a/backend/app/models/archive.py +++ b/backend/app/models/archive.py @@ -106,6 +106,41 @@ class PrintArchive(Base): failure_reason: Mapped[str | None] = mapped_column(String(100)) # For failed prints quantity: Mapped[int] = mapped_column(Integer, default=1) # Number of items printed + # Post-print outcome confirmation (#1898). user_verdict is the USER's + # quality judgement ('good' / 'reject'), deliberately orthogonal to the + # machine-reported `status`: completed + reject means "printer finished + # it, part is scrap". confirm_requested is copied from the queue item's + # opt-in flag at dispatch (like plate_id) and drives the prompt + the + # "unconfirmed" badge; confirm_token is a per-archive capability for the + # one-tap verdict links in push notifications, minted when the prompt + # fires. A landed verdict RETIRES the token by stamping + # confirm_token_used_at rather than clearing the value: the link stays + # resolvable so a second tap can say "already answered, here is what was + # recorded" instead of the bare "invalid or already used" 404. + # user_verdict_source records how the verdict arrived ('dialog', 'link', + # 'plate_clear', 'printer_card', 'api', 'reaction') so the UI can explain + # a verdict nobody remembers giving. + user_verdict: Mapped[str | None] = mapped_column(String(10), nullable=True) + user_verdict_source: Mapped[str | None] = mapped_column(String(16), nullable=True) + # When the verdict on file was recorded (#1898). Written with every verdict, + # unlike `confirm_token_used_at`, which marks the one moment the one-tap + # capability was spent — a verdict changed later in the app must not be + # dated by that older event. + user_verdict_at: Mapped[datetime | None] = mapped_column(DateTime, nullable=True) + # Nullable to match the ALTER that adds it to an existing install: a fresh + # database would otherwise get NOT NULL while an upgraded one gets a + # nullable column, and the two would disagree about the same table. The + # default still means every row written by Bambuddy is True or False; only + # `is_(True)` and truthiness read it, both of which treat NULL as off. + confirm_requested: Mapped[bool | None] = mapped_column(Boolean, nullable=True, default=False, server_default="0") + # Indexed and unique: the one-tap route is reachable with no credential + # at all, so an unindexed lookup would let anyone turn a stream of + # garbage tokens into a stream of full scans of this table. Uniqueness + # costs nothing (the only writer is secrets.token_urlsafe(32)) and keeps + # scalar_one_or_none from ever raising MultipleResultsFound. + confirm_token: Mapped[str | None] = mapped_column(String(64), nullable=True, unique=True, index=True) + confirm_token_used_at: Mapped[datetime | None] = mapped_column(DateTime, nullable=True, default=None) + # Energy tracking energy_kwh: Mapped[float | None] = mapped_column(Float) # Energy consumed in kWh energy_cost: Mapped[float | None] = mapped_column(Float) # Cost of energy consumed diff --git a/backend/app/models/notification.py b/backend/app/models/notification.py index ef1253b3a..428225b93 100644 --- a/backend/app/models/notification.py +++ b/backend/app/models/notification.py @@ -101,6 +101,11 @@ class NotificationProvider(Base): # Off by default: fires after every print, alongside the print-complete alert (#2525) on_plate_clear_required = Column(Boolean, default=False) # Print ended, queue gated until plate is confirmed clear + # Print asked for an outcome verdict (#1898). Defaults ON: it only ever + # fires for prints where the user opted in per-job, so the provider-level + # toggle exists to silence a channel, not to enable the feature. + on_print_confirm_request = Column(Boolean, default=True) + # Event triggers - Bed cooled after print on_bed_cooled = Column(Boolean, default=False) # Bed cooled below threshold after print on_first_layer_complete = Column(Boolean, default=False) # First layer finished printing diff --git a/backend/app/models/notification_template.py b/backend/app/models/notification_template.py index 9e0be4ba9..7a20d7aad 100644 --- a/backend/app/models/notification_template.py +++ b/backend/app/models/notification_template.py @@ -55,6 +55,20 @@ DEFAULT_TEMPLATES = [ "title_template": "Print {progress}% Complete", "body_template": "{printer}: {filename}\nRemaining: {remaining_time}", }, + { + # Post-print outcome confirmation (#1898). The body carries + # {confirm_url}, which only opens the archive in Bambuddy: anything + # that walks a URL out of a message body — a preview card, a mail + # gateway, a proxy — then reaches a page that changes nothing. + # {good_url} / {reject_url} are the single-use capability links and + # stay available as variables, but they travel by default only in the + # affordances nothing prefetches: the ntfy action buttons and the + # Telegram inline keyboard, both built in notification_service. + "event_type": "print_confirm_request", + "name": "Print Outcome Confirmation", + "title_template": "How did your print come out?", + "body_template": "{printer}: {filename}\nConfirm: {confirm_url}", + }, { "event_type": "print_missing_spool_assignment", "name": "Missing Spool Assignment", diff --git a/backend/app/models/print_log.py b/backend/app/models/print_log.py index 82e479cbd..512c32a73 100644 --- a/backend/app/models/print_log.py +++ b/backend/app/models/print_log.py @@ -45,6 +45,10 @@ class PrintLogEntry(Base): energy_kwh: Mapped[float | None] = mapped_column(Float) energy_cost: Mapped[float | None] = mapped_column(Float) failure_reason: Mapped[str | None] = mapped_column(String(100)) + # User quality verdict ('good' / 'reject'), mirrored from the archive on + # PATCH exactly like status/failure_reason (#1444 mirror) so verdict-aware + # statistics can stay on this table (#1898). + user_verdict: Mapped[str | None] = mapped_column(String(10), nullable=True) thumbnail_path: Mapped[str | None] = mapped_column(String(500)) created_by_id: Mapped[int | None] = mapped_column(ForeignKey("users.id", ondelete="SET NULL"), nullable=True) created_by_username: Mapped[str | None] = mapped_column(String(100)) diff --git a/backend/app/models/print_queue.py b/backend/app/models/print_queue.py index 351e48c3b..fa029b47f 100644 --- a/backend/app/models/print_queue.py +++ b/backend/app/models/print_queue.py @@ -118,6 +118,9 @@ class PrintQueueItem(Base): use_ams: Mapped[bool] = mapped_column(Boolean, default=True) # Nozzle offset calibration — dual-nozzle printers only, MQTT-gated (#1682) nozzle_offset_cali: Mapped[str] = mapped_column(String(8), default="auto") + # Ask for a post-print outcome verdict when this job completes (#1898). + # Copied onto the archive as confirm_requested at dispatch, like plate_id. + confirm_outcome: Mapped[bool] = mapped_column(Boolean, default=False, server_default="0") # Preheat / heat-soak override (#1468). 'inherit' uses the global # preheat_enabled setting; 'on' / 'off' force the per-item decision. The diff --git a/backend/app/schemas/archive.py b/backend/app/schemas/archive.py index 91b628ef6..5906f35dc 100644 --- a/backend/app/schemas/archive.py +++ b/backend/app/schemas/archive.py @@ -32,6 +32,14 @@ class ArchiveUpdate(ArchiveBase): project_id: int | None = None # Allow changing status (e.g., clearing failed flag) status: str | None = None + # Post-print quality verdict (#1898): 'good' / 'reject'; null clears it. + user_verdict: str | None = Field(default=None, pattern="^(good|reject)$") + # Which surface the verdict came from, for the "recorded when the plate was + # cleared" hint. Only the sources a client can honestly claim are accepted; + # 'link', 'plate_clear' and 'reaction' are stamped server-side by the paths + # that own them and must not be forgeable over this route. Omitted means + # 'api' — some script or integration did it. + user_verdict_source: str | None = Field(default=None, pattern="^(dialog|printer_card|api)$") # Editable because a print archived without its 3MF has no figure at all, # and nothing else can supply one after the fact -- rescan needs a file # this archive does not have (#1820). Bounded because it feeds the filament @@ -109,6 +117,12 @@ class ArchiveResponse(BaseModel): failure_reason: str | None quantity: int = 1 # Number of items printed + # Post-print outcome confirmation (#1898) + user_verdict: str | None = None + user_verdict_source: str | None = None + user_verdict_at: datetime | None = None + confirm_requested: bool = False + # Energy tracking energy_kwh: float | None = None energy_cost: float | None = None diff --git a/backend/app/schemas/notification.py b/backend/app/schemas/notification.py index 22aff3682..6f0c05f59 100644 --- a/backend/app/schemas/notification.py +++ b/backend/app/schemas/notification.py @@ -82,6 +82,12 @@ class NotificationProviderBase(BaseModel): default=False, description="Notify when a finished print is waiting for plate-clear confirmation" ) + # Event triggers - Post-print outcome confirmation (#1898) + on_print_confirm_request: bool = Field( + default=True, + description="Notify with one-tap verdict links when a print that opted in asks for its outcome", + ) + # Event triggers - Bed cooled on_bed_cooled: bool = Field(default=False, description="Notify when bed cools after print") @@ -188,6 +194,9 @@ class NotificationProviderUpdate(BaseModel): on_plate_not_empty: bool | None = None on_plate_clear_required: bool | None = None + # Event triggers - Post-print outcome confirmation (#1898) + on_print_confirm_request: bool | None = None + # Event triggers - Bed cooled on_bed_cooled: bool | None = None diff --git a/backend/app/schemas/print_log.py b/backend/app/schemas/print_log.py index df48fe1b4..f6c492652 100644 --- a/backend/app/schemas/print_log.py +++ b/backend/app/schemas/print_log.py @@ -28,6 +28,7 @@ class PrintLogEntrySchema(BaseModel): energy_kwh: float | None = None energy_cost: float | None = None failure_reason: str | None = None + user_verdict: str | None = None # Post-print quality verdict (#1898) thumbnail_path: str | None = None created_by_id: int | None = None created_by_username: str | None = None diff --git a/backend/app/schemas/print_queue.py b/backend/app/schemas/print_queue.py index f53557e17..cafe9c2ce 100644 --- a/backend/app/schemas/print_queue.py +++ b/backend/app/schemas/print_queue.py @@ -103,6 +103,8 @@ class PrintQueueItemCreate(BaseModel): # Nozzle offset calibration — dual-nozzle printers only (#1682). The MQTT # layer ignores the value on single-nozzle printers so the wire stays "skip". nozzle_offset_cali: TriState = "auto" + # Ask for a post-print outcome verdict when this job completes (#1898) + confirm_outcome: bool = False # Preheat / heat-soak per-item override (#1468). 'inherit' uses the global # preheat_enabled setting; 'on' / 'off' force the decision. The chamber # target falls through: this override → max(filament-map[loaded tray]) → 0. @@ -155,6 +157,7 @@ class PrintQueueItemUpdate(BaseModel): timelapse: bool | None = None use_ams: bool | None = None nozzle_offset_cali: TriState | None = None + confirm_outcome: bool | None = None preheat_override: Literal["inherit", "on", "off"] | None = None preheat_chamber_target_override: int | None = Field(default=None, ge=0, le=MAX_CHAMBER_TEMP_C) # Auto-print G-code injection @@ -215,6 +218,7 @@ class PrintQueueItemResponse(BaseModel): timelapse: bool = False use_ams: bool = True nozzle_offset_cali: TriState = "auto" + confirm_outcome: bool = False preheat_override: Literal["inherit", "on", "off"] = "inherit" preheat_chamber_target_override: int | None = None status: Literal["pending", "printing", "completed", "failed", "skipped", "cancelled"] @@ -339,6 +343,7 @@ class PrintQueueBulkUpdate(BaseModel): timelapse: bool | None = None use_ams: bool | None = None nozzle_offset_cali: TriState | None = None + confirm_outcome: bool | None = None preheat_override: Literal["inherit", "on", "off"] | None = None preheat_chamber_target_override: int | None = Field(default=None, ge=0, le=MAX_CHAMBER_TEMP_C) # Auto-print G-code injection diff --git a/backend/app/schemas/settings.py b/backend/app/schemas/settings.py index 65e65a890..b6357f9e4 100644 --- a/backend/app/schemas/settings.py +++ b/backend/app/schemas/settings.py @@ -397,6 +397,24 @@ class AppSettings(BaseModel): default="auto", description="Default nozzle offset calibration option for new prints (dual-nozzle printers only)", ) + default_confirm_outcome: bool = Field( + default=False, + description="Default for asking for a post-print outcome verdict on new prints (#1898)", + ) + confirm_outcome_external_prints: bool = Field( + default=False, + description=( + "Also ask for the outcome of prints Bambuddy archived but did not dispatch — started at " + "the printer, in Bambu Studio or in the Handy app (#1898)" + ), + ) + confirm_default_good_on_plate_clear: bool = Field( + default=False, + description=( + "When the build plate is released (manual acknowledgment or next dispatch) with the " + "outcome prompt still unanswered, record the print as a good part (#1898)" + ), + ) # Staggered batch start for multi-printer jobs stagger_group_size: int = Field( @@ -737,6 +755,9 @@ class AppSettingsUpdate(BaseModel): default_layer_inspect: bool | None = None default_timelapse: bool | None = None default_nozzle_offset_cali: TriState | None = None + default_confirm_outcome: bool | None = None + confirm_outcome_external_prints: bool | None = None + confirm_default_good_on_plate_clear: bool | None = None stagger_group_size: int | None = Field(default=None, ge=1, le=50) stagger_interval_minutes: int | None = Field(default=None, ge=1, le=60) billing_enabled: bool | None = None diff --git a/backend/app/services/bambu_mqtt.py b/backend/app/services/bambu_mqtt.py index 3da2b9ba3..7b42760eb 100644 --- a/backend/app/services/bambu_mqtt.py +++ b/backend/app/services/bambu_mqtt.py @@ -179,6 +179,40 @@ def a2l_lite_wire_ids(ams_id: int, tray_id: int) -> tuple[int, int, int] | None: ) +def resolve_external_spools_in_mapping(ams_mapping: object, ams_mapping2: object, is_dual_nozzle: bool) -> object: + """Put the external spool back into a captured flat ``ams_mapping`` (#3166). + + The firmware rejects 254/255 in the flat list, so BambuStudio and our own + dispatch both write an external spool there as -1 -- the same value as a + slot that isn't fed at all -- and carry the real target only in + ``ams_mapping2`` as ``{ams_id: 254|255, slot_id: 0}``. Capturing the flat + list alone turns "printed from the external spool" into "unmapped", and + usage then lands on whatever tray the fallback guesses. + + Each -1 whose ``ams_mapping2`` entry names an external spool becomes the + global tray the rest of Bambuddy uses: the ams_id itself on dual-nozzle + printers (254 = left/deputy, 255 = right/main), 254 on single-nozzle + printers, which have one external spool that the wire always calls 255. + ``{255, 255}`` is the unmapped marker and stays -1. Every other entry is + left exactly as captured. Without a usable ``ams_mapping2`` the input is + returned unchanged. + """ + if not isinstance(ams_mapping, list) or not isinstance(ams_mapping2, list): + return ams_mapping + if len(ams_mapping2) != len(ams_mapping): + return ams_mapping + resolved = list(ams_mapping) + for i, (flat, detail) in enumerate(zip(ams_mapping, ams_mapping2, strict=True)): + if flat != -1 or not isinstance(detail, dict): + continue + ams_id = detail.get("ams_id") + slot_id = detail.get("slot_id") + if ams_id not in (254, 255) or slot_id == 255: + continue + resolved[i] = ams_id if is_dual_nozzle else 254 + return resolved + + def apply_tray_exist_bits( units: list, tray_exist_bits_str: str | int | None, @@ -1988,7 +2022,7 @@ class BambuMQTTClient: self.state.current_project_url = url self.state.last_project_url = url if "ams_mapping" in print_data: - self._captured_ams_mapping = print_data["ams_mapping"] + self._captured_ams_mapping = self._resolve_captured_mapping(print_data) logger.info( "[%s] Captured ams_mapping from print command: %s", self.serial_number, @@ -2068,13 +2102,24 @@ class BambuMQTTClient: # already captured this print's mapping that copy is the slicer's own, # and the echo can arrive without the field at all. if self._captured_ams_mapping is None and isinstance(print_data.get("ams_mapping"), list): - self._captured_ams_mapping = print_data["ams_mapping"] + self._captured_ams_mapping = self._resolve_captured_mapping(print_data) logger.info( "[%s] Captured ams_mapping from print response: %s", self.serial_number, self._captured_ams_mapping, ) + def _resolve_captured_mapping(self, print_data: dict) -> object: + """The ``ams_mapping`` of a project_file, with external spools resolved + from its ``ams_mapping2`` (#3166).""" + from backend.app.utils.printer_models import is_dual_nozzle_model + + return resolve_external_spools_in_mapping( + print_data.get("ams_mapping"), + print_data.get("ams_mapping2"), + self._is_dual_nozzle or is_dual_nozzle_model(self.model), + ) + @staticmethod def _project_file_key(print_data: dict) -> str: """Identity of a project_file dispatch, for telling ours from a slicer's. @@ -3185,15 +3230,16 @@ class BambuMQTTClient: # slot (typically 0) when the active feed is actually the # external spool. X1C / P1S / A1 correctly report 254 in # that case; H2S does not. When the slicer-captured - # ams_mapping is all-external (every entry == -1), the - # print can only be feeding from the external spool, so - # promote tray_now to 254. Mixed (e.g. [5, -1]) and + # ams_mapping is all-external (every entry is 254, or -1 + # when the command carried no ams_mapping2 to resolve it + # from, #3166), the print can only be feeding from the + # external spool, so promote tray_now to 254. Mixed (e.g. [5, 254]) and # AMS-only mappings are NOT overridden — there's no # evidence the firmware misreports in those cases. Prints # started without a captured mapping (printer-screen start, # or before Bambuddy connected) fall through unchanged. captured = self._captured_ams_mapping - if captured and all(s == -1 for s in captured): + if captured and all(s in (-1, 254, 255) for s in captured): if self.state.tray_now != 254: logger.debug( f"[{self.serial_number}] tray_now external-spool override (#1822): " diff --git a/backend/app/services/failure_analysis.py b/backend/app/services/failure_analysis.py index eae246f87..d99cb410d 100644 --- a/backend/app/services/failure_analysis.py +++ b/backend/app/services/failure_analysis.py @@ -103,6 +103,29 @@ class FailureAnalysisService: outcome_prints = successful_prints + failed_prints failure_rate = (failed_prints / outcome_prints * 100) if outcome_prints > 0 else 0 + # Quality dimension (#1898): a completed print the user marked as + # reject is machine-success but scrap. Kept OUT of failure_rate — that + # number stays the machine's — and reported separately, with a yield + # rate that treats rejects as non-good output. + rejected_result = await self.db.execute( + select(func.count(PrintLogEntry.id)).where( + and_(*base_filter, PrintLogEntry.status == "completed", PrintLogEntry.user_verdict == "reject") + ) + ) + rejected_prints = rejected_result.scalar() or 0 + yield_rate = ((successful_prints - rejected_prints) / outcome_prints * 100) if outcome_prints > 0 else 0 + + rejects_reason_result = await self.db.execute( + select( + PrintLogEntry.failure_reason, + func.count(PrintLogEntry.id).label("count"), + ) + .where(and_(*base_filter, PrintLogEntry.status == "completed", PrintLogEntry.user_verdict == "reject")) + .group_by(PrintLogEntry.failure_reason) + .order_by(func.count(PrintLogEntry.id).desc()) + ) + rejects_by_reason = {(row[0] or "Unknown"): row[1] for row in rejects_reason_result.fetchall()} + # Failures by reason reason_result = await self.db.execute( select( @@ -257,6 +280,9 @@ class FailureAnalysisService: "total_prints": total_prints, "failed_prints": failed_prints, "failure_rate": round(failure_rate, 1), + "rejected_prints": rejected_prints, + "yield_rate": round(yield_rate, 1), + "rejects_by_reason": rejects_by_reason, "failures_by_reason": failures_by_reason, "failures_by_filament": failures_by_filament, "failures_by_printer": failures_by_printer, diff --git a/backend/app/services/notification_service.py b/backend/app/services/notification_service.py index 278a3fc9f..944deb540 100644 --- a/backend/app/services/notification_service.py +++ b/backend/app/services/notification_service.py @@ -19,6 +19,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from backend.app.models.notification import NotificationDigestQueue, NotificationLog, NotificationProvider from backend.app.models.notification_template import NotificationTemplate +from backend.app.services.print_confirmation import one_tap_url logger = logging.getLogger(__name__) @@ -318,11 +319,13 @@ class NotificationService: else: return False, f"HTTP {response.status_code}: {response.text[:200]}" - async def _send_bark(self, config: dict, title: str, message: str) -> tuple[bool, str]: + async def _send_bark(self, config: dict, title: str, message: str, url: str | None = None) -> tuple[bool, str]: """Send notification via Bark, the self-hostable iOS push service (#1495). POSTs JSON to {server}/push. Defaults to the official api.day.app relay; a self-hosted bark-server works by overriding the server URL. + ``url`` opens on tap — the outcome confirmation (#1898) deep-links + into the archive's confirmation dialog with it. """ server = (config.get("server") or "https://api.day.app").strip().rstrip("/") device_key = (config.get("device_key") or "").strip() @@ -348,6 +351,8 @@ class NotificationService: level = (config.get("level") or "").strip() if level in ("active", "timeSensitive", "critical", "passive"): payload["level"] = level + if url: + payload["url"] = url client = await self._get_client() response = await client.post(f"{server}/push", json=payload) @@ -375,8 +380,14 @@ class NotificationService: message: str, image_data: bytes | None = None, event_type: str | None = None, + actions: str | None = None, ) -> tuple[bool, str]: - """Send notification via ntfy.""" + """Send notification via ntfy. + + ``actions`` is a pre-built value for ntfy's Actions header (simple + format), used by the outcome-confirmation event (#1898) to put + one-tap Good/Reject buttons directly into the push notification. + """ server = config.get("server", "https://ntfy.sh").rstrip("/") topic = config.get("topic", "").strip() auth_token = config.get("auth_token", "").strip() @@ -420,6 +431,9 @@ class NotificationService: if auth_token: headers["Authorization"] = f"Bearer {auth_token}" + if actions: + headers["Actions"] = actions + client = await self._get_client() if image_data: @@ -452,7 +466,13 @@ class NotificationService: return False, _opaque_http_failure(response, label="ntfy server") async def _send_pushover( - self, config: dict, title: str, message: str, image_data: bytes | None = None + self, + config: dict, + title: str, + message: str, + image_data: bytes | None = None, + url: str | None = None, + url_title: str | None = None, ) -> tuple[bool, str]: """Send notification via Pushover. @@ -461,6 +481,8 @@ class NotificationService: title: Notification title message: Notification body image_data: Optional JPEG image bytes to attach (max 2.5MB) + url: Optional supplementary URL shown under the message + url_title: Optional label for that URL """ user_key = config.get("user_key", "").strip() app_token = config.get("app_token", "").strip() @@ -472,7 +494,7 @@ class NotificationService: if not user_key or not app_token: return False, "User key and app token are required" - url = "https://api.pushover.net/1/messages.json" + api_url = "https://api.pushover.net/1/messages.json" data = { "token": app_token, "user": user_key, @@ -480,6 +502,10 @@ class NotificationService: "message": message, "priority": priority, } + if url: + data["url"] = url + if url_title: + data["url_title"] = url_title # Emergency priority (2) keeps re-alerting until acknowledged, so # Pushover *requires* retry (how often, >= 30s) and expire (when to @@ -502,9 +528,9 @@ class NotificationService: if image_data: # Pushover supports image attachments via multipart form-data files = {"attachment": ("photo.jpg", image_data, "image/jpeg")} - response = await client.post(url, data=data, files=files) + response = await client.post(api_url, data=data, files=files) else: - response = await client.post(url, data=data) + response = await client.post(api_url, data=data) if response.status_code == 200: return True, "Message sent successfully" @@ -516,8 +542,21 @@ class NotificationService: except Exception: return False, f"HTTP {response.status_code}: {response.text[:200]}" - async def _send_telegram(self, config: dict, message: str, image_data: bytes | None = None) -> tuple[bool, str]: - """Send notification via Telegram bot.""" + async def _send_telegram( + self, + config: dict, + message: str, + image_data: bytes | None = None, + buttons: list[dict] | None = None, + link_preview: bool = True, + ) -> tuple[bool, str]: + """Send notification via Telegram bot. + + ``buttons`` is one row of inline URL buttons (``{"text", "url"}`` + entries), used by the outcome-confirmation event (#1898) to put + one-tap Good/Reject under the message. ``link_preview=False`` asks + Telegram not to fetch the first URL in the text for a preview card. + """ bot_token = config.get("bot_token", "").strip() chat_id = config.get("chat_id", "").strip() @@ -548,36 +587,60 @@ class NotificationService: client = await self._get_client() - if image_data: - # Use sendPhoto to attach the thumbnail with the caption - url = f"https://api.telegram.org/bot{bot_token}/sendPhoto" - form: dict[str, Any] = {"chat_id": chat_id, "caption": message, "parse_mode": "Markdown"} - if message_thread_id is not None: - form["message_thread_id"] = message_thread_id - response = await client.post( - url, - data=form, - files={"photo": ("photo.jpg", image_data, "image/jpeg")}, - ) - else: + async def _post(with_buttons: bool): + if image_data: + # Use sendPhoto to attach the thumbnail with the caption + url = f"https://api.telegram.org/bot{bot_token}/sendPhoto" + form: dict[str, Any] = {"chat_id": chat_id, "caption": message, "parse_mode": "Markdown"} + if message_thread_id is not None: + form["message_thread_id"] = message_thread_id + if with_buttons: + # Multipart form fields are strings — reply_markup goes JSON-encoded. + form["reply_markup"] = json.dumps({"inline_keyboard": [buttons]}) + return await client.post( + url, + data=form, + files={"photo": ("photo.jpg", image_data, "image/jpeg")}, + ) + url = f"https://api.telegram.org/bot{bot_token}/sendMessage" - data: dict[str, Any] = { + payload: dict[str, Any] = { "chat_id": chat_id, "text": message, "parse_mode": "Markdown", } + if not link_preview: + payload["disable_web_page_preview"] = True if message_thread_id is not None: - data["message_thread_id"] = message_thread_id - response = await client.post(url, json=data) + payload["message_thread_id"] = message_thread_id + if with_buttons: + payload["reply_markup"] = {"inline_keyboard": [buttons]} + return await client.post(url, json=payload) - if response.status_code == 200: - result = response.json() + def _failure(resp) -> str | None: + """What Telegram objected to, or None when the send went through.""" + if resp.status_code != 200: + return f"HTTP {resp.status_code}: {resp.text[:200]}" + result = resp.json() if result.get("ok"): - return True, "Message sent successfully" - else: - return False, f"Telegram error: {result.get('description', 'Unknown error')}" - else: - return False, f"HTTP {response.status_code}: {response.text[:200]}" + return None + return f"Telegram error: {result.get('description', 'Unknown error')}" + + response = await _post(bool(buttons)) + failure = _failure(response) + if failure and buttons: + # Telegram validates every inline-keyboard URL and refuses the whole + # send when one of them is not a URL it accepts — which is what an + # install without a public external_url produces for the #1898 + # verdict links. Dropping the buttons is survivable; dropping the + # message the user is waiting for is not. + logger.warning("Telegram refused the message with inline buttons (%s); retrying without them", failure) + response = await _post(False) + failure = _failure(response) + + if failure: + return False, failure + return True, "Message sent successfully" async def _send_email( self, @@ -777,6 +840,21 @@ class NotificationService: if payload_format == "slack": # Slack/Mattermost format - just text field data = {"text": f"*{title}*\n{message}"} + if event_type == "print_confirm_request": + # Slack and Mattermost fetch every URL in the text to build + # preview cards, and the outcome prompt (#1898) is the one + # message whose links are single-use capabilities — that fetch + # would be a machine answering the operator's question. Off + # here for the same reason Telegram's preview is. + # + # Only here: the slack payload never attaches image bytes (the + # base64 attach below is generic-format only), so unfurling is + # the only way a {finish_photo_url} in a print_complete body + # can render as a photo in the channel. Switching it off for + # every event would quietly take that away with no setting to + # get it back. + data["unfurl_links"] = False + data["unfurl_media"] = False else: # Generic format with custom field names custom_field_title = config.get("field_title", "title").strip() or "title" @@ -947,11 +1025,85 @@ class NotificationService: if provider.provider_type == "callmebot": return await self._send_callmebot(config, f"{title}\n{message}") elif provider.provider_type == "ntfy": - return await self._send_ntfy(config, title, message, image_data=image_data, event_type=event_type) + # Outcome confirmation (#1898): render the verdict capability + # links as one-tap buttons on the notification itself. http so + # no browser needs to open; POST because that is the method + # that records — a GET only opens the confirmation page, which + # is what keeps unfurlers from answering the prompt. + # clear=true dismisses the notification once a button was tapped. + ntfy_actions = None + good_url = (variables or {}).get("good_url") + reject_url = (variables or {}).get("reject_url") + # Buttons need absolute URLs; without a configured external_url + # the links are relative, and the body's deep link into the + # archive has to do. + if ( + event_type == "print_confirm_request" + and good_url + and reject_url + and good_url.startswith("http") + and reject_url.startswith("http") + ): + ntfy_actions = ( + f"http, Good, {good_url}, method=POST, clear=true; " + f"http, Reject, {reject_url}, method=POST, clear=true" + ) + return await self._send_ntfy( + config, title, message, image_data=image_data, event_type=event_type, actions=ntfy_actions + ) elif provider.provider_type == "pushover": - return await self._send_pushover(config, title, message, image_data=image_data) + # Outcome confirmation (#1898): Pushover has no arbitrary + # buttons, but supports one supplementary URL — deep-link into + # the archive's confirmation dialog. + supplement_url = None + supplement_url_title = None + _confirm_url = (variables or {}).get("confirm_url") + if event_type == "print_confirm_request" and _confirm_url and _confirm_url.startswith("http"): + supplement_url = _confirm_url + supplement_url_title = "Confirm print outcome" + return await self._send_pushover( + config, title, message, image_data=image_data, url=supplement_url, url_title=supplement_url_title + ) elif provider.provider_type == "telegram": - return await self._send_telegram(config, f"*{title}*\n{message}", image_data=image_data) + # Outcome confirmation (#1898): inline URL buttons under the + # message — one tap records the verdict via the capability + # link. Same absolute-URL requirement as the ntfy actions. + # Telegram has no way to POST, so this is the one affordance + # that opens a browser, and it is the one that gets the one-tap + # marker: the page submits itself only for a URL that came off + # a button. Telegram does not fetch inline-keyboard URLs and + # nothing else can read them, so the marker never reaches a + # scanner — which is the difference between this and trusting + # the User-Agent. + tg_buttons = None + _tg_good = (variables or {}).get("good_url") + _tg_reject = (variables or {}).get("reject_url") + if ( + event_type == "print_confirm_request" + and _tg_good + and _tg_reject + and _tg_good.startswith("http") + and _tg_reject.startswith("http") + ): + tg_buttons = [ + {"text": "\U0001f44d Good", "url": one_tap_url(_tg_good)}, + {"text": "\U0001f44e Reject", "url": one_tap_url(_tg_reject)}, + ] + # Telegram's servers GET the first URL in the text to build a + # preview card. An outcome prompt whose edited body still + # carries {good_url} would have that fetch answer the question + # before the operator saw it, so the preview is off for this + # event. Only for this one, for the same reason as the Slack + # unfurl in _send_webhook: when the finish photo is too large to attach, + # the preview is how a {finish_photo_url} in a print_complete + # body still shows up as a photo in the chat. + return await self._send_telegram( + config, + f"*{title}*\n{message}", + image_data=image_data, + buttons=tg_buttons, + link_preview=event_type != "print_confirm_request", + ) elif provider.provider_type == "email": # finish_photo_url is pulled from the rendered template variables # so _send_email can detect whether the template referenced the @@ -969,7 +1121,13 @@ class NotificationService: elif provider.provider_type == "homeassistant": return await self._send_homeassistant(config, title, message, db=db) elif provider.provider_type == "bark": - return await self._send_bark(config, title, message) + # Outcome confirmation (#1898): Bark opens one URL on tap — + # deep-link into the confirmation dialog, like Pushover. + bark_url = None + _bark_confirm = (variables or {}).get("confirm_url") + if event_type == "print_confirm_request" and _bark_confirm and _bark_confirm.startswith("http"): + bark_url = _bark_confirm + return await self._send_bark(config, title, message, url=bark_url) else: return False, f"Unknown provider type: {provider.provider_type}" except Exception as e: @@ -1311,6 +1469,62 @@ class NotificationService: variables=variables, ) + async def on_print_confirm_request( + self, + printer_id: int, + printer_name: str, + data: dict, + db: AsyncSession, + archive_data: dict | None = None, + good_url: str | None = None, + reject_url: str | None = None, + confirm_url: str | None = None, + ): + """Ask for a post-print outcome verdict (#1898). + + Fires only for completed prints whose queue item opted in — the + provider-level toggle exists to mute a channel, not to enable the + feature. good_url / reject_url are the one-tap capability links + (rendered as ntfy action buttons), confirm_url deep-links into the + archive's confirmation dialog in the web UI. + """ + providers = await self._get_providers_for_event(db, "on_print_confirm_request", printer_id) + if not providers: + return + + subtask_name = data.get("subtask_name") + if subtask_name: + filename = subtask_name.replace("_", " ") + else: + filename = self._clean_filename(data.get("filename", "Unknown")) + + variables = {"printer": printer_name, "filename": filename} + if good_url: + variables["good_url"] = good_url + if reject_url: + variables["reject_url"] = reject_url + if confirm_url: + variables["confirm_url"] = confirm_url + + image_data = None + if archive_data: + if archive_data.get("finish_photo_url"): + variables["finish_photo_url"] = archive_data["finish_photo_url"] + image_data = archive_data.get("image_data") + + title, message = await self._build_message_from_template(db, "print_confirm_request", variables) + await self._send_to_providers( + providers, + title, + message, + db, + "print_confirm_request", + printer_id, + printer_name, + image_data=image_data, + variables=variables, + ) + async def on_print_progress( self, printer_id: int, diff --git a/backend/app/services/print_batch.py b/backend/app/services/print_batch.py index 1d28ed568..02e8a0d1d 100644 --- a/backend/app/services/print_batch.py +++ b/backend/app/services/print_batch.py @@ -72,6 +72,7 @@ CLONED_SETTING_COLUMNS = ( "timelapse", "use_ams", "nozzle_offset_cali", + "confirm_outcome", "preheat_override", "preheat_chamber_target_override", "skip_filament_check", diff --git a/backend/app/services/print_confirmation.py b/backend/app/services/print_confirmation.py new file mode 100644 index 000000000..87c5342d4 --- /dev/null +++ b/backend/app/services/print_confirmation.py @@ -0,0 +1,193 @@ +"""Post-print outcome confirmation helpers (#1898).""" + +import logging +from collections.abc import Mapping +from datetime import datetime, timezone + +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from backend.app.models.archive import PrintArchive +from backend.app.models.print_log import PrintLogEntry + +logger = logging.getLogger(__name__) + +# How a verdict reached the archive. 'reaction' is written by the Telegram +# reaction handler (#3046), which lives on its own branch — listed here so the +# vocabulary is complete and the UI can label it the day that lands. +VERDICT_SOURCES = ("dialog", "link", "plate_clear", "printer_card", "api", "reaction") + +# Link-preview unfurlers and mail-security scanners fetch every URL they find in +# a message, unattended, within seconds of it being sent. Nothing they can do +# with a GET records a verdict any more -- that is the POST route's job -- so +# this list is the second layer: it decides whether the confirmation page +# submits its own form, which is what keeps a human at one tap. A scanner that +# runs JavaScript would otherwise press the button on the operator's behalf. +# Matched as case-insensitive substrings of the User-Agent; the generic "bot" +# token covers TelegramBot, Discordbot, Slackbot-LinkExpanding, Twitterbot and +# LinkedInBot in one go. +UNATTENDED_FETCH_AGENTS = ( + "bot", + "crawler", + "spider", + "facebookexternalhit", + "whatsapp", + "skypeuripreview", + "bingpreview", + "safelinks", + "urldefense", + "proofpoint", + "mimecast", + "barracuda", + "forcepoint", +) + +# Prefetch / preload hints. A finger on a notification button is never one. +UNATTENDED_FETCH_HEADERS = { + "purpose": ("prefetch", "preview"), + "x-purpose": ("prefetch", "preview"), + "x-moz": ("prefetch",), + "sec-purpose": ("prefetch",), +} + + +def is_unattended_fetch(method: str, headers: Mapping[str, str]) -> bool: + """Whether a request for a one-tap verdict link came from a machine. + + Decides whether the confirmation page submits itself. False positives are + deliberately cheap -- a browser mistaken for a bot gets the same page with + a button to press -- so the lists above err towards catching more. + """ + if method.upper() != "GET": + # Anything that is not the page load is not a page load: only the GET + # route renders, and only a GET can be widened to HEAD by a future + # router change. + return True + for header, markers in UNATTENDED_FETCH_HEADERS.items(): + value = (headers.get(header) or "").lower() + if value and any(marker in value for marker in markers): + return True + agent = (headers.get("user-agent") or "").lower() + return any(marker in agent for marker in UNATTENDED_FETCH_AGENTS) + + +# The one-tap marker. The confirmation page submits its own form only when the +# URL it was opened from carries this, and the marker is put on exactly one +# thing: the Telegram inline keyboard's buttons -- an affordance no unfurler, +# gateway or proxy reads, for the same reason the capability URLs themselves no +# longer travel in message text. +# +# It is what closes the gap the User-Agent list above cannot: a mail-security +# sandbox that renders HTML and runs JavaScript sends an ordinary Chrome string +# (so does literal HeadlessChrome), and a verdict URL that reached it did so out +# of the message BODY -- where the marker never appears. That fetch now gets the +# page with a button on it and records nothing. The operator's tap on the +# notification button still costs exactly one tap. +ONE_TAP_PARAM = "tap" + + +def one_tap_url(url: str) -> str: + """Mark a verdict URL as one a human is about to press. + + Only for the affordances a person taps directly. A URL that goes into text + anybody's machine might follow is left unmarked on purpose. + """ + return f"{url}{'&' if '?' in url else '?'}{ONE_TAP_PARAM}=1" + + +def is_one_tap_request(query_params: Mapping[str, str]) -> bool: + """Whether this page load came from a button rather than from message text.""" + return (query_params.get(ONE_TAP_PARAM) or "") == "1" + + +def stamp_verdict(archive: PrintArchive, source: str) -> None: + """Record a verdict's provenance and the moment it landed (#1898). + + Both fields move together on every verdict write, which is what keeps the + "already answered" page from pairing a new source with the timestamp of an + older decision. `retire_confirm_token` is separate on purpose: spending the + one-tap capability happens once, recording a verdict can happen again. + """ + archive.user_verdict_source = source + archive.user_verdict_at = datetime.now(timezone.utc) + + +def retire_confirm_token(archive: PrintArchive) -> None: + """Spend the one-tap capability token without destroying it. + + The token is still single-use: once ``confirm_token_used_at`` is stamped, + no verdict path accepts it again. Keeping the VALUE is what lets the + one-tap route recognise a link belonging to an already-answered print and + say so, instead of 404ing as if the link had never been real (the live-farm + case: the plate-clear default answered the prompt, then the user tapped the + Telegram button and got "invalid or already used"). + """ + if archive.confirm_token and archive.confirm_token_used_at is None: + archive.confirm_token_used_at = datetime.now(timezone.utc) + + +async def resolve_pending_confirmation_as_good(db: AsyncSession, printer_id: int) -> int | None: + """Mark the printer's latest pending-confirmation archive as good. + + Backs the opt-in ``confirm_default_good_on_plate_clear`` setting: releasing + the build plate is the moment the operator moves on to the next job, so an + unanswered outcome prompt can default to "good part" right there instead of + lingering as unconfirmed. Only the LATEST pending archive is resolved — the + plate release refers to the print that just came off the plate, not to + older unanswered prompts. + + Mirrors the verdict onto the latest PrintLogEntry (the #1444 mirror) and + retires the one-tap capability token. Deliberately does NOT commit — both + callers (the clear-plate route and the queue dispatcher) manage their own + transaction. + + Returns the resolved archive id, or None when nothing was pending. + """ + archive = await db.scalar( + select(PrintArchive) + .where( + PrintArchive.printer_id == printer_id, + PrintArchive.status == "completed", + PrintArchive.confirm_requested.is_(True), + PrintArchive.user_verdict.is_(None), + ) + .order_by(PrintArchive.id.desc()) + .limit(1) + ) + if archive is None: + return None + + archive.user_verdict = "good" + stamp_verdict(archive, "plate_clear") + retire_confirm_token(archive) + + latest_entry = await db.scalar( + select(PrintLogEntry).where(PrintLogEntry.archive_id == archive.id).order_by(PrintLogEntry.id.desc()).limit(1) + ) + if latest_entry is not None: + latest_entry.user_verdict = "good" + + logger.info("[#1898] Plate clear defaulted archive %s to 'good' (printer %s)", archive.id, printer_id) + return archive.id + + +async def confirm_outcome_for_new_queue_item(db: AsyncSession, *, started_outside_bambuddy: bool = False) -> bool: + """The ask-for-outcome flag for a queue item created without the print dialog. + + The dialog seeds its own per-job toggle from ``default_confirm_outcome``. + Every other queue-creation path -- the virtual printer, the library bulk + add, the webhook, a pipeline run -- has no toggle to seed and used to leave + the column at its ``False`` default, so "Ask for Outcome" only ever reached + jobs queued by hand. + + ``started_outside_bambuddy`` additionally honours + ``confirm_outcome_external_prints``: a plate sent from Bambu Studio to a + virtual printer is one of the prints that setting's description names, but + it arrives with a queue item, so ``on_print_start`` never sees it as + external and the setting could not otherwise reach it. + """ + from backend.app.api.routes.settings import get_setting, setting_is_true + + if setting_is_true(await get_setting(db, "default_confirm_outcome")): + return True + return started_outside_bambuddy and setting_is_true(await get_setting(db, "confirm_outcome_external_prints")) diff --git a/backend/app/services/print_scheduler.py b/backend/app/services/print_scheduler.py index 482701b43..742128c5f 100644 --- a/backend/app/services/print_scheduler.py +++ b/backend/app/services/print_scheduler.py @@ -6379,6 +6379,12 @@ class PrintScheduler: if archive.plate_id is None and item.plate_id is not None: archive.plate_id = item.plate_id + # Ask-for-outcome opt-in rides from the queue item to the archive + # the same way (#1898); never cleared here so a reprint of an + # archive that already asked keeps asking. + if item.confirm_outcome: + archive.confirm_requested = True + file_path = settings.base_dir / archive.file_path filename = archive.filename @@ -6439,6 +6445,8 @@ class PrintScheduler: ) if archive: item.archive_id = archive.id + if item.confirm_outcome: + archive.confirm_requested = True # ask-for-outcome opt-in (#1898) if budget_reservation is not None: budget_reservation.print_archive_id = archive.id if item.cleanup_library_after_dispatch and not library_file.is_external: @@ -6968,6 +6976,16 @@ class PrintScheduler: # Clear the awaiting-plate-clear flag now that we're starting a new print printer_manager.set_awaiting_plate_clear(item.printer_id, False) + + # #1898: with the opt-in default-good setting, moving on to the next + # print resolves the previous print's unanswered outcome prompt as + # "good" — this path also covers the camera-based plate detection, + # which releases the gate by allowing dispatch rather than by an + # explicit acknowledgment. Rides on the dispatch transaction. + if await self._get_bool_setting(db, "confirm_default_good_on_plate_clear", default=False): + from backend.app.services.print_confirmation import resolve_pending_confirmation_as_good + + await resolve_pending_confirmation_as_good(db, item.printer_id) logger.info("Queue item %s: Status set to 'printing', sending print command...", item.id) # Capture state before dispatch so the watchdog can detect whether the diff --git a/backend/app/services/usage_tracker.py b/backend/app/services/usage_tracker.py index d8b6981f0..b05efc78b 100644 --- a/backend/app/services/usage_tracker.py +++ b/backend/app/services/usage_tracker.py @@ -1507,6 +1507,24 @@ async def _track_from_3mf( mapping_source or "none", ) + # A mapping that names no tray for any slot the print used carries no + # evidence at all -- typically a flat ams_mapping whose external spool is + # -1 and came without an ams_mapping2 to resolve it (#3166). Drop it so the + # tray_now evidence below decides, rather than leaving every slot unfed. + if slot_to_tray: + used_slots = [u.get("slot_id", 0) for u in filament_usage if u.get("used_g", 0) > 0] + if used_slots and all( + not (0 < s <= len(slot_to_tray) and isinstance(slot_to_tray[s - 1], int) and slot_to_tray[s - 1] >= 0) + for s in used_slots + ): + logger.info( + "[UsageTracker] 3MF: mapping %s names no tray for used slots %s — ignoring it", + slot_to_tray, + used_slots, + ) + slot_to_tray = None + mapping_source = None + # 5. For single-filament non-queue prints, use tray_now from printer state # Priority: tray_change_log (multi-tray split) > tray_now_at_start > current tray_now # > last_loaded_tray > vt_tray check @@ -1779,6 +1797,18 @@ async def _track_from_3mf( mapped = slot_to_tray[slot_id - 1] if isinstance(mapped, int) and mapped >= 0: global_tray_id = mapped + elif mapped == -1: + # The mapping says this slot isn't fed from any tray. The + # position-based guess below would hand it the Nth loaded + # tray anyway -- an AMS spool that never moved (#3166), or + # the tray another slot really used, which that slot then + # finds already handled and drops (#2880). + logger.info( + "[UsageTracker] 3MF: slot_id=%d is unmapped (-1) — nothing charged (used_g=%.1f)", + slot_id, + used_g, + ) + continue # Position-based default: sort available tray IDs so external spools (254/255) # naturally follow standard AMS trays, matching slicer slot numbering. # diff --git a/backend/app/services/virtual_printer/manager.py b/backend/app/services/virtual_printer/manager.py index 6718804ba..dd2a95c6b 100644 --- a/backend/app/services/virtual_printer/manager.py +++ b/backend/app/services/virtual_printer/manager.py @@ -838,6 +838,7 @@ class VirtualPrinterInstance: from backend.app.models.print_queue import PrintQueueItem from backend.app.services.archive import ArchiveService from backend.app.services.filament_requirements import extract_filament_requirements + from backend.app.services.print_confirmation import confirm_outcome_for_new_queue_item async with self._session_factory() as db: name_source = await get_setting(db, "virtual_printer_archive_name_source") @@ -905,6 +906,14 @@ class VirtualPrinterInstance: ) timelapse = _slicer_or("timelapse", _bool_setting(await get_setting(db, "default_timelapse"), False)) + # "Ask for Outcome" is a default print option like the ones + # above. A plate sent here from Bambu Studio is also one of the + # prints `confirm_outcome_external_prints` names — but it + # arrives with a queue item, so on_print_start resumes the + # archive created below instead of treating it as external, and + # without this the setting could never reach it (#1898). + confirm_outcome = await confirm_outcome_for_new_queue_item(db, started_outside_bambuddy=True) + # H2C dual-nozzle-rack slicer-pick preservation (#1780). # BambuStudio's project_file MQTT command for rack-swap models # (O1C2 today) carries `nozzle_mapping` — a per-filament array @@ -1133,6 +1142,7 @@ class VirtualPrinterInstance: vibration_cali=vibration_cali, layer_inspect=layer_inspect, timelapse=timelapse, + confirm_outcome=confirm_outcome, # Per-VP opt-in for auto-print G-code injection (#1516). # Default off; when on, the scheduler still no-ops unless # gcode_snippets are configured for the target model, so it's diff --git a/backend/tests/integration/test_confirm_outcome_queue_defaults_1898.py b/backend/tests/integration/test_confirm_outcome_queue_defaults_1898.py new file mode 100644 index 000000000..21b422ab7 --- /dev/null +++ b/backend/tests/integration/test_confirm_outcome_queue_defaults_1898.py @@ -0,0 +1,128 @@ +"""Every queue-creation path has to be able to ask for the outcome (#1898). + +``confirm_outcome`` rides from a queue item onto the archive at dispatch, and +the only place that ever set it was the print dialog. Jobs created anywhere +else -- a plate sent from Bambu Studio to a virtual printer, the Library's bulk +"Add to queue", the webhook, a pipeline run -- carried the column default and +were never asked about, however the install's settings were configured. + +That mattered most for the virtual printer: a Bambu Studio plate is one of the +prints ``confirm_outcome_external_prints`` names in its own description, but it +arrives with a queue item, so ``on_print_start`` resumes its archive instead of +treating it as external and the setting could not reach it at all. +""" + +from pathlib import Path +from uuid import uuid4 + +import pytest +from httpx import AsyncClient +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from backend.app.core.config import settings as app_settings +from backend.app.models.print_queue import PrintQueueItem +from backend.app.models.settings import Settings +from backend.app.services.print_confirmation import confirm_outcome_for_new_queue_item + +pytestmark = pytest.mark.integration + + +async def _set_setting(db_session, key: str, value: str) -> None: + db_session.add(Settings(key=key, value=value)) + await db_session.commit() + + +class TestTheDefaultForAQueueItemNobodyToggled: + @pytest.mark.asyncio + async def test_off_when_nothing_is_configured(self, db_session): + assert await confirm_outcome_for_new_queue_item(db_session) is False + assert await confirm_outcome_for_new_queue_item(db_session, started_outside_bambuddy=True) is False + + @pytest.mark.asyncio + async def test_the_per_job_default_covers_every_path(self, db_session): + """Same setting the print dialog seeds its own toggle from.""" + await _set_setting(db_session, "default_confirm_outcome", "true") + + assert await confirm_outcome_for_new_queue_item(db_session) is True + assert await confirm_outcome_for_new_queue_item(db_session, started_outside_bambuddy=True) is True + + @pytest.mark.asyncio + async def test_the_external_setting_only_covers_external_origins(self, db_session): + """A Bambu Studio plate counts; a pipeline run Bambuddy started itself + does not -- that one follows the per-job default like any queued job.""" + await _set_setting(db_session, "confirm_outcome_external_prints", "true") + + assert await confirm_outcome_for_new_queue_item(db_session, started_outside_bambuddy=True) is True + assert await confirm_outcome_for_new_queue_item(db_session) is False + + @pytest.mark.asyncio + async def test_an_odd_stored_value_counts_as_off(self, db_session): + await _set_setting(db_session, "default_confirm_outcome", "None") + + assert await confirm_outcome_for_new_queue_item(db_session) is False + + +@pytest.fixture +async def sliced_library_file(db_session): + """A library file that passes add-to-queue's gates: the filename has to look + sliced and the bytes have to exist under ``base_dir``.""" + from backend.app.models.library import LibraryFile + + # Unique per test: the suite runs with xdist and the teardown below would + # otherwise delete the bytes a sibling worker is still relying on, which the + # route answers with a 400 for a bulk add that queued nothing (#3112). + name = f"confirm_outcome_probe_{uuid4().hex}.gcode.3mf" + rel_path = f"archive/library/files/{name}" + abs_path = Path(app_settings.base_dir) / rel_path + abs_path.parent.mkdir(parents=True, exist_ok=True) + abs_path.write_bytes(b"probe") + + lib_file = LibraryFile( + filename=name, + file_path=rel_path, + file_size=5, + file_type="3mf", + ) + db_session.add(lib_file) + await db_session.commit() + await db_session.refresh(lib_file) + + yield lib_file + + abs_path.unlink(missing_ok=True) + + +async def _read_item(test_engine, item_id: int) -> PrintQueueItem: + """Fresh-session read: the route ran on its own ``Depends(get_db)`` session.""" + maker = async_sessionmaker(test_engine, class_=AsyncSession, expire_on_commit=False) + async with maker() as fresh: + return (await fresh.execute(select(PrintQueueItem).where(PrintQueueItem.id == item_id))).scalar_one() + + +class TestTheLibraryBulkAdd: + """The route has no per-job toggle at all, so the install-wide default is + the only thing that can decide.""" + + async def _add(self, async_client: AsyncClient, file_id: int) -> int: + response = await async_client.post("/api/v1/library/files/add-to-queue", json={"file_ids": [file_id]}) + assert response.status_code == 200 + added = response.json()["added"] + assert len(added) == 1 + return added[0]["queue_item_id"] + + @pytest.mark.asyncio + async def test_follows_the_default(self, async_client, db_session, test_engine, sliced_library_file): + await _set_setting(db_session, "default_confirm_outcome", "true") + + item_id = await self._add(async_client, sliced_library_file.id) + + assert (await _read_item(test_engine, item_id)).confirm_outcome is True + + @pytest.mark.asyncio + async def test_stays_off_for_an_install_that_never_asked_for_it( + self, async_client, db_session, test_engine, sliced_library_file + ): + item_id = await self._add(async_client, sliced_library_file.id) + + assert (await _read_item(test_engine, item_id)).confirm_outcome is False diff --git a/backend/tests/integration/test_external_print_confirmation_1898.py b/backend/tests/integration/test_external_print_confirmation_1898.py new file mode 100644 index 000000000..e85596822 --- /dev/null +++ b/backend/tests/integration/test_external_print_confirmation_1898.py @@ -0,0 +1,657 @@ +"""Ask for the outcome of prints Bambuddy did not start (#1898 follow-up). + +The ask-for-outcome flag rides from a queue item onto the archive at dispatch. +On a farm where most jobs are started at the printer's screen, in Bambu Studio +or in the Handy app there is no queue item to ride from, so every archive +``on_print_start`` created had ``confirm_requested`` false and the feature +looked broken. ``confirm_outcome_external_prints`` is the missing source. + +Both archive-creating branches of ``on_print_start`` are driven here against a +real database: the no-3MF fallback (what a P1S/A1 farm actually hits) and the +normal path that archives a downloaded 3MF. +""" + +from contextlib import ExitStack +from datetime import datetime, timezone +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from backend.app.main import ( + _active_prints, + _expected_print_creators, + _expected_print_registered_at, + _expected_prints, + _print_ams_mappings, +) +from backend.app.models.archive import PrintArchive +from backend.app.models.print_queue import PrintQueueItem +from backend.app.models.settings import Settings + +DISPATCH = "/data/Metadata/plate_1.gcode" +SUBTASK = "Bracket_plate_1" + + +@pytest.fixture(autouse=True) +def _clear_print_state(): + dicts = ( + _expected_prints, + _expected_print_registered_at, + _expected_print_creators, + _print_ams_mappings, + _active_prints, + ) + for d in dicts: + d.clear() + yield + for d in dicts: + d.clear() + + +class _StubArchiveService: + """Stands in for ArchiveService on the downloaded-3MF branch. + + Writes the row the real service would write, without needing a parseable + 3MF on disk. Everything this test asserts happens *after* the row exists. + """ + + def __init__(self, db): + self.db = db + + async def archive_print(self, **kwargs): + archive = PrintArchive( + printer_id=kwargs.get("printer_id"), + filename=f"{SUBTASK}.gcode.3mf", + file_path=f"archives/test/{SUBTASK}.gcode.3mf", + file_size=2048, + print_name=SUBTASK, + status="printing", + started_at=datetime.now(timezone.utc), + ) + self.db.add(archive) + await self.db.commit() + await self.db.refresh(archive) + return archive + + +async def _drive_print_start(test_engine, printer, *, download_ok: bool, extra_patches=()) -> dict: + """Run ``on_print_start`` against the test database. + + ``download_ok`` picks the branch: False leaves the 3MF unreachable and the + no-3MF fallback archive is written inline; True takes the ArchiveService + branch. + """ + session_maker = async_sessionmaker(test_engine, class_=AsyncSession, expire_on_commit=False) + state = MagicMock( + current_project_url=f"ftp://{SUBTASK}.gcode.3mf", + sdcard=True, + sdcard_reported=True, + ) + + patches = [ + patch("backend.app.main.async_session", session_maker), + patch("backend.app.core.database.async_session", session_maker), + patch("backend.app.main.download_file_async", new=AsyncMock(return_value=download_ok)), + patch("backend.app.main.download_file_try_paths_async", new=AsyncMock(return_value=None)), + patch("backend.app.main.get_cached_3mf", return_value=None), + patch("backend.app.main.cache_3mf_download"), + patch("backend.app.main.peek_plate_index_in_3mf", return_value=None), + patch("backend.app.main.ArchiveService", _StubArchiveService), + # Imported inside the function, so patching it anywhere else lets the + # directory walk open real sockets. + patch("backend.app.services.bambu_ftp.list_files_async", new=AsyncMock(return_value=[])), + patch("backend.app.main.ftps_handshake_blocked", return_value=False), + patch("backend.app.main.get_ftp_retry_settings", new=AsyncMock(return_value=(False, 3, 2.0, 30))), + patch("backend.app.main._record_energy_start", new_callable=AsyncMock), + patch("backend.app.main._send_print_start_notification", new_callable=AsyncMock), + patch("backend.app.main._maybe_start_layer_timelapse"), + patch("backend.app.main._capture_timelapse_baseline_at_start", new_callable=AsyncMock), + # Real, it would spawn a task that outlives the test by a minute. + patch("backend.app.main._schedule_fallback_3mf_retry"), + patch("backend.app.main._store_spoolman_print_data", new_callable=AsyncMock), + # Imported inside on_print_start; it would try to persist the mocked + # printer state's AMS trays and fail on the MagicMock values. + patch("backend.app.services.usage_tracker.on_print_start", new_callable=AsyncMock), + ] + + mocks: dict = {} + with ExitStack() as stack: + for p in [*patches, *extra_patches]: + mocks[p.attribute] = stack.enter_context(p) + notif = stack.enter_context(patch("backend.app.main.notification_service")) + plug = stack.enter_context(patch("backend.app.main.smart_plug_manager")) + ws = stack.enter_context(patch("backend.app.main.ws_manager")) + relay = stack.enter_context(patch("backend.app.main.mqtt_relay")) + pm = stack.enter_context(patch("backend.app.main.printer_manager")) + + notif.on_print_start = AsyncMock() + plug.on_print_start = AsyncMock() + ws.send_print_start = AsyncMock() + ws.send_archive_created = AsyncMock() + ws.send_archive_updated = AsyncMock() + relay.on_print_start = AsyncMock() + relay.on_archive_created = AsyncMock() + pm.get_status = MagicMock(return_value=state) + pm.get_printer = MagicMock(return_value=MagicMock(serial_number="TEST1898")) + + from backend.app.main import on_print_start + + await on_print_start(printer.id, {"filename": DISPATCH, "subtask_name": SUBTASK}) + return mocks + + +async def _set_setting(db_session, key: str, value: str) -> None: + db_session.add(Settings(key=key, value=value)) + await db_session.commit() + + +async def _created_archive(db_session, printer_id: int) -> PrintArchive: + db_session.expire_all() + archive = await db_session.scalar( + select(PrintArchive).where(PrintArchive.printer_id == printer_id).order_by(PrintArchive.id.desc()).limit(1) + ) + assert archive is not None, "on_print_start created no archive" + return archive + + +class TestExternalPrintGetsTheOutcomePrompt: + @pytest.mark.asyncio + @pytest.mark.integration + async def test_no_3mf_fallback_asks_when_the_setting_is_on(self, test_engine, db_session, printer_factory): + """The P1S/A1 case: the 3MF cannot be fetched, the archive is written + inline — and that is the row the completion path reads the flag off.""" + printer = await printer_factory() + await _set_setting(db_session, "confirm_outcome_external_prints", "true") + + await _drive_print_start(test_engine, printer, download_ok=False) + + archive = await _created_archive(db_session, printer.id) + assert archive.extra_data.get("no_3mf_available") is True + assert archive.confirm_requested is True + # What the completion path gates the prompt on. + assert archive.user_verdict is None + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_the_archive_reaches_the_rest_of_the_1898_machinery(self, test_engine, db_session, printer_factory): + """An externally started print is now a pending confirmation like any + other: the prompt the completion path emits is gated on exactly these + two fields, and the plate-release default resolves the same row.""" + from backend.app.services.print_confirmation import resolve_pending_confirmation_as_good + + printer = await printer_factory() + printer_id = printer.id + await _set_setting(db_session, "confirm_outcome_external_prints", "true") + + await _drive_print_start(test_engine, printer, download_ok=False) + + archive = await _created_archive(db_session, printer_id) + archive.status = "completed" + await db_session.commit() + + assert await resolve_pending_confirmation_as_good(db_session, printer_id) == archive.id + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_downloaded_3mf_archive_asks_when_the_setting_is_on(self, test_engine, db_session, printer_factory): + """The other creation branch: the 3MF arrived and ArchiveService wrote + the row. The flag is set on the row afterwards rather than passed into + archive_print, which also serves the queue dispatcher.""" + printer = await printer_factory() + await _set_setting(db_session, "confirm_outcome_external_prints", "true") + + await _drive_print_start(test_engine, printer, download_ok=True) + + archive = await _created_archive(db_session, printer.id) + assert archive.file_path.endswith(".3mf") + assert archive.confirm_requested is True + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_default_leaves_external_prints_alone(self, test_engine, db_session, printer_factory): + """Default off: nothing about today's behaviour changes for an install + that never touches the new setting — no row for it at all.""" + printer = await printer_factory() + + await _drive_print_start(test_engine, printer, download_ok=False) + + archive = await _created_archive(db_session, printer.id) + assert archive.confirm_requested is False + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_setting_off_does_not_ask(self, test_engine, db_session, printer_factory): + printer = await printer_factory() + await _set_setting(db_session, "confirm_outcome_external_prints", "false") + + await _drive_print_start(test_engine, printer, download_ok=False) + + assert (await _created_archive(db_session, printer.id)).confirm_requested is False + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_an_odd_stored_value_counts_as_off(self, test_engine, db_session, printer_factory): + """Settings live in a VARCHAR column and every other reader treats + anything that is not "true" as off.""" + printer = await printer_factory() + await _set_setting(db_session, "confirm_outcome_external_prints", "None") + + await _drive_print_start(test_engine, printer, download_ok=False) + + assert (await _created_archive(db_session, printer.id)).confirm_requested is False + + +class TestAFailureHereCostsOnlyThePrompt: + """The prompt is optional; the rest of print start is not. + + After a failed statement, a rollback of the whole transaction expires every + object the session holds, and on an async session the next read of one + raises instead of reloading. The print start below the check reads both + the printer and the archive, so the check runs in a savepoint, and the flag + write reloads what it used after its own rollback. + """ + + @staticmethod + def _assert_print_start_finished(mocks, archive_id: int) -> None: + assert archive_id in _active_prints.values() + mocks["_record_energy_start"].assert_awaited() + assert mocks["_record_energy_start"].await_args.args[0].id == archive_id + mocks["_capture_timelapse_baseline_at_start"].assert_awaited() + assert mocks["_capture_timelapse_baseline_at_start"].await_args.kwargs["archive_id"] == archive_id + + @pytest.mark.asyncio + @pytest.mark.integration + @pytest.mark.parametrize("download_ok", [True, False]) + async def test_a_failing_query_in_the_check(self, test_engine, db_session, printer_factory, download_ok): + from sqlalchemy import text + + from backend.app.api.routes import settings as settings_routes + + real_get_setting = settings_routes.get_setting + + async def _broken_for_this_key(db, key): + if key == "confirm_outcome_external_prints": + # A real failed statement, not just an exception: this is what + # leaves the transaction needing a rollback. + await db.execute(text("SELECT no_such_column FROM settings")) + return await real_get_setting(db, key) + + printer = await printer_factory() + await _set_setting(db_session, "confirm_outcome_external_prints", "true") + + mocks = await _drive_print_start( + test_engine, + printer, + download_ok=download_ok, + extra_patches=[patch.object(settings_routes, "get_setting", _broken_for_this_key)], + ) + + archive = await _created_archive(db_session, printer.id) + assert archive.confirm_requested is False + self._assert_print_start_finished(mocks, archive.id) + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_a_failing_flag_write(self, test_engine, db_session, printer_factory): + async def _yes_but_poison_the_commit(db, printer_id, observed_name=None): + # Two rows for one unique key: the commit that carries the flag fails. + db.add(Settings(key="x_1898_duplicate", value="a")) + db.add(Settings(key="x_1898_duplicate", value="b")) + return True + + printer = await printer_factory() + + mocks = await _drive_print_start( + test_engine, + printer, + download_ok=True, + extra_patches=[patch("backend.app.main._ask_outcome_for_external_print", _yes_but_poison_the_commit)], + ) + + archive = await _created_archive(db_session, printer.id) + # archive_print committed the row; only the flag is lost. + assert archive.confirm_requested is False + self._assert_print_start_finished(mocks, archive.id) + + +class TestAQueuedPrintStillDecidesForItself: + @pytest.mark.asyncio + @pytest.mark.integration + async def test_a_dispatched_job_is_not_overridden_by_the_setting( + self, test_engine, db_session, printer_factory, archive_factory + ): + """A queue item that deliberately has the ask-for-outcome flag off must + stay off. A restart mid-print empties the expected-print registry, so + the queue row — which the scheduler commits to "printing" before the + MQTT send — is the durable record that Bambuddy started this.""" + printer = await printer_factory() + # Dispatched but not yet reported as started: the archive the scheduler + # attached to the row only turns "printing" once on_print_start runs, so + # the name-match resume above cannot find it and the queue row is the + # only record that Bambuddy sent this print. + source = await archive_factory(printer.id, filename=f"{SUBTASK}.gcode.3mf", status="pending", with_run=False) + db_session.add( + PrintQueueItem( + printer_id=printer.id, + archive_id=source.id, + status="printing", + confirm_outcome=False, + ) + ) + await db_session.commit() + await _set_setting(db_session, "confirm_outcome_external_prints", "true") + + await _drive_print_start(test_engine, printer, download_ok=False) + + archive = await _created_archive(db_session, printer.id) + assert archive.confirm_requested is False + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_a_queue_items_own_yes_survives_print_start( + self, test_engine, db_session, printer_factory, archive_factory + ): + """The dispatcher copies ``confirm_outcome`` onto the archive before + the print starts; the expected-print branch of ``on_print_start`` must + leave that alone even while the external-print setting is off.""" + from backend.app.main import register_expected_print + + printer = await printer_factory() + archive = await archive_factory( + printer.id, + filename=f"{SUBTASK}.gcode.3mf", + status="pending", + confirm_requested=True, + with_run=False, + ) + db_session.add( + PrintQueueItem( + printer_id=printer.id, + archive_id=archive.id, + status="printing", + confirm_outcome=True, + ) + ) + await db_session.commit() + archive_id, printer_id = archive.id, printer.id + register_expected_print(printer_id, f"{SUBTASK}.gcode.3mf", archive_id) + + await _drive_print_start(test_engine, printer, download_ok=False) + + db_session.expire_all() + refreshed = await db_session.get(PrintArchive, archive_id) + assert refreshed.status == "printing" + assert refreshed.confirm_requested is True + # No second row for the same print. + rows = (await db_session.scalars(select(PrintArchive).where(PrintArchive.printer_id == printer_id))).all() + assert len(rows) == 1 + + +class TestSettingsRoundTrip: + @pytest.mark.asyncio + @pytest.mark.integration + async def test_defaults_to_off_and_round_trips(self, async_client): + response = await async_client.get("/api/v1/settings/") + assert response.status_code == 200 + assert response.json()["confirm_outcome_external_prints"] is False + + response = await async_client.put("/api/v1/settings/", json={"confirm_outcome_external_prints": True}) + assert response.status_code == 200 + assert response.json()["confirm_outcome_external_prints"] is True + + assert (await async_client.get("/api/v1/settings/")).json()["confirm_outcome_external_prints"] is True + + response = await async_client.put("/api/v1/settings/", json={"confirm_outcome_external_prints": False}) + assert response.status_code == 200 + assert (await async_client.get("/api/v1/settings/")).json()["confirm_outcome_external_prints"] is False + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_updating_it_leaves_the_per_job_default_alone(self, async_client): + """Two different questions: one seeds the per-job toggle in the print + dialog, the other covers prints that never see that dialog.""" + await async_client.put("/api/v1/settings/", json={"default_confirm_outcome": True}) + + body = (await async_client.put("/api/v1/settings/", json={"confirm_outcome_external_prints": True})).json() + assert body["default_confirm_outcome"] is True + assert body["confirm_outcome_external_prints"] is True + + +class TestAStrandedQueueRowDoesNotMuteTheSetting: + """A ``printing`` row is not proof that Bambuddy started what is printing now. + + ``_completion_belongs_to_queue_item`` deliberately leaves a row open when a + completion's subtask name disagrees with the file it was dispatched with, + and the scheduler's stranded sweep only takes it back once the printer has + sat connected and terminal for minutes. Treating any such row as "we + dispatched this" switched the setting off for every screen-started print in + between -- the exact symptom the setting exists to cure. + """ + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_a_row_left_open_for_another_file_still_asks( + self, test_engine, db_session, printer_factory, archive_factory + ): + printer = await printer_factory() + stranded_for = await archive_factory( + printer.id, filename="SomeOtherJob.gcode.3mf", status="printing", with_run=False + ) + db_session.add( + PrintQueueItem( + printer_id=printer.id, + archive_id=stranded_for.id, + status="printing", + confirm_outcome=False, + ) + ) + await db_session.commit() + await _set_setting(db_session, "confirm_outcome_external_prints", "true") + stranded_filename = stranded_for.filename + + await _drive_print_start(test_engine, printer, download_ok=False) + + archive = await _created_archive(db_session, printer.id) + assert archive.filename != stranded_filename + assert archive.confirm_requested is True + + +class TestAReprintAsksAgain: + """The reprint reuses the archive row, and the completion prompt is gated on + ``user_verdict is None`` -- so without a reset the second run inherits the + first run's answer and is never asked about.""" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_the_previous_runs_verdict_does_not_carry_over( + self, test_engine, db_session, printer_factory, archive_factory + ): + from backend.app.main import register_expected_print + + printer = await printer_factory() + archive = await archive_factory( + printer.id, + filename=f"{SUBTASK}.gcode.3mf", + status="completed", + confirm_requested=True, + user_verdict="good", + confirm_token="token-from-the-first-run", + with_run=False, + ) + archive_id, printer_id = archive.id, printer.id + register_expected_print(printer_id, f"{SUBTASK}.gcode.3mf", archive_id) + + await _drive_print_start(test_engine, printer, download_ok=False) + + db_session.expire_all() + refreshed = await db_session.get(PrintArchive, archive_id) + assert refreshed.status == "printing" + assert refreshed.confirm_requested is True + assert refreshed.user_verdict is None + assert refreshed.confirm_token is None + + # And the completion really does ask again, which is the point. + refreshed.status = "completed" + await db_session.commit() + sent, ws, notif = await _dispatch(db_session, printer_id, archive_id) + assert sent is True + ws.send_print_confirm_request.assert_awaited_once() + notif.on_print_confirm_request.assert_awaited_once() + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_an_archive_that_was_never_asked_about_keeps_its_verdict( + self, test_engine, db_session, printer_factory, archive_factory + ): + """The reset is scoped to rows that ask. An archive answered once and + later reprinted with the flag off must keep the answer it has.""" + from backend.app.main import register_expected_print + + printer = await printer_factory() + archive = await archive_factory( + printer.id, + filename=f"{SUBTASK}.gcode.3mf", + status="completed", + confirm_requested=False, + user_verdict="reject", + with_run=False, + ) + archive_id = archive.id + register_expected_print(printer.id, f"{SUBTASK}.gcode.3mf", archive_id) + + await _drive_print_start(test_engine, printer, download_ok=False) + + db_session.expire_all() + assert (await db_session.get(PrintArchive, archive_id)).user_verdict == "reject" + + +async def _dispatch(db_session, printer_id: int, archive_id: int, **kwargs): + """Run the completion path's outcome dispatch with the two emitters mocked.""" + from backend.app.main import dispatch_outcome_confirmation + + with ( + patch("backend.app.main.ws_manager") as ws, + patch("backend.app.main.notification_service") as notif, + ): + ws.send_print_confirm_request = AsyncMock() + notif.on_print_confirm_request = AsyncMock() + sent = await dispatch_outcome_confirmation( + db_session, + printer_id, + "Bench P1S", + {"subtask_name": SUBTASK}, + archive_id, + **kwargs, + ) + return sent, ws, notif + + +class TestTheCompletionEmitsThePrompt: + """The half of the feature the user actually sees. + + Everything else here asserts a column value; this drives the block + ``on_print_complete``'s notification task runs -- which is wrapped in a bare + ``except Exception`` that only logs, so a regression in it is invisible + unless something pins it. + """ + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_an_external_print_gets_a_prompt_with_one_tap_links(self, test_engine, db_session, printer_factory): + printer = await printer_factory() + printer_id = printer.id + await _set_setting(db_session, "confirm_outcome_external_prints", "true") + await _set_setting(db_session, "external_url", "https://farm.example.com/") + + await _drive_print_start(test_engine, printer, download_ok=False) + + archive = await _created_archive(db_session, printer_id) + archive_id = archive.id + assert archive.confirm_requested is True + archive.status = "completed" + await db_session.commit() + + sent, ws, notif = await _dispatch(db_session, printer_id, archive_id) + + assert sent is True + ws.send_print_confirm_request.assert_awaited_once() + assert ws.send_print_confirm_request.await_args.args[1]["archive_id"] == archive_id + + notif.on_print_confirm_request.assert_awaited_once() + kwargs = notif.on_print_confirm_request.await_args.kwargs + token = (await db_session.get(PrintArchive, archive_id)).confirm_token + assert token, "the one-tap links need a minted capability token" + assert kwargs["good_url"] == f"https://farm.example.com/api/v1/archives/confirm/{token}/good" + assert kwargs["reject_url"] == f"https://farm.example.com/api/v1/archives/confirm/{token}/reject" + assert kwargs["confirm_url"] == f"https://farm.example.com/archives?confirm={archive_id}" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_the_links_stay_absolute_without_an_external_url(self, test_engine, db_session, printer_factory): + """Telegram's inline keyboard and ntfy's action buttons are both dropped + for a relative URL, so an install that never set external_url used to + get a message carrying two unusable paths and no buttons at all.""" + printer = await printer_factory() + printer_id = printer.id + await _set_setting(db_session, "confirm_outcome_external_prints", "true") + + await _drive_print_start(test_engine, printer, download_ok=False) + + archive = await _created_archive(db_session, printer_id) + archive_id = archive.id + archive.status = "completed" + await db_session.commit() + + _, _, notif = await _dispatch(db_session, printer_id, archive_id) + + kwargs = notif.on_print_confirm_request.await_args.kwargs + assert kwargs["good_url"].startswith("http") + assert kwargs["reject_url"].startswith("http") + assert kwargs["confirm_url"].startswith("http") + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_a_failed_prompt_leaves_the_session_usable_for_the_email(self, db_session): + """The completion task sends the per-user print email on the same + session right after the prompt, so a prompt that dies mid-flush must + not take the email with it.""" + from backend.app.main import _dispatch_outcome_confirmation_safely + + async def _dies_mid_flush(db, *args, **kwargs): + db.add(Settings(key="x_1898_duplicate", value="a")) + db.add(Settings(key="x_1898_duplicate", value="b")) + await db.flush() + + with patch("backend.app.main.dispatch_outcome_confirmation", _dies_mid_flush): + await _dispatch_outcome_confirmation_safely(db_session, 1, "X1C", {}, 1, {}) + + # What _dispatch_user_print_email does next: read from the session. + # Without the rollback this raises PendingRollbackError. + await db_session.execute(select(Settings).limit(1)) + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_an_already_answered_archive_is_not_asked_again(self, db_session, printer_factory, archive_factory): + printer = await printer_factory() + archive = await archive_factory( + printer.id, status="completed", confirm_requested=True, user_verdict="good", with_run=False + ) + + sent, ws, notif = await _dispatch(db_session, printer.id, archive.id) + + assert sent is False + ws.send_print_confirm_request.assert_not_awaited() + notif.on_print_confirm_request.assert_not_awaited() + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_an_archive_that_never_opted_in_is_not_asked(self, db_session, printer_factory, archive_factory): + printer = await printer_factory() + archive = await archive_factory(printer.id, status="completed", confirm_requested=False, with_run=False) + + sent, _, notif = await _dispatch(db_session, printer.id, archive.id) + + assert sent is False + notif.on_print_confirm_request.assert_not_awaited() diff --git a/backend/tests/integration/test_print_confirmation.py b/backend/tests/integration/test_print_confirmation.py new file mode 100644 index 000000000..31302648f --- /dev/null +++ b/backend/tests/integration/test_print_confirmation.py @@ -0,0 +1,605 @@ +"""Integration tests for the post-print outcome confirmation (#1898). + +Covers the verdict PATCH (incl. the #1444-style mirror to the latest +PrintLogEntry and token retirement), the unauthenticated capability-token +endpoint, response defaults, and the verdict-aware statistics. +""" + +import re + +import pytest +from httpx import AsyncClient +from sqlalchemy import select + +from backend.app.models.print_log import PrintLogEntry + + +class TestOutcomeVerdictPatch: + @pytest.mark.asyncio + @pytest.mark.integration + async def test_defaults_present_on_response(self, async_client: AsyncClient, archive_factory, printer_factory): + """Archives created without any verdict expose the new fields with + their defaults — no verdict, no pending confirmation.""" + printer = await printer_factory() + archive = await archive_factory(printer.id) + + response = await async_client.get(f"/api/v1/archives/{archive.id}") + assert response.status_code == 200 + body = response.json() + assert body["user_verdict"] is None + assert body["confirm_requested"] is False + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_patch_verdict_mirrors_to_latest_log_entry( + self, async_client: AsyncClient, archive_factory, printer_factory, db_session + ): + """Setting the verdict via PATCH lands on the archive AND the latest + PrintLogEntry (verdict-aware statistics read the log), and retires a + pending confirmation token.""" + printer = await printer_factory() + archive = await archive_factory(printer.id, confirm_requested=True, confirm_token="test-token-mirror") + + response = await async_client.patch(f"/api/v1/archives/{archive.id}", json={"user_verdict": "reject"}) + assert response.status_code == 200 + assert response.json()["user_verdict"] == "reject" + + entry = await db_session.scalar( + select(PrintLogEntry).where(PrintLogEntry.archive_id == archive.id).order_by(PrintLogEntry.id.desc()) + ) + assert entry is not None + assert entry.user_verdict == "reject" + + await db_session.refresh(archive) + assert archive.user_verdict == "reject" + # Retired by stamping, not by dropping the value: the link stays + # resolvable so a later tap can be told it is already answered. + assert archive.confirm_token == "test-token-mirror" + assert archive.confirm_token_used_at is not None + assert archive.user_verdict_source == "api" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_patch_rejects_unknown_verdict(self, async_client: AsyncClient, archive_factory, printer_factory): + printer = await printer_factory() + archive = await archive_factory(printer.id) + + response = await async_client.patch(f"/api/v1/archives/{archive.id}", json={"user_verdict": "meh"}) + assert response.status_code == 422 + + +class TestConfirmTokenEndpoint: + @pytest.mark.asyncio + @pytest.mark.integration + async def test_token_records_verdict_and_retires_token( + self, async_client: AsyncClient, archive_factory, printer_factory, db_session + ): + """The one-tap link from a push notification records the verdict + without auth, mirrors it to the log entry, and single-uses the token.""" + printer = await printer_factory() + archive = await archive_factory(printer.id, confirm_requested=True, confirm_token="test-token-good") + + response = await async_client.post("/api/v1/archives/confirm/test-token-good/good") + assert response.status_code == 200 + assert "text/html" in response.headers["content-type"] + + await db_session.refresh(archive) + assert archive.user_verdict == "good" + assert archive.user_verdict_source == "link" + assert archive.confirm_token == "test-token-good" + assert archive.confirm_token_used_at is not None + + entry = await db_session.scalar( + select(PrintLogEntry).where(PrintLogEntry.archive_id == archive.id).order_by(PrintLogEntry.id.desc()) + ) + assert entry is not None + assert entry.user_verdict == "good" + + # Second use of the same token: spent, and said so rather than 404. + response = await async_client.get("/api/v1/archives/confirm/test-token-good/reject") + assert response.status_code == 200 + assert "Already answered" in response.text + await db_session.refresh(archive) + assert archive.user_verdict == "good" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_spent_link_reports_the_recorded_verdict_without_changing_it( + self, async_client: AsyncClient, archive_factory, printer_factory, db_session + ): + """The live-farm case (#1898): the plate-clear default answered the + prompt, then the Telegram button was tapped. The link must name the + verdict on file, say how it got there, and leave it alone.""" + from backend.app.services.print_confirmation import resolve_pending_confirmation_as_good + + printer = await printer_factory() + archive = await archive_factory(printer.id, confirm_requested=True, confirm_token="plate-cleared-token") + + assert await resolve_pending_confirmation_as_good(db_session, printer.id) == archive.id + await db_session.commit() + + response = await async_client.get("/api/v1/archives/confirm/plate-cleared-token/reject") + assert response.status_code == 200 + body = response.text + assert "Already answered" in body + assert "Good part" in body + assert "plate was cleared" in body + assert f"/archives?confirm={archive.id}" in body + + await db_session.refresh(archive) + assert archive.user_verdict == "good" + assert archive.user_verdict_source == "plate_clear" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_spent_link_after_the_verdict_was_cleared_again( + self, async_client: AsyncClient, archive_factory, printer_factory + ): + """Clearing the verdict in the app does not un-spend the link: the + capability was used, so the page explains rather than re-opening it.""" + printer = await printer_factory() + archive = await archive_factory(printer.id, confirm_requested=True, confirm_token="cleared-again-token") + + assert (await async_client.post("/api/v1/archives/confirm/cleared-again-token/good")).status_code == 200 + assert ( + await async_client.patch(f"/api/v1/archives/{archive.id}", json={"user_verdict": None}) + ).status_code == 200 + + response = await async_client.get("/api/v1/archives/confirm/cleared-again-token/good") + assert response.status_code == 200 + assert "Already answered" in response.text + assert (await async_client.get(f"/api/v1/archives/{archive.id}")).json()["user_verdict"] is None + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_unknown_token_and_garbage_verdict(self, async_client: AsyncClient): + assert (await async_client.get("/api/v1/archives/confirm/no-such-token/good")).status_code == 404 + assert (await async_client.get("/api/v1/archives/confirm/whatever/maybe")).status_code == 400 + assert (await async_client.post("/api/v1/archives/confirm/no-such-token/good")).status_code == 404 + assert (await async_client.post("/api/v1/archives/confirm/whatever/maybe")).status_code == 400 + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_a_get_records_nothing_whoever_sends_it( + self, async_client: AsyncClient, archive_factory, printer_factory, db_session + ): + """The blocker. Telegram and Slack GET the URLs in a message to build a + preview card, mail gateways detonate them before delivery, proxies and + browsers prefetch. While GET was the route that recorded, any of those + settled the outcome before the operator read the question — always + towards 'good', because good_url came first — and spent the token, so + the real tap landed on "already answered". A scrap part counted as a + success for good, in the statistics this branch adds. + + The heuristic below decides how the page behaves, not whether the + verdict is written: nothing a GET can say records anything. + """ + printer = await printer_factory() + archive = await archive_factory(printer.id, confirm_requested=True, confirm_token="unfurler-token") + + for agent in ( + "TelegramBot (like TwitterBot)", + "Slackbot-LinkExpanding 1.0", + "Mimecast-Link-Protect", + # The one that used to be allowed straight through to the write. + "Mozilla/5.0 (iPhone; CPU iPhone OS 17_5 like Mac OS X) AppleWebKit/605.1.15 " + "(KHTML, like Gecko) Version/17.5 Mobile/15E148 Safari/604.1", + ): + response = await async_client.get( + "/api/v1/archives/confirm/unfurler-token/good", headers={"user-agent": agent} + ) + assert response.status_code == 200, agent + assert "Confirm this outcome" in response.text, agent + assert "