From 4ffebdd5be7df1f0ce054ba4c5907bf525366efc Mon Sep 17 00:00:00 2001 From: Jesse_Chen Date: Mon, 20 Jul 2026 14:44:24 +0800 Subject: [PATCH] fix: close chat deletion and transport errors --- .superpowers/sdd/task-1-report.md | 81 +++++-------------- .../src/lib/birth-time-client-transport.ts | 9 ++- ...ete_and_dynamic_candidate_confirmation.sql | 7 ++ .../tests/birth-time-client-transport.test.ts | 31 +++++++ .../chat-session-delete-contract.test.ts | 13 +++ 5 files changed, 78 insertions(+), 63 deletions(-) create mode 100644 frontend/supabase/migrations/20260720000000_chat_delete_and_dynamic_candidate_confirmation.sql create mode 100644 frontend/tests/birth-time-client-transport.test.ts create mode 100644 frontend/tests/chat-session-delete-contract.test.ts diff --git a/.superpowers/sdd/task-1-report.md b/.superpowers/sdd/task-1-report.md index f4f99f45..f779a417 100644 --- a/.superpowers/sdd/task-1-report.md +++ b/.superpowers/sdd/task-1-report.md @@ -1,75 +1,34 @@ -# Task 1 — Dynamic Choice Contracts and Stop Policy +# Task 1 Report: Chat deletion and browser transport errors ## Implementation -- Added browser-safe dynamic choice and time-range Zod schemas. Public question parsing is strict and rejects hidden partition fields. -- Added internal-only dynamic choice contracts, persisted/private question schemas, candidate-difference packet schemas, and an explicit public projection helper. -- Added pure deterministic stop policy with the specified precedence and a material-change calculation for candidate range, representative time, and two-point margin changes. -- Added separate `DynamicNextAction` and `DynamicJourneyProgress` schemas, preserving the legacy guided-v1 `NextAction` and `JourneyProgress` parser path. -- Kept the internal contract module dependency-free as resolved by the user. A source-contract test scans components, hooks, client transports, and response schemas to prohibit imports of the private module. -- Dynamic IDs are opaque nonempty server-issued strings, rather than being overconstrained to UUIDs. +- Added the owner-only `chat_sessions` DELETE policy and authenticated DELETE grant. +- Classified both native `SyntaxError` and WebKit `DOMException` values named `SyntaxError` as JSON parse/lost-response errors. +- Non-OK responses with malformed JSON now return `payload: null`; retry logic recognizes the WebKit error form. -## Files changed +## Files -- `frontend/src/lib/birth-time-dynamic-choice.ts` -- `frontend/src/lib/birth-time-dynamic-choice-internal.ts` -- `frontend/src/lib/birth-time-dynamic-stop-policy.ts` -- `frontend/src/lib/birth-time-journey-turn-protocol.ts` -- `frontend/src/lib/birth-time-journey-turn.ts` -- `frontend/tests/birth-time-dynamic-choice.test.ts` -- `frontend/tests/birth-time-dynamic-stop-policy.test.ts` +- `frontend/supabase/migrations/20260720000000_chat_delete_and_dynamic_candidate_confirmation.sql` +- `frontend/tests/chat-session-delete-contract.test.ts` +- `frontend/tests/birth-time-client-transport.test.ts` +- `frontend/src/lib/birth-time-client-transport.ts` -## RED +## TDD evidence -1. `/Users/jesse/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin/node --test tests/birth-time-dynamic-choice.test.ts` - - Failed as expected before the public contract existed: `ERR_MODULE_NOT_FOUND` for `birth-time-dynamic-choice.ts`. -2. `/Users/jesse/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin/node --test tests/birth-time-dynamic-stop-policy.test.ts` - - Failed as expected before the policy existed: `ERR_MODULE_NOT_FOUND` for `birth-time-dynamic-stop-policy.ts`. -3. After the boundary resolution, the dynamic choice test failed as expected while the obsolete `server-only` marker remained: `ERR_MODULE_NOT_FOUND: Cannot find package 'server-only'`. -4. The opaque-ID regression initially failed because the first implementation required UUIDs. +- RED: `node --import ./frontend/node_modules/tsx/dist/loader.mjs --test frontend/tests/chat-session-delete-contract.test.ts frontend/tests/birth-time-client-transport.test.ts` failed as expected: the migration file was absent and `DOMException("SyntaxError")` escaped; the native non-JSON case already passed. +- GREEN: the same command passed all 3 tests after the minimal implementation. -## GREEN +## Verification -1. `/Users/jesse/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin/node --test tests/birth-time-dynamic-choice.test.ts tests/birth-time-dynamic-stop-policy.test.ts tests/birth-time-journey-turn.test.ts` - - `14` passed, `0` failed. -2. `/Users/jesse/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin/node --test tests/birth-time-*.test.ts` - - `194` passed, `0` failed, duration `1449ms`. -3. `git diff --check` - - Passed with no whitespace errors. +- Focused suite: 3 passed, 0 failed. +- Full frontend suite: `node --import ./frontend/node_modules/tsx/dist/loader.mjs --test frontend/tests/*.test.ts` completed with 461 passed and 1 failed (462 total). +- The sole failure is the known baseline in `frontend/tests/health-deployment.test.ts`: its expected `DEPLOY_GIT_SHA` expression differs from the existing deployment workflow. It is outside Task 1 scope. +- Self-review: inspected the migration against existing owner-scoped RLS patterns, reviewed the four-file diff, and ran `git diff --check` successfully. -## Self-review +## Commit -- Public choices are strict, require 2–4 primary options plus exactly one unknown and one unmatched option, reject duplicate IDs, cap labels at 80 characters, and reject private fields. -- Persisted primary choices require nonempty partitions and finite score maps. Unknown/unmatched choices require both private fields to be `null`. -- The public projection parses through the public schema, so partition IDs and candidate scores cannot cross the browser boundary. -- Stop ordering is high confidence, effective-answer safety cap, plateau, no information gain, repeated partition, then continue. Non-effective answers retain the prior plateau count. -- Legacy schemas and turn behavior remain unchanged; v2 schemas use distinct dynamic names and are re-exported from the turn module. -- All created/modified source files are within the 250 pure-LOC threshold (largest: `birth-time-journey-turn.ts`, 229 lines; new internal contract, 208 lines). +- `ba8ece0 fix: close chat deletion and transport errors` ## Concerns -- Full `tsc --noEmit --incremental false` remains blocked by an unrelated existing error in `frontend/tests/profile-persistence.test.ts:7`: the project targets ES2017 while that test uses an ES2018 regular-expression flag. None of the Task 1 files produced a TypeScript error. -- The supplied no-excuse checker could not run because it is outside the frontend dependency tree and cannot resolve its own `typescript` package. The focused runtime suite, full birth-time suite, diff check, and manual forbidden-pattern scan completed successfully. - -## Review fixes - -- `DynamicStopInput.result` is now nullable, so a dynamic flow can finish before its first score. It also carries the explicit `forcedReason` union: `user_finished`, `generation_unavailable`, or `null`. -- Forced terminal reasons now win over every score-derived condition. A null result preserves the current plateau count instead of attempting score comparison. -- Added and re-exported `dynamicJourneyTurnStateSchema` / `DynamicJourneyTurnState`. The schema is strict and explicitly requires `journeyProtocol: "dynamic-choice-v2"`, a nonnegative turn version, a dynamic action, dynamic progress, and the existing permissions shape. The legacy `journeyTurnStateSchema` is unchanged. -- Added regressions for both forced terminal reasons, their high-confidence precedence, the dynamic discriminator, and rejection of a valid legacy action under the v2 schema. - -### Review RED - -`/Users/jesse/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin/node --test tests/birth-time-dynamic-choice.test.ts tests/birth-time-dynamic-stop-policy.test.ts` - -- Failed before implementation because `dynamicJourneyTurnStateSchema` was not exported. -- Existing stop policy threw on `result: null` and returned `high_confidence` instead of the forced `user_finished` reason. - -### Review GREEN - -1. `/Users/jesse/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin/node --test tests/birth-time-dynamic-choice.test.ts tests/birth-time-dynamic-stop-policy.test.ts tests/birth-time-journey-turn.test.ts` - - `16` passed, `0` failed. -2. `/Users/jesse/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin/node --test tests/birth-time-*.test.ts` - - `196` passed, `0` failed, duration `1472ms`. -3. `/Users/jesse/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin/node ./node_modules/typescript/bin/tsc --noEmit --incremental false` - - Still reports only the existing `tests/profile-persistence.test.ts:7` ES2018-regexp/ES2017-target incompatibility; no Task 1 diagnostic was emitted. +- The repository pre-work gate remains blocked by its documented host Python 3.9/fragment-scan baseline; it did not affect this frontend-only task. diff --git a/frontend/src/lib/birth-time-client-transport.ts b/frontend/src/lib/birth-time-client-transport.ts index 9365fd3c..eff9369c 100644 --- a/frontend/src/lib/birth-time-client-transport.ts +++ b/frontend/src/lib/birth-time-client-transport.ts @@ -15,9 +15,14 @@ function isAbort(error: unknown, signal?: AbortSignal): boolean { || (error instanceof DOMException && error.name === "AbortError"); } +function isJsonSyntaxError(error: unknown): boolean { + return error instanceof SyntaxError + || (error instanceof DOMException && error.name === "SyntaxError"); +} + function isLostResponse(error: unknown, signal?: AbortSignal): boolean { return !isAbort(error, signal) - && (error instanceof TypeError || error instanceof SyntaxError); + && (error instanceof TypeError || isJsonSyntaxError(error)); } async function postOnce(input: JsonPostInput): Promise { @@ -30,7 +35,7 @@ async function postOnce(input: JsonPostInput): Promise { try { return { response, payload: await response.json() }; } catch (error) { - if (!response.ok && isLostResponse(error, input.signal)) { + if (!response.ok && isJsonSyntaxError(error)) { return { response, payload: null }; } throw error; diff --git a/frontend/supabase/migrations/20260720000000_chat_delete_and_dynamic_candidate_confirmation.sql b/frontend/supabase/migrations/20260720000000_chat_delete_and_dynamic_candidate_confirmation.sql new file mode 100644 index 00000000..3287e94f --- /dev/null +++ b/frontend/supabase/migrations/20260720000000_chat_delete_and_dynamic_candidate_confirmation.sql @@ -0,0 +1,7 @@ +begin; +drop policy if exists chat_sessions_delete_own on public.chat_sessions; +create policy chat_sessions_delete_own + on public.chat_sessions for delete to authenticated + using ((select auth.uid()) = user_id); +grant delete on table public.chat_sessions to authenticated; +commit; diff --git a/frontend/tests/birth-time-client-transport.test.ts b/frontend/tests/birth-time-client-transport.test.ts new file mode 100644 index 00000000..c07e5d83 --- /dev/null +++ b/frontend/tests/birth-time-client-transport.test.ts @@ -0,0 +1,31 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { postJson } from "../src/lib/birth-time-client-transport.ts"; + +test("non-json 502 returns a null payload instead of leaking WebKit syntax text", async () => { + const original = globalThis.fetch; + globalThis.fetch = async () => new Response("bad gateway", { status: 502 }); + try { + const result = await postJson({ url: "/x", body: "{}", retryLostResponse: false }); + assert.equal(result.response.status, 502); + assert.equal(result.payload, null); + } finally { + globalThis.fetch = original; + } +}); + +test("DOMException SyntaxError is classified as a lost response", async () => { + const original = globalThis.fetch; + let attempts = 0; + globalThis.fetch = async () => { + attempts += 1; + if (attempts === 1) throw new DOMException("pattern", "SyntaxError"); + return Response.json({ ok: true }); + }; + try { + const result = await postJson({ url: "/x", body: "{}", retryLostResponse: true }); + assert.deepEqual(result.payload, { ok: true }); + } finally { + globalThis.fetch = original; + } +}); diff --git a/frontend/tests/chat-session-delete-contract.test.ts b/frontend/tests/chat-session-delete-contract.test.ts new file mode 100644 index 00000000..ffaed5ac --- /dev/null +++ b/frontend/tests/chat-session-delete-contract.test.ts @@ -0,0 +1,13 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import test from "node:test"; + +const sql = readFileSync(new URL( + "../supabase/migrations/20260720000000_chat_delete_and_dynamic_candidate_confirmation.sql", + import.meta.url, +), "utf8"); + +test("chat sessions expose owner-only delete", () => { + assert.match(sql, /create policy chat_sessions_delete_own[\s\S]*for delete[\s\S]*auth\.uid\(\).*user_id/i); + assert.match(sql, /grant delete on table public\.chat_sessions to authenticated/i); +});