From 9868f1035f726cad80b6df61bca6f0f0607a7c2f Mon Sep 17 00:00:00 2001 From: Matysh Date: Tue, 28 Jul 2026 21:11:11 +0300 Subject: [PATCH] v1.46.6: the detach promise, actually kept this time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v1.46.4 and v1.46.5 documented that detaching a plan leaves the image on disk, added guards for it, and shipped tests. The guards were never reached: they sit behind 'not superseded', and a file that left the configuration was called superseded. From old_refs - new_refs alone, replacing a plan, detaching one and deleting its space are indistinguishable — so all three deleted the file, at the moment of the save, before any scheduled pass ever ran. Every test I wrote for this called collect_plans(d, cfg, cfg): old config equal to new, i.e. only the scheduled pass. The transition that mattered was never exercised. Codex reproduced it in four lines. Classification is by owner now: space in both, plan A -> plan B : the user picked another image -> removed space in both, plan -> none : detached -> kept space gone : kept (the image was imported; a thirty-day grace measured from file age is meaningless anyway, it was uploaded months ago) space has a plan, other file : rejected upload -> 1 h Attachments follow the same shape: dropped from a device that still exists -> removed (a trash button promises nothing); device gone -> kept; staging folder -> 1 h. Tests: a matrix per rule in the pure module, and — the part that was missing — test_detaching_a_plan_keeps_the_file, which goes through real config/set calls: attach, detach, assert the file is there, restart, assert again, re-attach, replace, assert the replaced one is gone, delete the space, assert the plan survives. Also strengthened the sweep/save race test to assert the save actually succeeded and the config points at the specific expected file, per the report. --- custom_components/houseplan/const.py | 2 +- .../houseplan/frontend/houseplan-card.js | 2 +- custom_components/houseplan/manifest.json | 2 +- custom_components/houseplan/plans.py | 82 +++++--- demo/srv/assets/houseplan-card.js | 2 +- dist/houseplan-card.js | 2 +- docs/ARCHITECTURE.md | 20 +- docs/CHANGELOG.md | 15 ++ docs/CHANGELOG.ru.md | 15 ++ docs/STATUS.md | 4 +- docs/TESTING.md | 13 +- package.json | 2 +- src/houseplan-card.ts | 2 +- tests_backend/test_ha_websocket.py | 59 +++++- tests_backend/test_validation.py | 189 +++++++++++------- 15 files changed, 288 insertions(+), 123 deletions(-) diff --git a/custom_components/houseplan/const.py b/custom_components/houseplan/const.py index ad40247..d56f3eb 100755 --- a/custom_components/houseplan/const.py +++ b/custom_components/houseplan/const.py @@ -34,7 +34,7 @@ PLAN_ORPHAN_TTL_S = 3600 SCHEDULED_GRACE_S = 30 * 24 * 3600 FILES_DIR = "houseplan/files" CONF_ADMIN_ONLY = "admin_only" -VERSION = "1.46.5" +VERSION = "1.46.6" DEFAULT_CONFIG: dict = { "spaces": [], diff --git a/custom_components/houseplan/frontend/houseplan-card.js b/custom_components/houseplan/frontend/houseplan-card.js index 477ebbe..c84e417 100755 --- a/custom_components/houseplan/frontend/houseplan-card.js +++ b/custom_components/houseplan/frontend/houseplan-card.js @@ -2561,4 +2561,4 @@ const t=globalThis,e=t.ShadowRoot&&(void 0===t.ShadyCSS||t.ShadyCSS.nativeShadow `} - `}}ps.properties={hass:{attribute:!1},_config:{state:!0},_space:{state:!0},_layout:{state:!0},_devices:{state:!0},_tip:{state:!0},_selId:{state:!0},_toast:{state:!0},_serverCfg:{state:!0},_mode:{state:!0},_tool:{state:!0},_path:{state:!0},_cursorPt:{state:!0},_mergeSel:{state:!0},_openingDialog:{state:!0},_openingInfo:{state:!0},_mergeDialog:{state:!0},_splitSel:{state:!0},_decorTool:{state:!0},_decorStyle:{state:!0},_decorDraft:{state:!0},_decorSel:{state:!0},_decorTextDialog:{state:!0},_kioskDialog:{state:!0},_kioskDots:{state:!0},_areaSel:{state:!0},_nameSel:{state:!0},_roomDialog:{state:!0},_roomEditId:{state:!0},_roomFill:{state:!0},_roomTempSrc:{state:!0},_roomHumSrc:{state:!0},_roomSrcOpen:{state:!0},_roomSrcFilter:{state:!0},_roomNameScale:{state:!0},_roomLabelScale:{state:!0},_spaceDialog:{state:!0},_infoCard:{state:!0},_rulesDialog:{state:!0},_settingsDialog:{state:!0},_importDialog:{state:!0},_markerDialog:{state:!0},_zoom:{state:!0},_view:{state:!0}},ps._touchSeen=!1,ps._noHoverMq="undefined"!=typeof window&&"function"==typeof window.matchMedia&&window.matchMedia("(hover: none)").matches,ps.styles=Ui,customElements.get("houseplan-card")||customElements.define("houseplan-card",ps),window.customCards=window.customCards||[],window.customCards.find(t=>"houseplan-card"===t.type)||window.customCards.push({type:"houseplan-card",name:"House Plan Card",description:"Interactive house plan: spaces, rooms and devices with live states and drag layout."}),console.info("%c HOUSEPLAN-CARD %c v1.46.5 ","background:#3ea6ff;color:#04121f;font-weight:700",""); + `}}ps.properties={hass:{attribute:!1},_config:{state:!0},_space:{state:!0},_layout:{state:!0},_devices:{state:!0},_tip:{state:!0},_selId:{state:!0},_toast:{state:!0},_serverCfg:{state:!0},_mode:{state:!0},_tool:{state:!0},_path:{state:!0},_cursorPt:{state:!0},_mergeSel:{state:!0},_openingDialog:{state:!0},_openingInfo:{state:!0},_mergeDialog:{state:!0},_splitSel:{state:!0},_decorTool:{state:!0},_decorStyle:{state:!0},_decorDraft:{state:!0},_decorSel:{state:!0},_decorTextDialog:{state:!0},_kioskDialog:{state:!0},_kioskDots:{state:!0},_areaSel:{state:!0},_nameSel:{state:!0},_roomDialog:{state:!0},_roomEditId:{state:!0},_roomFill:{state:!0},_roomTempSrc:{state:!0},_roomHumSrc:{state:!0},_roomSrcOpen:{state:!0},_roomSrcFilter:{state:!0},_roomNameScale:{state:!0},_roomLabelScale:{state:!0},_spaceDialog:{state:!0},_infoCard:{state:!0},_rulesDialog:{state:!0},_settingsDialog:{state:!0},_importDialog:{state:!0},_markerDialog:{state:!0},_zoom:{state:!0},_view:{state:!0}},ps._touchSeen=!1,ps._noHoverMq="undefined"!=typeof window&&"function"==typeof window.matchMedia&&window.matchMedia("(hover: none)").matches,ps.styles=Ui,customElements.get("houseplan-card")||customElements.define("houseplan-card",ps),window.customCards=window.customCards||[],window.customCards.find(t=>"houseplan-card"===t.type)||window.customCards.push({type:"houseplan-card",name:"House Plan Card",description:"Interactive house plan: spaces, rooms and devices with live states and drag layout."}),console.info("%c HOUSEPLAN-CARD %c v1.46.6 ","background:#3ea6ff;color:#04121f;font-weight:700",""); diff --git a/custom_components/houseplan/manifest.json b/custom_components/houseplan/manifest.json index 53b05b2..87fdb85 100755 --- a/custom_components/houseplan/manifest.json +++ b/custom_components/houseplan/manifest.json @@ -16,5 +16,5 @@ "issue_tracker": "https://github.com/Matysh/houseplan-card/issues", "requirements": [], "single_config_entry": true, - "version": "1.46.5" + "version": "1.46.6" } diff --git a/custom_components/houseplan/plans.py b/custom_components/houseplan/plans.py index a269a04..58c4b8e 100644 --- a/custom_components/houseplan/plans.py +++ b/custom_components/houseplan/plans.py @@ -121,14 +121,21 @@ def collect_attachments( ) -> int: """The same commit-scoped rule as `collect_plans`, for marker attachments. - A file the old revision referenced and the new one does not was superseded - by this commit and goes. Otherwise a staging folder (`up_*` — only ever a - dialog that was never saved) is collected after PLAN_ORPHAN_TTL_S, and - anything else waits out SCHEDULED_GRACE_S. Never raises: it runs behind a - durable write. + A file the old revision referenced and the new one does not, whose marker + still exists, was removed on purpose — the dialog has a trash button and + promises nothing. It goes. If the marker itself is gone, that is a different + transition and the files are kept, like a deleted space's plan. A staging + folder (`up_*` — only ever a dialog that was never saved) is collected after + PLAN_ORPHAN_TTL_S, anything else after SCHEDULED_GRACE_S. Never raises: it + runs behind a durable write. """ new_refs = attachment_refs(new_cfg) old_refs = attachment_refs(old_cfg) + # Removing an attachment from a device that still exists is the user saying + # "drop this one" — a trash button, no promise that anything is kept. A + # device that is GONE is a different transition, and its files follow the + # same rule as a deleted space's plan: kept. + live_markers = {str(m.get("id")) for m in (new_cfg or {}).get("markers") or []} # Same distinction as for plans. A staging folder (`up_*`) is different: it # only ever holds an upload from a dialog that was never saved, so the short # rule is exactly right there even on the timer. @@ -144,10 +151,9 @@ def collect_attachments( removed += sweep_upload_temps(files_dir, now) for folder in folders: # A staging folder only ever holds an upload from a dialog that was never - # saved — unambiguous, so an hour is right. A marker's own folder is not: - # removing an attachment is deliberate, but so is re-adding one, and the - # file may have been detached rather than abandoned. Give it a month. - limit = staging_cutoff if folder.name.startswith("up_") else cutoff + # saved — unambiguous, so an hour is right, and no device owns it. + staging = folder.name.startswith("up_") + limit = staging_cutoff if staging else cutoff try: items = sorted(p for p in folder.iterdir() if p.is_file()) except OSError: @@ -156,7 +162,15 @@ def collect_attachments( rel = f"{folder.name}/{item.name}" if rel in new_refs: continue - if rel not in old_refs: # not superseded: absence alone is weak evidence + dropped = rel in old_refs and folder.name in live_markers + if not dropped: + if not staging and folder.name not in live_markers: + # No device owns this folder any more. Whether the file was + # attached once or is a leftover upload we cannot tell, and + # by the standing rule that means we keep it. (A staging + # folder is exempt above: it has no device by construction.) + continue + # the device is there and never listed this file: a rejected upload try: if item.stat().st_mtime >= limit: continue @@ -196,6 +210,14 @@ def plan_refs(cfg: dict[str, Any] | None) -> set[str]: return out +def plan_by_space(cfg: dict[str, Any] | None) -> dict[str, str]: + """space id -> the plan file it references ('' when it has none).""" + return { + str(sp.get("id")): plan_basename(sp.get("plan_url")) + for sp in (cfg or {}).get("spaces") or [] + } + + def is_plan_file(name: str) -> bool: """Does this look like a plan we wrote: . or ..?""" parts = name.split(".") @@ -242,10 +264,19 @@ def collect_plans( # nothing) destroyed two detached plans on 2026-07-28. # The short rule fits exactly one case: a space that HAS a plan, where any # other file of its own can only be a superseded or rejected upload. - spaces = {str(sp.get("id")): sp.get("plan_url") for sp in (new_cfg or {}).get("spaces") or []} + old_by_space = plan_by_space(old_cfg) + new_by_space = plan_by_space(new_cfg) + # A file that left the configuration tells us nothing on its own: replacing a + # plan, detaching one and deleting a space all look identical from + # `old_refs - new_refs`. Only the first is a deletion the user asked for + # (HP-1465-01 — the guards below were written and then never reached, + # because the code decided "superseded" before asking why). + replaced = { + name for space, name in old_by_space.items() + if new_by_space.get(space) and new_by_space[space] != name + } now_s = time.time() if now is None else now reject_cutoff = now_s - PLAN_ORPHAN_TTL_S - cutoff = now_s - SCHEDULED_GRACE_S removed = 0 try: items = sorted(plans_dir.iterdir()) if plans_dir.is_dir() else [] @@ -258,21 +289,24 @@ def collect_plans( for item in items: if not item.is_file() or item.name in new_refs or not is_plan_file(item.name): continue - superseded = item.name in old_refs - if not superseded: + if item.name not in replaced: + # Not a replacement. PRODUCT RULE (owner's decision, 2026-07-28): + # a file we were not told to delete is kept, however long it sits + # there. Detaching a plan is one click to undo and the editor says + # the image stays; deleting a space is deliberate but the image is + # usually something the user imported and may not have elsewhere. + # The two errors are not symmetrical — a few unnecessary megabytes + # can always be removed by hand, a file we should not have removed + # cannot be brought back. + # + # The single exception: a space that HAS a plan, and another file of + # its own that has never been the plan. That can only be an upload + # whose save was rejected, and an hour is plenty for it. space = item.name.split(".")[0] - if space in spaces and not spaces[space]: - # PRODUCT RULE (owner's decision, 2026-07-28): a detached plan - # is never deleted, at any age. The space is there and currently - # has no plan — the image was detached, one click undoes that, - # and the editor says the file stays. The two errors are not - # symmetrical: a few megabytes we did not need can always be - # removed by hand, a file we should not have removed cannot be - # brought back. When in doubt, keep it. + if not new_by_space.get(space) or item.name in old_by_space.values(): continue - limit = reject_cutoff if space in spaces else cutoff try: - if item.stat().st_mtime >= limit: + if item.stat().st_mtime >= reject_cutoff: continue except OSError: continue diff --git a/demo/srv/assets/houseplan-card.js b/demo/srv/assets/houseplan-card.js index 477ebbe..c84e417 100755 --- a/demo/srv/assets/houseplan-card.js +++ b/demo/srv/assets/houseplan-card.js @@ -2561,4 +2561,4 @@ const t=globalThis,e=t.ShadowRoot&&(void 0===t.ShadyCSS||t.ShadyCSS.nativeShadow `} - `}}ps.properties={hass:{attribute:!1},_config:{state:!0},_space:{state:!0},_layout:{state:!0},_devices:{state:!0},_tip:{state:!0},_selId:{state:!0},_toast:{state:!0},_serverCfg:{state:!0},_mode:{state:!0},_tool:{state:!0},_path:{state:!0},_cursorPt:{state:!0},_mergeSel:{state:!0},_openingDialog:{state:!0},_openingInfo:{state:!0},_mergeDialog:{state:!0},_splitSel:{state:!0},_decorTool:{state:!0},_decorStyle:{state:!0},_decorDraft:{state:!0},_decorSel:{state:!0},_decorTextDialog:{state:!0},_kioskDialog:{state:!0},_kioskDots:{state:!0},_areaSel:{state:!0},_nameSel:{state:!0},_roomDialog:{state:!0},_roomEditId:{state:!0},_roomFill:{state:!0},_roomTempSrc:{state:!0},_roomHumSrc:{state:!0},_roomSrcOpen:{state:!0},_roomSrcFilter:{state:!0},_roomNameScale:{state:!0},_roomLabelScale:{state:!0},_spaceDialog:{state:!0},_infoCard:{state:!0},_rulesDialog:{state:!0},_settingsDialog:{state:!0},_importDialog:{state:!0},_markerDialog:{state:!0},_zoom:{state:!0},_view:{state:!0}},ps._touchSeen=!1,ps._noHoverMq="undefined"!=typeof window&&"function"==typeof window.matchMedia&&window.matchMedia("(hover: none)").matches,ps.styles=Ui,customElements.get("houseplan-card")||customElements.define("houseplan-card",ps),window.customCards=window.customCards||[],window.customCards.find(t=>"houseplan-card"===t.type)||window.customCards.push({type:"houseplan-card",name:"House Plan Card",description:"Interactive house plan: spaces, rooms and devices with live states and drag layout."}),console.info("%c HOUSEPLAN-CARD %c v1.46.5 ","background:#3ea6ff;color:#04121f;font-weight:700",""); + `}}ps.properties={hass:{attribute:!1},_config:{state:!0},_space:{state:!0},_layout:{state:!0},_devices:{state:!0},_tip:{state:!0},_selId:{state:!0},_toast:{state:!0},_serverCfg:{state:!0},_mode:{state:!0},_tool:{state:!0},_path:{state:!0},_cursorPt:{state:!0},_mergeSel:{state:!0},_openingDialog:{state:!0},_openingInfo:{state:!0},_mergeDialog:{state:!0},_splitSel:{state:!0},_decorTool:{state:!0},_decorStyle:{state:!0},_decorDraft:{state:!0},_decorSel:{state:!0},_decorTextDialog:{state:!0},_kioskDialog:{state:!0},_kioskDots:{state:!0},_areaSel:{state:!0},_nameSel:{state:!0},_roomDialog:{state:!0},_roomEditId:{state:!0},_roomFill:{state:!0},_roomTempSrc:{state:!0},_roomHumSrc:{state:!0},_roomSrcOpen:{state:!0},_roomSrcFilter:{state:!0},_roomNameScale:{state:!0},_roomLabelScale:{state:!0},_spaceDialog:{state:!0},_infoCard:{state:!0},_rulesDialog:{state:!0},_settingsDialog:{state:!0},_importDialog:{state:!0},_markerDialog:{state:!0},_zoom:{state:!0},_view:{state:!0}},ps._touchSeen=!1,ps._noHoverMq="undefined"!=typeof window&&"function"==typeof window.matchMedia&&window.matchMedia("(hover: none)").matches,ps.styles=Ui,customElements.get("houseplan-card")||customElements.define("houseplan-card",ps),window.customCards=window.customCards||[],window.customCards.find(t=>"houseplan-card"===t.type)||window.customCards.push({type:"houseplan-card",name:"House Plan Card",description:"Interactive house plan: spaces, rooms and devices with live states and drag layout."}),console.info("%c HOUSEPLAN-CARD %c v1.46.6 ","background:#3ea6ff;color:#04121f;font-weight:700",""); diff --git a/dist/houseplan-card.js b/dist/houseplan-card.js index 477ebbe..c84e417 100755 --- a/dist/houseplan-card.js +++ b/dist/houseplan-card.js @@ -2561,4 +2561,4 @@ const t=globalThis,e=t.ShadowRoot&&(void 0===t.ShadyCSS||t.ShadyCSS.nativeShadow `} - `}}ps.properties={hass:{attribute:!1},_config:{state:!0},_space:{state:!0},_layout:{state:!0},_devices:{state:!0},_tip:{state:!0},_selId:{state:!0},_toast:{state:!0},_serverCfg:{state:!0},_mode:{state:!0},_tool:{state:!0},_path:{state:!0},_cursorPt:{state:!0},_mergeSel:{state:!0},_openingDialog:{state:!0},_openingInfo:{state:!0},_mergeDialog:{state:!0},_splitSel:{state:!0},_decorTool:{state:!0},_decorStyle:{state:!0},_decorDraft:{state:!0},_decorSel:{state:!0},_decorTextDialog:{state:!0},_kioskDialog:{state:!0},_kioskDots:{state:!0},_areaSel:{state:!0},_nameSel:{state:!0},_roomDialog:{state:!0},_roomEditId:{state:!0},_roomFill:{state:!0},_roomTempSrc:{state:!0},_roomHumSrc:{state:!0},_roomSrcOpen:{state:!0},_roomSrcFilter:{state:!0},_roomNameScale:{state:!0},_roomLabelScale:{state:!0},_spaceDialog:{state:!0},_infoCard:{state:!0},_rulesDialog:{state:!0},_settingsDialog:{state:!0},_importDialog:{state:!0},_markerDialog:{state:!0},_zoom:{state:!0},_view:{state:!0}},ps._touchSeen=!1,ps._noHoverMq="undefined"!=typeof window&&"function"==typeof window.matchMedia&&window.matchMedia("(hover: none)").matches,ps.styles=Ui,customElements.get("houseplan-card")||customElements.define("houseplan-card",ps),window.customCards=window.customCards||[],window.customCards.find(t=>"houseplan-card"===t.type)||window.customCards.push({type:"houseplan-card",name:"House Plan Card",description:"Interactive house plan: spaces, rooms and devices with live states and drag layout."}),console.info("%c HOUSEPLAN-CARD %c v1.46.5 ","background:#3ea6ff;color:#04121f;font-weight:700",""); + `}}ps.properties={hass:{attribute:!1},_config:{state:!0},_space:{state:!0},_layout:{state:!0},_devices:{state:!0},_tip:{state:!0},_selId:{state:!0},_toast:{state:!0},_serverCfg:{state:!0},_mode:{state:!0},_tool:{state:!0},_path:{state:!0},_cursorPt:{state:!0},_mergeSel:{state:!0},_openingDialog:{state:!0},_openingInfo:{state:!0},_mergeDialog:{state:!0},_splitSel:{state:!0},_decorTool:{state:!0},_decorStyle:{state:!0},_decorDraft:{state:!0},_decorSel:{state:!0},_decorTextDialog:{state:!0},_kioskDialog:{state:!0},_kioskDots:{state:!0},_areaSel:{state:!0},_nameSel:{state:!0},_roomDialog:{state:!0},_roomEditId:{state:!0},_roomFill:{state:!0},_roomTempSrc:{state:!0},_roomHumSrc:{state:!0},_roomSrcOpen:{state:!0},_roomSrcFilter:{state:!0},_roomNameScale:{state:!0},_roomLabelScale:{state:!0},_spaceDialog:{state:!0},_infoCard:{state:!0},_rulesDialog:{state:!0},_settingsDialog:{state:!0},_importDialog:{state:!0},_markerDialog:{state:!0},_zoom:{state:!0},_view:{state:!0}},ps._touchSeen=!1,ps._noHoverMq="undefined"!=typeof window&&"function"==typeof window.matchMedia&&window.matchMedia("(hover: none)").matches,ps.styles=Ui,customElements.get("houseplan-card")||customElements.define("houseplan-card",ps),window.customCards=window.customCards||[],window.customCards.find(t=>"houseplan-card"===t.type)||window.customCards.push({type:"houseplan-card",name:"House Plan Card",description:"Interactive house plan: spaces, rooms and devices with live states and drag layout."}),console.info("%c HOUSEPLAN-CARD %c v1.46.6 ","background:#3ea6ff;color:#04121f;font-weight:700",""); diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 9cf1294..2cffbc9 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -214,14 +214,22 @@ we should not have removed cannot be brought back.** When the evidence is weak, keep the file. Owner's decision, 2026-07-28, after the one-hour rule applied to every unreferenced file destroyed two detached plans. +The classification is by **owner**, not by "is it referenced". A file leaving +the configuration looks identical whether the plan was replaced, detached, or +its space deleted — and only the first is a deletion the user asked for. Reading +`old_refs - new_refs` and calling it "superseded" deleted a plan the moment it +was detached, under documentation promising the opposite (HP-1465-01). + | Case | What it means | Rule | |---|---|---| -| In the old revision, not in the new | a save replaced it | removed immediately | -| Space exists, has **no** plan | detached, one click to undo | **never removed** | -| Space exists, has a plan | its own rejected upload | `PLAN_ORPHAN_TTL_S` (1 h) | -| Space no longer exists | deliberate deletion, but people misclick | `SCHEDULED_GRACE_S` (30 d) | -| Attachment in `up_*` | a dialog that was never saved | `PLAN_ORPHAN_TTL_S` (1 h) | -| Attachment in a marker folder | unreferenced, but may come back | `SCHEDULED_GRACE_S` (30 d) | +| Space in both, plan A → plan B | the user picked another image | removed immediately | +| Space in both, plan → none | detached; one click undoes it | **kept** | +| Space gone | deliberate, but the image was imported and may be nowhere else | **kept** | +| Space has a plan, plus another file of its own | an upload whose save was rejected | `PLAN_ORPHAN_TTL_S` (1 h) | +| Marker in both, attachment dropped from its list | a trash button, promising nothing | removed immediately | +| Marker gone | same call as a deleted space's plan | **kept** | +| Attachment in `up_*` | a dialog that was never saved; no device owns it | `PLAN_ORPHAN_TTL_S` (1 h) | +| Marker there, file it never listed | a rejected upload | `SCHEDULED_GRACE_S` (30 d) | **Config writes are serialized** (HP-1454-03). `_writeConfig()` chains onto a single promise: one `config/set` in flight, each carrying the revision the diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 97f38c2..d352b01 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## v1.46.6 — 2026-07-28 (the detach promise, actually kept this time) +- **Switching a space to "draw" no longer deletes its image.** v1.46.4 and + v1.46.5 said it did not, and the scheduled cleanup indeed left detached plans + alone — but the save itself deleted the file the moment the reference was + cleared, before any of those guards were reached. The cause: a file that left + the configuration was called "superseded", and from that difference alone + replacing a plan, detaching one and deleting its space are indistinguishable. + Only the first is a deletion anybody asked for. The transition is now + classified by the space that owned the file, and the same distinction applies + to attachments: dropping one from a device that still exists removes it, + deleting the device keeps its manuals. +- **A plan whose space was deleted is kept**, rather than the thirty days + v1.46.5 promised — thirty days measured from the file's age is meaningless + anyway, since it was usually uploaded months earlier. + ## v1.46.5 — 2026-07-28 (audit of every automatic deletion) - **A detached plan is never deleted, at any age.** v1.46.4 gave it a month; this makes it permanent and writes the reason down where the next change will diff --git a/docs/CHANGELOG.ru.md b/docs/CHANGELOG.ru.md index 898145a..a21603b 100755 --- a/docs/CHANGELOG.ru.md +++ b/docs/CHANGELOG.ru.md @@ -6,6 +6,21 @@ > **Правило проекта:** оба файла пополняются в одном коммите с самим > изменением — как и остальная документация (см. docs/STATUS.md). +## v1.46.6 — 2026-07-28 (обещание про отцепление, теперь выполненное) +- **Переключение пространства в «нарисовать» больше не удаляет его картинку.** + v1.46.4 и v1.46.5 утверждали, что не удаляет, и плановая уборка действительно + отцеплённые планы не трогала — но само сохранение удаляло файл в тот момент, + когда снималась ссылка, ещё до всех этих проверок. Причина: файл, покинувший + конфигурацию, считался «заменённым», а по одной этой разнице замену плана, + отцепление и удаление пространства различить невозможно. Удалением, о котором + просили, является только первое. Теперь переход классифицируется по + пространству-владельцу, и та же разница применяется к вложениям: убрали файл у + существующего устройства — он удаляется, удалили устройство — его инструкции + остаются. +- **План удалённого пространства сохраняется**, а не тридцать дней, как обещала + v1.46.5: тридцать дней по возрасту файла всё равно бессмысленны — обычно он + загружен месяцы назад. + ## v1.46.5 — 2026-07-28 (ревизия всех автоматических удалений) - **Отцеплённый план не удаляется никогда, ни в каком возрасте.** В v1.46.4 ему давался месяц; теперь это навсегда, и причина записана там, где её увидит diff --git a/docs/STATUS.md b/docs/STATUS.md index 8cc5031..5d1d0fd 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -15,12 +15,12 @@ | Item | State | |---|---| -| Version | **v1.46.5** everywhere (manifest, const.py, package.json, CARD_VERSION); deployed to the home instance | +| Version | **v1.46.6** everywhere (manifest, const.py, package.json, CARD_VERSION); deployed to the home instance | | Workflow | Since 2026-07-22: minor changes go to branch **`dev`** (build + smokes → deploy home → commit → push, NO release); releases are batched on the owner's command (merge dev→main, one tag, one release with a summary changelog, CI checked on dev beforehand) | | GitHub | https://github.com/Matysh/houseplan-card — **`main` carries every published release, the latest tag is the current version above**; `dev` is where work lands and is merged into `main` at release time (so `dev` is normally equal to or ahead of `main`, never behind). Push via SSH key `ha_jb` (remote git@github.com:…); API releases via the fine-grained PAT in `~/.git-credentials` (Contents R/W, issued 2026-07-23) | | CI | validate.yml (hacs + hassfest + frontend + backend) green; release.yml attaches the bundle on release publish | | HACS | Custom repository works. **Inclusion PR: hacs/default#9004** — open, valid, labeled; ~864 older open PRs but merge rate ≈180/mo; realistic ETA 1–3 months (checked 2026-07-24) | -| Home instance | ha.jbstudio.pro (SSH port 323, key `ha_jb`), deployed **v1.46.5** via direct copy (HACS custom repo also installed) | +| Home instance | ha.jbstudio.pro (SSH port 323, key `ha_jb`), deployed **v1.46.6** via direct copy (HACS custom repo also installed) | | Localization | UI en/ru (src/i18n/*.json), everything user-visible localized incl. kiosk popover | | Tests | Four layers: frontend unit (`npm test`, node:test over `test-build/`), pure backend (`pytest tests_backend`, runs anywhere), HA-harness backend (same folder, CI only — needs py3.13 + pytest-homeassistant-custom-component), and browser smokes (`demo/smoke_*.mjs`, headless chromium). **Counts are not written down here** — they went stale within two releases while the version line beside them was kept current, which reads as less coverage than exists (review R5-2). Run `npm run inventory` for the current numbers, or read them off the last CI run | | Community | **Telegram chat: https://t.me/ha_houseplan** (created 2026-07-27) — the primary user-facing support channel; GitHub issues stay for bugs/features. Link it from any new release notes and posts | diff --git a/docs/TESTING.md b/docs/TESTING.md index 3484da8..411d8c5 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -239,11 +239,14 @@ Run the *core flows* (marked ★ below) in each environment at least once per mi on the plan after a reload. Same for each tap action and each fill mode [auto: backend test_every_display_mode_the_editor_offers_is_accepted and neighbours, test_a_marker_showing_its_value_can_be_saved] -- [ ] Detaching a plan keeps the file (v1.46.4/v1.46.5): switch a space to - "draw", restart, wait — the image stays in `config/houseplan/plans/` - indefinitely and can be re-attached. Replacing a plan still removes the one - it replaced, immediately - [auto: unit: test_scheduled_collection_never_takes_a_detached_plan] +- [ ] Detaching a plan keeps the file (v1.46.6): switch a space to "draw" and + SAVE — the image is still in `config/houseplan/plans/` right afterwards, + and after a restart, and can be re-attached. Deleting the space keeps it + too. Replacing a plan still removes the one it replaced, immediately. + Check straight after the save: the earlier bug deleted the file at that + moment, while every scheduled-pass test passed + [auto: unit: test_plan_collection_matrix, test_attachment_collection_matrix, + backend test_detaching_a_plan_keeps_the_file] - [ ] Rebinding a device does not eat its manuals (v1.46.5): attach two files to a device, rebind it to another HA device — both are readable afterwards. If a copy failed, the file it failed on is still there rather than deleted diff --git a/package.json b/package.json index 1ab7c4f..b61d0b0 100755 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "houseplan-card", - "version": "1.46.5", + "version": "1.46.6", "description": "Interactive house plan Lovelace card for Home Assistant", "license": "MIT", "type": "module", diff --git a/src/houseplan-card.ts b/src/houseplan-card.ts index 294a1ca..6de5340 100755 --- a/src/houseplan-card.ts +++ b/src/houseplan-card.ts @@ -35,7 +35,7 @@ import './space-card'; import { cardStyles } from './styles'; import { langOf, t, type I18nKey } from './i18n'; -const CARD_VERSION = '1.46.5'; +const CARD_VERSION = '1.46.6'; const LS_KEY = 'houseplan_card_layout_v1'; const LS_CFG = 'houseplan_card_cfg_v1'; // cache of the server config+layout for instant rendering const LS_ZOOM = 'houseplan_card_zoom_v1'; diff --git a/tests_backend/test_ha_websocket.py b/tests_backend/test_ha_websocket.py index d364f18..d063cba 100644 --- a/tests_backend/test_ha_websocket.py +++ b/tests_backend/test_ha_websocket.py @@ -889,7 +889,7 @@ def _paths(hass): "kept_file": os.path.join(files, "m5", "kept.pdf"), "kept_plan": os.path.join(plans, "s5.tok.png"), "orphan_file": os.path.join(files, "up_cancelled", "manual.pdf"), - "orphan_plan": os.path.join(plans, "deleted_space.orphan.png"), + "orphan_plan": os.path.join(plans, "s5.reject.png"), } @@ -992,18 +992,21 @@ async def test_sweep_and_a_config_write_do_not_race( cfg2["spaces"][0]["plan_url"] = "/api/houseplan/content/plans/_/s5.newcomer.png" entry = hass.config_entries.async_entries(DOMAIN)[0] - await asyncio.gather( + _reload, saved = await asyncio.gather( hass.config_entries.async_reload(entry.entry_id), # runs the sweep _save(client, cfg2, rev), ) await hass.async_block_till_done() + # assert the CONCRETE outcome: a save that came back `not_ready` would leave + # the old config pointing at the old file and satisfy a vaguer check + assert saved["success"], saved.get("error") await client.send_json_auto_id({"type": "houseplan/config/get"}) stored = (await client.receive_json())["result"]["config"] - referenced = stored["spaces"][0]["plan_url"].rsplit("/", 1)[-1] + assert stored["spaces"][0]["plan_url"].endswith("s5.newcomer.png") assert await hass.async_add_executor_job( - os.path.isfile, os.path.join(plans, referenced) - ), f"the accepted config points at {referenced}, which must exist" + os.path.isfile, os.path.join(plans, "s5.newcomer.png") + ), "the file the accepted config points at must exist" async def test_files_cleanup_keeps_referenced_files( @@ -1048,3 +1051,49 @@ async def test_files_cleanup_keeps_referenced_files( "a file the configuration still references survives a cleanup of its folder" ) assert not await hass.async_add_executor_job(os.path.isfile, os.path.join(folder, "orphan.pdf")) + + +async def test_detaching_a_plan_keeps_the_file( + hass: HomeAssistant, hass_ws_client: WebSocketGenerator +) -> None: + """HP-1465-01, through the real save — where the earlier tests never looked. + + Every check for this lived in the pure collector with old and new config + equal, i.e. the scheduled pass. The transition that matters is a save, and + there the file was deleted the moment the reference was cleared. + """ + import os + + from custom_components.houseplan.const import PLANS_DIR + + await _setup(hass) + client = await hass_ws_client(hass) + plans = hass.config.path(PLANS_DIR) + + url, name = await _upload(client, "d1", b"PLAN", ext="png") + rev = (await _save(client, await _cfg([{"id": "d1", "plan_url": url}]), 0))["result"]["rev"] + assert await hass.async_add_executor_job(os.path.isfile, os.path.join(plans, name)) + + # detach: the space stays, its plan does not + rev = (await _save(client, await _cfg([{"id": "d1", "plan_url": None}]), rev))["result"]["rev"] + assert await hass.async_add_executor_job(os.path.isfile, os.path.join(plans, name)), ( + "the editor says the image stays on disk — it has to actually stay" + ) + + # a restart does not change its mind either + entry = hass.config_entries.async_entries(DOMAIN)[0] + assert await hass.config_entries.async_reload(entry.entry_id) + await hass.async_block_till_done() + assert await hass.async_add_executor_job(os.path.isfile, os.path.join(plans, name)) + + # re-attach, then replace: THAT removes the one it replaced + rev = (await _save(client, await _cfg([{"id": "d1", "plan_url": url}]), rev))["result"]["rev"] + url2, name2 = await _upload(client, "d1", b"NEWPLAN", ext="png") + rev = (await _save(client, await _cfg([{"id": "d1", "plan_url": url2}]), rev))["result"]["rev"] + assert await hass.async_add_executor_job(os.path.isfile, os.path.join(plans, name2)) + assert not await hass.async_add_executor_job(os.path.isfile, os.path.join(plans, name)) + + # and deleting the space keeps its plan + await _save(client, await _cfg([]), rev) + await hass.async_block_till_done() + assert await hass.async_add_executor_job(os.path.isfile, os.path.join(plans, name2)) diff --git a/tests_backend/test_validation.py b/tests_backend/test_validation.py index bed5941..58db416 100644 --- a/tests_backend/test_validation.py +++ b/tests_backend/test_validation.py @@ -265,17 +265,6 @@ def test_collect_plans_keeps_a_fresh_unreferenced_upload(tmp_path): assert (d / "f1.inflight.png").is_file() -def test_collect_plans_takes_an_aged_orphan(tmp_path): - collect_plans = plans.collect_plans - - d = _plans(tmp_path, ["f1.keep.png"]) - # past the long grace, and belonging to no space in the config - _plans(tmp_path, ["f1.abandoned.png"], age=const.SCHEDULED_GRACE_S + 60) - removed = collect_plans(d, _cfg("/p/f1.keep.png"), _cfg("/p/f1.keep.png")) - assert removed == 1 - assert (d / "f1.keep.png").is_file() and not (d / "f1.abandoned.png").exists() - - def test_collect_plans_never_touches_a_referenced_or_foreign_file(tmp_path): PLAN_ORPHAN_TTL_S = const.PLAN_ORPHAN_TTL_S collect_plans = plans.collect_plans @@ -477,31 +466,6 @@ def test_attachment_refs_reads_marker_urls(): assert plans.attachment_refs(cfg) == set() -def test_collect_attachments_supersedes_and_ages(tmp_path): - import os - import time - - collect_attachments = plans.collect_attachments - files = tmp_path / "files" - (files / "m1").mkdir(parents=True) - for n in ("old.pdf", "new.pdf", "cancelled.pdf"): - (files / "m1" / n).write_bytes(b"x") - - # the commit swapped old.pdf for new.pdf; cancelled.pdf is a fresh upload - # nobody saved — it may belong to a dialog that is still open - removed = collect_attachments(files, _acfg("m1/old.pdf"), _acfg("m1/new.pdf")) - assert removed == 1 - assert not (files / "m1" / "old.pdf").exists() - assert (files / "m1" / "new.pdf").is_file() - assert (files / "m1" / "cancelled.pdf").is_file() - - old = time.time() - const.SCHEDULED_GRACE_S - 60 - os.utime(files / "m1" / "cancelled.pdf", (old, old)) - assert collect_attachments(files, _acfg("m1/new.pdf"), _acfg("m1/new.pdf")) == 1 - assert not (files / "m1" / "cancelled.pdf").exists() - assert (files / "m1" / "new.pdf").is_file() - - def test_collect_attachments_removes_the_empty_folder_and_never_raises(tmp_path): import os import time @@ -554,53 +518,130 @@ def test_legacy_segments_are_dropped_by_the_server(): assert "segments" not in out -def test_scheduled_collection_never_takes_a_detached_plan(tmp_path): - """2026-07-28, on the author's own instance: two plans were deleted. - - Detaching a plan (switching a space to "draw") is reversible and the editor - says the file stays on disk. The timer only knows "nothing points at it", - applied the one-hour orphan rule, and removed images that had been detached - weeks earlier. A commit may still collect what it superseded — it knows it - replaced something. The timer may not. - """ +def _aged(path, seconds): import os import time + t = time.time() - seconds + os.utime(path, (t, t)) + + +def _sp(sid, url): + return {"id": sid, "plan_url": f"/api/houseplan/content/plans/_/{url}" if url else None} + + +def test_plan_collection_matrix(tmp_path): + """Which config transition means "the user asked for this file to go"? + + `old_refs - new_refs` cannot tell replace, detach and delete-space apart — + they look identical. v1.46.4/v1.46.5 added guards for detach and only ever + reached them on the scheduled pass, so the commit itself still deleted a + detached plan the moment it was detached (HP-1465-01). One case is a + deletion the user asked for; the rest are kept. + """ + collect = plans.collect_plans d = tmp_path / "plans" d.mkdir() - for n in ("f1.svg", "f2.tok.png", "gone.old.png"): - (d / n).write_bytes(b"x") - t = time.time() - const.SCHEDULED_GRACE_S - 60 - os.utime(d / n, (t, t)) - cfg = {"spaces": [{"id": "f1", "plan_url": None}, # detached, space alive - {"id": "f2", "plan_url": None}]} # same - assert plans.collect_plans(d, cfg, cfg) == 1 - assert (d / "f1.svg").is_file(), "a detached plan of a live space is kept, at any age" - assert (d / "f2.tok.png").is_file() - assert not (d / "gone.old.png").exists(), "a plan of a deleted space ages out after a month" + def seed(*names): + for n in names: + (d / n).write_bytes(b"x") + _aged(d / n, const.SCHEDULED_GRACE_S * 2) # old enough for any rule - # a space that HAS a plan: its other files can only be its own rejects - import os - import time + # 1. replace: the user picked a different image for the same space + seed("f1.old.png", "f1.new.png") + assert collect(d, {"spaces": [_sp("f1", "f1.old.png")]}, + {"spaces": [_sp("f1", "f1.new.png")]}) == 1 + assert not (d / "f1.old.png").exists() and (d / "f1.new.png").is_file() - (d / "f9.current.png").write_bytes(b"x") - (d / "f9.reject.png").write_bytes(b"x") - hour = time.time() - const.PLAN_ORPHAN_TTL_S - 60 - os.utime(d / "f9.reject.png", (hour, hour)) - live = {"spaces": [{"id": "f1", "plan_url": None}, {"id": "f2", "plan_url": None}, - {"id": "f9", "plan_url": "/p/f9.current.png"}]} - assert plans.collect_plans(d, live, live) == 1 - assert (d / "f9.current.png").is_file() - assert not (d / "f9.reject.png").exists() + # 2. detach: same space, switched to "draw" + seed("f2.png") + assert collect(d, {"spaces": [_sp("f2", "f2.png")]}, + {"spaces": [_sp("f2", None)]}) == 0 + assert (d / "f2.png").is_file(), "the editor says the file stays — it stays" - # a commit still removes what it SUPERSEDED — that it knows for certain - old = {"spaces": [{"id": "f1", "plan_url": "/p/f1.svg"}, {"id": "f2", "plan_url": None}]} - new = {"spaces": [{"id": "f1", "plan_url": "/p/f1.new.png"}, {"id": "f2", "plan_url": None}]} - (d / "f1.new.png").write_bytes(b"x") - assert plans.collect_plans(d, old, new) == 1 - assert not (d / "f1.svg").exists() - assert (d / "f2.tok.png").is_file(), "and still touches nothing else of a live space" + # 3. the space is deleted outright + seed("f3.png") + assert collect(d, {"spaces": [_sp("f3", "f3.png")]}, {"spaces": []}) == 0 + assert (d / "f3.png").is_file() + + # 4. the scheduled pass, later, still keeps both + cfg = {"spaces": [_sp("f2", None)]} + assert collect(d, cfg, cfg) == 0 + assert (d / "f2.png").is_file() and (d / "f3.png").is_file() + + # 5. a rejected upload: the space HAS a plan, this file never was one + seed("f4.current.png", "f4.reject.png") + live = {"spaces": [_sp("f4", "f4.current.png")]} + assert collect(d, live, live) == 1 + assert (d / "f4.current.png").is_file() and not (d / "f4.reject.png").exists() + + # …but only once it is old; a fresh one may be a transaction in flight + (d / "f4.fresh.png").write_bytes(b"x") + assert collect(d, live, live) == 0 + assert (d / "f4.fresh.png").is_file() + + # 6. the same file still referenced by another space is never touched + seed("shared.png") + assert collect(d, {"spaces": [_sp("a", "shared.png"), _sp("b", "shared.png")]}, + {"spaces": [_sp("a", None), _sp("b", "shared.png")]}) == 0 + assert (d / "shared.png").is_file() + + +def test_attachment_collection_matrix(tmp_path): + """Removing an attachment is a trash button; deleting the device is not.""" + collect = plans.collect_attachments + + def case(name): + d = tmp_path / name + d.mkdir() + return d + + def seed(root, folder, fname, age=None): + (root / folder).mkdir(parents=True, exist_ok=True) + p = root / folder / fname + p.write_bytes(b"x") + if age: + _aged(p, age) + return p + + def cfg(*markers): + return {"markers": [ + {"id": mid, "pdfs": [{"url": f"/api/houseplan/content/files/{mid}/{n}"} for n in names]} + for mid, names in markers + ]} + + # 1. the user removed one attachment from a device that still exists + d = case("dropped") + seed(d, "m1", "dropped.pdf") + seed(d, "m1", "kept.pdf") + assert collect(d, cfg(("m1", ["dropped.pdf", "kept.pdf"])), cfg(("m1", ["kept.pdf"]))) == 1 + assert not (d / "m1" / "dropped.pdf").exists() + assert (d / "m1" / "kept.pdf").is_file() + + # 2. the device itself is gone: its manuals are not ours to throw away + d = case("device_gone") + seed(d, "m2", "manual.pdf", age=const.SCHEDULED_GRACE_S * 2) + assert collect(d, cfg(("m2", ["manual.pdf"])), cfg()) == 0 + assert (d / "m2" / "manual.pdf").is_file() + # and the scheduled pass, later, agrees + assert collect(d, cfg(), cfg()) == 0 + assert (d / "m2" / "manual.pdf").is_file() + + # 3. a dialog that was never saved, in its own staging folder + d = case("staging") + seed(d, "up_x", "manual.pdf", age=const.PLAN_ORPHAN_TTL_S + 60) + assert collect(d, cfg(), cfg()) == 1 + assert not (d / "up_x").exists() + + # 4. an upload into a live device's folder whose save was rejected + d = case("reject") + seed(d, "m3", "current.pdf") + seed(d, "m3", "rejected.pdf", age=const.SCHEDULED_GRACE_S + 60) + live = cfg(("m3", ["current.pdf"])) + assert collect(d, live, live) == 1 + assert (d / "m3" / "current.pdf").is_file() + assert not (d / "m3" / "rejected.pdf").exists() def test_attachment_grace_is_a_month_outside_a_staging_folder(tmp_path):