Skip to content

fix(mcp): expose native goal state in thread read and list - #18513

Closed
ddonathan wants to merge 2 commits into
pingdotgg:mainfrom
ddonathan:fix/mcp-native-goal-state
Closed

ddonathan wants to merge 2 commits into
pingdotgg:mainfrom
ddonathan:fix/mcp-native-goal-state

Conversation

@ddonathan

Copy link
Copy Markdown

Problem

T3 displays native /goal state, but t3_thread_read and t3_thread_list omit it. MCP consumers see execution status without the active provider conversation's objective and disposition, so they cannot distinguish an ended turn from an unfinished native goal.

Change

Expose the active provider thread's native goal separately from execution status. Thread reads return the full reported goal; thread lists bound the objective to 1,000 characters and evaluator feedback to 2,000, with goalTextTruncated indicating clipping. A thread without an active native goal returns null. The new response fields are optional for compatibility with older servers.

Tool descriptions and MCP usage documentation describe the new fields and how to recover full goal text from a bounded list result.

Scope and approval

This is a focused correction to existing thread-state reporting for the established native /goal capability added in #15592. It exposes already-persisted state through the existing read/list operations. It does not create goals, resume them, change execution policy, add controls, or alter provider selection. I believe it fits the focused obvious-bug exception; there is no claim of prior maintainer approval for a new feature.

Verification

Based on current upstream main at d514a8c617:

  • vp test run apps/server/src/mcp/OrchestratorMcpService.activity.test.ts: all 10 tests passed. Added cases verify full read versus bounded list output, missing/previous provider-conversation goals, and response-schema serialization.
  • Scoped server TypeScript and targeted lint passed.
  • node apps/server/scripts/cli.ts build-exe --target linux-x64 --verbose: executable build passed with Node 26.8.2.

Synthetic service fixtures exercise the reporting and wire contracts. No new native goal was created in a live user conversation for this PR. No UI changes or browser verification.

Model: GPT-6.1-Sol through the Codex harness in T3 Code; independent review by Claude Opus 5.5 through T3 Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 12, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 12, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at a90af6f

Macroscope's review found this PR approvable — This is a focused, additive MCP reporting fix that surfaces already-persisted native goal state without changing execution, provider selection, defaults, or existing response requirements. List output is bounded and covered by targeted tests, while the contract remains backward-compatible.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

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

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 12, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 12, 2026 05:13

Dismissing prior approval to re-evaluate a90af6f

@coderabbitai

coderabbitai Bot commented Oct 12, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 66103e03-b534-4dc5-ac94-f44f539f3e1a

📥 Commits

Reviewing files that changed from the base of the PR and between 6f6197c and a90af6f.


📒 Files selected for processing (5)
  • apps/server/src/mcp/OrchestratorMcpService.activity.test.ts
  • apps/server/src/mcp/OrchestratorMcpService.ts
  • apps/server/src/mcp/toolkits/orchestrator/tools.ts
  • docs/orchestration-v2/orchestrator-mcp-server.md
  • packages/contracts/src/orchestratorMcp.ts

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



📝 Walkthrough

Walkthrough

MCP thread read and list results now expose provider-reported goals. Thread reads return the full goal, while thread lists bound goal text and indicate when truncation occurs. Tool descriptions and documentation describe the fields and older-server compatibility.

Changes

Provider goal exposure

Layer / File(s) Summary
Expose provider goals in thread results
packages/contracts/src/orchestratorMcp.ts, apps/server/src/mcp/OrchestratorMcpService.ts, apps/server/src/mcp/OrchestratorMcpService.activity.test.ts, apps/server/src/mcp/toolkits/orchestrator/tools.ts, docs/orchestration-v2/orchestrator-mcp-server.md
Thread result contracts and service output now include provider goals. Thread reads return the full goal; thread lists limit objective text to 1,000 characters and last-check text to 2,000, with a flag indicating truncation. Tests cover supported statuses, cleared and omitted goals, and idle thread results. Tool descriptions and documentation describe the fields and older-server compatibility.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge


Merge Risk | ⚪ Minimal · up to a90af

Merge Risk: ⚪ Minimal · up to a90af

Thread results expose provider goals as intended, and no actionable merge-blocking issue remains after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to a90af

The change exposes additional goal text through existing thread-reading permissions without changing execution authority or thread access scope. No security vulnerability was established. Full-text detail responses and unvalidated production access boundaries leave limited residual uncertainty.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — Authorized tool callers receive additional provider-derived text within the pre-existing access scope: thread reads can target threads across the environment, while lists use the selected project. Environment-wide read scope predates this PR; it is not a newly introduced project-isolation bypass.

Trust Boundaries and Controls

  • observed — The tools retain their existing capability-checked read routing and caller/target resolution. Goal projection occurs after thread selection, rather than introducing a separate provider-session lookup or a new authorization path.

Resilience and Maintainability Implications

  • observed — List projection bounds the newly exposed text without modifying the shared goal object. Detail projection deliberately has no goal-text clipping in this code; the supplied evidence does not establish an end-to-end response-size limit.

Hardening Proposals

  • proposed — MCP consumers should treat objective and evaluator feedback as untrusted reported content, not as authorization or autonomous execution instructions. This is a consumer-side hardening proposal, not a verified injection finding.

Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: exposing native goal state in MCP thread read and list operations.
Description check Passed The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains compatibility, scope limits, tests, build verification, and unverified live-conversation b…
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.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • 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.

@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Thanks for identifying the gap in MCP goal visibility. Adding native goal fields and bounded summaries expands what the thread tools expose rather than correcting their existing execution-status result, so this needs prior approval. The PR explicitly states that no prior maintainer approval exists. Please preserve the use case in an Ideas discussion and obtain approval for the response contract and scope before requesting reconsideration.

@maria-rcks maria-rcks closed this Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:S 10-29 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.

3 participants