Skip to content

feat(sync): bounded tail-first thread sync (v2) with media externalization and transport negotiation#3849

Open
snipemanmike wants to merge 1 commit into
pingdotgg:mainfrom
snipemanmike:pr/thread-sync-v2
Open

feat(sync): bounded tail-first thread sync (v2) with media externalization and transport negotiation#3849
snipemanmike wants to merge 1 commit into
pingdotgg:mainfrom
snipemanmike:pr/thread-sync-v2

Conversation

@snipemanmike

@snipemanmike snipemanmike commented Jul 10, 2026

Copy link
Copy Markdown

Problem

Opening a large thread still transfers the entire thread body — every message, activity, and inline base64 blob since the thread began. #3719 moved the snapshot off the socket (great!), but the payload itself is still unbounded. Measured on a physical Android device over Tailscale:

  • One media-heavy thread's snapshot was ~25 MB on the wire; gzip only buys ~1.6× on inline base64 (it's already-compressed data).
  • The legacy full-thread client cache reached 268 MB for one thread; parsing it OOM-killed the mobile app on every open (Failed to allocate a 268501000 byte allocation).
  • Full-thread re-sends on reconnect were 21,528 KB per reconnect; with a sequence cursor they drop to 36–54 KB (~400×) — same idea as afterSequence, which this builds on.

With this change the same media-heavy thread renders in seconds over 5G, and live send/receive works normally on threads of any size.

What this adds

Everything is capability-gated via the environment descriptor, so old/new client-server pairs interoperate; the existing flows are untouched when v2 isn't negotiated.

  • subscribeThreadV2 — a small thread head (metadata, latest turn, session, active proposed plan, pending approval/input requests, counts) plus the last 32 messages / 128 compact activities under a 512 KiB inline budget, emitted as ≤256 KiB chunks with keepalives between them, then live events strictly after the snapshot watermark. Catch-ups are epoch-checked and watermark-bounded; a bounded (2,048) live buffer emits an explicit resync-required instead of queueing unboundedly behind a slow cellular consumer.
  • getThreadHistoryPage — keyset pagination for older messages/activities (scroll-up on mobile), guarded by a history epoch so pages can never mix pre/post-thread.reverted history.
  • Activity payload externalization — base64 data-URLs and oversized payloads are stored as content-addressed attachments served by the existing /api/assets route; the sync stream carries references and mobile renders tap-to-load placeholders. (Message attachments already work this way — activity payloads were the unbounded route.)
  • Transport negotiation — the descriptor advertises rpcTransports and threadSyncVersions (with decode defaults so new clients read old descriptors). /ws stays byte-for-byte plain JSON; an optional gzip transport lives at /ws-compressed behind an RpcCompressionCodec reference that defaults to null (only opted-in platforms provide one). Cached-token reuse treats the negotiation probe as best-effort so offline reconnects keep working.
  • Client windowed state — a distinct WindowedOrchestrationThread (never passed through APIs typed as a complete OrchestrationThread), atomic snapshot staging committed together with its cursor, resyncs that restart the subscription with a logged reason, and thread caches that store either a legacy detail snapshot or a v2 window (older records evict as cache misses).

Mobile-specific finding worth knowing

Hermes on current retail Android devices lacks Array.prototype.toSorted — it fails as a silent fiber defect rather than a visible error. This PR avoids array-by-copy methods in mobile-bound code; there's an existing toReversed in apps/mobile/src/features/keyboard/hardwareKeyboardCommands.ts that may be affected too.

Tests

Contract decode/wire round-trips, transactional tail + keyset page queries with revert invalidation, a wire-round-trip → staging integration test, windowed merge/revert reducers, mobile feed windowing, and an opt-in production-scale replay suite (T3_THREAD_SYNC_REALDATA_DB=path/to/state.sqlite) that pushes real thread data through the full pipeline.

