From da016e1b94d562b7cce0d4fd056c4fa789d158b1 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Fri, 4 Sep 2026 22:55:39 +0200 Subject: [PATCH] !dodaj lands in a queue; the management page approves it into the album A photo sent to the chat no longer enters the album by itself. pack_add strips it, checks it against the album and the queue, and writes it to pending/ with a sidecar naming the people and labels the sender proposed; the reply is "czeka na zatwierdzenie". The management page lists the queue above the tagger, with the proposal pre-ticked, and Approve moves the file in, mints the labels, records approved_at and asks the bot to rebuild its table through /_reload -- the same publish path every other data change takes, so PackHolder.reload is gone. A reload the bot refuses is reported on the page, not rolled back: the photo is in the album either way. The personal-data gate now reads pending/*.json too, so a label that is still waiting for approval is forbidden in the tree already. The e2e run boots the management page beside the bot with the same environment, proves the queue is stripped and never reaches media/, approves it through the real routes, and has !losowe answer with the photo afterwards without a restart; exiftool is found through media_strip.locate() there rather than PATH, which lacks vendor_perl on Arch. !gadaj is dropped from !odcisz: it was within a typo of !dodaj, and the nearest-command suggestion must never be a coin toss. Co-Authored-By: Claude Opus 4.8 --- DOCS-add-photo.md | 66 ++++-- DOCS-commands.md | 13 +- DOCS-photo-pack.md | 42 ++-- DOCS-web-ui.md | 60 +++--- .../approved/dodaj_again_is_a_duplicate.yaml | 9 + .../random_finds_the_approved_label.yaml | 9 + fake_signal/scenarios/pack_add_custom.yaml | 12 ++ fake_signal/scenarios/pack_add_disabled.yaml | 13 -- fake_signal/scenarios/pack_add_heic.yaml | 14 ++ .../scenarios/pack_add_no_attachment.yaml | 9 + fake_signal/scenarios/pack_add_person.yaml | 10 + fake_signal/scenarios/pack_add_rejected.yaml | 10 + fake_signal/scenarios/pack_add_scoping.yaml | 10 + fake_signal/scenarios/pack_add_svg.yaml | 12 ++ fake_signal/scenarios/pack_add_video.yaml | 15 ++ scripts/e2e.sh | 80 +++++--- scripts/e2e_proofs.sh | 67 ++++++ scripts/personal_data_sources.py | 9 +- signal_bot/aliases.json | 8 +- signal_bot/manage/app.py | 2 + signal_bot/manage/bot_reload.py | 41 ++++ signal_bot/manage/routes_aliases.py | 21 +- signal_bot/manage/routes_pending.py | 103 ++++++++++ signal_bot/pack_add.py | 155 +++++++++----- signal_bot/pack_holder.py | 55 ++--- signal_bot/pack_ingest.py | 116 ++++------- signal_bot/pack_pending.py | 193 ++++++++++++++++++ signal_bot/pack_replies.py | 20 +- signal_bot/wiring.py | 12 +- tests/test_help_album.py | 3 +- tests/test_manage_pending.py | 171 ++++++++++++++++ tests/test_multiple_albums.py | 11 +- tests/test_pack_add.py | 134 +++++++----- tests/test_pack_add_invariants.py | 168 ++++++++------- tests/test_pack_holder.py | 56 ----- tests/test_pack_pending.py | 179 ++++++++++++++++ tests/test_pack_random.py | 7 +- tests/test_personal_data_sources.py | 13 ++ tests/test_wiring_ungated.py | 10 +- tool/e2e_approve.py | 64 ++++++ tool/e2e_fixtures.py | 12 +- tool/e2e_media.py | 22 +- web/src/api/client.ts | 12 ++ web/src/api/types.ts | 21 ++ web/src/app.test.tsx | 3 +- web/src/test/fixtures.ts | 12 ++ web/src/views/album.test.tsx | 38 +++- web/src/views/album.tsx | 16 +- web/src/views/pending.test.tsx | 132 ++++++++++++ web/src/views/pending.tsx | 128 ++++++++++++ 50 files changed, 1853 insertions(+), 545 deletions(-) create mode 100644 fake_signal/approved/dodaj_again_is_a_duplicate.yaml create mode 100644 fake_signal/approved/random_finds_the_approved_label.yaml create mode 100644 fake_signal/scenarios/pack_add_custom.yaml delete mode 100644 fake_signal/scenarios/pack_add_disabled.yaml create mode 100644 fake_signal/scenarios/pack_add_heic.yaml create mode 100644 fake_signal/scenarios/pack_add_no_attachment.yaml create mode 100644 fake_signal/scenarios/pack_add_person.yaml create mode 100644 fake_signal/scenarios/pack_add_rejected.yaml create mode 100644 fake_signal/scenarios/pack_add_scoping.yaml create mode 100644 fake_signal/scenarios/pack_add_svg.yaml create mode 100644 fake_signal/scenarios/pack_add_video.yaml create mode 100644 signal_bot/manage/bot_reload.py create mode 100644 signal_bot/manage/routes_pending.py create mode 100644 signal_bot/pack_pending.py create mode 100644 tests/test_manage_pending.py delete mode 100644 tests/test_pack_holder.py create mode 100644 tests/test_pack_pending.py create mode 100755 tool/e2e_approve.py create mode 100644 web/src/views/pending.test.tsx create mode 100644 web/src/views/pending.tsx diff --git a/DOCS-add-photo.md b/DOCS-add-photo.md index 1cf447c..c17aa08 100644 --- a/DOCS-add-photo.md +++ b/DOCS-add-photo.md @@ -1,24 +1,48 @@ # Adding a photo from the chat: `!add_photo` -> **Unwired since 2026-09-04.** `!dodaj` is not a command until photos land -> in an approval queue instead of straight in the album. Everything below -> describes the ingest path as it was wired, and as the queue will reuse it. +How a member of an album's group proposes a photo for it by sending one, +how the operator approves it, and what keeps that path from leaking anything. +The album itself — where it lives, what its manifests look like, how several +albums coexist — is `DOCS-photo-pack.md`. -How a member of an album's group puts a photo into it by sending one, and -what keeps that path from leaking anything. The album itself — where it -lives, what its manifests look like, how several albums coexist — is -`DOCS-photo-pack.md`. +Send a photo captioned `!dodaj alfa wakacje2019` in one of the album's groups. +`alfa` resolves to that person's key; `wakacje2019` matches nobody and travels +as a *proposed* label. The photo is stripped and lands in the album's +`pending/`, and the reply is `czeka na zatwierdzenie: 1`. It is **not in the +album yet**: `!alfa` and `!losowe wakacje2019` cannot find it until somebody +approves it. -Send a photo captioned `!add_photo alfa wakacje2019` in one of the album's -groups. `alfa` resolves to that person's key; `wakacje2019` matches nobody and -is minted as a custom tag. The photo is retrievable through `!alfa` and -`!random wakacje2019` **immediately**, with no restart. +### Approval: the management page, and nothing else + +`./web.sh` shows a **Waiting for approval** section above the tagger whenever +`pending/` is not empty (`web/src/views/pending.tsx`). Each queued photo is a +card with the album's people as checkboxes — the ones the sender named are +pre-ticked — and one checkbox per proposed label. **Approve** moves the file +into `media/`, writes `tags.json`, mints any ticked label in `custom.json`, +records the sender in `adds.json` (with `approved_at`), and asks the bot for +`POST /_reload`. From that moment `!losowe wakacje2019` answers with it, with +no restart. **Reject** deletes the file and its sidecar; nothing else to undo, +because nothing else was written. + +The queue is the only door in. The bot's own process never approves anything; +it writes `pending/.` plus a `.json` sidecar (`keys`, +`labels`, `by`, `at`, `group`) and reads the result back at the reload. The +page can only approve a label the sender actually proposed (anything else is +`400`), and it can keep fewer — an unticked label is simply not minted. If the +bot then refuses the reload (a stale alias, say) the page **reports** it and +does not roll back: the photo is in the album with the tags the approver +chose, and the bot's reason is about something else. What holds: - **No typed label travels.** Labels become keys in the registry's `arg_resolver`, at the dispatcher, *before* the line that logs the command - name. The reply is counts and pseudonyms — `dodano · p1: 4 · t1: 1`. + name; a new word becomes a per-message token (`new1`, `new2`) and is claimed + back by its own handler, so concurrent frames cannot swap labels. The reply + is counts only — `czeka na zatwierdzenie: 2 · 1 już było` — and the proposed + word exists in exactly one place, the sidecar inside the album's `0700`. + `scripts/check_personal_data.sh` reads those sidecars too, so a proposed + name is on the forbidden list before it is ever approved. - **No cap.** No cooldown, no size limit, no daily limit. `!random` takes the ordinary pack cooldown; `!add_photo` takes no decorator at all. - **At most six labels**, the same ceiling `!alfa beta …` has. @@ -26,15 +50,19 @@ What holds: carries the *quoted* message's media in `quote.attachments`, and taking it would let anyone re-file somebody else's photo. - **A duplicate changes nothing.** The stored name is a content hash, so a - photo already in the album is reported and skipped; a mixed batch stores the - new files and says how many were already there. -- **Nothing is written until it works.** Media, `tags.json`, `custom.json` and - `adds.json` are written, the album is reloaded, and only a load that - succeeds is published. A load that fails rolls all four back and keeps - serving the album that was already working. + photo already in the album is `już było`, one already waiting is `już + czeka` — the same photo twice in one message counts as waiting too — and a + mixed batch queues the new files and says how many were which. +- **Approval is whole or not at all.** Media, `tags.json`, `custom.json` and + `adds.json` are written, the album is loaded again, and only a load that + succeeds keeps them. A load that fails rolls all four back and leaves the + photo in the queue with a `400` naming the reason. - **The second copy goes.** signal-cli keeps its own copy of every inbound attachment outside the album's `0700`. `!add_photo` deletes it once the file - is stored, and also when the upload was refused. + is queued, and also when the upload was refused. +- **Stripping happens before the wait.** The file in `pending/` is already + the stripped one; `scripts/e2e.sh` re-reads the queue with exiftool before + approving anything, and the album after. ### Stripping: exiftool, with a positive check diff --git a/DOCS-commands.md b/DOCS-commands.md index d955229..c5b05ee 100644 --- a/DOCS-commands.md +++ b/DOCS-commands.md @@ -31,12 +31,13 @@ This file is the trigger reference — every spelling that resolves, which | command_stats | `!analityka` `!command_stats` `!commandstats` `!analytics` `!uzycie` `!użycie` `!ranking` | | help | `!pomoc` `!pomocy` `!help` `!komendy` `!commands` | | random | `!losowe` `!losuj` `!random` `!rand` | +| add_photo | `!dodaj` `!dodaj_zdjecie` `!dodajzdjecie` `!add_photo` `!addphoto` | | suggest | `!zaproponuj` `!propozycja` `!sugestia` `!suggest` `!pomysl` | | good_bot | `!good bot` `!dobry bot` `!goodbot` `!dobrybot` `!good_bot` `!dobry_bot` | | food_today | `!co_jadł` `!co_jadl` `!cojadł` `!cojadl` `!jedzenie` `!dieta` + the operator's own in `aliases_custom.json` | | workout_today | `!co_ćwiczył` `!co_cwiczyl` `!coćwiczył` `!cocwiczyl` `!trening` `!workout` + the operator's own in `aliases_custom.json` | | mute | `!cisza` `!cicho` `!mute` | -| unmute | `!odcisz` `!gadaj` `!wracaj` | +| unmute | `!odcisz` `!wracaj` | Aliases live in `signal_bot/aliases.json`, not in source, because they are data: adding one should never mean touching Python. Two constraints on a new @@ -116,10 +117,12 @@ for `TRUSTED_INVITERS`; anyone else is refused without a reply, because a refusal is a reply. With `TRUSTED_INVITERS` unset, anybody may. There is no `!unmute` alias: it sits one edit from `!mute`, inside the near-miss budget. -`add_photo` (`!dodaj`) is **unwired since 2026-09-04**: a photo posted to a -group must not reach the album without somebody approving it, and no approval -queue exists yet. It comes back when one does; `DOCS-add-photo.md` describes -the path as it was. +`add_photo` (`!dodaj alfa wakacje` + an attachment) exists in the same groups +as `random` and under the same silence elsewhere. It does **not** put the +photo in the album: it strips it and queues it for approval on the management +page (`./web.sh`), and answers with a count — `czeka na zatwierdzenie: 1`. +Only an approved photo is retrievable, and it is from the moment it is +approved. No cooldown, no size cap, no daily limit. `DOCS-add-photo.md`. Any command can be **switched off** from the management page (Commands tab), outright or until a moment, and given its own cooldown. The bot reads the diff --git a/DOCS-photo-pack.md b/DOCS-photo-pack.md index 0e787bd..26cd523 100644 --- a/DOCS-photo-pack.md +++ b/DOCS-photo-pack.md @@ -15,18 +15,18 @@ gamma — so the mechanism can be public while the album stays private. ~/.local/share/signal-bot/pack/ people.json # the commands, and the only place names exist tags.json # which file shows whom - custom.json # labels !add_photo minted (optional) + custom.json # labels approved from !add_photo (optional) adds.json # who added what, read by nothing (optional) media/ # the photos and videos themselves + pending/ # !add_photo sends waiting for approval (optional) ``` Found in this order: `PHOTO_PACK_DIR`, then `$STATE_DIRECTORY/pack` (what systemd hands a **system** unit), then `$XDG_DATA_HOME/signal-bot/pack`. -`StateDirectory=` is deliberately **not** in the unit this repo installs: -for a *user* unit systemd resolves it below `$XDG_STATE_HOME`, which is not -where the album is. Discovery finds the same directory, and leaving -`PHOTO_PACK_DIR` unset is what keeps a not-yet-filled-in pack from turning -`Restart=always` into a restart loop. +`StateDirectory=` is deliberately **not** in the unit this repo installs: for +a *user* unit systemd resolves it below `$XDG_STATE_HOME`, which is not where +the album is. Discovery finds the same directory, and leaving `PHOTO_PACK_DIR` +unset keeps a not-yet-filled-in pack from turning `Restart=always` into a loop. Two absences, deliberately different: @@ -130,14 +130,14 @@ startup rather than shadowing the command it collides with. ## adds.json ```json -{"a3ae52c7….jpg": {"by": "", "at": 1756600000}} +{"a3ae52c7….jpg": {"by": "", "at": 1756600000, "approved_at": 1756700000}} ``` -Who added which file. **Nothing reads it** — `pack_load` ignores the file -entirely — and it exists only so `!undo_photo` can be built later without a -gap in its history; the adder has to be recorded at add time or not at all. -It lives inside the album, which is `0700` and outside this repository, so it -is handled exactly like the photos. +Who added which file, and when it was approved. **Nothing reads it** — +`pack_load` ignores it — and it exists only so `!undo_photo` can be built +later without a gap in its history; the adder is recorded at add time or not +at all. It lives inside the album (`0700`, outside this repository), so it is +handled exactly like the photos. ## sources.json @@ -169,11 +169,14 @@ at startup would be a restart loop under `Restart=always`. ## Adding from the chat: !add_photo -A member of one of the album's groups can send a photo captioned -`!add_photo alfa wakacje` and have it land in that album, tagged, stripped -and retrievable immediately. That path has its own document, because the -metadata handling and the privacy invariants it has to keep are longer than -the album format itself: **`DOCS-add-photo.md`**. +A member of one of the album's groups sends a photo captioned +`!add_photo alfa wakacje`. It is stripped and lands in **`pending/`** — under +its content hash, beside a `.json` sidecar with the proposed `keys`, +`labels`, sender and group — until the operator approves it on `./web.sh`, +which is when it is tagged, moved into `media/` and retrievable. `pack_load` +never reads `pending/`. The path has its own document, because its metadata +handling and privacy invariants are longer than the album format itself: +**`DOCS-add-photo.md`**. ## Filling it in: ./web.sh @@ -242,5 +245,6 @@ the group tries it. offline and with no Signal account, that one name posts a photo, two names intersect, an impossible combination gets one line and no attachment, a video leaves as a video, and the same command in another group produces **zero** -sends. It also drives `!add_photo` against the real exiftool, on media it -generates at run time, so no photo enters this repository. +sends. It also drives `!dodaj` against the real exiftool on media generated at +run time (no photo enters this repository), approves the queue through the +management page it boots beside the bot, and has `!losowe` answer, no restart. diff --git a/DOCS-web-ui.md b/DOCS-web-ui.md index df3098e..cc27453 100644 --- a/DOCS-web-ui.md +++ b/DOCS-web-ui.md @@ -77,13 +77,12 @@ account's number and is edited from a tab that can be closed mid-save. ### Group settings need a group listing -`GROUP_IDS` and `SELFTEST_GROUPS` are editable, but only while `/api/groups` -has actually returned something. `parse_group_ids` checks *syntax*; startup -additionally requires each listed group to be one the bot really joined, -judged against a listing that was really read. So with the container down, a -syntactically valid write can name a group the next startup rejects — turning -a scoping mistake into a unit that will not start. The page says so and leaves -the other keys editable. +`GROUP_IDS` and `SELFTEST_GROUPS` are editable only while `/api/groups` has +actually returned something. `parse_group_ids` checks *syntax*; startup also +requires each listed group to be one the bot really joined, judged against a +listing that was really read. With the container down, a syntactically valid +write can name a group the next startup rejects — a scoping mistake turned +into a unit that will not start. The page says so and leaves the rest editable. Group ids are **picked, never typed** — `GROUP_IDS` and `SELFTEST_GROUPS` render as a ` { + setKeys((current) => flip(current, person.key)); + }} + type="checkbox" + /> + {person.triggers[0] ?? person.key} + + + ); + })} + {props.item.labels.map((label) => { + return ( +
  • + +
    • + ); + })} + + + + {error !== "" &&

    {error}

    } + + ); +}