Skip to content

fix(platform): stop shared chats rendering as blocked and accept ?new=1 - #3204

Merged
larryro merged 4 commits into
mainfrom
fix/chat-shared-snapshot-and-new-flag
Sep 4, 2026
Merged

fix(platform): stop shared chats rendering as blocked and accept ?new=1#3204
larryro merged 4 commits into
mainfrom
fix/chat-shared-snapshot-and-new-flag

Conversation

@larryro

@larryro larryro commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Two product fixes that were committed inside the docs PR #3199 (docs/align-ghost-feature-pages) by the screenshot rig, shipped here as their own reviewable change with regression tests. #3199 will drop them separately.

1. Shared chat snapshots rendered every message as a blocked, failed reply

Symptom. Opening a shared conversation link showed a "blocked by your organization's content policy" notice and a "Something went wrong" error on every message.

Root cause. getSharedThread (backend/domains/chat/threads.ts) passed the SQL null of blocked_reason / error straight into the view, and c.json serialized it as null. The message view tests !== undefined (message-item.tsx, message-toolbar.tsx), and the frontend contract types both fields as undefined | string. The live thread read (shim.ts) already normalizes the same two columns with ?? undefined; the snapshot read did not.

Fix. blockedReason: message.blockedReason ?? undefined, error: message.error ?? undefined; SharedThreadView types the two fields string | undefined. Still needed on main: origin/main line 853 still passes the raw value (#3182 touched the file but not this mapping); the cherry-pick auto-merged with no conflict.

Tests.

  • backend/domains/chat/threads.test.tsgetSharedThread fed one plain row (NULL flags) and one genuinely blocked row through the fake sql: the plain row's blockedReason/error are undefined and absent from the JSON wire form; the blocked row keeps 'content_policy' / 'upstream refused'. Red on main's threads.ts: expected null to be undefined (1 failed / 7 passed) → green with the fix (8/8).
  • backend/integration-check.ts — the share-links lane gains share links: snapshot carries no null blocked reason or error over the two NULL-flag rows it already seeds (fails on main by construction: the wire carries "blockedReason": null).

2. /chat?new=1 resumed the last thread instead of opening a fresh composer

Symptom. A hand-typed or bookmarked /dashboard/<org>/chat?new=1 silently resumed the caller's most recent thread; a message typed into what looked like a fresh composer landed in that old thread.

Root cause. TanStack Router's default search parser is JSON-based, so ?new=1 arrives as the number 1. The index route's validateSearch accepted only true | '1' | 'true' and dropped it. Verified against the installed router (1.168): on main, router.load() for /chat?new=1 answers 200 and every match — the layout's useSearch({ strict: false }) included — sees search = { new: 1 }, so the layout's startFresh check fails and it resumes. With the fix the validator returns { new: true }, the router answers with a 307 to the canonical /chat?new=true, and every match then reads the boolean the layout already checks.

Fix. Accept search.new === 1 alongside the boolean the in-app links send and the string forms a custom parser would deliver (app/routes/dashboard/$id/chat/index.tsx). Still needed on main: origin/main line 26 still lacks the number form; cherry-pick applied cleanly.

Tests.

  • app/routes/dashboard/$id/chat/index.search.test.ts — drives the route's real Route.options.validateSearch: true, 1, '1', 'true' all normalize to { new: true }; an absent param, false, 0, 2, 'false', '0', 'yes', null leave the flag absent; projectId passthrough pinned. Red on main's index.tsx: expected {} to deeply equal { new: true } (1 failed / 5 passed) → green with the fix (6/6, under both the test:ui config and the server project).

Gates observed (platform workspace, final tree)

  • bunx tsc --noEmit — exit 0
  • bunx oxlint --type-aware — exit 0
  • bunx oxfmt --check on the five touched files — clean
  • bunx knip (repo root) — exit 0 (one pre-existing config hint about cron-parser, unrelated)
  • vitest: threads.test.ts 8/8 (server); index.search.test.ts 6/6 (ui config) and 6/6 (server)
  • backend:integration on a fresh throwaway tale-db (54329) + MinIO (59001): [itest] 448/451 checks passed, no fatal, both share-links records PASS (incl. the new snapshot carries no null blocked reason or error — flags=absent/absent,absent/absent). The 3 FAILs are in lanes this change does not touch and are pre-existing on main: blob-ref authority … compose → 403 FORBIDDEN (want 403 attachment_not_owned) and outbound send lane … memberDoors=403/403/403 (want 404s) are the fix(platform): gate conversation writes and audit 2FA successes #3187 first-line gate vs. probe drift that test(platform): make a truncated itest run fail loudly #3198 corrects; webdav re-home is the known red. Not re-run on base (tally unambiguous).

No user-visible strings, env vars, or schema changed — no locale or migration impact.

@larryro
larryro marked this pull request as ready for review September 4, 2026 05:34
getSharedThread passed the SQL null of blocked_reason and error straight
through, while the message view tests `!== undefined` — so every message
in a shared chat carried a "blocked by your organization's content
policy" notice and a "Something went wrong" error. Normalize both to
undefined, the way the live thread read already does.
The router parses search params as JSON, so a hand-typed `?new=1`
arrives as the number 1, which the index route's validator dropped —
the layout then resumed the caller's last thread and a message typed
into the "fresh" composer landed there. Accept the number alongside the
boolean the in-app links send.
getSharedThread must map a NULL blocked_reason/error to an ABSENT key,
because the message view tests `!== undefined` — a SQL null rendered
every shared message as a blocked, failed reply. The unit test feeds
one plain and one genuinely blocked row through the fake sql and pins
both shapes (plus the JSON wire form); the harness's share-links lane
now also records that the two snapshot rows carry no null flags.

Red on main's threads.ts: "expected null to be undefined".
The router parses search params as JSON, so a hand-typed ?new=1 reaches
the index route's validator as the number 1. Pin every spelling that
means "fresh composer" (true, 1, '1', 'true') to the one value the layout
reads, and that an absent or no-ish param leaves the flag absent.

Red on main's index.tsx: "expected {} to deeply equal { new: true }".
@larryro
larryro force-pushed the fix/chat-shared-snapshot-and-new-flag branch from e3e6358 to 64d7bcc Compare September 4, 2026 05:47
@larryro
larryro merged commit 9337ff1 into main Sep 4, 2026
48 checks passed
@larryro
larryro deleted the fix/chat-shared-snapshot-and-new-flag branch September 4, 2026 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant