Skip to content

Revert "feat(server): thread tools return a ready t3-thread link" - #24791

Open
juliusmarminge wants to merge 1 commit into
mainfrom
revert-24784-feat/thread-tools-return-link
Open

juliusmarminge wants to merge 1 commit into
mainfrom
revert-24784-feat/thread-tools-return-link

Conversation

@juliusmarminge

Copy link
Copy Markdown
Member

Reverts #24784

thread titles are not constant and should not be treated as such 😭 😭 😭 titles change and then the links will show stale data

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 25, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 43b5cf1

Macroscope's review found this PR approvable — This is a small, explicit revert of the ready-made thread-link feature, removing stale title-based link fields while preserving thread IDs and core MCP behavior. The contract and documentation updates are consistent with the revert and introduce no broader product or infrastructure change.

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

@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.9 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB −24 B (−0.5%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB −24 B (−2.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB −41 B (−0.2%) 29.3 KiB ✅
Claude Live turn messages 2 1 −1 (−50.0%) 8 ✅

Baseline: a96cf08 · PR result: 43b5cf1 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@coderabbitai

coderabbitai Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between a96cf08 and 43b5cf1.

Files selected for processing: 11
  • apps/server/src/mcp/OrchestratorMcpService.test.ts
  • apps/server/src/mcp/OrchestratorMcpService.ts
  • apps/server/src/mcp/toolkits/orchestrator/tools.ts
  • apps/server/src/mcp/toolkits/project/handlers.test.ts
  • apps/server/src/mcp/toolkits/project/handlers.ts
  • apps/server/src/mcp/toolkits/project/tools.ts
  • apps/server/src/mcp/toolkits/thread/handlers.ts
  • apps/server/src/mcp/toolkits/thread/tools.ts
  • docs/orchestration-v2/orchestrator-mcp-server.md
  • packages/contracts/src/orchestratorMcp.ts
  • packages/shared/src/threadLinks.test.ts
Files with no reviewable changes: 6
  • apps/server/src/mcp/toolkits/project/handlers.ts
  • packages/shared/src/threadLinks.test.ts
  • apps/server/src/mcp/toolkits/thread/tools.ts
  • apps/server/src/mcp/toolkits/project/handlers.test.ts
  • packages/contracts/src/orchestratorMcp.ts
  • apps/server/src/mcp/OrchestratorMcpService.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

Walkthrough

MCP thread responses no longer include generated link fields. Tool descriptions and documentation now instruct callers to construct thread links from returned thread IDs.

Changes: MCP thread link response changes

Layer / File(s) Summary
Remove generated links from MCP results
packages/contracts/src/orchestratorMcp.ts, apps/server/src/mcp/OrchestratorMcpService.ts, apps/server/src/mcp/toolkits/project/*, apps/server/src/mcp/toolkits/thread/*, apps/server/src/mcp/OrchestratorMcpService.test.ts, packages/shared/src/threadLinks.test.ts
The thread response contracts and handlers no longer include generated link fields. Thread search returns its result directly. Related tests no longer expect the removed fields.
Update thread link guidance
apps/server/src/mcp/toolkits/orchestrator/tools.ts, apps/server/src/mcp/toolkits/project/tools.ts, docs/orchestration-v2/orchestrator-mcp-server.md
Tool descriptions and documentation tell callers to construct thread links from the returned thread ID.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

Suggested labels: bugfix

Suggested reviewers: t3dotgg

Merge Risk: Minimal · up to 43b5c

The PR consistently replaces generated thread links with caller-constructed links without an established repository-side regression.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description explains the problem and identifies the reverted change, but it omits the required Problem, Change, Scope and approval, and Verification sections. Add the required section headings. Describe the expected behavior and reproduction context under Problem, explain the affected components under Change, provide the linked issue or maintainer approval and scope rationale under Scope and appr…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly identifies the primary change: reverting the feature that returned ready-to-use t3-thread links.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Resolution

Add the required section headings. Describe the expected behavior and reproduction context under Problem, explain the affected components under Change, provide the linked issue or maintainer approval and scope rationale under Scope and approval, and list focused tests or manual checks with observed results under Verification.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@chanatundev

Copy link
Copy Markdown

you are goated for reading the code and finding this. i really hope that this gets merged soon or my threads will start breaking cause i have a habit of renaming em lol. i havent installed the latest version

This branch has not been deployed

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

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants