Sign up
View stargazers13Sign up

feat(server): add bounded thread content search#8721

closed
t3code/codex-turn-mapping
agents/mcp-thread-search/search
openedAug 30, 2026byJJulius Marminge
View on GitHubTwo-way sync enabledPR last imported 4d ago

Problem

The existing thread search RPC is global, active-only, and cannot return stable anchors into the visible V2 timeline. MCP callers need project-scoped search without changing that established client wire behavior.

Change

Add an opt-in, bounded ProjectionSnapshotQuery.searchThreadContent path that searches durable titles plus visible user and assistant messages. The query:

  • scopes every result to one project and an optional thread
  • defaults to active threads with an explicit archived opt-in
  • excludes deleted projects and threads, streaming output, rolled-back runs, cancelled queued messages, and inherited-only fork content
  • treats V2 metadata as authoritative while retaining legacy-only threads
  • bounds title and message fragments in SQL before decoding
  • escapes literal %, _, and !, rejects NUL, and preserves valid Unicode and internal whitespace
  • returns live offset pagination and real source identities when available

The existing searchThreads RPC and cache shape are unchanged.

Behavior

Results are deterministic within each live query page. Legacy hits keep nullable anchors instead of inventing V2 identifiers. V2 title, archive, and deletion state cannot be shadowed by stale legacy rows.

Validation

  • vp test run apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.search.test.ts (3/3)
  • real in-memory SQLite coverage for V2 and legacy metadata authority, archived/deleted scope, literal wildcard and whitespace matching, Unicode bounds, queued visibility, and pagination
  • scoped t3, @t3tools/contracts, and @t3tools/shared typechecks
  • targeted lint, format, and git diff --check

Dependency

Bottom of native stack #8785. Based on t3code/codex-turn-mapping at 415ed0f73b97f1655b6282492f81d0b2bba3a9cc. The dependent MCP tool is #8722.

Implemented by GPT-5.6-Sol via Codex in T3 Code.

CcoderabbitaiBotAug 30, 2026
Synced from GitHub

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 Plus

Run ID: 518d8c0a-3725-4031-bd73-87965881274a

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

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.

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

