forked from pingdotgg/t3code
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: consolidated provider architecture (PR #28 base + PR #27 cherry-picks) #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
ranvier2d2
merged 16 commits into
main
from
devin/1774740175-provider-arch-consolidated
Mar 29, 2026
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
777a220
feat: Codex harness-only cutover with T3CODE_CODEX_LEGACY flag
ranvier2d2 2117acc
feat: expand capability model with graduated CapabilityLevel fields
ranvier2d2 c222a97
feat: error taxonomy with category classification on provider errors
ranvier2d2 3f5d32d
feat: telemetry baseline with structured metrics for session lifecycle
ranvier2d2 d1aebef
feat: McpConfigService with reference + snapshot persistence model
ranvier2d2 d4197f1
feat: adapter MCP translators with per-provider config translation
ranvier2d2 6ae1544
feat: contract test suite + provider behaviour + onboarding playbook
ranvier2d2 93b4aca
Add provider capability and MCP runtime support
ranvier2d2 83ce718
fix: address Devin review — missing McpConfigServiceLive + log spam
ranvier2d2 eb9432b
Address Devin PR review feedback
ranvier2d2 b8af408
feat: cherry-pick PR #27 additions onto PR #28 base
devin-ai-integration[bot] 0fe0876
fix: preserve existing config.toml when appending MCP servers
devin-ai-integration[bot] f5824ed
style: fix formatting for failure_matrix.md and serverLayers.ts
devin-ai-integration[bot] a23c342
fix: clean generated MCP dir before recreating to prevent duplicate T…
devin-ai-integration[bot] 1ef9048
Merge a23c34209621ffab09363426e25fcdc4dce73fdf into c48e1c848879ad4a3…
ranvier2d2 34fc7d4
Resolve PR 29 CI quality follow-ups
ranvier2d2 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| # Provider Failure Matrix | ||
|
|
||
| Populated failure matrix mapping operations, providers, error types, and recovery behavior. | ||
|
|
||
| ## Error Categories | ||
|
|
||
| | Category | Description | Recovery Strategy | | ||
| | --------------- | --------------------------------------- | -------------------------------------- | | ||
| | `transient` | Temporary failure, may succeed on retry | Retry with exponential backoff (max 3) | | ||
| | `permanent` | Unrecoverable error | Fail immediately, surface to user | | ||
| | `configuration` | Misconfigured provider/environment | Re-resolve config, prompt user to fix | | ||
| | `unavailable` | Provider is down or unreachable | Degrade gracefully, suggest fallback | | ||
|
|
||
| ## Failure Matrix | ||
|
|
||
| | Operation | Provider | Error Class | Category | Current Behavior | Desired Behavior | | ||
| | ---------------------- | ------------------ | ------------------------------------- | --------------- | -------------------------------------- | -------------------------------------- | | ||
| | `startSession` | codex (harness) | `ProviderAdapterProcessError` | `configuration` | Error propagated to transport | Check binary exists before spawn | | ||
| | `startSession` | codex (harness) | `ProviderAdapterRequestError` | `transient` | Error propagated to transport | Retry spawn with backoff | | ||
| | `startSession` | claudeAgent | `ProviderAdapterProcessError` | `configuration` | Error propagated to transport | Validate API key before spawn | | ||
| | `startSession` | claudeAgent | `ProviderAdapterRequestError` | `transient` | Error propagated to transport | Retry with backoff | | ||
| | `startSession` | cursor (harness) | `ProviderAdapterProcessError` | `configuration` | Error propagated to transport | Check cursor binary path | | ||
| | `startSession` | opencode (harness) | `ProviderAdapterProcessError` | `configuration` | Error propagated to transport | Check opencode binary path | | ||
| | `startSession` | any | `ProviderValidationError` | `permanent` | Error propagated to transport | Correct (no change needed) | | ||
| | `startSession` | any | `ProviderUnsupportedError` | `configuration` | Error propagated to transport | Suggest enabling provider in settings | | ||
| | `sendTurn` | codex (harness) | `ProviderAdapterRequestError` | `transient` | Error propagated to transport | Auto-retry once, then surface | | ||
| | `sendTurn` | codex (harness) | `ProviderAdapterSessionNotFoundError` | `permanent` | Recovery via `recoverSessionForThread` | Correct (recovery already implemented) | | ||
| | `sendTurn` | claudeAgent | `ProviderAdapterRequestError` | `transient` | Error propagated to transport | Auto-retry once, then surface | | ||
| | `sendTurn` | claudeAgent | `ProviderAdapterSessionClosedError` | `permanent` | Error propagated to transport | Prompt user to start new session | | ||
| | `sendTurn` | any | `ProviderValidationError` | `permanent` | Error propagated to transport | Correct (validation is permanent) | | ||
| | `interruptTurn` | codex (harness) | `ProviderAdapterRequestError` | `transient` | Error propagated to transport | Best-effort interrupt, log failure | | ||
| | `interruptTurn` | claudeAgent | `ProviderAdapterRequestError` | `transient` | Error propagated to transport | Best-effort interrupt, log failure | | ||
| | `respondToRequest` | codex (harness) | `ProviderAdapterRequestError` | `transient` | Error propagated to transport | Retry once for approval responses | | ||
| | `respondToRequest` | claudeAgent | `ProviderAdapterRequestError` | `transient` | Error propagated to transport | Retry once for approval responses | | ||
| | `stopSession` | codex (harness) | `ProviderAdapterRequestError` | `transient` | Error propagated to transport | Force-stop on timeout | | ||
| | `stopSession` | claudeAgent | `ProviderAdapterProcessError` | `unavailable` | Error propagated to transport | Kill process, mark session closed | | ||
| | `rollbackConversation` | codex (harness) | `ProviderAdapterRequestError` | `transient` | Recovery via resume + rollback | Correct (already recovers) | | ||
| | `rollbackConversation` | claudeAgent | `ProviderAdapterRequestError` | `transient` | Error propagated to transport | Resume session first, then rollback | | ||
| | `listSessions` | any | (no errors expected) | - | Returns empty on failure | Correct | | ||
| | `getCapabilities` | any | `ProviderUnsupportedError` | `configuration` | Error propagated to transport | Return empty capabilities for unknown | | ||
|
|
||
| ## Error Classification by `_tag` | ||
|
|
||
| | Error `_tag` | Default Category | Notes | | ||
| | ------------------------------------------ | ---------------- | ----------------------------------------------------------------------------------- | | ||
| | `ProviderAdapterRequestError` | `transient` | Promoted to `permanent` if detail contains "not found", "unauthorized", "forbidden" | | ||
| | `ProviderAdapterProcessError` | `transient` | Promoted to `configuration` if ENOENT/permission; `unavailable` if crashed/signal | | ||
| | `ProviderAdapterValidationError` | `permanent` | Invalid input to adapter API | | ||
| | `ProviderAdapterSessionNotFoundError` | `permanent` | Session ID does not exist | | ||
| | `ProviderAdapterSessionClosedError` | `permanent` | Session exists but is closed | | ||
| | `ProviderValidationError` | `permanent` | Invalid input to ProviderService API | | ||
| | `ProviderUnsupportedError` | `configuration` | Provider not registered | | ||
| | `ProviderSessionNotFoundError` | `permanent` | Thread not found in session directory | | ||
| | `ProviderSessionDirectoryPersistenceError` | `transient` | SQLite/persistence layer failure | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,164 @@ | ||
| # Provider Onboarding Playbook | ||
|
|
||
| Guide for adding a new coding agent provider to T3 Code. | ||
|
|
||
| ## Architecture Overview | ||
|
|
||
| T3 Code uses a layered adapter architecture: | ||
|
|
||
| ```text | ||
| Transport (WebSocket/RPC) | ||
| | | ||
| ProviderService (cross-provider facade) | ||
| | | ||
| ProviderAdapterRegistry (adapter lookup) | ||
| | | ||
| ProviderAdapter (provider-specific runtime) | ||
| | | ||
| Provider CLI/SDK (codex, claude, cursor, opencode, ...) | ||
| ``` | ||
|
|
||
| **Two runtime paths**: | ||
|
|
||
| - **Node SDK adapters** (ClaudeAdapter, CodexAdapter) -- run in-process via JS/TS SDKs | ||
| - **Harness adapters** (HarnessClientAdapter) -- bridge to Elixir GenServer sessions via WebSocket | ||
|
|
||
| Most new providers will use the **harness path** (Elixir GenServer) unless they have a first-class Node.js SDK. | ||
|
|
||
| ## Step 1: Declare Provider Kind | ||
|
|
||
| Add the new provider to the `ProviderKind` schema in `packages/contracts/src/orchestration.ts`: | ||
|
|
||
| ```typescript | ||
| export const ProviderKind = Schema.Literal( | ||
| "codex", | ||
| "claudeAgent", | ||
| "cursor", | ||
| "opencode", | ||
| "YOUR_PROVIDER", | ||
| ); | ||
| ``` | ||
|
|
||
| ## Step 2: Choose Runtime Path | ||
|
|
||
| ### Path A: Elixir Harness (recommended for CLI-based providers) | ||
|
|
||
| 1. **Create session module**: `apps/harness/lib/harness/providers/your_session.ex` | ||
| - Implement `@behaviour Harness.Providers.ProviderBehaviour` | ||
| - Implement the SessionManager-facing callbacks: `start_link/1`, `wait_for_ready/2`, `send_turn/2`, `interrupt_turn/3`, `respond_to_approval/3`, `respond_to_user_input/3`, `read_thread/2`, and `rollback_thread/3` | ||
| - If your module exposes `stop/1`, document it as a provider-owned shutdown helper; supervisor shutdown is not dispatched through SessionManager | ||
| - Use `GenServer, restart: :temporary` | ||
| - Register via `{:via, Registry, {Harness.SessionRegistry, thread_id, "your_provider"}}` | ||
|
|
||
| 2. **Register in SessionManager**: `apps/harness/lib/harness/session_manager.ex` | ||
| - Add clause to `provider_module/1`: `defp provider_module("your_provider"), do: {:ok, YourSession}` | ||
|
|
||
| 3. **Declare capabilities**: Add entry in `HARNESS_PROVIDER_CAPABILITIES` in `apps/server/src/provider/Layers/HarnessClientAdapter.ts` | ||
|
|
||
| 4. **Register in serverLayers.ts**: Add the provider to the `HARNESS_PROVIDERS` array in `makeServerProviderLayer()` | ||
|
|
||
| ### Path B: Node SDK Adapter (for providers with JS/TS SDKs) | ||
|
|
||
| 1. **Create service tag**: `apps/server/src/provider/Services/YourAdapter.ts` | ||
| - Follow `ClaudeAdapter.ts` pattern | ||
| - Extend `ProviderAdapterShape<ProviderAdapterError>` | ||
|
|
||
| 2. **Create layer**: `apps/server/src/provider/Layers/YourAdapter.ts` | ||
| - Implement all methods of `ProviderAdapterShape` | ||
| - Include `translateMcpConfig` (return null if provider manages its own MCP) | ||
|
|
||
| 3. **Register in adapter registry**: Update `ProviderAdapterRegistryLive` or `makeServerProviderLayer()` | ||
|
|
||
| ## Step 3: Implement ProviderAdapterShape Methods | ||
|
|
||
| Every adapter must implement these methods (see `apps/server/src/provider/Services/ProviderAdapter.ts`): | ||
|
|
||
| | Method | Description | | ||
| | -------------------- | ---------------------------------------------- | | ||
| | `startSession` | Start a provider-backed session | | ||
| | `sendTurn` | Send a conversational turn | | ||
| | `interruptTurn` | Interrupt an active turn | | ||
| | `respondToRequest` | Respond to approval requests | | ||
| | `respondToUserInput` | Respond to user input requests | | ||
| | `stopSession` | Stop one session | | ||
| | `listSessions` | List active sessions | | ||
| | `hasSession` | Check session ownership | | ||
| | `readThread` | Read thread snapshot | | ||
| | `rollbackThread` | Roll back N turns | | ||
| | `stopAll` | Stop all sessions | | ||
| | `translateMcpConfig` | Translate MCP config to provider-native format | | ||
|
|
||
| ## Step 4: Declare Capabilities | ||
|
|
||
| Set `ProviderAdapterCapabilities` for your provider. In shared contracts this shape is represented by `ProviderCapabilities`, and the capability level fields use `ProviderCapabilityLevel`: | ||
|
|
||
| ```typescript | ||
| { | ||
| sessionModelSwitch: "in-session" | "restart-session" | "unsupported", | ||
| supportsUserInput: boolean, | ||
| supportsRollback: boolean, | ||
| supportsFileChangeApproval: boolean, | ||
| resume: "none" | "basic" | "full", | ||
| subagents: "none" | "basic" | "full", | ||
| attachments: "none" | "basic" | "full", | ||
| replay: "none" | "basic" | "full", | ||
| mcpConfig: "none" | "basic" | "full", | ||
| } | ||
| ``` | ||
|
|
||
| The contract test suite (`apps/server/integration/contract.integration.test.ts`) uses these to auto-skip tests for unsupported capabilities. | ||
|
|
||
| ## Step 5: Runtime Event Mapping | ||
|
|
||
| Your adapter must emit `ProviderRuntimeEvent` objects with these core event types: | ||
|
|
||
| - `turn.started` / `turn.completed` -- turn lifecycle | ||
| - `content.delta` -- streaming text output | ||
| - `item.started` / `item.completed` -- tool/file-change items | ||
| - `request.opened` / `request.resolved` -- approval flow | ||
| - `session.status` -- session state changes | ||
|
|
||
| See `packages/contracts/src/providerRuntime.ts` for the full event schema. | ||
|
|
||
| ## Step 6: MCP Configuration | ||
|
|
||
| If your provider supports external MCP servers: | ||
|
|
||
| - Implement `translateMcpConfig()` to convert `ResolvedMcpConfig` to provider-native format | ||
| - The harness path stores `mcp_config` in the session state (see `codex_session.ex`) | ||
|
|
||
| If your provider manages its own MCP (like Claude): | ||
|
|
||
| - Return `null` from `translateMcpConfig()` | ||
|
|
||
| ## Step 7: Settings Integration | ||
|
|
||
| Add provider entry in `packages/contracts/src/settings.ts` server settings schema so users can enable/disable and configure the provider. | ||
|
|
||
| ## Step 8: Testing | ||
|
|
||
| 1. **Unit tests**: Add adapter-level tests in `apps/server/src/provider/Layers/YourAdapter.test.ts` | ||
| 2. **Contract tests**: The contract suite auto-includes all registered providers | ||
| 3. **Integration tests**: Add provider-specific integration scenarios if needed | ||
|
|
||
| ## Step 9: Model Discovery (Optional) | ||
|
|
||
| If your provider supports model listing: | ||
|
|
||
| - Elixir: implement model discovery in the session module or a separate GenServer | ||
| - Node: expose via the adapter shape | ||
|
|
||
| Register in `ProviderRegistry` (`apps/server/src/provider/Services/ProviderRegistry.ts`) for the UI model picker. | ||
|
|
||
| ## Checklist | ||
|
|
||
| - [ ] Provider kind added to `ProviderKind` schema | ||
| - [ ] Session module created (Elixir) or adapter layer created (Node) | ||
| - [ ] `@behaviour ProviderBehaviour` implemented (Elixir path) | ||
| - [ ] Capabilities declared | ||
| - [ ] Runtime event mapping implemented | ||
| - [ ] MCP config translator implemented | ||
| - [ ] Registered in SessionManager (Elixir) or adapter registry (Node) | ||
| - [ ] Settings entry added | ||
| - [ ] Unit tests passing | ||
| - [ ] Contract tests passing (or properly skipping unsupported capabilities) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| defmodule Harness.ProviderSession do | ||
| @moduledoc """ | ||
| Behaviour implemented by provider-backed session processes. | ||
|
|
||
| `Harness.SessionManager` dispatches to this surface regardless of whether the | ||
| provider uses a persistent stdio process, a turn-scoped CLI, or an HTTP/SSE | ||
| bridge. | ||
| """ | ||
|
|
||
| @type request_id :: term() | ||
| @type decision :: term() | ||
| @type answers :: term() | ||
| @type params :: map() | ||
| @type thread_id :: String.t() | ||
| @type turn_id :: term() | ||
| @type snapshot :: term() | ||
|
|
||
| @callback start_link(keyword() | map()) :: GenServer.on_start() | ||
| @callback wait_for_ready(GenServer.server(), timeout()) :: :ok | {:error, term()} | ||
| @callback send_turn(GenServer.server(), params()) :: term() | ||
| @callback interrupt_turn(GenServer.server(), thread_id(), turn_id() | nil) :: term() | ||
| @callback respond_to_approval(GenServer.server(), request_id(), decision()) :: term() | ||
| @callback respond_to_user_input(GenServer.server(), request_id(), answers()) :: term() | ||
| @callback read_thread(GenServer.server(), thread_id()) :: {:ok, snapshot()} | {:error, term()} | ||
| @callback rollback_thread(GenServer.server(), thread_id(), non_neg_integer()) :: | ||
| {:ok, snapshot()} | {:error, term()} | ||
| end |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Non-critical Metrics GenServer placed before critical services in rest_for_one supervision tree
In
application.ex:16,Harness.Metricsis inserted betweenHarness.StorageandHarness.SnapshotServerin the children list. The supervisor usesstrategy: :rest_for_one(line 20), which means if Metrics crashes, bothHarness.SnapshotServerandHarnessWeb.Endpointare terminated and restarted. Metrics is a non-critical observability service — its crash should not cascade-restart the web endpoint and snapshot server. PerAGENTS.md: "Reliability first. Keep behavior predictable under load and during failures."Prompt for agents
Was this helpful? React with 👍 or 👎 to provide feedback.