Happy to split this into smaller PRs (externalization / windowed sync / negotiation) if you'd prefer — and to rework the v2 tail as an extension of the #3719 HTTP snapshot endpoint instead of a WS RPC if that fits your direction better.

🤖 Generated with Claude Code


Note

High Risk
Large cross-cutting change to orchestration sync, WebSocket RPC transport, and persistence/cache formats; incorrect staging, epoch, or catch-up logic could cause silent hangs, stale history, or broken reconnects on production-scale threads.

Overview
This PR replaces full-thread downloads with a bounded tail-first sync (v2) when the environment descriptor advertises threadSyncVersions: [1, 2]. The server exposes subscribeThreadV2 (chunked snapshot start/chunk/complete, optional incremental catch-up, then live events with resync-required on overflow/revert) and getThreadHistoryPage (keyset paging guarded by a revert history epoch). ThreadSyncQuery reads bounded tails and pages from SQLite; ThreadSyncWire trims/chunks snapshots and externalizes oversized activity payloads to attachment references.

Transport negotiation adds descriptor rpcTransports (/ws plain JSON vs /ws-compressed gzip-json), RpcCompressionCodec (pako on mobile/web, zlib on server), and client logic to pick transport/sync version and rewrite the WebSocket path. Durable subscriptions can pass fresh sinceSequence on reconnect for delta catch-up.

Clients persist thread cache v3: legacy detail snapshots or WindowedOrchestrationThread, with stale v2 records evicted on load. Mobile gains scroll-up loadOlder, tap-to-load message/work-log media with timeouts, and feed logic so v2 messages and activities paginate independently.

Risk note: thread snapshot cache schema bumps to v3; older cached thread bodies are dropped and force a resync.

Reviewed by Cursor Bugbot for commit 7a9e6e0. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add bounded tail-first v2 thread sync with gzip transport negotiation and media externalization

  • Introduces a v2 thread sync protocol (subscribeThreadV2) that streams a bounded tail window (32 messages / 128 activities) to clients, with loadOlder/hasOlder pagination via a new getThreadHistoryPage RPC.
  • Adds transport negotiation: server advertises /ws (JSON) and /ws-compressed (gzip-JSON) endpoints; clients select the best transport based on codec availability using pako (web/mobile) or node:zlib (server).
  • Implements ThreadSyncQueryLive on the server to query projection tables for thread tails and paginated history, backed by a new per-thread readThreadFromSequence stream on the event store.
  • Adds windowedThread.ts on the client with WindowedOrchestrationThreadState, converters (fromWindowSnapshot/toWindowSnapshot), and mergeWindowHistoryPage for history merging.
  • Mobile and web thread feeds gain upward-scroll pagination and lazy-load asset placeholders (tap-to-load) for message attachments and activity payload assets.
  • Risk: storage schema version bumped from 2 to 3 for both web and mobile caches; existing v2 snapshots will fail decode and be evicted, requiring a full re-sync on first load after upgrade.

Macroscope summarized 7a9e6e0.

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 153d72da-acae-4afe-bc37-403fd436f3c3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added size:XXL 1,000+ changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Jul 10, 2026
Comment thread apps/mobile/src/lib/runtime.ts
Comment thread apps/mobile/src/features/threads/ThreadFeed.tsx Outdated
Comment thread .npmrc Outdated
Comment thread apps/server/src/ws.ts
Comment thread apps/server/src/ws.ts
updatedAt: head.updatedAt,
archivedAt: head.archivedAt,
deletedAt: head.deletedAt,
messages: window.messages,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High state/windowedThread.ts:84

fromWindowSnapshot copies window.messages directly into state without stripping the ["Older content omitted from the synced window."] marker from truncated entries. When a streaming assistant message arrives via thread.message-sent, upsertMessage appends the new chunk onto the truncated entry.text, producing corrupted content with the omission marker embedded in the middle and permanently missing bytes. Consider replacing the marker with an empty string (or a sentinel the reducer recognizes) for messages that are still streaming so appended chunks concatenate onto the real prefix rather than the marker.

