docs(project): record future-work P1-P8 execution and launcher fix in plan progress

Author: rschlaefli

project/2026-07-23-pr-5109-assistant-production-readiness-plan.md

@@ -233,3 +233,7 @@ Architecture note before scaling tool count: lecturer READ tools talk directly t
 - 2026-07-24: **Finish gates run.** Fresh verification re-confirmed: `check:all` 25/25, chat `test:run` 124/124, frontend-manage tsx assert 3/3, Playwright `Y-manage-assistant` 7/7. `$security-review` (Opus, range `b58112c23^..d20348b11`): no high-confidence vulnerabilities — postMessage sends target validated concrete origins (never `*` except the content-free ready ping), receivers validate `event.origin`+`event.source`, the create-question payload rides a server-signed 15-min JWT re-verified at confirm (`sub` bound to userId), untrusted tool output is zod-`safeParse`d before render, no secret/env leakage in new client code; one deferred defense-in-depth item — chat caches the parent origin from any framing parent (proper fix = env-driven manage-origin allowlist + `frame-ancestors` CSP; pre-existing embed surface, untouched by this change set). Independent branch review (agy, gemini-3.6-flash-high, git-show-only on `d20348b11`): 3 findings — F1 (validate cached parent origin against an allowlist) DEFERRED as that same defense-in-depth item; F2 (`'*'` on the ready ping) REJECTED (parent origin unknown at ping time, payload content-free, parent validates origin+source on receipt); F3 (derive `ManageProposalPreview` initial `useSingleStudentResponse` type from `element.elementType` instead of hardcoded `FreeText`) REJECTED (mirrors 5 sibling preview components; `useSingleStudentResponse` corrects it in a mount effect; E2E confirms correct SC/MC/FREE_TEXT render). `$thermo-nuclear-code-quality-review` (Opus, rubric-delegated): CHANGES-REQUESTED on Finding #1 only — the `[300,1000,2500]` retry burst + ack protocol (`nextMessageIdRef`/`ackedMessageIdRef`/`MANAGE_CONTEXT_ACK_MESSAGE_TYPE`) in `ManageAssistantWidget.tsx` is now redundant belt-and-suspenders since Slice 1 added the deterministic ready-ping resend (the code comment itself notes the burst "can fully elapse before a slow-hydrating iframe is able to receive anything"). DEFERRED with rationale: the remedy removes a defensive retry net from a cross-origin handshake with documented hydration fragility, the retry+ack are coupled (all-or-nothing), and the headless Playwright suite always wins the ready-ping race so it cannot exercise the tail cases the net defends — a release-risk change not taken unilaterally in the finish gate; booked as a PR follow-up and surfaced to Roland for veto. Findings #2 (extract `ThreadWelcomeCapabilities` out of the pre-existing 1382-line `thread.tsx`) and #4 (shared postMessage-protocol constant module) DEFERRED as nice-to-have. Finding #3 (client sanitizer `Number.isFinite(id)` drifted from server `z.number().int().positive()`) FIXED — `manageElementCreatedMessage.ts` now rejects fractional/non-positive ids, assert test green (commit `bc4bf4b53`). Next: PR #5109 body update, then housekeeping (`docker rm -f pw-pg-fwd`).
 - 2026-07-24: **SonarCloud gate resolved.** The PR quality gate failed on "D Security Rating on New Code"; the SonarCloud issues API (`componentKeys=uzh-bf_klicker-uzh&pullRequest=5109&types=VULNERABILITY`) returned a single CRITICAL: `useEmbeddedManageContext.ts:75` posted the readiness ping with a `'*'` targetOrigin (javascript:S2819). This effectively reverses the agy-F2 disposition on SonarCloud's demand. Fix (commit `8edb24744`, `fix(chat)`): the Manage parent now forwards its own origin as a `parentOrigin` query param (`manageAssistantConfig.ts` + `ManageAssistantWidget.tsx` `assistantUrl` memo, param always present since the iframe is client-only); the child (`useEmbeddedManageContext.ts`) parses it through a strict `parseOrigin` guard (bare well-formed origin only, never `'*'`/a full URL) and targets the ping there, skipping the proactive ping entirely when the param is absent. No regression: the parent's existing `[300,1000,2500]` retry burst still delivers the context in the skip case. Verification — new `manageAssistantConfig.test.ts` case (parent emits the encoded param) 3/3; chat tsc 0 errors; chat `test:run` 124/124; pre-commit `check:all` 25/25 + pre-push `build` 23/23 green. Runtime (agent-browser, standalone embed load): child reads the concrete `parentOrigin` from the URL, and a postMessage with that exact origin targets the concrete Manage origin with no `'*'` (a `pm-spy` init-script captured `{type:'klicker:manage-context-ready', targetOrigin:'https://manage.klicker.…localhost'}`). Full live cross-origin handshake was not exercisable here — the worktree's `auth` app (port 3010) is crashed pre-existing (502 since the first health check, unrelated to this change set) so delegated login is blocked, and a standalone chat load does not hydrate React to auto-fire the effect; risk covered by the unit test + retry-burst fallback + code inspection. **Re-scan confirmed:** after CI re-analyzed head `f79bf4e10`, the SonarCloud quality gate is `OK` with 0 open vulnerabilities (all conditions OK); the CI `SonarCloud` job and the `SonarCloud Code Analysis` status check both pass (a stale `SonarCloud` check-run from a prior head still shows red in `gh pr checks` but is superseded). PR #5109 body updated to head `f79bf4e10`. Next: none — plan slices 1-8 + all finish gates complete; PR left non-draft (pre-existing) with no merge instruction.
 - 2026-07-24: **Final branch review + forward roadmap.** Independent Opus audit of the full slice range vs this plan: verdict CLEAN — every claimed deliverable present at HEAD, no drift, no debris introduced by the range, and all five deferred items (thermo #1/#2/#4, agy F1, proposal-token replay) confirmed genuinely still open. One real gap found OUTSIDE the audit's static scope, via CI history: `Y-manage-assistant.spec.ts` had never passed on any completed CI run (deterministic fail on `d20348b11` and `bf32b231d`; cancelled/superseded elsewhere) while passing 7/7 locally — the workflow's build step set every `APP_ORIGIN_*` except `APP_ORIGIN_CHAT` (which `apps/frontend-manage/.env.test` maps to `NEXT_PUBLIC_CHAT_URL`) and never set `NEXT_PUBLIC_MANAGE_ASSISTANT_ENABLED`, so the widget compiled out of the CI manage bundle and all six UI tests timed out on their first click. Fixed in `d0046c581` (`ci(playwright)`): both vars wired into the build step, `APP_ORIGIN_CHAT` added to `turbo.json` `globalEnv` next to its siblings (cache-keyed; the flag was already listed). Host-side incident during the commit: the `turbo.json` edit invalidated all turbo caches and exposed a poisoned `packages/util/dist` (tsbuildinfo-only, zero emitted files — stale incremental marker restored from cache); direct `pnpm --filter @klicker-uzh/util build` fixed it, unrelated to the change; `check` 25/25 green after. CI proof of the spec now passing on head `d0046c581` pending at the time of this entry. Forward roadmap added as the `Future Work (post-PR roadmap)` section above (two Opus investigations: prioritized hardening P1-P9 with the notable corrections that prod/stg `frame-ancestors` CSP already ships and that the manage chat route skips Langfuse telemetry; plus a 14-candidate, 3-tier lecturer-MCP extension map with the read-path-authZ-convergence note).
+
+- 2026-07-24: **Future-work P1-P8 executed + review-thread batch.** Roland approved working the roadmap with Opus subagents, explicitly excluding P9 i18n (separate chat design workstream — also parks the de.ts "Assistant" CTA review thread 3342859759). Six-agent fleet over disjoint file clusters, every diff personally reviewed before commit: `6107df65f` (build: `@klicker-uzh/prisma#build` now declares `src/prisma/client/**` as turbo outputs — CI `check-types` failed on cache-restored builds missing the generated client; proven locally via mv-away + cache-restore, confirmed green in CI), `bb3d8f714` (P1 telemetry toggle `CHAT_ENABLE_AI_TELEMETRY`; P2 in-memory sliding-window rate limiters, 30/5min chat + 10/5min confirm, 429+Retry-After, per-pod caveat documented; P4 replay protection — mcp-lecturer signs a `randomUUID` `jti`, confirm claims it once via in-memory TTL map, reuse → 409, no-jti tokens pass for rollout compat; OPENAI_API_KEY fail-fast; `APP_ORIGIN_API` throws in production instead of localhost fallback; `@/src/...` aliases — CodeRabbit's literal `@/services/...` suggestions were wrong for the tsconfig), `c087d2e11` (server-side `sanitizeAsPath` via zod transform + Template/Instance/Quiz ID prompt lines + manage-side `MAX_ASPATH_LENGTH` clamp parity + stale-context clear on failed sanitization), `aa700873e` (dialog keyboard-modal: focus-into-panel, Tab trap, aria-modal; new-tab link uses `embed:false` URL), `384c1ad3d` (P8 dismiss action with terminal collapsed state + pure `applyDismiss`/`isDismissible`; thread inline-margin cleanup; `ThreadWelcomeCapabilities` extracted = thermo #2), `527a34fcd` (P3: tampered-token 403, confirm 401, mid-stream error, malformed envelope E2E), `2a569dbba` (pwa chatbot error redirects locale-prefixed), `80cca1c8b` (P6 dev CSP: `ALLOWED_FRAME_ANCESTORS=https://*.localhost` — host wildcard covers namespaced worktree origins). Rebutted with evidence (no change): mcp-lecturer Dockerfile pnpm pattern (13/13 repo convention), workflow hardening (0/32 convention; repo-wide task chip spawned), CourseChatDrawer dist/ops import (459-file convention), formatToolName + query-key cap (already addressed). Task chips: missing frontend-pwa `/error` page; `packages/util` jwt `expiresIn: 0` bug.
+- 2026-07-24: **Launcher pointer-interception regression fixed (shards 2/4/7).** CI on `6107df65f`: check-types green, `Y-manage-assistant` passed in CI for the first time — but the now-compiled floating launcher broke three other specs, each `<button data-cy="manage-assistant-open" class="... fixed ... z-30 ..."> intercepts pointer events` on an end-of-page bottom-right control (`groupActivity-save-submission-grading`, `live-quiz-template-submit`, `submit-create-answer-collection`) — a real UX bug for lecturers, not a test artifact. Fix `533a657f8` took three verified iterations driven by an agent-browser rect-intersection harness against this worktree's own stack: (1) container `pb-20 md:pb-24` → ZERO effect — browsers do not extend a scroll container's scrollable overflow area by its own end padding (every overlap case measured exactly `viewport − 61px` footer, padding swallowed); (2) in-flow end-spacer in Layout → still clipped on pages whose root wrapper is `h-full w-full`: the fixed 100%-height box with visible overflow positions following flow content under the overflowing region; (3) final: Layout end-spacer (`h-20 md:h-24`, gated on the assistant flag) + page-root wrappers (AnswerCollections/Chatbots/MediaLibrary/UserGroupsManagement) switched `h-full` → `min-h-full shrink-0` — `shrink-0` is load-bearing: without it the overflowing column-flex container compresses a `min-h-full` item straight back to 100% (explicit min-height discards the content-based automatic minimum). Final measurements: 93px clearance in both prior worst cases (1280×800 16-entry form, 1280×577), wrapper `offsetHeight === scrollHeight` exactly; cancel-click works; course list unaffected. Ops: this worktree's auth app (3010) was crashed since stack boot — respawned in-container by cloning the live turbo process env from `/proc/<pid>/environ` (keeps the workspace-namespaced `APP_ORIGIN_AUTH`/`NEXTAUTH_URL`); delegated login verified after.
+- 2026-07-24: **P5+P7 handshake simplification + stream-error UI (`53e1473e9`).** Ack protocol (`klicker:manage-context-ack`, `nextMessageIdRef`/`ackedMessageIdRef`) and the `[300,1000,2500]`ms retry burst removed on BOTH sides — the ready→resend handshake plus send-on-load and send-on-change is the whole protocol now (resolves the deferred thermo #1 with the E2E safety net it demanded). Message-type literals centralized in `packages/types/src/manageAssistant.ts` (thermo #4; `@klicker-uzh/types` already a dependency of both apps, zero lockfile churn). thread.tsx wires `MessagePrimitive.Error` → muted inline `AssistantMessageError` note (`data-cy="chat-assistant-message-error"`, raw SDK error text deliberately not shown) — closes the silent-stream-failure product gap found during P3; subagent verified against installed `@assistant-ui/react@0.12.10` + `@assistant-ui/core` source AND an empirical throwaway vitest run of the real `Chat`+`DefaultChatTransport` against the exact mock SSE bodies: mid-stream error attaches `{type:'incomplete', reason:'error'}` to the partial message, malformed envelope synthesizes an empty assistant message via `createErrorAssistantMessage` — one wiring covers both. Y-manage-assistant now 13 tests: + dismiss flow (confirm-request spy asserts zero calls), + slow-hydration (script-fetch delay pushes hydration past the parent's load-time send; course-dashboard suggestions still arrive via handshake alone), + strengthened error asserts (both error tests assert the visible note; corrected a factually wrong pre-existing `toHaveCount(0)` assertion for the malformed case). Verified: chat check + 140/140, manage check + 3/3 assert tests, types build/check, playwright tsc + `--list` 13. Next: consolidated agy review of `64da087f5..53e1473e9`, push, thread replies (22 of 23; de.ts parked), PR body refresh, CI watch.