● fix(backup): Gitea wraps GitCommit in Commit schema — extract tree SHA from both shapes (issue #1224 follow-up)

Subsequent backups against Gitea 1.24+ failed with the opaque
  "Backup failed: 'tree'" message after the initial-backup fix landed in
  7ee89b56. Root cause: Gitea's GET /repos/{owner}/{repo}/git/commits/{sha}
  returns the wrapped Commit schema where the tree lives at
  data["commit"]["tree"]["sha"], whereas GitHub's same-named Git Database
  endpoint returns the unwrapped GitCommit schema with tree at the top
  level. The bare commit_response.json()["tree"]["sha"] lookup at
  gitea.py:109 raised KeyError: 'tree' and the broad except in push_files
  surfaced it as the opaque "Backup failed: 'tree'" string — masking the
  real shape mismatch.

  Adds a _commit_tree_sha() helper that tries the flat shape first
  (GitHub-compatible / older Gitea) and falls back to the wrapped shape
  (Gitea 1.24+, Forgejo). Returns None on truly malformed responses;
  push_files maps that to a clear "Failed to extract tree SHA from commit
  response" instead of leaking a KeyError repr. Keeps the existing-files
  diff working on both shapes so subsequent backups don't re-upload every
  blob — preferred over the .get()-and-skip approach which would have
  required also dropping base_tree from the tree POST and re-uploading
  unchanged files on every backup.
This commit is contained in:
maziggy
2026-05-08 07:00:22 +02:00
parent 61314cf20b
commit 233808956b
3 changed files with 94 additions and 2 deletions
+1 -1
View File
@@ -12,7 +12,7 @@ All notable changes to Bambuddy will be documented in this file.
- **3D Preview returned `{"detail":"Not Found"}` in Docker installs** ([#1218](https://github.com/maziggy/bambuddy/issues/1218)) — The embedded GCode viewer's static assets (`gcode_viewer/`) were not copied into the production Docker image, so clicking "3D Preview" on any archive loaded an iframe at `/gcode-viewer/?archive=<id>` that returned a bare FastAPI 404 — Firefox / Chrome rendered the JSON response inside the iframe area while the outer Bambuddy layout looked normal, masking the failure unless the user actually inspected the iframe. The Vite production build doesn't stage `gcode_viewer/` into `static/` either (the dev server serves it via a `configureServer` middleware that's dev-only), and the only integration test for the route accepted `404` as a valid outcome ("`assert response.status_code in (200, 404)`") so CI never caught the missing files. Affected every Docker build since the embedded viewer landed in 0.2.4b1 (commit `3adce435`, 2026-04-22). **Fix:** `Dockerfile` now copies the `gcode_viewer/` directory alongside the React build output. **Defence in depth:** `backend/app/main.py` logs an ERROR at startup when `_gcode_viewer_dir / "index.html"` is missing so future packaging gaps surface in `docker logs` and the support bundle instead of as silent runtime 404s. **Test guard:** `backend/tests/integration/test_gcode_viewer.py` adds `test_gcode_viewer_index_served_when_assets_present` which skips when the directory is intentionally absent (unit-test environments) but asserts `200 OK` + a non-empty HTML body when the assets do exist on disk — so a future broken `COPY` fails CI loudly rather than continuing to ship a broken image.
- **Slice button no longer enabled before the preview slice resolves** — Until the preview slice (or embedded-metadata read for already-sliced 3MFs) returned the per-plate filament list, the SliceModal rendered a synthetic single-slot fallback so the auto-pick had something to bind against. That made the Slice button enabled the moment the modal opened, even before the slicer had told us which AMS slots the plate actually consumes — clicking would dispatch against opaque defaults and the real-life print would either pick the wrong filament or fail with a slot-mismatch error after the fact. Adds `filamentReqsQuery.isSuccess` to the `isReady` chain so the button stays disabled while the preview slice is in flight (or before the backend's `/filament-requirements` call settles for sliced files) and flips to enabled the moment the real slot list lands and auto-pick fills it.
- **New AMS RFID rolls auto-named to the wrong colour when the hex is shared across material variants** ([#1227](https://github.com/maziggy/bambuddy/issues/1227)) — Inserting an Ivory White (PLA Matte) roll always created a spool named "Jade White" because the colour-catalog lookup in `create_spool_from_tray` filtered by manufacturer + hex only, with no `ORDER BY`. Three Bambu Lab catalog rows share `#FFFFFF` — Jade White (PLA Basic), Ivory White (PLA Matte), White (PLA Silk) — and SQLite returned them in rowid order, so the first-inserted entry (Jade White) won every time regardless of the actual material the AMS reported. Same class of bug bites any other shared-hex pair across PLA Basic / Matte / Silk; the whites were just the most visible. **Fix:** `spool_tag_matcher.py::create_spool_from_tray` now filters the catalog by `tray_sub_brands` too — the printer-reported material variant ("PLA Matte" / "PLA Basic" / "PLA Silk") matches the catalog's `material` column directly. The query also gets an explicit `ORDER BY id` so the fallback path (when `tray_sub_brands` is empty — third-party spools / OpenTag tags) is deterministic across SQLite + PostgreSQL instead of DB-implementation-defined. The catalog lookup uses the *raw* `tray_sub_brands` value (before the gradient/dual/tri-color subtype upgrade at lines 73-87) because the catalog stores `"PLA Basic"` for gradient rolls too — the upgraded subtype lives on the spool, not the catalog row. **Note for affected users:** spools already in the database under the wrong colour name (e.g. four Ivory White rolls labelled "Jade White") don't auto-correct on next AMS read — the matcher only fires when *creating* a new spool from RFID. Existing rows need a manual rename in Inventory after upgrading. **Tests:** 4 new in `test_spool_tag_matcher.py` — `test_ivory_white_pla_matte_resolves_to_ivory_not_jade` (the #1227 regression pin), `test_pla_silk_white_resolves_to_white_not_jade` (the third collision), `test_jade_white_pla_basic_still_resolves_correctly` (happy-path guard with all three #FFFFFF entries seeded), and `test_unknown_material_falls_back_to_hex_only_lookup` (third-party / empty `tray_sub_brands` path stays deterministic via ORDER BY).
- **Backups to Gitea / Forgejo failed with "Failed to create tree" on empty repos and "list indices must be integers or slices, not str" on populated repos** ([#1224](https://github.com/maziggy/bambuddy/issues/1224), [#1225](https://github.com/maziggy/bambuddy/issues/1225)) — Two interacting bugs in the Gitea/Forgejo backend, both inherited from `GitHubBackend` because PR #1160's class docstring assumed Gitea's Git Data API was fully GitHub-compatible. (1) **List-shaped ref response:** `GET /api/v1/repos/{owner}/{repo}/git/refs/heads/{branch}` returns a *list* of matching refs on Gitea/Forgejo even when only one matches (`[{"ref": ..., "object": {"sha": ...}}]`), whereas GitHub returns a single object. The inherited `push_files` and `_create_branch_and_push` did `ref_response.json()["object"]["sha"]` and crashed with `list indices must be integers or slices, not str` — surfacing as the failure at the top of any push against a populated Gitea repo (#1225's symptom, and #1224's symptom once the user committed any file before the first backup). (2) **Empty-repo writes refused:** GitHub's Git Data API accepts `POST /git/blobs` against a brand-new empty repo and creates the initial commit + branch implicitly. Gitea refuses every blob/tree/commit POST with 404 until the underlying git repo has at least one commit — so the inherited `_create_initial_commit` (which posts blobs → tree → commit → ref in that order) silently failed: every blob POST returned 404, `tree_items` ended up empty, and the next tree POST also returned 404 ("Failed to create tree" — #1224's symptom on a freshly-created empty Gitea repo). **Fix:** `GiteaBackend` now overrides `push_files`, `_create_branch_and_push`, and `_create_initial_commit` directly instead of inheriting them. The Git Data API path uses a `_ref_sha()` helper that accepts both list and dict shapes; the empty-repo bootstrap route uses Gitea's Contents API (`POST /api/v1/repos/{owner}/{repo}/contents` with a `files` array, `branch=<target>`, `new_branch=<target>`) which seeds the initial commit + branch in a single transaction — Contents API is documented to work on empty repos because it goes through Gitea's higher-level repo-init path. `GitHubBackend` is **untouched** — the GitHub backup path is proven working, the fix is fully isolated to the Gitea side. `ForgejoBackend(GiteaBackend)` inherits both fixes automatically; tests pin that. **Tests:** 10 new tests in `test_git_providers.py` — `TestGiteaBackendListShapeRefResponse` (4 tests: `_ref_sha` accepts list/dict/empty-list, plus full `push_files` happy paths against list-shaped branch ref and list-shaped default-branch ref), `TestGiteaBackendEmptyRepoInitialCommit` (4 tests: empty repo routes through Contents API exclusively with no blob/tree/commit/ref Git Data API calls, payload shape verified field-by-field against Gitea's documented schema, error truncation works, empty file dict returns `skipped` without firing a useless API call), and `TestForgejoInheritsGiteaFixes` (2 tests: list-shape and empty-repo paths both work via inheritance). Existing 6 `TestGiteaBackendPushFiles` tests still pass since `_ref_sha` accepts dict-shaped responses too. Total: 78 tests pass across the backup unit + integration suites; ruff clean.
- **Backups to Gitea / Forgejo failed with "Failed to create tree" on empty repos and "list indices must be integers or slices, not str" on populated repos** ([#1224](https://github.com/maziggy/bambuddy/issues/1224), [#1225](https://github.com/maziggy/bambuddy/issues/1225)) — Two interacting bugs in the Gitea/Forgejo backend, both inherited from `GitHubBackend` because PR #1160's class docstring assumed Gitea's Git Data API was fully GitHub-compatible. (1) **List-shaped ref response:** `GET /api/v1/repos/{owner}/{repo}/git/refs/heads/{branch}` returns a *list* of matching refs on Gitea/Forgejo even when only one matches (`[{"ref": ..., "object": {"sha": ...}}]`), whereas GitHub returns a single object. The inherited `push_files` and `_create_branch_and_push` did `ref_response.json()["object"]["sha"]` and crashed with `list indices must be integers or slices, not str` — surfacing as the failure at the top of any push against a populated Gitea repo (#1225's symptom, and #1224's symptom once the user committed any file before the first backup). (2) **Empty-repo writes refused:** GitHub's Git Data API accepts `POST /git/blobs` against a brand-new empty repo and creates the initial commit + branch implicitly. Gitea refuses every blob/tree/commit POST with 404 until the underlying git repo has at least one commit — so the inherited `_create_initial_commit` (which posts blobs → tree → commit → ref in that order) silently failed: every blob POST returned 404, `tree_items` ended up empty, and the next tree POST also returned 404 ("Failed to create tree" — #1224's symptom on a freshly-created empty Gitea repo). **Fix:** `GiteaBackend` now overrides `push_files`, `_create_branch_and_push`, and `_create_initial_commit` directly instead of inheriting them. The Git Data API path uses a `_ref_sha()` helper that accepts both list and dict shapes; the empty-repo bootstrap route uses Gitea's Contents API (`POST /api/v1/repos/{owner}/{repo}/contents` with a `files` array, `branch=<target>`, `new_branch=<target>`) which seeds the initial commit + branch in a single transaction — Contents API is documented to work on empty repos because it goes through Gitea's higher-level repo-init path. `GitHubBackend` is **untouched** — the GitHub backup path is proven working, the fix is fully isolated to the Gitea side. `ForgejoBackend(GiteaBackend)` inherits both fixes automatically; tests pin that. **Tests:** 10 new tests in `test_git_providers.py` — `TestGiteaBackendListShapeRefResponse` (4 tests: `_ref_sha` accepts list/dict/empty-list, plus full `push_files` happy paths against list-shaped branch ref and list-shaped default-branch ref), `TestGiteaBackendEmptyRepoInitialCommit` (4 tests: empty repo routes through Contents API exclusively with no blob/tree/commit/ref Git Data API calls, payload shape verified field-by-field against Gitea's documented schema, error truncation works, empty file dict returns `skipped` without firing a useless API call), and `TestForgejoInheritsGiteaFixes` (2 tests: list-shape and empty-repo paths both work via inheritance). Existing 6 `TestGiteaBackendPushFiles` tests still pass since `_ref_sha` accepts dict-shaped responses too. Total: 78 tests pass across the backup unit + integration suites; ruff clean. **Follow-up fix (still under #1224):** subsequent backups against Gitea 1.24+ then failed with the opaque "Backup failed: 'tree'" because Gitea's `GET /repos/{owner}/{repo}/git/commits/{sha}` returns the wrapped `Commit` schema (tree at `commit.tree.sha`), whereas GitHub's same-named Git Database endpoint returns the unwrapped `GitCommit` schema (tree at top level). The bare `commit_response.json()["tree"]["sha"]` lookup at `gitea.py:109` raised `KeyError: 'tree'` and the broad `except` surfaced it as the opaque message. **Fix:** `_commit_tree_sha()` helper that tries the flat shape first (GitHub-compatible / older Gitea) and falls back to the wrapped shape (Gitea 1.24+, Forgejo) — keeps the existing-files diff working on both shapes so subsequent backups don't re-upload every blob. **Tests:** new `TestGiteaBackendWrappedCommitResponse` (4 tests: helper accepts flat / wrapped / missing shapes, full `push_files` succeeds against a wrapped commit response, failure path surfaces a clear error message instead of `KeyError` when the tree SHA can't be extracted).
- **Docker data-volume ownership normalised at startup via gosu entrypoint** ([#1211](https://github.com/maziggy/bambuddy/issues/1211)) — Two long-standing failure modes have been biting Docker users repeatedly: (1) Docker named volumes are created by the daemon as `root:root`, and the previous `chmod 777 /app/data` Dockerfile workaround only covered the named-volume root — so subdirs Bambuddy creates at runtime (`virtual_printer/uploads`, `virtual_printer/certs`, etc.) inherited wrong ownership when the container ran as `1000:1000`. (2) The shipped `docker-compose.yml` ships `./virtual_printer:/app/data/virtual_printer` uncommented, and dockerd creates a missing bind-mount source on the host as root before the container starts — leaving the host directory unwritable by uid 1000 inside the container even though the named volume above it had the chmod-777 workaround. Symptom either way: `[Errno 13] Permission denied: '/app/data/virtual_printer/uploads'`, no virtual printer ever starts, "VP doesn't work" support reports follow. **Replaces the chmod-777 hack with a proper entrypoint:** `deploy/docker-entrypoint.sh` runs as root, chowns `/app/data` and `/app/logs` (and `/app/data/virtual_printer` when bind-mounted) to `PUID:PGID`, then drops to that uid via `gosu` before `exec`'ing the app. The chown is gated behind a top-level ownership check so subsequent restarts skip the recursive traversal — no multi-second startup penalty on multi-GB archive directories. A sentinel `.bambuddy` file in each data path prevents Docker from re-syncing image directory metadata on every mount (otherwise empty volumes have their ownership reverted from the image on each restart, defeating the idempotency). When the container is started with an explicit `user:` directive or `--user` flag the entrypoint detects it isn't root and falls through to direct `exec` — preserving compatibility for users who pin a specific uid. **Compose template changes:** removes `user: "${PUID:-1000}:${PGID:-1000}"` (the entrypoint owns privilege drop now), adds `PUID` / `PGID` env vars with the same defaults, and comments out the `./virtual_printer:/app/data/virtual_printer` bind mount by default with explicit "only needed if you also run a native install of Bambuddy on the same host and want both to share the VP CA cert" guidance. The entrypoint chowns the host-side dir through the bind mount the first time it sees wrong ownership, so existing uncommented installs continue to work and #1211 specifically gets fixed.
- **Label picker modal clipped the 4th template option and Cancel button on short viewports** ([#1230](https://github.com/maziggy/bambuddy/issues/1230), reported by @elit3ge) — Clicking "Print labels" from Inventory opened the picker with only 3 of the 4 templates visible (Avery 5160 was half-cut at the bottom) and no Cancel button reachable, with no way to scroll to them. Surfaced reliably on Windows 11 + Brave at 1080p with browser chrome / DPI scaling shrinking the effective viewport, but the layout bug hits anywhere the modal's `max-h-[90vh]` lands below ~770 px. **Cause:** `LabelTemplatePickerModal.tsx` uses a flex column with `overflow-hidden` on the outer modal, the spool list as the `flex-1` shrinkable child, and the templates section + footer as fixed siblings below it. The spool list had `min-h-[160px]`, which combined with the default `min-height: auto` for flex items meant the spool list couldn't yield space when the modal was tight — the templates and footer overflowed the modal's bottom edge and got clipped. **Fix:** `min-h-[160px]` → `min-h-0` on the spool list scroller, which both removes the fixed floor and overrides the implicit `min-height: auto` so flex shrinking actually works; the spool list now yields height to keep all four templates and the Cancel button visible on constrained viewports. On larger viewports the behaviour is identical (`flex-1` still grows to fill). Pre-existing on `dev` since 0.2.4b2 (commit `864e5c99`, the original PR #809 that introduced the modal); not a regression from the spoolman-inventory rebase. **Test:** new regression test in `LabelTemplatePickerModal.test.tsx` asserts all four template names + the Cancel button render in the DOM, and pins the structural fix by checking the spool list scroller has `min-h-0` and no `min-h-[…]` literal — so a future refactor that re-introduces a fixed floor on that element fails CI.
+20 -1
View File
@@ -40,6 +40,23 @@ class GiteaBackend(GitHubBackend):
return ref_data[0]["object"]["sha"]
return ref_data["object"]["sha"]
@staticmethod
def _commit_tree_sha(commit_data: dict) -> str | None:
"""Extract the tree SHA from a commit response.
GitHub's ``GET /git/commits/{sha}`` returns the GitCommit schema with
``tree`` at the top level. Gitea's same-named endpoint returns the
wrapped Commit schema where ``tree`` lives under ``commit``. Try the
flat shape first (GitHub-compatible deployments / Gitea ≤ 1.23) then
fall back to the wrapped shape (Gitea 1.24+, Forgejo).
"""
tree_node = commit_data.get("tree")
if not isinstance(tree_node, dict):
tree_node = (commit_data.get("commit") or {}).get("tree")
if isinstance(tree_node, dict):
return tree_node.get("sha")
return None
def parse_repo_url(self, url: str) -> tuple[str, str]:
"""Return (owner, repo) — accepts both https:// and http:// for self-hosted instances."""
if not url or len(url) > 500:
@@ -106,7 +123,9 @@ class GiteaBackend(GitHubBackend):
if commit_response.status_code != 200:
return {"status": "failed", "message": "Failed to get current commit"}
current_tree_sha = commit_response.json()["tree"]["sha"]
current_tree_sha = self._commit_tree_sha(commit_response.json())
if not current_tree_sha:
return {"status": "failed", "message": "Failed to extract tree SHA from commit response"}
tree_response = await client.get(
f"{api_base}/repos/{owner}/{repo}/git/trees/{current_tree_sha}?recursive=1", headers=headers
+73
View File
@@ -329,6 +329,79 @@ class TestGiteaBackendListShapeRefResponse:
assert result["status"] == "success"
class TestGiteaBackendWrappedCommitResponse:
"""#1224 regression: Gitea wraps the GitCommit fields under ``commit``.
GitHub's ``GET /git/commits/{sha}`` returns the unwrapped GitCommit schema
(``tree`` at top level). Gitea's same-named endpoint returns the wrapped
Commit schema where ``tree`` lives at ``commit.tree`` (Gitea 1.24+).
Pre-fix code did ``commit_response.json()["tree"]["sha"]`` and raised
``KeyError: 'tree'`` on every backup *after* the initial one — surfaced to
the user as the opaque ``Backup failed: 'tree'`` message.
"""
def setup_method(self):
self.backend = GiteaBackend()
self.repo_url = "https://git.example.com/owner/repo"
self.token = "gitea-token"
self.branch = "bambuddy-backup"
def test_commit_tree_sha_reads_flat_shape(self):
"""GitHub-compatible / older Gitea: ``tree`` at top level."""
assert self.backend._commit_tree_sha({"tree": {"sha": "abc"}}) == "abc"
def test_commit_tree_sha_reads_wrapped_shape(self):
"""Gitea 1.24+ / Forgejo: ``tree`` nested under ``commit``."""
assert self.backend._commit_tree_sha({"sha": "c1", "commit": {"tree": {"sha": "abc"}}}) == "abc"
def test_commit_tree_sha_returns_none_on_missing(self):
assert self.backend._commit_tree_sha({"sha": "c1", "commit": {}}) is None
assert self.backend._commit_tree_sha({}) is None
@pytest.mark.asyncio
async def test_push_files_handles_wrapped_commit_response(self):
"""Subsequent backup against Gitea 1.24+ — commit endpoint returns wrapped shape."""
client = AsyncMock()
client.get = AsyncMock(
side_effect=[
_make_mock_response(200, [{"object": {"sha": "base-commit"}}]),
# Wrapped Gitea commit response — tree under "commit", not top level
_make_mock_response(200, {"sha": "base-commit", "commit": {"tree": {"sha": "base-tree"}}}),
_make_mock_response(200, {"tree": []}),
]
)
client.post = AsyncMock(
side_effect=[
_make_mock_response(201, {"sha": "blob1"}),
_make_mock_response(201, {"sha": "new-tree"}),
_make_mock_response(201, {"sha": "new-commit"}),
]
)
client.patch = AsyncMock(return_value=_make_mock_response(200, {}))
result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client)
assert result["status"] == "success"
assert result["commit_sha"] == "new-commit"
@pytest.mark.asyncio
async def test_push_files_fails_cleanly_when_tree_sha_missing(self):
"""Defensive: malformed/unexpected commit response surfaces a clear error, not KeyError."""
client = AsyncMock()
client.get = AsyncMock(
side_effect=[
_make_mock_response(200, [{"object": {"sha": "base-commit"}}]),
_make_mock_response(200, {"sha": "base-commit"}), # no tree at all
]
)
result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client)
assert result["status"] == "failed"
assert "tree SHA" in result["message"]
class TestGiteaBackendEmptyRepoInitialCommit:
"""#1224 regression: Git Data API refuses writes against empty Gitea repos.