镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

feat(omp): add native RPC provider and session imports - #33065

Closed
klukacin wants to merge 2 commits into
pingdotgg:mainfrom
klukacin:feat/omp-native-rpc
Closed

klukacin wants to merge 2 commits into
pingdotgg:mainfrom
klukacin:feat/omp-native-rpc

Conversation

@klukacin

@klukacin klukacin commented Nov 8, 2026

Copy link
Copy Markdown

Problem

OMP users cannot select a native OMP provider or import their existing OMP conversations in T3 Code. This adds an opt-in native RPC provider on the current V2 architecture, targeting OMP 18.16.0 or newer.

Change

Adds native model/thinking and command/skill discovery, streaming tools and messages, interactive questions and permissions, T3 MCP tools, background-session settlement, compaction and usage. Session imports are idempotent snapshots; continuation and forks use T3-owned copies so original transcripts remain unchanged. Profile-aware discovery and install-aware version/update actions use the existing T3 service paths. Required shared contracts and client registrations are included; unrelated NetBird work is excluded.

Scope and approval

This revisits #11791 and #11973. #11791 declined bespoke OMP scope and its maintenance burden; #11973 was closed during the V2 rewrite with an invitation to reassess afterward. That invitation is not prior approval of this implementation. We recognize that explicit upstream direction approval is still missing and that draft status does not waive contribution requirements.

Our team is willing to maintain this integration and compatibility fixes as OMP and T3 evolve. This draft presents the concrete implementation for consideration; it does not claim upstream acceptance.

Verification

  • TMPDIR=/private/tmp vp run check:changed: passed all gates, including 168 tests, 8 compiler tests and affected typechecks for 41 packages. Canonical TMPDIR avoids an existing macOS /var versus /private/var scanner test issue, reproduced separately on the base revision.
  • Isolated installed OMP 18.16.0 probes verified native dialogs, MCP extension loading/tool inventory, fork boundaries and unchanged source hashes.
  • A real Luna turn through the T3 adapter read an isolated fixture, returned LIVE_OMP_OK 73, and completed normally.
  • Independent review identified and fixes cover native restore isolation, visible built-in slash output/config changes, profile-specific discovery, and native compaction mode arguments.

No full integrated web/desktop/mobile UI acceptance or screenshots are supplied yet. Actual OMP upgrade execution and continuity during a live upgrade are untested; no installed provider was updated. Child thread controls/approval forwarding, structured plan/TODO panels, custom terminal UI and live session synchronization are not supported. User documentation describes these limits.

Implemented using GPT-6-Astra through the Codex harness in T3 Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). labels Nov 8, 2026

@binjamshid binjamshid 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.

A thorough draft. The RPC framing, process-group teardown, and the launch-arg reservation list are careful. My comments are on the OMP-specific session-copy logic and on scope.

  1. Low: the session-copy prefix check assumes / separators (packages/provider-omp/src/server/adapter.ts:2565).
    registerThread builds ${host.paths.stateDir}/omp-sessions by string concatenation and compares with startsWith(${sessionDir}/). On Windows stateDir uses backslashes, so if OMP reports sessionFile in native form (I couldn't confirm this), the T3-owned copy will never match. Each resume would then copy the copy under a new sourceIdentity name. Using path.join and comparing normalized paths (or path.relative) avoids depending on OMP's path format. The fork path at :3418 builds its path the same way.

  2. Low: fork leaves its intermediate copy behind, and the "distinct file" check compares against the wrong file (adapter.ts:3418-3420, :3467).
    forkCopy exists only so OMP can resume from it without touching the source. After fork or new_session runs, OMP writes a new session file, and forkCopy stays in omp-sessions on success and on failure. Over time each fork leaves an orphan file. The check file === sourceFile also can't catch OMP returning the copy it resumed from, so compare against forkCopy too. Removing forkCopy in a finalizer would fix the leak.

  3. Maintenance: most of the adapter mirrors provider-pi. adapter.ts is about 3.8k lines, and a rough normalized diff against packages/provider-pi/src/server/adapter.ts differs in about 1.3k lines. The same goes for rpc.ts, usage.ts and mcpExtensionSource.ts. #11791 declined OMP mainly over maintenance cost. Making OMP a configuration or variant of the pi driver (binary, config root, session-dir rules, the RPC-UI dialog additions), or extracting the shared pieces into provider-core, would shrink this a lot and probably make the upstream case easier.

Process: as the description says, there is no maintainer sign-off on direction yet, and the PR bundles the provider, session import, usage and docs. Splitting import/usage into a follow-up after the provider question is settled may be easier to review.

@github-actions

Copy link
Copy Markdown
Contributor

This PR conflicts with main. Rebase it or merge main into it. If it still conflicts after 7 days with no activity, it will be closed automatically.

See the conflict window.

@github-actions github-actions Bot added the needs-rebase Conflicts with its base branch. Closes after 7 days with no activity. label Nov 12, 2026
@t3dotgg

t3dotgg commented Nov 12, 2026

Copy link
Copy Markdown
Member

Note

🤖 Claude Opus 5.5 responding on behalf of Theo

Closing. We are not taking new providers from outside PRs. Adding a provider is a product decision that needs maintainer approval before any code is written, and this PR has none. See prior approval.

If you want to make the case for this provider, open an Ideas discussion.

@t3dotgg t3dotgg closed this Nov 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase Conflicts with its base branch. Closes after 7 days with no activity. 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.

3 participants