|
|
|
|
@@ -0,0 +1,231 @@
|
|
|
|
|
---
|
|
|
|
|
phase: quick-261018-qbs
|
|
|
|
|
plan: 01
|
|
|
|
|
quick_id: 261018-qbs
|
|
|
|
|
slug: fix-19-1-review-findings-wr-04-chatbots-
|
|
|
|
|
type: execute
|
|
|
|
|
mode: quick
|
|
|
|
|
wave: 1
|
|
|
|
|
depends_on: []
|
|
|
|
|
files_modified:
|
|
|
|
|
- /media/nvme/dev/golem15/summercms.io/summercms/sm-chatbots-plugin/classes/dispatcher.go
|
|
|
|
|
- /media/nvme/dev/golem15/summercms.io/summercms/sm-chatbots-plugin/map_support_test.go
|
|
|
|
|
- /media/nvme/dev/golem15/summercms.io/summercms/sm-chatbots-plugin/http_tracer_test.go
|
|
|
|
|
- /media/nvme/dev/golem15/summercms.io/summercms/sm-chatbots-plugin/README.md
|
|
|
|
|
- /media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin/classes/video_service.go
|
|
|
|
|
- /media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin/classes/user_deletion.go
|
|
|
|
|
- /media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin/boot.go
|
|
|
|
|
- /media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin/user_deletion_test.go
|
|
|
|
|
- /media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin/README.md
|
|
|
|
|
- /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/scripts/check-phase19.1.sh
|
|
|
|
|
- /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/docs/plugins/extending.md
|
|
|
|
|
autonomous: true
|
|
|
|
|
requirements: [WR-04, WR-05]
|
|
|
|
|
source:
|
|
|
|
|
- .planning/phases/19.1-chat-satellites/19.1-REVIEW.md (WR-04 line 149, WR-05 line 162)
|
|
|
|
|
- .planning/phases/19.1-chat-satellites/19.1-SECURITY.md (Unregistered findings, rows WR-04 and WR-05)
|
|
|
|
|
- docs/plugins/extending.md ("Optional dependencies")
|
|
|
|
|
|
|
|
|
|
estimate:
|
|
|
|
|
tokens: 75000
|
|
|
|
|
raw_tokens: 75000
|
|
|
|
|
tasks: 3
|
|
|
|
|
confidence: low
|
|
|
|
|
|
|
|
|
|
must_haves:
|
|
|
|
|
truths:
|
|
|
|
|
- "POST /_chatbots/api/v1/dispatch for a registered command whose IsAvailable is false for the JWT actor answers 422 with a body byte-identical to an unknown trigger ({error: Validation failed, errors: {input: [unknown_command]}}), and the command's Handle never runs (WR-04)."
|
|
|
|
|
- "Available commands and /help dispatch exactly as before; GET /commands and every response shape are unchanged."
|
|
|
|
|
- "Deleting a user who started a video session that has other participants keeps the session row and the other participants' rows; started_by moves to the other participant who joined earliest (joined_at ASC NULLS LAST, id ASC) (WR-05)."
|
|
|
|
|
- "Deleting a user who started a session with no other participant removes that session."
|
|
|
|
|
- "A live session (pending/active) the deleted user started, or was the last joined participant of, is ended inside the delete transaction (status ended, ended_at, duration_seconds, active_conversation_id NULL); its LiveKit room is deleted only after the commit, best-effort, logged on failure, never failing the delete."
|
|
|
|
|
- "The deleted user's own participant rows are removed on ev.Tx; in a live call that continues without them, their LiveKit connection is removed after the commit, best-effort."
|
|
|
|
|
- "No schema change: sm-chatvideo-plugin updates/ is untouched and started_by stays NOT NULL."
|
|
|
|
|
- "scripts/check-phase19.1.sh requires TestUserDeletionReassignsSession in the chatvideo postgres stage and its self-test refuses a script without that token."
|
|
|
|
|
- "docs/plugins/extending.md says an optional dependency is optional at activation, not at compile time: the consumer still has it in go.mod when it imports the package declaring the type it looks up; only party.Plugin.Requires leaves it out."
|
|
|
|
|
- "go vet ./... and go test -short ./... are green in sm-chatbots-plugin, sm-chatvideo-plugin and summercms.go; go.mod and go.sum of both plugins are unchanged."
|
|
|
|
|
artifacts:
|
|
|
|
|
- "sm-chatbots-plugin/classes/dispatcher.go — Dispatch refuses a command whose IsAvailable(actor, cmdCtx) is false, returning nil like a missing trigger"
|
|
|
|
|
- "sm-chatbots-plugin/map_support_test.go — TestDispatchRefusesUnavailable (-short, no DB)"
|
|
|
|
|
- "sm-chatbots-plugin/http_tracer_test.go — TestHTTPTracer extended: restricted command over HTTP equals the unknown-trigger 422, absent from GET /commands"
|
|
|
|
|
- "sm-chatvideo-plugin/classes/user_deletion.go — VideoService.OnUserDeleting(ctx, tx, userID)"
|
|
|
|
|
- "sm-chatvideo-plugin/boot.go — boot step listeners: user.Deleting on ev.Tx"
|
|
|
|
|
- "sm-chatvideo-plugin/user_deletion_test.go — TestUserDeletionReassignsSession (testcontainers postgres)"
|
|
|
|
|
- "summercms.go/scripts/check-phase19.1.sh — PG_CHATVIDEO_TESTS, self-test token list and NOT RUN banner name TestUserDeletionReassignsSession"
|
|
|
|
|
- "summercms.go/docs/plugins/extending.md — one short paragraph in Optional dependencies"
|
|
|
|
|
key_links:
|
|
|
|
|
- "chatbots: api.Dispatch -> Dispatcher.Dispatch -> nil -> writeValidation input unknown_command. The fix lives only in Dispatcher.Dispatch so the API handler and its 422 path stay untouched."
|
|
|
|
|
- "chatvideo: user plugin FormAfterDelete runs inside cabana's lagoon.Transaction and fires DeletingEvent{User, Tx} with the transaction ctx; chatvideo's listener must pass that same ctx and ev.Tx to lagoon.AfterCommit, otherwise the callback is either run too early or skipped."
|
|
|
|
|
- "chatvideo: listener order is activation order; golem15.chat (required) runs first and reassigns conversation created_by, so the conversation survives the delete; the chatvideo listener must run its work before the user row's FK CASCADE fires (it does: FormAfterDelete fires the event, then deletes the user)."
|
|
|
|
|
- "gate: go_named in check-phase19.1.sh runs each PG_CHATVIDEO_TESTS name with -run and refuses a skipped or missing name; the self-test greps the script for each required token."
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
<objective>
|
|
|
|
|
Close the two open 19.1 code-review warnings in the sibling plugin repos and add one clarifying paragraph to the framework's plugin-extension docs.
|
|
|
|
|
|
|
|
|
|
1. WR-04 (`sm-chatbots-plugin`): `POST /_chatbots/api/v1/dispatch` runs a command without `IsAvailable`, while `GET /commands` filters on it. A restricted host command becomes invocable by any JWT user. Fix: an unavailable command is refused exactly like a missing trigger (existing 422 `unknown_command`); no response shape changes.
|
|
|
|
|
2. WR-05 (`sm-chatvideo-plugin`): there is no `user.Deleting` listener, so `started_by ... ON DELETE CASCADE` drops whole sessions including other users' participant rows, and the LiveKit room is never deleted. Fix: listen `user.Deleting` on `ev.Tx`, reassign `started_by` to an heir (sm-userfriends-plugin heir pattern), delete only sessions nobody else was on, end live sessions, remove the user's own rows, and delete LiveKit rooms after commit. No migration.
|
|
|
|
|
3. Docs (`summercms.go`): `docs/plugins/extending.md` "Optional dependencies" gains a line saying an optional dependency is optional at activation, not at compile time.
|
|
|
|
|
|
|
|
|
|
Purpose: WR-04 is a latent elevation of privilege on a core plugin; WR-05 lets one user's GDPR delete destroy other users' call history and leaves a live room reachable. Both are listed as unregistered findings in 19.1-SECURITY.md.
|
|
|
|
|
Output: one commit in sm-chatbots-plugin, one in sm-chatvideo-plugin, two in summercms.go (gate script, docs). Nothing is pushed or tagged.
|
|
|
|
|
</objective>
|
|
|
|
|
|
|
|
|
|
<execution_context>
|
|
|
|
|
@~/.claude/msd-core/workflows/execute-plan.md
|
|
|
|
|
@~/.claude/msd-core/templates/summary.md
|
|
|
|
|
</execution_context>
|
|
|
|
|
|
|
|
|
|
<context>
|
|
|
|
|
@/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/CLAUDE.md
|
|
|
|
|
@/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/.planning/phases/19.1-chat-satellites/19.1-REVIEW.md
|
|
|
|
|
@/media/nvme/dev/golem15/summercms.io/summercms/sm-chatbots-plugin/classes/dispatcher.go
|
|
|
|
|
@/media/nvme/dev/golem15/summercms.io/summercms/sm-chatbots-plugin/api/commands.go
|
|
|
|
|
@/media/nvme/dev/golem15/summercms.io/summercms/sm-chatbots-plugin/map_support_test.go
|
|
|
|
|
@/media/nvme/dev/golem15/summercms.io/summercms/sm-chatbots-plugin/http_tracer_test.go
|
|
|
|
|
@/media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin/boot.go
|
|
|
|
|
@/media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin/classes/video_service.go
|
|
|
|
|
@/media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin/classes/livekit.go
|
|
|
|
|
@/media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin/harness_test.go
|
|
|
|
|
@/media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin/reconcile_test.go
|
|
|
|
|
@/media/nvme/dev/golem15/summercms.io/summercms/sm-userfriends-plugin/boot.go
|
|
|
|
|
@/media/nvme/dev/golem15/summercms.io/summercms/sm-userfriends-plugin/classes/group_service.go
|
|
|
|
|
@/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/lagoon/transaction.go
|
|
|
|
|
@/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/scripts/check-phase19.1.sh
|
|
|
|
|
@/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/docs/plugins/extending.md
|
|
|
|
|
|
|
|
|
|
Facts verified at planning time (2026-10-18, live reads):
|
|
|
|
|
- Both plugin repos have a clean worktree. summercms.go has other sessions' uncommitted `.planning/state.json`, `.msd/` and `17-VERIFICATION.md`: leave them alone, stage by explicit path only.
|
|
|
|
|
- chatbots `Dispatcher.Dispatch` (classes/dispatcher.go:30-47) is the only caller path; its single caller is `api.Dispatch` (api/commands.go:57), which already maps nil to `writeValidation(input: unknown_command)`. `Registry.Catalog` (dispatcher.go:85) already filters `IsAvailable`.
|
|
|
|
|
- chatbots tests live in package `chatbots_test` at the plugin root. `gatedCommand{trigger, avail, mode}` already exists in map_support_test.go:15-31 (IsAvailable returns `avail`). `env.reg`, `env.disp` are on `botsEnv`; `TestDispatcherAnnounce` registers on `env.reg` after Boot, so post-Boot registration works. `TestHTTPTracer` (http_tracer_test.go:100) is Docker-backed and already required by the gate.
|
|
|
|
|
- chatvideo `endSession` (classes/video_service.go:333-361) writes status/ended_at/duration_seconds/active_conversation_id on `s.db`, then calls `s.rooms.DeleteRoom` and `s.notify.Ended` inline. `sdkRooms.DeleteRoom` returns nil when LiveKit is not configured and swallows a missing room; `sdkRooms.RemoveParticipant` returns `ErrNotConfigured` when not configured. `FakeRooms.DeleteRoom` removes the name from `ActiveRoomNames`.
|
|
|
|
|
- chatvideo `invite` creates the starter's participant row as `joined` (with `joined_at`) and every other conversation member as `invited`. Live sessions are `status IN (pending, active) AND active_conversation_id IS NOT NULL` (Reconcile, video_service.go:292-294).
|
|
|
|
|
- chatvideo go.mod already requires `git.golem15.com/golem15/sm-user-plugin` (notifier.go imports its models). `userclasses.DeletingEvent{User *models.User; Tx *gorm.DB}`. The user plugin fires it from `usersAdminController.FormAfterDelete` inside cabana's `lagoon.Transaction` (cabana crud.go:134), passing the transaction ctx, then deletes the user row with `Unscoped()`; a listener error rolls the delete back.
|
|
|
|
|
- golem15.chat also listens `user.Deleting` (sm-chat-plugin boot.go:183) and reassigns conversation `created_by` to another participant; conversations with no other participant are left to CASCADE.
|
|
|
|
|
- `lagoon.AfterCommit(ctx, db, fn)`: inside a `lagoon.Transaction` (buffer found through ctx/db) the callback runs after commit; outside any transaction it runs immediately; inside a foreign plain GORM transaction it logs a warning and SKIPS fn.
|
|
|
|
|
- Both plugins have `TestFixtureAbsentFromProduction`, which refuses production `.go`, `.yaml` and `.md` files containing the words queststream, family, parent, child, wavepath or fonoteka (case-insensitive, whole word). New comments and README text must avoid them (say "the other participant", "earlier row", not "parent/child").
|
|
|
|
|
- check-phase19.1.sh: `PG_CHATVIDEO_TESTS=(TestHTTPTracer TestReconcileEmptyRoom)` at line 86; self-test required-token list at lines 246-258; the NOT RUN postgres banner at lines 402-412 lists each plugin's postgres tests. Modes: `--self-test`, `--plugins`, `--docs`, `--postgres`, `--all`.
|
|
|
|
|
- Docker: if the socket is denied although the user is in the `docker` group, run the Docker-backed command through `sudo -u golem` (CLAUDE.md).
|
|
|
|
|
</context>
|
|
|
|
|
|
|
|
|
|
<tasks>
|
|
|
|
|
|
|
|
|
|
<task type="tracer" tdd="true">
|
|
|
|
|
<name>Task 1: WR-04 end-to-end: dispatch refuses an unavailable command like an unknown trigger (sm-chatbots-plugin)</name>
|
|
|
|
|
<files>/media/nvme/dev/golem15/summercms.io/summercms/sm-chatbots-plugin/classes/dispatcher.go, /media/nvme/dev/golem15/summercms.io/summercms/sm-chatbots-plugin/map_support_test.go, /media/nvme/dev/golem15/summercms.io/summercms/sm-chatbots-plugin/http_tracer_test.go, /media/nvme/dev/golem15/summercms.io/summercms/sm-chatbots-plugin/README.md</files>
|
|
|
|
|
<behavior>
|
|
|
|
|
- Dispatching "/secret" where the registered command's IsAvailable(actor, ctx) is false returns nil, and its Handle is never called.
|
|
|
|
|
- Dispatching "/open" (IsAvailable true) still returns the command's ephemeral result.
|
|
|
|
|
- Over HTTP, POST /dispatch {input: "/secret"} answers 422 with a body byte-identical to POST /dispatch {input: "/no-such-trigger"}; GET /commands does not list "secret"; the existing /ping and /help assertions still pass.
|
|
|
|
|
</behavior>
|
|
|
|
|
<action>
|
|
|
|
|
RED first: in map_support_test.go add TestDispatchRefusesUnavailable (runs under -short, no database). Build a fresh classes.NewRegistry, register a restricted command (IsAvailable false) and an open one (IsAvailable true), build classes.NewDispatcher(reg, nil) and dispatch both with any actor and an empty classes.CommandContext. The restricted command must count Handle calls (add a small test type with a pointer counter next to gatedCommand, or extend gatedCommand with an optional counter pointer without changing its existing uses) so the test asserts zero Handle calls and a nil result; the open command must return a non-nil ephemeral result. Run it and see it fail on the current dispatcher.
|
|
|
|
|
|
|
|
|
|
GREEN: in classes/dispatcher.go Dispatch, after the registry lookup, treat a command whose IsAvailable(actor, cmdCtx) is false exactly like a missing trigger: return nil before Handle runs (the review's fix: cmd nil or not available returns nil). Do not touch api/commands.go: its nil branch already writes the 422 input unknown_command body, and response shapes are the PHP frontend contract. Update the Dispatch doc comment to say a missing trigger or one not available to actor in ctx returns nil.
|
|
|
|
|
|
|
|
|
|
HTTP check (cheap, same file the gate already requires): extend TestHTTPTracer in http_tracer_test.go. After the /ping assertions, register a restricted command (IsAvailable false, trigger "secret") on env.reg, POST /dispatch with input "/secret" and with an unknown trigger such as "/no-such-trigger" using the same token, and assert both are 422 and their bodies are byte-identical (no existence oracle between "restricted" and "unknown"). In the existing GET /commands loop also assert "secret" is absent. Update the test's doc comment to mention the refusal.
|
|
|
|
|
|
|
|
|
|
README.md (same commit, CLAUDE.md docs rule): the Dispatcher feature bullet (line 28) says unrecognized or unavailable-for-the-actor input returns nil; the HTTP API bullet (line 30) adds that POST /dispatch refuses a command not IsAvailable for the JWT actor with the same 422 unknown_command as an unknown trigger; the API reference row for classes.Dispatcher.Dispatch (line 79) says an unrecognized or unavailable trigger returns nil. Keep the words refused by TestFixtureAbsentFromProduction out of the text.
|
|
|
|
|
|
|
|
|
|
Verify, then commit in the chatbots repo only, staging the four files by path: message "fix(19.1): refuse unavailable commands on chatbots dispatch" with a body naming WR-04. No co-author tag, no push.
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/sm-chatbots-plugin && go vet ./... && go test -short -count=1 ./... && go test -short -count=1 -run '^TestDispatchRefusesUnavailable$' -v . && go test -count=1 -run '^(TestHTTPTracer|TestDispatchRefusesUnavailable|TestDispatcherAnnounce)$' -v .</automated>
|
|
|
|
|
</verify>
|
|
|
|
|
<done>TestDispatchRefusesUnavailable fails before the dispatcher change and passes after; TestHTTPTracer (Docker) passes with the restricted-vs-unknown byte-identical 422 assertion; go vet and go test -short ./... are green; `git -C ../sm-chatbots-plugin diff HEAD~1 --stat` lists only dispatcher.go, map_support_test.go, http_tracer_test.go and README.md; go.mod/go.sum unchanged; one commit, not pushed.</done>
|
|
|
|
|
</task>
|
|
|
|
|
|
|
|
|
|
<task type="auto" tdd="true">
|
|
|
|
|
<name>Task 2: WR-05: user.Deleting keeps other users' call history and closes the room after commit (sm-chatvideo-plugin), then require the test in the 19.1 gate</name>
|
|
|
|
|
<files>/media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin/classes/video_service.go, /media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin/classes/user_deletion.go, /media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin/boot.go, /media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin/user_deletion_test.go, /media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin/README.md, /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/scripts/check-phase19.1.sh</files>
|
|
|
|
|
<behavior>
|
|
|
|
|
- heir: a starts a call in a conversation with b, b joins; deleting a (event plus user-row delete in one lagoon.Transaction) leaves the session row with started_by = b, status ended, active_conversation_id NULL; a's participant rows are gone, b's row remains; the room is still present inside the transaction and absent from FakeRooms after commit.
|
|
|
|
|
- empty: c starts a call in a conversation with only c; deleting c removes the session row and, after commit, the room.
|
|
|
|
|
- last participant: d starts with e, e joins, d leaves; deleting e ends the session (started_by stays d, d's row remains, e's rows gone) and deletes the room after commit.
|
|
|
|
|
- continuing call: f starts with g and h, both join; deleting h leaves the session active with started_by f and the room present; h's row is gone.
|
|
|
|
|
</behavior>
|
|
|
|
|
<action>
|
|
|
|
|
RED first: create user_deletion_test.go (package chatvideo_test) with TestUserDeletionReassignsSession, following reconcile_test.go: call ensurePostgres(t), bootVideo(t, nil), create users with env.user, conversations with env.conversation(createdBy, participants...), calls with env.svc.Start / Join / Leave, counts with env.count, and the injected env.rooms (classes.FakeRooms). Use one subtest per behavior case (heir, empty, last_participant, continuing) on the same env with distinct users. Add a local helper that deletes a user the way the admin does: lagoon.Transaction(t.Context(), env.gdb, fn) where fn fires env.app.Events.Fire(ctx, &userclasses.DeletingEvent{User: u, Tx: tx}) with the ctx fn received, then, still inside fn, checks that the rooms expected to be deleted are still listed by env.rooms.ActiveRoomNames (proves no RoomAPI call inside the transaction), then deletes the user row with tx.Session(&gorm.Session{NewDB: true}).Unscoped().Where("id = ?", u.ID).Delete(&usermodels.User{}) so the real FK CASCADE runs. Assertions per behavior block above. Without the listener the heir case must fail (CASCADE drops the session), so run it and see it fail.
|
|
|
|
|
|
|
|
|
|
GREEN, production code:
|
|
|
|
|
(a) classes/video_service.go: factor the column map endSession writes (status ended, ended_at now, duration_seconds from started_at, active_conversation_id nil) into one unexported helper that both endSession and the new path use, so the two cannot drift. endSession's behavior (inline DeleteRoom swallowing errors, inline Ended notify) stays exactly as it is.
|
|
|
|
|
(b) new classes/user_deletion.go: exported method OnUserDeleting(ctx context.Context, tx *gorm.DB, userID uint) error on *VideoService, the counterpart of userfriends GroupService.OnUserDeleting (group_service.go:729-766). Nil service or userID 0 returns nil; a nil tx falls back to s.db. Build every query from tx.Session(&gorm.Session{NewDB: true}).WithContext(ctx) so no statement state leaks from the caller's handle. All writes go to that handle; nothing commits on its own. Order:
|
|
|
|
|
1. Live sessions (status IN pending/active AND active_conversation_id IS NOT NULL) that userID started or has a participant row in. For each: when userID started it, or no participant other than userID has status joined, end it with the shared column helper and remember it as "ended". Otherwise remember its room name as "remove userID from this room".
|
|
|
|
|
2. Every session (any status) with started_by = userID. Pick the heir: the participant of that session with user_id other than userID, ordered joined_at ASC NULLS LAST, then id ASC (deterministic, never a MySQL FIELD() expression). Heir found: update started_by to the heir's user_id. No heir: delete the session row (its participant rows go with the video_session_id CASCADE) and mark it as deleted so no Ended event is published for it.
|
|
|
|
|
3. Delete every participant row with user_id = userID.
|
|
|
|
|
4. If anything was remembered, register ONE lagoon.AfterCommit(ctx, handle, fn) with the ctx the listener received (it carries lagoon's buffer) — never call RoomAPI inside the transaction. In fn, using a short bounded context (about 10 seconds) per external call: for each ended session call s.rooms.DeleteRoom(room) when s.rooms is non-nil; for each "remove" room call s.rooms.RemoveParticipant(room, user id as decimal string, the same identity Leave uses). Treat nil and errors.Is(err, ErrNotConfigured) as silent; log any other error with slog.Default().Warn naming golem15.chatvideo, the room name and err (no email or display name). Then, for ended sessions that still exist (reassigned, or started by someone else), publish s.notify.Ended(ctx, session, userID) when s.notify is non-nil. The callback never returns an error and never panics on a nil rooms/notify.
|
|
|
|
|
Doc comment: why (other users' history survives the delete; the room is closed after commit so a rolled-back delete never kills a live call) and that the caller must hand over the delete transaction's ctx and ev.Tx, because lagoon.AfterCommit skips callbacks inside a foreign plain GORM transaction.
|
|
|
|
|
(c) boot.go: add registerBootStep("listeners", bootListeners) after "service" in init. bootListeners returns nil when app or app.Events is nil, else app.Events.Listen[*userclasses.DeletingEvent](PluginID, p.onUserDeleting), importing git.golem15.com/golem15/sm-user-plugin/classes as userclasses (already a go.mod requirement). onUserDeleting(ctx, ev) nil-guards p, ev, ev.User and p.service (service is set in the OnDatabase callback) and returns p.service.OnUserDeleting(ctx, ev.Tx, ev.User.ID). Mirror sm-userfriends-plugin boot.go:75-88. A returned error rolls the user delete back, as the user plugin documents.
|
|
|
|
|
Hard limits: NO migration, NO edit of updates/ (started_by stays NOT NULL, the baseline is frozen), no API/response-shape change, no new dependency, no change to Start/Join/Leave/End/Reconcile behavior.
|
|
|
|
|
|
|
|
|
|
README.md (same commit): add a Features bullet in the style of sm-userfriends-plugin README line 36, e.g. "**GDPR.** Boot Listens `user.Deleting` (`DeletingEvent`) on `ev.Tx`. Sessions the user started go to the other participant who joined first, else are deleted; live calls the user started or was last in are ended and their LiveKit room is deleted after the commit, best-effort; the user's participant rows are removed. No schema change." Add an API reference row for `classes.VideoService.OnUserDeleting` ("user.Deleting cleanup on the delete transaction"). In Testing, add TestUserDeletionReassignsSession to the -run regex and one sentence saying it needs Docker and uses the RoomAPI fake. Keep the TestFixtureAbsentFromProduction words out.
|
|
|
|
|
|
|
|
|
|
Verify the plugin, then commit in the chatvideo repo only, staging the five chatvideo files by path: "fix(19.1): keep call history when a chatvideo starter is deleted" with a body naming WR-05. No co-author tag, no push.
|
|
|
|
|
|
|
|
|
|
Then, in summercms.go scripts/check-phase19.1.sh: append TestUserDeletionReassignsSession to PG_CHATVIDEO_TESTS (line 86), add it to the self-test required-token loop (lines 246-253, next to the other TestUserDeletion tokens), and add it to the sm-chatvideo-plugin list in the NOT RUN postgres banner (lines 409-410). Run the self-test and plugins stages, then commit in summercms.go staging only scripts/check-phase19.1.sh: "chore(19.1): require chatvideo user-deletion test in phase 19.1 gate". This is a separate commit after the chatvideo commit.
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin && go vet ./... && go test -short -count=1 ./... && go test -count=1 -run '^(TestUserDeletionReassignsSession|TestReconcileEmptyRoom|TestHTTPTracer|TestFixtureAbsentFromProduction)$' -v . && git diff --quiet HEAD -- updates/ go.mod go.sum && cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && bash scripts/check-phase19.1.sh --self-test && bash scripts/check-phase19.1.sh --plugins && grep -c 'TestUserDeletionReassignsSession' scripts/check-phase19.1.sh</automated>
|
|
|
|
|
</verify>
|
|
|
|
|
<done>TestUserDeletionReassignsSession fails on the heir case before the listener exists and passes with all four subtests after; TestReconcileEmptyRoom, TestHTTPTracer and TestFixtureAbsentFromProduction still pass; go vet and go test -short ./... green; updates/, go.mod and go.sum unchanged; the grep count is 3 (PG list, self-test token, banner); --self-test and --plugins pass; when Docker is reachable `bash scripts/check-phase19.1.sh --postgres` passes too; two commits (chatvideo repo, then summercms.go gate), not pushed.</done>
|
|
|
|
|
</task>
|
|
|
|
|
|
|
|
|
|
<task type="auto">
|
|
|
|
|
<name>Task 3: Docs: optional dependencies are optional at activation, not at compile time (summercms.go)</name>
|
|
|
|
|
<files>/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/docs/plugins/extending.md</files>
|
|
|
|
|
<action>
|
|
|
|
|
In docs/plugins/extending.md, section "Optional dependencies" (lines 51-58), add one short paragraph after the line "This replaces `PluginManager::exists` checks in WinterCMS." House style: plain, short sentences, no consuming-application names. Content: an optional dependency is optional at activation, not at compile time; a plugin that looks up a type declared in another plugin's package imports that package, so the other plugin still appears in its `go.mod`; only `party.Plugin.Requires` leaves it out. End by pointing to `pact.OptionalMessage` as the way to integrate without that import, so the paragraph agrees with the existing bullet that says OptionalMessage needs no import. Suggested wording (adjust only for flow): "Optional here means optional at activation, not at compile time. A plugin that looks up a type declared in another plugin's package imports that package, so the other plugin still appears in its `go.mod`. Only `party.Plugin.Requires` leaves it out. `pact.OptionalMessage` is the way to integrate without that import."
|
|
|
|
|
Use only code spans the docs checker already accepts in this file (`party.Plugin.Requires`, `pact.OptionalMessage`, `go.mod`); do not introduce new identifiers.
|
|
|
|
|
Run the docs checker, then commit in summercms.go staging only docs/plugins/extending.md: "docs(plugins): note optional dependencies still sit in go.mod". No co-author tag, no push.
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go test -count=1 ./cmd/summer -run TestDocsTree && go run ./cmd/summer docs:build --check && grep -c 'optional at activation, not at compile time' docs/plugins/extending.md</automated>
|
|
|
|
|
</verify>
|
|
|
|
|
<done>The paragraph is in the Optional dependencies section and does not contradict the pact.OptionalMessage bullet; TestDocsTree and docs:build --check pass; grep count is 1; one docs-only commit in summercms.go, not pushed.</done>
|
|
|
|
|
</task>
|
|
|
|
|
|
|
|
|
|
</tasks>
|
|
|
|
|
|
|
|
|
|
<threat_model>
|
|
|
|
|
## Trust Boundaries
|
|
|
|
|
|
|
|
|
|
| Boundary | Description |
|
|
|
|
|
|----------|-------------|
|
|
|
|
|
| frontend JWT user -> chatbots POST /dispatch | Any authenticated user sends free-text `input`; the trigger selects host-registered code that may be restricted. |
|
|
|
|
|
| admin user delete -> plugin user.Deleting listeners | One user's GDPR delete runs every plugin's cleanup in one transaction; a listener can destroy or keep rows belonging to other users. |
|
|
|
|
|
| SummerCMS -> LiveKit server (after commit) | External call to close rooms; failure or slowness must not affect the database outcome. |
|
|
|
|
|
| issued LiveKit join token -> LiveKit room | A token minted earlier stays valid until `token_ttl` (default 900 s) while the room exists. |
|
|
|
|
|
|
|
|
|
|
## STRIDE Threat Register
|
|
|
|
|
|
|
|
|
|
Numbering continues after 19.1-SECURITY.md's highest registered ID (T-19.1-12); these rows register WR-04 and WR-05.
|
|
|
|
|
|
|
|
|
|
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
|
|
|
|
|-----------|----------|-----------|----------|-------------|-----------------|
|
|
|
|
|
| T-19.1-13 | Elevation of privilege | sm-chatbots-plugin `classes/dispatcher.go` Dispatch (WR-04) | high | mitigate | `IsAvailable(actor, cmdCtx)` is checked before `Handle`; an unavailable command returns nil and the API answers the existing 422 `unknown_command`, byte-identical to an unknown trigger (no existence oracle); actor still only from the JWT. Proven by TestDispatchRefusesUnavailable (-short) and TestHTTPTracer (gate-required). |
|
|
|
|
|
| T-19.1-14 | Tampering | sm-chatvideo-plugin `started_by ... ON DELETE CASCADE` / new `VideoService.OnUserDeleting` (WR-05) | medium | mitigate | `user.Deleting` listener on `ev.Tx` reassigns `started_by` to a deterministic heir before the user row's CASCADE fires, deletes only sessions with no other participant, removes only the deleted user's participant rows. TestUserDeletionReassignsSession runs the real FK CASCADE and is added to the 19.1 gate's postgres stage. |
|
|
|
|
|
| T-19.1-15 | Information disclosure | sm-chatvideo-plugin LiveKit room after the starter's or last participant's deletion (WR-05) | medium | mitigate | Live sessions the user started or was last in are ended in the transaction and their room is `DeleteRoom`'d after commit (`lagoon.AfterCommit`), which invalidates outstanding join tokens for that room; for calls that continue, the user's connection is removed via `RemoveParticipant` after commit. |
|
|
|
|
|
| T-19.1-16 | Information disclosure | LiveKit token of a deleted non-starter in a call that continues | low | accept | LiveKit has no per-token revocation; the window is bounded by `token_ttl` (default 900 s), the user's frontend JWT is gone with the account, and the live connection is removed after commit. |
|
|
|
|
|
| T-19.1-17 | Denial of service | after-commit LiveKit calls in the user-delete path | low | mitigate | No RoomAPI call inside the transaction (no row locks held on a network call, a LiveKit outage cannot roll back a GDPR delete); per-call bounded context; errors logged, never returned; `ErrNotConfigured` / unconfigured LiveKit is silent. |
|
|
|
|
|
| T-19.1-SC | Tampering | go.mod / go.sum of sm-chatbots-plugin and sm-chatvideo-plugin | high | mitigate | No package installs and no new dependency; Task 2 verify fails if chatvideo go.mod/go.sum change; Task 1 done-check confirms chatbots go.mod/go.sum unchanged. |
|
|
|
|
|
</threat_model>
|
|
|
|
|
|
|
|
|
|
<verification>
|
|
|
|
|
- sm-chatbots-plugin: `go vet ./...`, `go test -short ./...`, and with Docker `go test -run '^(TestHTTPTracer|TestDispatchRefusesUnavailable|TestDispatcherAnnounce)$' .`.
|
|
|
|
|
- sm-chatvideo-plugin: `go vet ./...`, `go test -short ./...`, and with Docker `go test -run '^(TestUserDeletionReassignsSession|TestReconcileEmptyRoom|TestHTTPTracer)$' -v .`; `git diff HEAD~1 -- updates/ go.mod go.sum` is empty.
|
|
|
|
|
- summercms.go: `bash scripts/check-phase19.1.sh --self-test`, `--plugins`, `--docs`; with Docker `--postgres` (runs every sibling's required postgres tests, including the new one); `go test ./cmd/summer -run TestDocsTree`; `go vet ./...` and `go test -short ./...` stay green.
|
|
|
|
|
- If the Docker socket is denied, run the Docker-backed commands through `sudo -u golem` (CLAUDE.md). If Docker is unreachable, record the Docker-backed tests as NOT RUN in the SUMMARY; do not claim them.
|
|
|
|
|
- Commits: exactly four (chatbots, chatvideo, summercms.go gate, summercms.go docs), files staged by path, no co-author tags, nothing pushed or tagged; other sessions' uncommitted files in summercms.go untouched.
|
|
|
|
|
</verification>
|
|
|
|
|
|
|
|
|
|
<success_criteria>
|
|
|
|
|
- WR-04 closed: a restricted command cannot be run through POST /dispatch, and the refusal is indistinguishable from an unknown trigger.
|
|
|
|
|
- WR-05 closed: deleting a call starter no longer erases other users' sessions or participant rows; live rooms are closed after commit; no migration.
|
|
|
|
|
- The 19.1 gate requires the new chatvideo test.
|
|
|
|
|
- The extending docs state that optional dependencies are optional at activation, not at compile time.
|
|
|
|
|
- All verify commands above pass (Docker-backed ones either pass or are reported NOT RUN).
|
|
|
|
|
</success_criteria>
|
|
|
|
|
|
|
|
|
|
<output>
|
|
|
|
|
Create `.planning/quick/261018-qbs-fix-19-1-review-findings-wr-04-chatbots-/261018-qbs-SUMMARY.md` when done, listing the four commit hashes per repo, which Docker-backed tests ran, and the new threat rows (T-19.1-13 .. T-19.1-17) for later registration in 19.1-SECURITY.md.
|
|
|
|
|
</output>
|
Jakub Zych