apps/server/src/orchestration/Services/ProjectionSnapshotQuery.tslines 74–76Outdated · 5d66e72Open thread
……@@ -52,6 +59,60 @@ export interface ProjectionFullThreadDiffContext {
73+
74+export class ProjectionThreadContentSearchInputError extends Error {
75+ readonly _tag = "ProjectionThreadContentSearchInputError";
76+}
MmacroscopeappBoton lines 74–76Aug 30, 2026

Services/ProjectionSnapshotQuery.ts:76

This new service failure is a plain Error subclass with a hand-written _tag and an unstructured message as its only data. Service failures here should be declared with Schema.TaggedErrorClass and structured attributes, with message derived from them (see apps/server/src/orchestration/Errors.ts and apps/server/src/persistence/Errors.ts). Capturing which bound was violated (e.g. field plus the offending value/limit) also makes the failure diagnosable instead of a single fixed sentence.

diff
1-export class ProjectionThreadContentSearchInputError extends Error {
2- readonly _tag = "ProjectionThreadContentSearchInputError";
3-}
4+export class ProjectionThreadContentSearchInputError extends Schema.TaggedErrorClass<ProjectionThreadContentSearchInputError>()(
5+ "ProjectionThreadContentSearchInputError",
6+ {
7+ field: Schema.Literals(["query", "limit", "offset", "snippetChars"]),
8+ value: Schema.Number,
9+ min: Schema.Number,
10+ max: Schema.Number,
11+ },
12+) {
13+ override get message(): string {
14+ return `Thread content search ${this.field} ${this.value} is outside the supported bounds (${this.min}-${this.max}).`;
15+ }
16+}

This also needs import * as Schema from "effect/Schema"; in this module and an update to the construction site in apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts, where the bounds check currently builds the error with a single message string — validating each bound separately lets it report the failing field.

No longer relevant as of 0fc6b466c87620156b8213855dd7403a83597cd6

apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.tsline 953Outdated · 5d66e72Open thread
……@@ -882,6 +928,183 @@ const makeProjectionSnapshotQuery = Effect.gen(function* () {
950+ threads.title AS thread_title,
951+ threads.updated_at AS thread_updated_at,
952+ CASE WHEN threads.archived_at IS NULL THEN 0 ELSE 1 END AS archived
953+ FROM projection_threads AS threads
MmacroscopeappBoton line 953Aug 30, 2026

🟠 High Layers/ProjectionSnapshotQuery.ts:953

V2-only threads never produce searchable hits, and V2-imported threads remain visible as archived: false after V2 archival or deletion. thread_meta is sourced only from legacy projection_threads, yet the V2 candidate is inner-joined to it and its archived/deleted filters are never taken from orchestration_v2_projection_threads. Build the V2 candidate metadata from the V2 thread projection (and apply its visibility state), while retaining the legacy metadata path for legacy candidates.

✅ Resolved in c68767960ef6d5a336345119f771642f9e65b16d

Ggithub-actionsBotAug 30, 2026
Synced from GitHub

Thread transfer impact

⚠️ The latest CI run did not produce a thread transfer result for 298b869.

This comment will update automatically after the next completed run.

apps/server/src/orchestration/Services/ProjectionSnapshotQuery.tslines 76–78Outdated · 0d5b88cOpen thread
……@@ -52,6 +60,63 @@ export interface ProjectionFullThreadDiffContext {
75+
76+export class ProjectionThreadContentSearchInputError extends Schema.TaggedErrorClass<ProjectionThreadContentSearchInputError>()(
77+ "ProjectionThreadContentSearchInputError",
78+ { message: Schema.String },
MmacroscopeappBoton lines 76–78Aug 30, 2026

Services/ProjectionSnapshotQuery.ts:78

ProjectionThreadContentSearchInputError is now a Schema.TaggedErrorClass, but its only attribute is a free-form message, and the single call site passes one fixed sentence. Service failures here should carry structured attributes with message derived from them (see PersistenceSqlError/PersistenceDecodeError in apps/server/src/persistence/Errors.ts), so a caller can tell which bound was violated. Something like:

ts
1export class ProjectionThreadContentSearchInputError extends Schema.TaggedErrorClass<ProjectionThreadContentSearchInputError>()(
2 "ProjectionThreadContentSearchInputError",
3 {
4 field: Schema.Literals(["query", "limit", "offset", "snippetChars"]),
5 bound: Schema.Int,
6 actual: Schema.Int,
7 },
8) {
9 override get message(): string {
10 return `Thread content search ${this.field} is outside the supported bounds (bound ${this.bound}, actual ${this.actual}).`;
11 }
12}

Keep the attributes bounded/safe — report the query's character count rather than the raw query text — and construct the error in searchThreadContent with the specific field that failed instead of a single combined check.

No longer relevant as of 0fc6b466c87620156b8213855dd7403a83597cd6

CcursorBotreviewed · 1 inline threadAug 30, 2026
Synced from GitHub

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

Fix All in Cursor

Bugbot Autofix is ON, but a cloud agent failed to start.

Reviewed by Cursor Bugbot for commit 0d5b88c5b5c898c171adb3d74c2069c1e8f46a26. Configure here.

apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.tsline 1044Outdated · 0d5b88cOpen thread
……@@ -882,6 +930,186 @@ const makeProjectionSnapshotQuery = Effect.gen(function* () {
1041+ items.turn_item_id AS identity_key
1042+ FROM orchestration_v2_projection_turn_items AS items
1043+ INNER JOIN thread_meta AS meta
1044+ ON meta.thread_id = items.thread_id
CcursorBoton line 1044Aug 30, 2026

V2 threads omitted from search scope

High Severity

searchThreadContent builds thread_meta only from projection_threads, then inner-joins V2 turn items to that CTE. Durable V2 threads, titles, and archive/delete flags are written to orchestration_v2_projection_threads, which existing searchThreads already reads. Native V2 threads never enter scope, so their titles and message hits are dropped, and V2 archive/delete state is ignored.

Additional Locations (1)

Reviewed by Cursor Bugbot for commit 0d5b88c5b5c898c171adb3d74c2069c1e8f46a26. Configure here.

JJulius MarmingeAug 30, 2026

Fixed in 0fc6b466c87620156b8213855dd7403a83597cd6. The query now builds V2-authoritative metadata from orchestration_v2_projection_threads and uses legacy metadata only when no V2 row exists for that thread ID. This makes native V2 threads searchable, applies V2 archive/delete state to imported threads, and prevents a legacy row from reviving or shadowing its V2 counterpart. The real SQLite coverage includes native V2, imported V2, archived/deleted V2, and legitimate legacy-only threads.

MmacroscopeappBotAug 30, 2026
Synced from GitHub

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR adds a new production thread-content search capability with a substantial cross-generation SQL query, visibility rules, deduplication, pagination, and snippet processing. It is opt-in and well-tested without changing existing callers or product defaults, but the scope and runtime complexity warrant human review.

No code changes detected at 298b869. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.tslines 280–280Outdated · b57d3d8Open thread
……@@ -236,6 +272,18 @@ function buildSearchSnippet(text: string, query: string): string {
277+ readonly fragmentStart: number;
278+ readonly totalChars: number;
279+}): string {
280+ const normalizedFragment = input.fragment.replace(/\s+/g, " ").trim();
MmacroscopeappBoton lines 280–280Aug 30, 2026

🟡 Medium Layers/ProjectionSnapshotQuery.ts:280

buildBoundedSearchSnippet changes a literal match such as "alpha\tbeta" to "alpha beta", so consumers cannot find or highlight the query in the returned snippet. searchThreadContent matches the caller’s trimmed query without collapsing internal whitespace; preserve that whitespace by trimming only the fragment boundaries.

✅ Resolved in d22f88dbc8d801f1761a3006c5167ae94aa08a0b

JJulius MarmingeAug 30, 2026

Fixed in d22f88dbc8d801f1761a3006c5167ae94aa08a0b. Snippet construction now trims only fragment boundaries. The real SQLite query test searches a literal tab and verifies the returned snippet retains it.

JJulius MarmingeSep 7, 2026

Superseded as the active MCP proposal by #10561 in the rebuilt MCP stack, following the maintainer request to review thin service wrappers separately from service hardening.

This is not a full feature-parity replacement. This filters the global top results, so a project can underfill the requested limit. It is not exhaustive or paginated. Project-first SQL pagination and search-service correctness changes belong in a separate service PR.

Closing this mixed proposal without merging. The remote branch agents/mcp-thread-search/search and exact head 298b869dc63809571f15ec9f92eaf1740c7b53f7 are retained so the service changes can be extracted for separate review later.