Merge remote-tracking branch 'origin/main'

Resolved CURRENT TODO conflict: kept Thomas's reformatted list and new
items, dropped the two scan-to-download lines this branch just finished.

CLAUDE.md

@@ -9,12 +9,10 @@ photos/calendar/whiteboard widget system to the device.
 CURRENT TODO
 -add more actions for buttons (i.e. change widget/layout)
 -Fix spurious button assignment stuff (probably but buttons on widget config with sane defaults)
--FIX Scan to download
 -make a weather widget
 -widget border option
 -battery life widget
 -sharing layouts with linked users
--when multiple photo widgets on layout, the "scan to download" should create a share with all the photos on 
 -a "coming up this week" widget
 -scan to download for non-immich photos too?
 -switch button reset action? and on reset dismiss the menu.
@@ -53,6 +51,16 @@ Start here, don't re-derive from scratch:
   tempted to exclude and why. This repo shipped a token gate once that
   covered `/api/*` but left `/frame/image` -- the actual photo bytes --
   open; caught immediately in production.
+- **Commit and push once a task is verified working, without waiting to
+  be asked separately.** Once tests pass (and, for UI changes, the
+  browser check has been done), stage the relevant files, write a normal
+  commit message, and push to the current branch -- the maintainer's
+  standing authorization for the commit/push step itself. This doesn't
+  relax anything else: still run `git status`/review the diff before
+  staging, still never force-push/amend a pushed commit/skip hooks, and
+  still surface anything that looks like it needs a real decision (e.g.
+  a change that would trigger `main`'s deploy workflow, see below)
+  instead of pushing through it silently.
 
 ## Working in this repo
 

server/README.md

@@ -121,9 +121,11 @@ Pages: `/` (routing hub), `/setup`, `/login`, `/claim`, `/settings`,
   this response may grow.
 - `GET /frame/photo-info` -- location/date overlay text for the manage
   menu (city + abbreviated US/CAN region or country, `MM/DD/YY`).
-- `GET /frame/share/{asset_id}` -- creates a 30-minute public Immich
+- `GET /frame/share/{manage_token}` -- creates a 30-minute public Immich
-  share link and 302s to it; scoped to the photo currently showing or
+  share link covering every photo widget's currently-showing photo on
-  queued on *this* frame only.
+  *this* frame and 302s to it. Authenticated by the frame's own
+  `manage_token` (see the manage QR below), not device credentials -- a
+  phone scanning the QR has no way to supply `?id=`/`?token=`.
 - `GET /frame/face-labels` -- up to 4 named faces with 800x480
   positions, flattened (`name_0`/`x_0`/`y_0`, ...) for the device's
   flat-scalar parser.
@@ -200,12 +202,12 @@ Pages: `/` (routing hub), `/setup`, `/login`, `/claim`, `/settings`,
   in the web UI (or using "Show next") only rearranges what's already in
   that lookahead; it doesn't add or remove photos from the album.
 - Auth in one breath: browsers use sessions (+CSRF), devices use
-  per-frame tokens (`?id=` + `?token=`), the manage QR uses its own
+  per-frame tokens (`?id=` + `?token=`), the manage QR and the
-  limited token, and `MANAGEMENT_TOKEN` survives only as the migration
+  scan-to-download QR both use the frame's own `manage_token` (device
-  credential for pre-multi-frame firmware. `/frame/share` stays scoped
+  tokens don't work for either -- neither is ever called by firmware,
-  to photos this frame is actually showing or has queued, not any
+  both are opened by a phone that has no way to supply `?id=`/`?token=`),
-  Immich asset ID someone might guess -- a second layer a leaked device
+  and `MANAGEMENT_TOKEN` survives only as the migration credential for
-  token alone wouldn't bypass.
+  pre-multi-frame firmware.
 - Calendar frame mode (`app/calendar_feed.py`) expands recurring events
   (RRULE/EXDATE/DST) via [`recurring-ical-events`](https://pypi.org/project/recurring-ical-events/),
   which is LGPL-3.0-or-later -- the only non-permissively-licensed

server/app/immich_client.py

@@ -80,10 +80,10 @@ class ImmichClient:
         resp.raise_for_status()
         return resp.json()
 
-    def create_share_link(self, asset_id: str, expires_in_s: int) -> str:
+    def create_share_link(self, asset_ids: list[str], expires_in_s: int) -> str:
-        """Creates a public, view-only Immich share link for a single
+        """Creates a public, view-only Immich share link covering one or
-        asset, expiring expires_in_s seconds from now, and returns its
+        more assets, expiring expires_in_s seconds from now, and returns
-        public URL. Used by the manage-button overlay's share QR --
+        its public URL. Used by the manage-button overlay's share QR --
         created lazily (only when someone actually scans it), not when
         the button's pressed, so the expiry clock starts when it's
         actually used."""
@@ -93,7 +93,7 @@ class ImmichClient:
             headers=self._headers,
             json={
                 "type": "INDIVIDUAL",
-                "assetIds": [asset_id],
+                "assetIds": list(asset_ids),
                 "expiresAt": expires_at,
                 "allowUpload": False,
                 "allowDownload": True,

server/app/routers/api_widgets.py

@@ -493,8 +493,8 @@ def api_widget_thumbnail(
     """Scoped to what this widget is actually showing/queuing -- a user
     merely linked to view this frame shouldn't be able to pull thumbnails
     for arbitrary asset ids in the owner's Immich library, only this
-    widget's own curated album. Same rule device.frame_share and
+    widget's own curated album. Same rule manage.manage_thumbnail
-    manage.manage_thumbnail already enforce."""
+    already enforces."""
     frame, widget = frame_widget
     _require_widget_type(widget, "photos")
     pcfg = db.get(PhotoWidgetConfig, widget.id)

server/app/routers/common.py

@@ -449,7 +449,15 @@ def build_manage_content(db: Session, frame: Frame, request) -> dict:
             exif = asset.get("exifInfo") or {}
             content["location_lines"] = _format_location(exif)
             content["taken_at"] = _format_taken_at(exif)
-            content["share_url"] = f"{base}/frame/share/{primary_cfg.current_asset_id}"
+
+    # Unlike location/date-taken (a single fixed corner, so tied to the
+    # one "primary" widget above), the share link covers every photo
+    # widget's current photo (see manage.manage_share) -- so it only
+    # needs *some* photo widget to have a current photo, not specifically
+    # the primary one, and doesn't depend on the EXIF fetch above
+    # succeeding.
+    if any(db.get(PhotoWidgetConfig, w.id).current_asset_id for w in photo_widgets):
+        content["share_url"] = f"{base}/frame/share/{frame.manage_token}"
 
     panel_w, panel_h = logical_render_size(frame.orientation)
     face_labels: list[dict] = []

server/app/routers/device.py

@@ -15,9 +15,8 @@ from __future__ import annotations
 import logging
 import time
 
-import httpx
 from fastapi import APIRouter, Depends, HTTPException, Request
-from fastapi.responses import FileResponse, RedirectResponse, Response
+from fastapi.responses import FileResponse, Response
 from pydantic import BaseModel
 from sqlalchemy import delete, func, select
 from sqlalchemy.orm import Session
@@ -27,7 +26,7 @@ from ..auth import get_server_settings, require_device
 from ..db import frame_locked, get_db
 from ..firmware import firmware_path
 from ..image_pipeline import logical_render_size, render_panel, render_placeholder
-from ..models import BatteryLog, Frame, FrameButtonAction, PhotoWidgetConfig, Widget
+from ..models import BatteryLog, Frame, FrameButtonAction, Widget
 from ..widgets import WIDGET_TYPES
 from .common import (
     BATTERY_HISTORY_MAX,
@@ -35,9 +34,6 @@ from .common import (
     RECHARGE_JUMP_PCT,
     RECHARGE_LOOKBACK,
     build_manage_content,
-    immich_client_for,
-    immich_creds,
-    photo_widgets_for_frame,
 )
 
 logger = logging.getLogger(__name__)
@@ -356,34 +352,3 @@ def frame_firmware(frame: Frame = Depends(require_device)):
     if not path.exists():
         raise HTTPException(404, "No firmware uploaded")
     return FileResponse(path, media_type="application/octet-stream")
-
-
-@router.get("/frame/share/{asset_id}")
-def frame_share(asset_id: str, frame: Frame = Depends(require_device), db: Session = Depends(get_db)):
-    """Creates a 30-minute public Immich share link for asset_id and
-    redirects to it -- what the manage overlay's bottom-left QR code
-    points to. The link is created lazily, when this actually gets hit
-    (i.e. when someone scans it), not when the manage button was
-    pressed, so the 30-minute window starts when it's actually used.
-    Also scoped to the photo currently showing or queued on one of THIS
-    frame's own photo widgets -- not any arbitrary Immich asset id -- as
-    a second layer even a leaked token wouldn't bypass."""
-    url, key = immich_creds(frame)
-    if not url or not key:
-        raise HTTPException(400, "Immich URL/API key not configured yet")
-
-    photo_widgets = photo_widgets_for_frame(db, frame)
-    showing_or_queued = any(
-        asset_id == cfg.current_asset_id or asset_id in cfg.queue
-        for cfg in (db.get(PhotoWidgetConfig, w.id) for w in photo_widgets)
-    )
-    if not showing_or_queued:
-        raise HTTPException(404, "That photo isn't currently showing or queued on this frame")
-
-    client = immich_client_for(frame)
-    try:
-        share_url = client.create_share_link(asset_id, expires_in_s=1800)
-    except httpx.HTTPError as e:
-        raise HTTPException(502, f"Could not create share link: {e}") from e
-
-    return RedirectResponse(share_url)

server/app/routers/manage.py

@@ -11,7 +11,7 @@ import logging
 
 import httpx
 from fastapi import APIRouter, Depends, HTTPException, Request
-from fastapi.responses import HTMLResponse, Response
+from fastapi.responses import HTMLResponse, RedirectResponse, Response
 from fastapi.templating import Jinja2Templates
 from pydantic import BaseModel
 from sqlalchemy import select
@@ -19,8 +19,14 @@ from sqlalchemy.orm import Session
 
 from .. import photo_queue, quiet_hours
 from ..db import get_db, widget_locked
-from ..models import Frame
+from ..models import Frame, PhotoWidgetConfig
-from .common import immich_client_for, list_assets, photo_widget_config_or_404
+from .common import (
+    immich_client_for,
+    immich_creds,
+    list_assets,
+    photo_widget_config_or_404,
+    photo_widgets_for_frame,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -120,3 +126,37 @@ def manage_thumbnail(asset_id: str, frame: Frame = Depends(require_manage), db:
     except httpx.HTTPError as e:
         raise HTTPException(502, f"Could not download thumbnail from Immich: {e}") from e
     return Response(content=content, media_type=content_type)
+
+
+@router.get("/frame/share/{manage_token}")
+def manage_share(frame: Frame = Depends(require_manage), db: Session = Depends(get_db)):
+    """Creates a 30-minute public Immich share link covering every photo
+    widget's currently-displayed asset on this frame, and redirects to it
+    -- what the manage overlay's bottom-left QR code points to. Lazily
+    created (only when someone actually scans it, not when the manage
+    button was pressed), so the 30-minute window starts at actual use.
+    Keyed on this frame's own manage_token, like the rest of this router,
+    rather than device credentials -- a phone scanning a QR code has no
+    way to supply the device's ?id=/?token=, which is why this used to
+    silently fall back to whichever frame happened to still carry the
+    legacy migration token instead of the frame that was actually
+    scanned."""
+    photo_widgets = photo_widgets_for_frame(db, frame)
+    asset_ids: list[str] = []
+    for widget in photo_widgets:
+        cfg = db.get(PhotoWidgetConfig, widget.id)
+        if cfg.current_asset_id and cfg.current_asset_id not in asset_ids:
+            asset_ids.append(cfg.current_asset_id)
+    if not asset_ids:
+        raise HTTPException(404, "No photos currently showing on this frame")
+
+    url, key = immich_creds(frame)
+    if not url or not key:
+        raise HTTPException(400, "Immich URL/API key not configured yet")
+
+    client = immich_client_for(frame)
+    try:
+        share_url = client.create_share_link(asset_ids, expires_in_s=1800)
+    except httpx.HTTPError as e:
+        raise HTTPException(502, f"Could not create share link: {e}") from e
+    return RedirectResponse(share_url)

server/tests/test_device_widget_dispatch.py

@@ -1,7 +1,7 @@
 """End-to-end HTTP tests for the widget-system cutover in
-routers/device.py -- /frame/image, /frame/advance, /frame/back, and
+routers/device.py -- /frame/image, /frame/advance, and /frame/back
-/frame/share against real widget rows (via the migration-backfilled
+against real widget rows (via the migration-backfilled frame #1, or a
-frame #1, or a purpose-built second frame), a real TestClient, real
+purpose-built second frame), a real TestClient, real
 render_panel/compose_into. Only Immich itself is mocked (monkeypatched
 at the app.widgets.photos module boundary, same pattern as
 test_widgets_photos.py) -- everything else in the pipeline is real.
@@ -23,7 +23,6 @@ from app.models import (
     Frame,
     FrameButtonAction,
     PhotoWidgetConfig,
-    User,
     Widget,
 )
 
@@ -145,33 +144,3 @@ def test_two_widget_frame_composites_both_and_next_targets_calendar(client, db_s
     photo_cfg = db_session.get(PhotoWidgetConfig, photo_widget.id)
     assert cal_cfg.browse_offset == 1
     assert photo_cfg.current_asset_id == ""  # untouched -- NEXT was never bound to it
-
-
-def test_frame_share_checks_widget_scoped_state(client, db_session, monkeypatch):
-    client.post("/setup", data={"username": "alice", "password": "hunter22"})
-    frame = db_session.get(Frame, 1)
-    frame.owner_user_id = db_session.query(User).filter_by(username="alice").one().id
-    widget = db_session.query(Widget).filter_by(frame_id=frame.id, widget_type="photos").one()
-    cfg = db_session.get(PhotoWidgetConfig, widget.id)
-    cfg.album_id = "album-1"
-    cfg.current_asset_id = "asset-1"
-    cfg.queue = ["asset-2"]
-    frame.immich_url = "http://immich.example.com"
-    frame.immich_api_key = "key"
-    db_session.commit()
-
-    # Not showing/queued -- rejected before ever touching Immich
-    resp = client.get("/frame/share/asset-not-on-this-frame")
-    assert resp.status_code == 404
-
-    # Currently showing -- allowed through to the (mocked) Immich call
-    monkeypatch.setattr(
-        "app.routers.device.immich_client_for",
-        lambda frame: type("C", (), {"create_share_link": lambda self, asset_id, expires_in_s: "https://immich.example.com/share/abc"})(),
-    )
-    resp = client.get("/frame/share/asset-1", follow_redirects=False)
-    assert resp.status_code in (302, 303, 307)
-
-    # Queued (not current) -- also allowed
-    resp = client.get("/frame/share/asset-2", follow_redirects=False)
-    assert resp.status_code in (302, 303, 307)

server/tests/test_manage_widget_dispatch.py

@@ -119,3 +119,106 @@ def test_manage_thumbnail_scoped_to_showing_or_queued(client, db_session, monkey
 def test_unknown_manage_token_404s(client, db_session):
     resp = client.get("/api/m/not-a-real-token/queue")
     assert resp.status_code == 404
+
+
+def test_manage_share_requires_current_photo(client, db_session):
+    client.post("/setup", data={"username": "alice", "password": "hunter22"})
+    frame, _ = _configure_photo_widget(db_session)
+    resp = client.get(f"/frame/share/{frame.manage_token}", follow_redirects=False)
+    assert resp.status_code == 404
+
+
+def test_manage_share_creates_link_and_redirects(client, db_session, monkeypatch):
+    client.post("/setup", data={"username": "alice", "password": "hunter22"})
+    frame, widget = _configure_photo_widget(db_session)
+    cfg = db_session.get(PhotoWidgetConfig, widget.id)
+    cfg.current_asset_id = "asset-1"
+    db_session.commit()
+
+    calls = []
+    monkeypatch.setattr(
+        "app.routers.manage.immich_client_for",
+        lambda frame: type("C", (), {
+            "create_share_link": lambda self, asset_ids, expires_in_s: (
+                calls.append(asset_ids) or "https://immich.example.com/share/abc"
+            ),
+        })(),
+    )
+    resp = client.get(f"/frame/share/{frame.manage_token}", follow_redirects=False)
+    assert resp.status_code in (302, 303, 307)
+    assert calls == [["asset-1"]]
+
+
+def test_manage_share_covers_every_photo_widget(client, db_session, monkeypatch):
+    from app.models import Widget
+
+    client.post("/setup", data={"username": "alice", "password": "hunter22"})
+    frame, widget = _configure_photo_widget(db_session)
+    cfg = db_session.get(PhotoWidgetConfig, widget.id)
+    cfg.current_asset_id = "asset-1"
+
+    second_widget = Widget(frame_id=frame.id, widget_type="photos", x=4, y=0, w=4, h=5,
+                           sort_order=1, created_at=0)
+    db_session.add(second_widget)
+    db_session.flush()
+    db_session.add(PhotoWidgetConfig(widget_id=second_widget.id, album_id="album-2", current_asset_id="asset-9"))
+    db_session.commit()
+
+    calls = []
+    monkeypatch.setattr(
+        "app.routers.manage.immich_client_for",
+        lambda frame: type("C", (), {
+            "create_share_link": lambda self, asset_ids, expires_in_s: (
+                calls.append(asset_ids) or "https://immich.example.com/share/abc"
+            ),
+        })(),
+    )
+    resp = client.get(f"/frame/share/{frame.manage_token}", follow_redirects=False)
+    assert resp.status_code in (302, 303, 307)
+    assert calls == [["asset-1", "asset-9"]]
+
+
+def test_manage_share_scoped_to_its_own_frame(client, db_session, monkeypatch):
+    """The regression test for the original bug: each frame's own share
+    QR must only ever surface that frame's own current photo, never
+    another frame's -- previously it silently resolved via whichever
+    frame happened to still carry the legacy migration token, regardless
+    of which frame's QR was actually scanned."""
+    from app.models import Widget
+
+    client.post("/setup", data={"username": "alice", "password": "hunter22"})
+    frame_a, widget_a = _configure_photo_widget(db_session)
+    cfg_a = db_session.get(PhotoWidgetConfig, widget_a.id)
+    cfg_a.current_asset_id = "asset-a"
+
+    frame_b = Frame(
+        name="Frame B", device_id="112233445566", device_token="devtok-b",
+        manage_token="mtok-b", orientation="landscape", created_at=0,
+        immich_url="http://immich.example.com", immich_api_key="key",
+    )
+    db_session.add(frame_b)
+    db_session.flush()
+    widget_b = Widget(frame_id=frame_b.id, widget_type="photos", x=0, y=0, w=4, h=5,
+                      sort_order=0, created_at=0)
+    db_session.add(widget_b)
+    db_session.flush()
+    db_session.add(PhotoWidgetConfig(widget_id=widget_b.id, album_id="album-b", current_asset_id="asset-b"))
+    db_session.commit()
+
+    calls = []
+    monkeypatch.setattr(
+        "app.routers.manage.immich_client_for",
+        lambda frame: type("C", (), {
+            "create_share_link": lambda self, asset_ids, expires_in_s: (
+                calls.append(asset_ids) or "https://immich.example.com/share/abc"
+            ),
+        })(),
+    )
+    client.get(f"/frame/share/{frame_a.manage_token}", follow_redirects=False)
+    client.get(f"/frame/share/{frame_b.manage_token}", follow_redirects=False)
+    assert calls == [["asset-a"], ["asset-b"]]
+
+
+def test_unknown_manage_share_token_404s(client, db_session):
+    resp = client.get("/frame/share/not-a-real-token")
+    assert resp.status_code == 404