Skip to content

fix: consolidate provider behaviour + fix multi-turn provider corruption - #37

Merged
ranvier2d2 merged 1 commit into
mainfrom
fix/provider-behaviour-consolidation
Mar 29, 2026
Merged

ranvier2d2 merged 1 commit into
mainfrom
fix/provider-behaviour-consolidation

Conversation

@ranvier2d2

@ranvier2d2 ranvier2d2 commented Mar 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Removed duplicate Harness.ProviderSession behaviour — all session modules now use only Harness.Providers.ProviderBehaviour, eliminating ~40 conflicting callback warnings at compile time. Added missing @impl annotations and stop/1 to MockSession.
  • Fixed toLegacyProvider in web store to recognize "cursor" and "opencode" — previously both were silently mapped to "codex", corrupting the session provider on second turn and causing "cannot switch provider" errors for OpenCode and Cursor sessions.
  • Hardcoded experimentalApi: true in Codex initialize (was conditional on providerOptions), fixing collaborationMode rejection on turn/start.

Test plan

  • mix compile --warnings-as-errors passes with zero warnings
  • OpenCode multi-turn works (second message no longer errors)
  • Cursor multi-turn works
  • Codex GPT-5.4 turn starts without collaborationMode error

🤖 Generated with Claude Code


Open with Devin

Summary by CodeRabbit

  • Refactor
    • Internal provider behavior contracts were streamlined across providers for clearer lifecycle handling.
  • Bug Fixes / Improvements
    • Legacy provider mapping now recognizes "cursor" and "opencode" so selections route as expected.
    • Provider initialization logic simplified to consistently enable experimental API capability for one provider, improving startup reliability.

@coderabbitai

coderabbitai Bot commented Mar 29, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 15d8f3da-8597-419f-88c3-9e63a3c44db6

📥 Commits

Reviewing files that changed from the base of the PR and between 25ab8e956e365b9a04873db224175a8c4163ade9 and 78b8644.

📒 Files selected for processing (7)
  • apps/harness/lib/harness/provider_session.ex
  • apps/harness/lib/harness/providers/claude_session.ex
  • apps/harness/lib/harness/providers/codex_session.ex
  • apps/harness/lib/harness/providers/cursor_session.ex
  • apps/harness/lib/harness/providers/mock_session.ex
  • apps/harness/lib/harness/providers/opencode_session.ex
  • apps/web/src/store.ts
💤 Files with no reviewable changes (1)
  • apps/harness/lib/harness/provider_session.ex
✅ Files skipped from review due to trivial changes (2)
  • apps/harness/lib/harness/providers/claude_session.ex
  • apps/harness/lib/harness/providers/cursor_session.ex
🚧 Files skipped from review as they are similar to previous changes (4)
  • apps/web/src/store.ts
  • apps/harness/lib/harness/providers/codex_session.ex
  • apps/harness/lib/harness/providers/mock_session.ex
  • apps/harness/lib/harness/providers/opencode_session.ex

📝 Walkthrough

Walkthrough

Deleted the Harness.ProviderSession behaviour module; updated provider session modules to use Harness.Providers.ProviderBehaviour (adding @impl annotations), simplified CodexSession initialization payload, added stop/1 to MockSession, and extended legacy provider recognition in the web store.

Changes

Cohort / File(s) Summary
Behaviour Module Removed
apps/harness/lib/harness/provider_session.ex
Module deleted — removed type aliases and all behaviour callback specifications for provider-backed session processes.
Provider Implementations Updated
apps/harness/lib/harness/providers/claude_session.ex, apps/harness/lib/harness/providers/codex_session.ex, apps/harness/lib/harness/providers/cursor_session.ex, apps/harness/lib/harness/providers/opencode_session.ex
Removed @behaviour Harness.ProviderSession and added @impl Harness.Providers.ProviderBehaviour on implementations (notably wait_for_ready/2). No public signatures changed.
CodexSession Init Simplified
apps/harness/lib/harness/providers/codex_session.ex
Initialization JSON-RPC now always includes "capabilities" => %{\"experimentalApi\" => true}, removing conditional capability construction from state params.
MockSession Lifecycle API
apps/harness/lib/harness/providers/mock_session.ex
Switched to Harness.Providers.ProviderBehaviour, annotated callbacks with @impl, and added exported stop(pid) delegating to GenServer.stop/1.
Frontend Provider Mapping
apps/web/src/store.ts
toLegacyProvider() now recognizes "cursor" and "opencode" as valid legacy provider names instead of defaulting to "codex".

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I nibbled a spec, then hopped away,
Behaviours trimmed to a brighter day.
Codex now whispers experimental cheer,
Mock session stops when a rabbit draws near,
Cursor and OpenCode hop into view — hip‑hop hooray!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main changes: consolidating provider behaviour and fixing multi-turn provider corruption.
Description check ✅ Passed The PR description provides comprehensive context covering what changed (behaviour consolidation, toLegacyProvider fix, hardcoded experimentalApi), why it matters (eliminating compile warnings and fixing session corruption), and includes a test plan validating the fixes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/provider-behaviour-consolidation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions github-actions Bot added size:M vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. labels Mar 29, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

… provider corruption

Two fixes:

1. Remove duplicate Harness.ProviderSession behaviour — all session modules
   now use only Harness.Providers.ProviderBehaviour, eliminating conflicting
   callback warnings. Added missing @impl annotations and stop/1 to MockSession.

2. Fix toLegacyProvider in web store to recognize "cursor" and "opencode" —
   previously both were silently mapped to "codex", corrupting the provider
   on second turn and causing "cannot switch provider" errors.

Also hardcodes experimentalApi: true in Codex initialize (was conditional on
providerOptions), fixing collaborationMode rejection on turn/start.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ranvier2d2
ranvier2d2 force-pushed the fix/provider-behaviour-consolidation branch from 25ab8e9 to 78b8644 Compare March 29, 2026 17:14
@ranvier2d2
ranvier2d2 merged commit 176948a into main Mar 29, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 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.

1 participant