Suggested change
messages: window.messages,
messages: window.messages.map((message) =>
message.text === '["Older content omitted from the synced window."]' && message.streaming
? { ...message, text: '' }
: message
),
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/windowedThread.ts around line 84:

`fromWindowSnapshot` copies `window.messages` directly into state without stripping the `["Older content omitted from the synced window."]` marker from truncated entries. When a streaming assistant message arrives via `thread.message-sent`, `upsertMessage` appends the new chunk onto the truncated `entry.text`, producing corrupted content with the omission marker embedded in the middle and permanently missing bytes. Consider replacing the marker with an empty string (or a sentinel the reducer recognizes) for messages that are still streaming so appended chunks concatenate onto the real prefix rather than the marker.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium

return decodeStoredThreadSnapshot(raw).pipe(

Bumping StoredThreadSnapshot.schemaVersion from 2 to 3 makes every existing web thread-cache entry written as version 2 fail to decode. The loadThread path catches the decode error and returns a cold cache, but it never removes the stale record, so every subsequent open of that thread re-decodes the same incompatible entry and hits the error path again until the thread is eventually resaved. Consider deleting the stored value when decoding fails so the incompatible entry is evicted on first access.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/connection/storage.ts around line 545:

Bumping `StoredThreadSnapshot.schemaVersion` from `2` to `3` makes every existing web thread-cache entry written as version `2` fail to decode. The `loadThread` path catches the decode error and returns a cold cache, but it never removes the stale record, so every subsequent open of that thread re-decodes the same incompatible entry and hits the error path again until the thread is eventually resaved. Consider deleting the stored value when decoding fails so the incompatible entry is evicted on first access.

Comment thread packages/client-runtime/src/environment/transport.ts
Comment thread apps/mobile/src/lib/threadActivity.ts
Comment thread apps/mobile/src/features/threads/ThreadFeed.tsx
Comment thread packages/client-runtime/src/state/windowedThread.ts Outdated
Comment thread apps/server/src/ws.ts
Comment on lines +2151 to +2155
const nodeGzipCodec: CompressionCodec = {
compressSync: (b) => zlib.gzipSync(b, { level: 4 }),
decompressSync: (b) => zlib.gunzipSync(b),
threshold: 1024,
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High src/ws.ts:2151

nodeGzipCodec.decompressSync calls zlib.gunzipSync without maxOutputLength, so any authenticated client on /ws-compressed can send a small gzip-compressed binary frame that decompresses to a buffer large enough to block the event loop or crash the process via OOM before Node's default buffer.kMaxLength limit kicks in. Consider passing maxOutputLength to gunzipSync with a sane cap sized to the largest legitimate message.

Suggested change
const nodeGzipCodec: CompressionCodec = {
compressSync: (b) => zlib.gzipSync(b, { level: 4 }),
decompressSync: (b) => zlib.gunzipSync(b),
threshold: 1024,
};
const nodeGzipCodec: CompressionCodec = {
compressSync: (b) => zlib.gzipSync(b, { level: 4 }),
decompressSync: (b) => zlib.gunzipSync(b, { maxOutputLength: 1 << 20 }),
threshold: 1024,
};
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/ws.ts around lines 2151-2155:

`nodeGzipCodec.decompressSync` calls `zlib.gunzipSync` without `maxOutputLength`, so any authenticated client on `/ws-compressed` can send a small gzip-compressed binary frame that decompresses to a buffer large enough to block the event loop or crash the process via OOM before Node's default `buffer.kMaxLength` limit kicks in. Consider passing `maxOutputLength` to `gunzipSync` with a sane cap sized to the largest legitimate message.

case "project.created":
case "project.meta-updated":
case "project.deleted":
case "thread.created":

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium state/windowedThread.ts:417

The thread.turn-diff-completed event falls through to { kind: "unchanged" } in applyWindowedThreadEvent, so in v2 mode every live checkpoint update is silently dropped. The caller treats unchanged as a no-op and advances the sequence, so the client never records the new checkpoint or the latest-turn settlement that the legacy reducer performs for this event. Consider handling thread.turn-diff-completed to update checkpoints and latestTurn (or returning { kind: "resync" } if the windowed state cannot reconstruct the checkpoint incrementally).

🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/windowedThread.ts around line 417:

The `thread.turn-diff-completed` event falls through to `{ kind: "unchanged" }` in `applyWindowedThreadEvent`, so in v2 mode every live checkpoint update is silently dropped. The caller treats `unchanged` as a no-op and advances the sequence, so the client never records the new checkpoint or the latest-turn settlement that the legacy reducer performs for this event. Consider handling `thread.turn-diff-completed` to update `checkpoints` and `latestTurn` (or returning `{ kind: "resync" }` if the windowed state cannot reconstruct the checkpoint incrementally).

Comment thread packages/client-runtime/src/state/threads.ts Outdated
SELECT activity_id FROM latest_user_input WHERE kind = 'user-input.requested'
)
)
ORDER BY activity.created_at ASC, activity.activity_id ASC

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium Layers/ThreadSyncQuery.ts:339

