fix(#242): honor configured interview adapter backend in pm

[srausser]

Summary

• route ouroboros pm through create_llm_adapter(use_case="interview") instead of constructing LiteLLMAdapter() directly
• make the PM CLI respect configured backends such as Codex via the existing provider factory
• add a focused regression test that verifies the factory-backed adapter path is used

Testing

• PYTHONPATH=src pytest tests/unit/cli/test_pm_runtime_adapter.py -q
• ruff check src/ouroboros/cli/commands/pm.py tests/unit/cli/test_pm_runtime_adapter.py

Closes #242.

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Reviewing commit 241466c | Triggered by: PR opened by @srausser

Branch: sam/issue-242-pm-codex-adapter | 2 files, +46/-2 | CI: MyPy Type Check pass 43s https://github.com/Q00/ouroboros/actions/runs/23671290163/job/68965148913

Issue #N Requirements

Requirement	Status
ouroboros pm ignores configured Codex backend and always uses LiteLLMAdapter	PARTIALLY VERIFIED — linked issue reviewed; evidence is called out in the blocking findings and changed files.

Previous Review Follow-up

First review — no previous findings.

Blocking Findings

#	File:Line	Severity	Confidence	Finding
1	src/ouroboros/cli/commands/pm.py:321	High	High	This now binds ouroboros pm to the globally configured LLM backend, but the command still defaults --model to the Anthropic-specific literal from line 59. The config layer already has backend-safe model normalization for Codex (get_clarification_model(backend=...) -> "default"), so a user with codex configured will hit the new backend with an Anthropic model string unless they remember to override --model manually. That is a real runtime regression introduced by switching away from the hardcoded LiteLLM adapter.
2	src/ouroboros/cli/commands/pm.py:321	Medium	High	create_llm_adapter() can now raise configuration errors here (Unsupported LLM backend, rejected opencode, bad permission config), and pm_command() only catches KeyboardInterrupt. Before this change the path was a direct LiteLLMAdapter() construction, so the PR adds a new uncaught failure mode that will surface as a raw traceback instead of a CLI-formatted error.
3	src/ouroboros/cli/commands/pm.py:321	Medium	Medium	The shared factory implicitly upgrades codex and claude_code interview runs to bypassPermissions. That may be the right default for MCP/server flows, but on the CLI this is a silent behavior change from the previous LiteLLM-only implementation and there is no PM-specific flag or prompt making the privilege jump explicit.
4	src/ouroboros/cli/commands/pm.py:321	Medium	Medium	The new factory call omits the interview-specific options used by the other CLI interview entry point (cwd, max_turns, on_message). For local agent backends that means PM interview requests fall back to max_turns=1 and no debug/stream callback, which is materially weaker than the existing interview path and increases the chance of low-context question generation or opaque failures.
5	tests/unit/cli/test_pm_runtime_adapter.py:14	Low	High	The new regression test proves only that _run_pm_interview() calls the factory on the resume/error path. It does not exercise the new-session path or assert any backend-sensitive behavior (model normalization, permission mode implications, max_turns/debug parity), so the regressions above can still ship while this test stays green.

Test Coverage

tests/unit/cli/test_pm_runtime_adapter.py:14 only covers resume_id + load_state() failure. There is no coverage for the new-session branch where PM actually starts an interview and persists state.

No test currently verifies that the PM CLI chooses a backend-safe default model when the configured backend is codex or another non-LiteLLM backend. The existing config tests already show this normalization exists elsewhere.

No CLI test asserts the factory call shape for PM interview mode beyond use_case="interview"; gaps remain around permission behavior, max_turns, cwd, and debug callback wiring.

Design Notes

The direction is good: PM should use the shared adapter factory so backend selection is consistent across entry points. The issue is that this command still carries LiteLLM-era assumptions, especially around model defaults and the absence of explicit backend/runtime options. Once PM becomes backend-aware, it should use the same backend-aware model resolution and interview runtime settings as the rest of the interview surfaces; otherwise the factory swap is only partial and introduces backend-specific regressions.

Follow-up Recommendations

• Re-run this review after the author addresses the blocking items so the current PR HEAD can be verified again.
• Add or expand regression tests for the code paths called out above before merge.

Files Reviewed

• src/ouroboros/cli/commands/pm.py
• tests/unit/cli/test_pm_runtime_adapter.py

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Reviewing commit 455d0ab | Triggered by: new push by @srausser

Branch: sam/issue-242-pm-codex-adapter | 2 files, +235/-7 | CI: MyPy Type Check pass; Ruff Lint pass; Test Python 3.12 pass; Test Python 3.13 pass; Test Python 3.14 pass

What Improved

This push addresses most of the first-round review well: PM now resolves a backend-safe default model, uses interview-specific runtime options (max_turns=5, debug callback, cwd), and adds tests around the new-session path plus the user-facing bypass-permissions warning.

Issue #242 Requirements

Requirement	Status
Route ouroboros pm through the provider factory instead of constructing LiteLLMAdapter() directly	MET — _run_pm_interview() now builds the adapter via create_llm_adapter(..., use_case="interview") at src/ouroboros/cli/commands/pm.py:362.
Respect configured backends such as Codex	PARTIALLY MET — the normal path now resolves backend + backend-safe model defaults in pm_command(), but invalid/unsupported backend configuration still escapes before the command’s CLI-formatted error handling runs (src/ouroboros/cli/commands/pm.py:98-103).

Previous Review Follow-up

Previous Finding	Status
Backend-safe default model was missing for non-LiteLLM backends	RESOLVED — pm_command() now uses get_clarification_model(resolved_backend) at src/ouroboros/cli/commands/pm.py:99.
Factory/config errors were not formatted as a clean CLI error	MODIFIED / STILL OPEN — the new except ValueError block helps for errors raised inside asyncio.run(...), but resolve_llm_backend() and resolve_llm_permission_mode() now run before the try, so a bad backend still bypasses the formatter (src/ouroboros/cli/commands/pm.py:98-103,111-121).
Permission escalation to bypassPermissions was silent	RESOLVED — the CLI now emits an explicit warning before starting the interview (src/ouroboros/cli/commands/pm.py:106-110).
PM interview path was missing the stronger interview runtime options (max_turns, callback, cwd)	RESOLVED — _run_pm_interview() now passes max_turns=5, _make_message_callback(debug), and cwd=Path.cwd() into create_llm_adapter() (src/ouroboros/cli/commands/pm.py:362-369).
Test coverage only exercised the resume/error path	IMPROVED — the new test file adds a new-session case and CLI entrypoint assertions, but there is still a gap around the real invalid-backend failure path noted below.

Blocking Findings

#	File:Line	Severity	Confidence	Finding
1	src/ouroboros/cli/commands/pm.py:98	Medium	High	resolve_llm_backend(get_llm_backend()) and resolve_llm_permission_mode(...) now run before the try/except ValueError block, so an unsupported configured backend still raises directly out of pm_command() as a raw exception instead of the CLI-formatted print_error(...) + typer.Exit(1) path this PR is trying to add. The new regression test masks that by mocking resolve_llm_backend() to succeed and forcing asyncio.run() to raise later (tests/unit/cli/test_pm_runtime_adapter.py:143-177), so the real failure path remains unverified.

Test Coverage

tests/unit/cli/test_pm_runtime_adapter.py:143-177 does not exercise the real invalid-backend path; it patches resolve_llm_backend() and resolve_llm_permission_mode() to succeed, then injects the ValueError from asyncio.run(). That means the current blocker can ship while the test still passes.

I could not re-run the project test file in this sandbox because creating a uv environment required downloading dependencies from PyPI and outbound package fetches were unavailable here. The review above is therefore based on current-HEAD code inspection plus the checked CI results already attached to the PR.

Design Notes

The overall direction is now correct: PM is using the shared provider factory, backend-safe defaults, and interview-oriented runtime settings instead of a hardcoded LiteLLM path. The remaining issue is mostly command-boundary hygiene — once backend resolution moved into the CLI entrypoint, the error-formatting guard needs to wrap that whole resolution block, not just the async interview execution.

Follow-up Recommendations

• After fixing the blocker, replace the mocked invalid-backend test with one that lets resolve_llm_backend() raise for real so the command-level UX is protected against future regressions.
• If you want PM CLI and PM MCP to stay behaviorally aligned, consider adding an explicit assertion around the chosen interview permission mode in this test file as well.

Files Reviewed

• src/ouroboros/cli/commands/pm.py
• tests/unit/cli/test_pm_runtime_adapter.py

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis