Skip to content

Commit eacb26e

Browse files
committed
docs(project): record slice 8 finish-gate closure
Security review clean (one deferred defense-in-depth origin-allowlist item), independent agy branch review dispositioned (F2/F3 rejected with evidence, F1 deferred), thermo-nuclear maintainability gate CHANGES-REQUESTED on the redundant context-delivery retry burst (deferred with release-risk rationale, booked as a follow-up), its nice-to-have findings deferred, and the id-bound sanitizer drift fixed.
1 parent bc4bf4b commit eacb26e

1 file changed

Lines changed: 1 addition & 0 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,3 +200,4 @@ agy (Gemini 3.6 Flash High, sandbox/plan mode) reviewed commit `b58112c23`. Veri
200200
- 2026-07-24: Slice 6 reviews closed. agy (git-show-only on `1eb519fec`): clean, no findings — v2 constructor correct with guards preserved, 2.9.0/0.220.0 are release twins satisfying all @langfuse/otel peers, lockfile changes strictly confined to the apps/chat importer (backend-docker byte-identical), syncpack group correctly scoped (exempts backend-docker, still binds future consumers), and the explicit exporter-trace-otlp-http dep confirmed required (without it pnpm binds backend-docker's v1 0.53.0 exporter to @langfuse/otel). Simplification lens was folded into the agy pass — the non-lockfile diff is ~20 lines, no separate subagent warranted. Next: Slice 8.
201201
- 2026-07-24: **Slice 8 E2E done.** New `playwright/tests/Y-manage-assistant.spec.ts` (7 tests) + `playwright/util/manageAssistant.ts` drive the REAL frontend-manage page and its embedded cross-origin chat iframe. Lecturer authed via the shared `next-auth.session-token` cookie set with `COOKIE_DOMAIN` scoped to the namespaced host so it reaches the same-site `api.*` subdomain the client `self` query hits. Only POST /api/manage/chat and POST /api/manage/proposals/confirm are mocked. Coverage: plain text bubble; signed-proposal tool call → auto-expanded card with the real StudentElement preview (`instance-question-content`/`sc-0-answer-option-N`, scoped past the collapsible "Show raw JSON"); confirm → success state + `klicker:manage-element-created` postMessage crossing the iframe boundary + design-system toast on the parent; welcome capability explainer; per-surface suggestions (question-pool vs course-dashboard); a pure envelope-shape guard. Three protocol facts were empirically required for the stock AI SDK v5 `DefaultChatTransport` (via `AssistantChatTransport`), proven with an instrumented diagnostic (mock hit + rendered bubble): SSE events separated by a blank line (`\n\n`), the `x-vercel-ai-ui-message-stream: v1` marker header, and browser-CONTEXT-level `route()` (request originates in an out-of-process cross-origin iframe). The student chatbot's lenient custom parser needs none of these (intentional, documented in comments). Composer input needs a native-setter + input event with a retry-until-enabled loop (assistant-ui's controlled textarea ignores pressSequentially/fill in the iframe). Verified: 7/7 pass repeatedly (single-worker, host Playwright vs the worktree's own devrouter stack, global-setup reseeds the namespaced DB over a socat bridge). Checks: playwright tsc clean, prettier clean, `check:all` 25/25 green. Commit `a79ac3160`. Next: Slice 8 finish gates.
202202
- 2026-07-24: Slice 8 reviews closed. Simplification (Opus, on `a79ac3160`): 3 findings — accepted removing the dead `chatUrl` export + unused `URL_CHAT` import and extracting the shared start/step/finish envelope into `stepStreamBody`; declined hoisting the per-surface mock into `beforeEach` (keeps tests self-contained; LIFO-route caveat). agy (gemini-3.6-flash-high, git-show-only on `a79ac3160`): 6 findings, all rejected/deferred after cross-checking against the passing suite — `data: [DONE]` IS emitted by the real AI SDK v5 stream (grep of `ai` dist) so the mock is faithful; `delta` (not `textDelta`) is the correct text-delta key (empirically renders); `sc-0-answer-option-0` is the hardcoded `elementIx={0}` in ManageProposalPreview, not a DB id; the `[data-cy=...]` frameLocator is required (frameLocator takes a string selector; testIdAttribute is `data-cy`); error/rejection-path coverage is out of this slice's happy-path scope; the co-located envelope-shape guard is a cheap intentional check. agy did not run the tests (stated in its own Q3). A one-off course-dashboard flake (parent→iframe context-handshake race after nav) was hardened with a 15s visibility timeout on its suggestion assertions; re-verified 7/7. Commit `c174536c9`. Next: finish gates (security-review, thermo-nuclear, PR body).
203+
- 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`).

0 commit comments

Comments
 (0)