getTail serializes every unresolved approval and pending user-input activity for the thread into head.pendingRequests with no limit on the number returned. listPendingRequests has no LIMIT clause, so a thread with many outstanding requests produces an unbounded pendingRequests array in the snapshot — defeating the size cap this PR introduces and potentially recreating the large-payload problem it avoids for messages and activities. Consider adding a LIMIT to listPendingRequests (e.g. matching the activity tail limit) so the pending-requests portion of the snapshot is also bounded.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ThreadSyncQuery.ts around line 339:

`getTail` serializes every unresolved approval and pending user-input activity for the thread into `head.pendingRequests` with no limit on the number returned. `listPendingRequests` has no `LIMIT` clause, so a thread with many outstanding requests produces an unbounded `pendingRequests` array in the snapshot — defeating the size cap this PR introduces and potentially recreating the large-payload problem it avoids for messages and activities. Consider adding a `LIMIT` to `listPendingRequests` (e.g. matching the activity tail limit) so the pending-requests portion of the snapshot is also bounded.

@macroscopeapp

macroscopeapp Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

14 blocking correctness issues found. This PR introduces a new thread sync protocol (v2) with compression, media externalization, and pagination across server and all clients. Multiple high-severity issues have been identified including potential DoS vulnerability, data corruption in streaming messages, incorrect activity ordering, and broken asset resolution, alongside numerous medium-severity bugs affecting core sync behavior.

You can customize Macroscope's approvability policy. Learn more.

…ation and transport negotiation

Opening a large thread still transfers the entire thread body — every
message, activity, and inline base64 blob since the thread began. On
cellular this never completes for media-heavy threads, and parsing the
resulting payload can OOM-kill the mobile app outright (observed: a
268MB thread snapshot). pingdotgg#3719 moved the snapshot off the socket; this
change bounds what is transferred at all.

Measured on a physical Android device over Tailscale:
- One media-heavy thread's snapshot was ~25MB on the wire; gzip only
  buys 1.6x on inline base64. With the bounded tail it renders in
  seconds over 5G.
- Sequence-cursor reconnects: 21,528KB re-sent per reconnect before,
  36-54KB after (~400x).

What this adds, all capability-gated so old peers are unaffected:

- subscribeThreadV2: a small thread head (metadata, latest turn,
  session, active plan, pending approval/input requests, counts) plus
  the last 32 messages / 128 compact activities under a 512KiB inline
  budget, emitted as <=256KiB chunks with keepalives between them, then
  live events after the snapshot watermark. Catch-up replays are
  epoch-checked and bounded to the watermark; a bounded (2048) live
  buffer signals an explicit resync instead of queueing unboundedly.
- getThreadHistoryPage: keyset pagination for older messages and
  activities (scroll-up on mobile), invalidated across thread.reverted
  via a history epoch so pages never mix pre/post-revert history.
- Activity payload externalization: base64 data-URLs and oversized
  payloads are stored as content-addressed attachments served by the
  existing /api/assets route; the wire carries references and mobile
  renders tap-to-load placeholders. (Message attachments already work
  this way — activity payloads were the unbounded route.)
- Transport negotiation: the environment descriptor advertises
  rpcTransports and threadSyncVersions (with decode defaults, so new
  clients read old descriptors); /ws stays byte-for-byte plain JSON and
  an optional gzip transport lives at /ws-compressed behind an optional
  RpcCompressionCodec (defaults to null; only opted-in platforms
  provide one). Cached-token reuse treats the negotiation probe as
  best-effort so offline reconnects keep working.
- Client: a distinct windowed thread state (never passed through APIs
  typed as a complete OrchestrationThread), atomic snapshot staging
  committed together with its cursor, resyncs that restart the
  subscription with a logged reason, and thread caches that store
  either a legacy detail snapshot or a v2 window (older records evict
  as cache misses). Array-by-copy methods are avoided in mobile-bound
  code: Hermes on current retail devices lacks toSorted and fails as a
  silent fiber defect.

Tests: v2 contract decode/round-trip, transactional tail and keyset
page queries with revert invalidation, wire round-trip -> staging
integration, windowed merge/revert reducers, mobile feed windowing, and
an opt-in real-data replay suite (T3_THREAD_SYNC_REALDATA_DB) that
replays production-scale threads through the pipeline.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@snipemanmike

Copy link
Copy Markdown
Author

Updated (force-pushed) with all review findings addressed — thanks Bugbot/CodeRabbit, several were genuinely good catches:

  • Transport negotiation: clients without a compression codec no longer select the gzip endpoint; applyRpcTransport preserves reverse-proxy path prefixes.
  • loadOlder race: the history page is merged into the post-RPC thread state and dropped entirely if the history epoch moved (revert) while in flight.
  • Pagination: gated on upward scroll movement so short (underflowing) threads don't page eagerly; feed no longer hides activities older than the oldest loaded message (they paginate independently in v2).
  • Media UX: tap-to-load now shows the inline thumbnail; stalled/failed loads fall back to the tappable placeholder after 10s; Linking.openURL rejections consumed.
  • Windowed reducer parity: terminal turn states (interrupted/error) are preserved; latestTurn only rebinds for the same turn.
  • Streaming truncation: oversized streaming messages keep their tail (leading marker) so client-side delta appends stay correct; surrogate-safe slicing.
  • Caches: web thread-cache records from older schema versions are evicted on first decode failure (mobile already did).
  • Deleted threads: a cached client reconnecting after a hard delete now receives the terminal event via a paged log replay (instead of retrying "not found" forever); archived threads keep serving a tail.
  • Payload externalization: base64 decoding now requires a declared non-text MIME type + strict charset + round-trip; anything else is externalized as verbatim UTF-8 (no way to corrupt plain text).
  • Dead isDeltaSafeThreadEvent removed (the windowed reducer only resyncs on thread.reverted, which the guard already checks); stray machine-local .npmrc dropped from the diff.

Three findings kept as-is with reasoning in the inline replies (pako gzip autodetect, watermark buffering, whole-payload asset references).

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 7a9e6e0. Configure here.

Comment thread apps/server/src/ws.ts
},
},
hasOlderMessages: snapshot.head.counts.messages > built.messages.length,
hasOlderActivities: snapshot.head.counts.activities > built.activities.length,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Older messages stuck without cursor

Medium Severity

The subscribeThreadV2 RPC's snapshot-complete item can incorrectly report hasOlderMessages as true while before.message is null. This occurs when the initial snapshot budget omits all messages. Consequently, getThreadHistoryPage cannot paginate to load older messages.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7a9e6e0. Configure here.

messages: window.messages,
activities: mergeActivities(window.activities, head.pendingRequests),
proposedPlans: head.activeProposedPlan === null ? [] : [head.activeProposedPlan],
checkpoints: [],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium state/windowedThread.ts:87

fromWindowSnapshot hard-codes checkpoints: [], so every v2/windowed thread loses all checkpoint summaries the moment it is loaded from a v2 snapshot or restored from cache. Downstream consumers reading thread.checkpoints directly see an empty array for all windowed threads, so checkpoint-based features disappear instead of showing server state. The WindowedOrchestrationThread type has no checkpoints field, so the data is never carried through. If checkpoints are intentionally excluded from the windowed sync payload, consider documenting that rationale; otherwise thread the server-provided checkpoints through fromWindowSnapshot (and toWindowSnapshot) so they survive round-trips.

🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/windowedThread.ts around line 87:

`fromWindowSnapshot` hard-codes `checkpoints: []`, so every v2/windowed thread loses all checkpoint summaries the moment it is loaded from a v2 snapshot or restored from cache. Downstream consumers reading `thread.checkpoints` directly see an empty array for all windowed threads, so checkpoint-based features disappear instead of showing server state. The `WindowedOrchestrationThread` type has no `checkpoints` field, so the data is never carried through. If checkpoints are intentionally excluded from the windowed sync payload, consider documenting that rationale; otherwise thread the server-provided checkpoints through `fromWindowSnapshot` (and `toWindowSnapshot`) so they survive round-trips.

Comment on lines +297 to +306
const listTailActivities = SqlSchema.findAll({
Request: ThreadLookup,
Result: ProjectionActivityDbRow,
execute: ({ threadId }) => sql`
SELECT activity_id AS "activityId", thread_id AS "threadId", turn_id AS "turnId", tone,
kind, summary, payload_json AS payload, sequence, created_at AS "createdAt"
FROM projection_thread_activities WHERE thread_id = ${threadId}
ORDER BY created_at DESC, activity_id DESC LIMIT ${THREAD_SYNC_TAIL_ACTIVITY_LIMIT}
`,
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High Layers/ThreadSyncQuery.ts:297

listTailActivities and listActivityPage order activities by created_at DESC, activity_id DESC instead of sequence DESC, activity_id DESC. When activities arrive with out-of-order createdAt values, these queries return them in timestamp order rather than event-sequence order, so clients receive a thread history that does not match the actual event sequence and paginate around the wrong cursor. Consider ordering by sequence DESC, activity_id DESC to match the existing projection snapshot behavior.

   const listTailActivities = SqlSchema.findAll({
     Request: ThreadLookup,
     Result: ProjectionActivityDbRow,
     execute: ({ threadId }) => sql`
       SELECT activity_id AS "activityId", thread_id AS "threadId", turn_id AS "turnId", tone,
         kind, summary, payload_json AS payload, sequence, created_at AS "createdAt"
       FROM projection_thread_activities WHERE thread_id = ${threadId}
-      ORDER BY created_at DESC, activity_id DESC LIMIT ${THREAD_SYNC_TAIL_ACTIVITY_LIMIT}
+      ORDER BY sequence DESC, activity_id DESC LIMIT ${THREAD_SYNC_TAIL_ACTIVITY_LIMIT}
     `,
   });
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ThreadSyncQuery.ts around lines 297-306:

`listTailActivities` and `listActivityPage` order activities by `created_at DESC, activity_id DESC` instead of `sequence DESC, activity_id DESC`. When activities arrive with out-of-order `createdAt` values, these queries return them in timestamp order rather than event-sequence order, so clients receive a thread history that does not match the actual event sequence and paginate around the wrong cursor. Consider ordering by `sequence DESC, activity_id DESC` to match the existing projection snapshot behavior.

case "resync-required":
yield* requestV2Resync(`server-resync-required (${item.reason})`);
return;
case "keepalive":

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium state/threads.ts:514

The keepalive handler in applyV2Item sets status to "live" unconditionally (except when already "deleted"), so a keepalive received before the first snapshot-complete transitions the thread to live while data is still Option.none() (or holds only stale cached data). subscribeThreadV2 is allowed to send keepalives between snapshot chunks, so on a cold open the UI sees live status before any v2 snapshot has been committed. Consider guarding the keepalive case to only set "live" when the thread already has data (e.g. Option.isSome(current.data)), leaving the status unchanged otherwise.

🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/threads.ts around line 514:

The `keepalive` handler in `applyV2Item` sets `status` to `"live"` unconditionally (except when already `"deleted"`), so a keepalive received before the first `snapshot-complete` transitions the thread to `live` while `data` is still `Option.none()` (or holds only stale cached data). `subscribeThreadV2` is allowed to send keepalives between snapshot chunks, so on a cold open the UI sees `live` status before any v2 snapshot has been committed. Consider guarding the `keepalive` case to only set `"live"` when the thread already has data (e.g. `Option.isSome(current.data)`), leaving the status unchanged otherwise.

Comment thread apps/server/src/ws.ts
import { failEnvironmentAuthInvalid, failEnvironmentInternal } from "./auth/http.ts";
import * as RelayClient from "@t3tools/shared/relayClient";
const isOrchestrationDispatchCommandError = Schema.is(OrchestrationDispatchCommandError);
const isOrchestrationGetSnapshotError = Schema.is(OrchestrationGetSnapshotError);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium src/ws.ts:133

Schema.is(OrchestrationGetSnapshotError) at line 133 returns false for OrchestrationGetSnapshotError instances that getThreadHistoryPage constructs with a plain object cause ({ requestedHistoryEpoch, currentHistoryEpoch }), because the schema declares cause as Schema.Defect() which does not validate plain objects. As a result, the Effect.mapError path wraps the original error in a generic OrchestrationGetSnapshotError, discarding the specific "history changed; reload the tail window" message and epoch details that clients need to trigger a targeted resync. Consider using a looser predicate (e.g., checking the _tag discriminant) or adjusting the schema so object-shaped causes pass validation.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/ws.ts around line 133:

`Schema.is(OrchestrationGetSnapshotError)` at line 133 returns `false` for `OrchestrationGetSnapshotError` instances that `getThreadHistoryPage` constructs with a plain object `cause` (`{ requestedHistoryEpoch, currentHistoryEpoch }`), because the schema declares `cause` as `Schema.Defect()` which does not validate plain objects. As a result, the `Effect.mapError` path wraps the original error in a generic `OrchestrationGetSnapshotError`, discarding the specific "history changed; reload the tail window" message and epoch details that clients need to trigger a targeted resync. Consider using a looser predicate (e.g., checking the `_tag` discriminant) or adjusting the schema so object-shaped causes pass validation.

? thread.latestTurn.assistantMessageId
: null,
}
: thread.latestTurn?.state === "running"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium state/windowedThread.ts:385

applyWindowedThreadEvent settles a running latestTurn to "completed" for every non-"error" thread.session-set event. When the session leaves running with status "interrupted" or "stopped", the turn is incorrectly reported as completed instead of interrupted. Consider mapping those statuses to "interrupted", matching the legacy reducer's behavior.

🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/windowedThread.ts around line 385:

`applyWindowedThreadEvent` settles a running `latestTurn` to `"completed"` for every non-`"error"` `thread.session-set` event. When the session leaves running with status `"interrupted"` or `"stopped"`, the turn is incorrectly reported as completed instead of interrupted. Consider mapping those statuses to `"interrupted"`, matching the legacy reducer's behavior.

Comment on lines +602 to +611
return !forceSnapshot &&
seq > 0 &&
Option.isSome(current.data) &&
isWindowedThread(current.data.value)
? {
threadId,
sinceSequence: seq,
historyEpoch: current.data.value.historyEpoch,
}
: { threadId };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium state/threads.ts:602

resolveV2SubscribeInput requires Option.isSome(current.data) before sending sinceSequence, so once the thread is deleted and data is Option.none(), every reconnect sends only { threadId } without sinceSequence. The server treats a missing thread without a cursor as not found instead of replaying the terminal delete, so the client enters an error/retry loop on each reconnect instead of staying deleted. Consider allowing sinceSequence to be sent when the status is "deleted" (e.g. checking seq > 0 && (Option.isNone(current.data) || isWindowedThread(current.data.value))), so the server replays the delete event instead of returning not found.

Suggested change
return !forceSnapshot &&
seq > 0 &&
Option.isSome(current.data) &&
isWindowedThread(current.data.value)
? {
threadId,
sinceSequence: seq,
historyEpoch: current.data.value.historyEpoch,
}
: { threadId };
return !forceSnapshot &&
seq > 0 &&
(Option.isNone(current.data) || isWindowedThread(current.data.value))
? {
threadId,
sinceSequence: seq,
historyEpoch: Option.isSome(current.data) && isWindowedThread(current.data.value)
? current.data.value.historyEpoch
: undefined,
}
: { threadId };
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/threads.ts around lines 602-611:

`resolveV2SubscribeInput` requires `Option.isSome(current.data)` before sending `sinceSequence`, so once the thread is deleted and `data` is `Option.none()`, every reconnect sends only `{ threadId }` without `sinceSequence`. The server treats a missing thread without a cursor as `not found` instead of replaying the terminal delete, so the client enters an error/retry loop on each reconnect instead of staying deleted. Consider allowing `sinceSequence` to be sent when the status is `"deleted"` (e.g. checking `seq > 0 && (Option.isNone(current.data) || isWindowedThread(current.data.value))`), so the server replays the delete event instead of returning `not found`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium

readonly onHeaderMaterialVisibilityChange?: (visible: boolean) => void;

The payloadAsset filter in ThreadWorkLog silently drops media-bearing rows: activities with status === "neutral" and toolLike === true are removed before rendering, so any attachment on an in-progress or informational tool activity is never shown and the user has no way to open it. Consider preserving rows that carry a payloadAsset regardless of their status, or excluding only toolLike && status === "neutral" rows when they have no asset.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around line 139:

The `payloadAsset` filter in `ThreadWorkLog` silently drops media-bearing rows: activities with `status === "neutral"` and `toolLike === true` are removed before rendering, so any attachment on an in-progress or informational tool activity is never shown and the user has no way to open it. Consider preserving rows that carry a `payloadAsset` regardless of their status, or excluding only `toolLike && status === "neutral"` rows when they have no asset.

// on the wire, impossible to corrupt.
return {
bytes: Buffer.from(data, "utf8"),
mediaType: "text/plain; charset=utf-8",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High orchestration/ThreadSyncWire.ts:109

externalizeActivityPayload externalizes large plain-text payloads as .txt files, but the attachment resolver only recognizes image extensions and .bin. Those asset references always 404 when the client requests them from /api/assets, silently making large text payloads unloadable. Consider using .bin for the plain-text fallback in externalizableData, or extend the attachment resolver to recognize .txt.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/ThreadSyncWire.ts around line 109:

`externalizeActivityPayload` externalizes large plain-text payloads as `.txt` files, but the attachment resolver only recognizes image extensions and `.bin`. Those asset references always 404 when the client requests them from `/api/assets`, silently making large text payloads unloadable. Consider using `.bin` for the plain-text fallback in `externalizableData`, or extend the attachment resolver to recognize `.txt`.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant