feat(harness): developer surface v1 — inspect, doctor, explain - #5
Merged
Conversation
Core transport-agnostic service modules for agent-driven debugging: - Harness.Dev.Inspect: sessions/0, session/1, bridge/0 — enriched runtime state with GenServer diagnostics and snapshot data - Harness.Dev.Doctor: full/0, check/1 — health probes for provider binaries (codex, claude, cursor, opencode) and infrastructure - Harness.Dev.Explain: 8 topics (startup, resume-fallback, bridge-contract, providers, wal, approval-flow, subagents, model-discovery) - Harness.Dev.BridgeContract: structured Node↔Elixir channel contract GenServer accessors: :get_diagnostics on all 4 provider sessions, SessionManager.get_diagnostics/1, SnapshotServer.get_wal_stats/0. Dual adapters: - HTTP /api/dev/* endpoints for live runtime inspection (curl-able) - Mix tasks for local-safe commands (explain, doctor binary checks) 15 new tests, all pass. Credo clean. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds developer-facing diagnostics and introspection: new Harness.Dev modules (bridge contract, diagnostics helpers, doctor, explain, inspect), provider sessions expose :get_diagnostics, SessionManager/SnapshotServer gain diagnostic endpoints, new dev HTTP routes and Mix tasks, tests, and assorted frontend/server wiring and formatting changes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
github-actions
Bot
added
size:XXL
vouch:trusted
System.cmd/3 does not support :timeout option. Use Task.async + Task.yield with 4s timeout instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/harness/lib/harness/providers/claude_session.ex (1)
129-154:⚠️ Potential issue | 🔴 CriticalCapture the completed state outside the
unlessblock.In Elixir, the
state = maybe_complete_turn(...)inside theunlessblock does not affect the outerstatevariable. The later branches operate on the pre-completion struct, leavingturn_stateuncleared until the next request.Suggested fix
def handle_info({port, {:exit_status, status}}, %{port: port} = state) do - unless state.stopped do - Logger.info("Claude process exited with status #{status} for thread #{state.thread_id}") - - # Complete any active turn - state = maybe_complete_turn(state, if(status == 0, do: "completed", else: "failed")) - - if status != 0 do - emit_event(state, :session, "session/exited", %{ - "exitStatus" => status, - "exitKind" => "error" - }) - end - end + state = + if state.stopped do + state + else + Logger.info("Claude process exited with status #{status} for thread #{state.thread_id}") + + state = maybe_complete_turn(state, if(status == 0, do: "completed", else: "failed")) + + if status != 0 do + emit_event(state, :session, "session/exited", %{ + "exitStatus" => status, + "exitKind" => "error" + }) + end + + state + end if status == 0 and not state.stopped do # Graceful exit — stay alive for next turn, clear port and buffer state = %{state | port: nil, buffer: ""}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/harness/lib/harness/providers/claude_session.ex` around lines 129 - 154, The state returned by maybe_complete_turn is currently assigned only inside the unless block in handle_info, so subsequent branches use the old state; move the call to maybe_complete_turn (or capture its result) so that state = maybe_complete_turn(state, if(status == 0, do: "completed", else: "failed")) executes regardless of the unless condition (or assign a new variable like completed_state and use that in later calls to cancel_all_pending/emit_event), then use that updated state for clearing buffer/port, cancel_all_pending, emit_event and in the {:noreply, state} / {:stop, :normal, state} returns so turn_state is cleared consistently.
🧹 Nitpick comments (2)
apps/harness/lib/mix/tasks/harness.doctor.ex (1)
12-23: Documentation doesn't mentionbeamas a valid target.The
@local_targetsincludes"beam", but the usage examples in the moduledoc only list provider binaries. Consider addingmix harness.doctor beamto the usage section for completeness.📝 Suggested doc update
mix harness.doctor codex # Check Codex binary only mix harness.doctor claude # Check Claude binary only mix harness.doctor cursor # Check Cursor binary only mix harness.doctor opencode # Check OpenCode binary only + mix harness.doctor beam # Check BEAM/OTP health """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/harness/lib/mix/tasks/harness.doctor.ex` around lines 12 - 23, The moduledoc usage examples omit "beam" though the module defines `@local_targets` ~w(codex claude cursor opencode beam); update the module documentation (the usage block in the moduledoc) to include "mix harness.doctor beam" as a valid target so docs match the `@local_targets` list and users know "beam" is supported.apps/harness/lib/harness/dev/doctor.ex (1)
110-121: Bridge status only reflects SnapshotServer health.The
statusis determined solely bysnapshot_alive, but other critical processes (Harness.PubSub,Harness.SessionRegistry,Harness.SessionSupervisor,HarnessWeb.Endpoint) being down would also indicate a degraded bridge. Consider factoring all process checks into the status determination.♻️ Suggested fix
defp check_bridge do snapshot_alive = Process.whereis(Harness.SnapshotServer) != nil + endpoint_running = Process.whereis(HarnessWeb.Endpoint) != nil + pubsub_alive = Process.whereis(Harness.PubSub) != nil + registry_alive = Process.whereis(Harness.SessionRegistry) != nil + supervisor_alive = Process.whereis(Harness.SessionSupervisor) != nil + + all_healthy = snapshot_alive and endpoint_running and pubsub_alive and registry_alive and supervisor_alive %{ - status: if(snapshot_alive, do: "healthy", else: "degraded"), - endpoint_running: Process.whereis(HarnessWeb.Endpoint) != nil, - pubsub_alive: Process.whereis(Harness.PubSub) != nil, - registry_alive: Process.whereis(Harness.SessionRegistry) != nil, - supervisor_alive: Process.whereis(Harness.SessionSupervisor) != nil, + status: if(all_healthy, do: "healthy", else: "degraded"), + endpoint_running: endpoint_running, + pubsub_alive: pubsub_alive, + registry_alive: registry_alive, + supervisor_alive: supervisor_alive, snapshot_server_alive: snapshot_alive } end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/harness/lib/harness/dev/doctor.ex` around lines 110 - 121, The current check_bridge function sets status based only on snapshot_alive; change it so status is "healthy" only when all monitored processes are alive (Harness.SnapshotServer, HarnessWeb.Endpoint, Harness.PubSub, Harness.SessionRegistry, Harness.SessionSupervisor) and "degraded" otherwise — compute a combined boolean (e.g. all_alive = snapshot_alive && endpoint_running && pubsub_alive && registry_alive && supervisor_alive) and use that for the status field while leaving the individual keys (endpoint_running, pubsub_alive, registry_alive, supervisor_alive, snapshot_server_alive) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/harness/lib/harness_web/endpoint.ex`:
- Around line 74-109: The dev endpoints under route/2 (paths matching
"/api/dev/...") must be explicitly gated; add a reusable check like
dev_access_allowed?/0 (reads config e.g. :harness, :dev_only or allowed IPs) and
an ensure_dev_access(conn) helper that returns a 403 response and halts if not
allowed, then call ensure_dev_access(conn) at the start of each route clause
handling "/api/dev/..." (or add a single catch-all clause that calls it before
delegating to Harness.Dev.* functions); implement using Plug.Conn.send_resp/3
and Plug.Conn.halt/1 so unauthorized requests never reach
Harness.Dev.Explain.topic/1, Harness.Dev.Doctor.check/1,
Harness.Dev.Inspect.session/1, etc.
In `@apps/harness/lib/harness/dev/diagnostics_helpers.ex`:
- Around line 20-23: sanitize_turn_state/1 currently only extracts :id and
:started_at from a turn map, which drops the in-flight identifier stored under
:turn_id; update the function (sanitize_turn_state) to include :turn_id in the
returned sanitized map (or include both :id and :turn_id if both may exist) so
diagnostics preserve the active turn identifier when given a map; locate
sanitize_turn_state and adjust the Map.take call to include :turn_id (and keep
:id and :started_at) so the diagnostics response exposes the current turn
identifier.
In `@apps/harness/lib/harness/dev/inspect.ex`:
- Around line 86-109: The call to Harness.SnapshotServer.get_snapshot() in
bridge/0 can crash the whole function if the SnapshotServer is down; wrap that
call in a try/catch (like the existing get_wal_stats call) and set snapshot to a
safe default (e.g. %{sequence: 0, sessions: %{}} or a map containing an :error
key) when an exit is caught so bridge/0 can continue and report
snapshot_server.alive: false and the WAL fallback payload; update uses of
snapshot (sequence and session_count) to rely on these safe defaults.
- Around line 19-48: The current Enum.map over raw_sessions calls
Harness.SessionManager.get_diagnostics/1 serially causing latency; replace the
Enum.map with Task.async_stream(raw_sessions, fn session -> ... end, timeout:
:infinity) (or a bounded :max_concurrency option) so diagnostics calls run
concurrently with back-pressure, then collect and map the async_stream results
back into the enriched list (preserving the same per-session merging logic for
diag_fields and snapshot_fields). Ensure you reference raw_sessions,
Harness.SessionManager.get_diagnostics/1, and the enriched mapping logic
(diag_fields, snapshot_fields, base |> Map.merge(...)) so each async result
merges best-effort diagnostics and snapshot info for that session.
In `@apps/harness/lib/harness/providers/claude_session.ex`:
- Around line 238-256: When spawn_claude_with_prompt returns {:error, reason}
you must not reply with {:ok,...}; instead return an error reply and ensure the
transient turn state is cleaned up. Change the case in which
spawn_claude_with_prompt returns {:error, reason} to update state (clear
state.port/start flags or mark the turn as failed), and return {:reply, {:error,
%{threadId: state.thread_id, turnId: turn_id, reason: inspect(reason)}},
updated_state} (or a similar error tuple your caller expects) rather than
falling through to the successful reply that uses build_resume_cursor; reference
spawn_claude_with_prompt, state.port, build_resume_cursor, turn_id and thread_id
to locate and modify the logic.
In `@apps/harness/lib/harness/providers/codex_session.ex`:
- Around line 789-792: The code computing is_user_input can crash if method
isn't a binary; update the second predicate to guard before calling
String.downcase/1 (e.g., ensure is_binary(method) &&
String.contains?(String.downcase(method), "ask_user")) so non-binary values skip
the String.downcase path; adjust the is_user_input expression in the same block
(the variable name is_user_input and the reference to String.downcase/1) to use
that binary guard.
- Around line 316-333: The diagnostics payload in CodexSession (the diagnostics
map in the code building diagnostics) currently exposes sensitive filesystem
fields (binary_path and codex_home) and is returned via the unprotected
/api/dev/* endpoint; update the code to prevent leaking these values by either
(A) adding environment guards around the dev endpoints so /api/dev/* is only
registered when Mix.env() == :dev (adjust the harness_web endpoint/router
registration), or (B) add authentication middleware to the /api/dev/* route
group (e.g., basic auth or API key check) and/or remove binary_path and
codex_home from the diagnostics map in the CodexSession module (remove the
binary_path and codex_home keys from diagnostics or replace with
sanitized/boolean flags), and ensure the reply path that returns diagnostics
(the {:reply, {:ok, diagnostics}, state} flow in CodexSession) only emits
non-sensitive fields unless the request is authenticated and running in dev.
In `@apps/harness/lib/mix/tasks/harness.explain.ex`:
- Around line 29-46: The invalid-input branches of run/1 (the run([]) and run(_)
clauses) currently call Mix.shell().error/1 which only prints to stderr; change
these to call Mix.raise/1 with the same usage message so the task exits with
non-zero status on invalid input; leave the successful run([topic]) clause (and
its error handling for Harness.Dev.Explain.topic) unchanged.
---
Outside diff comments:
In `@apps/harness/lib/harness/providers/claude_session.ex`:
- Around line 129-154: The state returned by maybe_complete_turn is currently
assigned only inside the unless block in handle_info, so subsequent branches use
the old state; move the call to maybe_complete_turn (or capture its result) so
that state = maybe_complete_turn(state, if(status == 0, do: "completed", else:
"failed")) executes regardless of the unless condition (or assign a new variable
like completed_state and use that in later calls to
cancel_all_pending/emit_event), then use that updated state for clearing
buffer/port, cancel_all_pending, emit_event and in the {:noreply, state} /
{:stop, :normal, state} returns so turn_state is cleared consistently.
---
Nitpick comments:
In `@apps/harness/lib/harness/dev/doctor.ex`:
- Around line 110-121: The current check_bridge function sets status based only
on snapshot_alive; change it so status is "healthy" only when all monitored
processes are alive (Harness.SnapshotServer, HarnessWeb.Endpoint,
Harness.PubSub, Harness.SessionRegistry, Harness.SessionSupervisor) and
"degraded" otherwise — compute a combined boolean (e.g. all_alive =
snapshot_alive && endpoint_running && pubsub_alive && registry_alive &&
supervisor_alive) and use that for the status field while leaving the individual
keys (endpoint_running, pubsub_alive, registry_alive, supervisor_alive,
snapshot_server_alive) intact.
In `@apps/harness/lib/mix/tasks/harness.doctor.ex`:
- Around line 12-23: The moduledoc usage examples omit "beam" though the module
defines `@local_targets` ~w(codex claude cursor opencode beam); update the module
documentation (the usage block in the moduledoc) to include "mix harness.doctor
beam" as a valid target so docs match the `@local_targets` list and users know
"beam" is supported.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3ba66fb-be4c-4b9f-b72e-2cab26a45671
📒 Files selected for processing (17)
apps/harness/lib/harness/dev/bridge_contract.exapps/harness/lib/harness/dev/diagnostics_helpers.exapps/harness/lib/harness/dev/doctor.exapps/harness/lib/harness/dev/explain.exapps/harness/lib/harness/dev/inspect.exapps/harness/lib/harness/providers/claude_session.exapps/harness/lib/harness/providers/codex_session.exapps/harness/lib/harness/providers/cursor_session.exapps/harness/lib/harness/providers/opencode_session.exapps/harness/lib/harness/session_manager.exapps/harness/lib/harness/snapshot_server.exapps/harness/lib/harness_web/endpoint.exapps/harness/lib/mix/tasks/harness.doctor.exapps/harness/lib/mix/tasks/harness.explain.exapps/harness/test/harness/dev/doctor_test.exsapps/harness/test/harness/dev/explain_test.exsapps/harness/test/harness/dev/inspect_test.exs
Comment on lines
+74
to
+109
| defp route(%{request_path: "/api/dev/explain/topics"} = conn, _opts) do | ||
| json_response(conn, 200, %{ok: true, data: Harness.Dev.Explain.topics()}) | ||
| end | ||
|
|
||
| defp route(%{request_path: "/api/dev/explain/" <> topic} = conn, _opts) when topic != "" do | ||
| case Harness.Dev.Explain.topic(topic) do | ||
| {:ok, data} -> json_response(conn, 200, %{ok: true, data: data}) | ||
| {:error, reason} -> json_response(conn, 404, %{ok: false, error: reason}) | ||
| end | ||
| end | ||
|
|
||
| defp route(%{request_path: "/api/dev/doctor/" <> target} = conn, _opts) when target != "" do | ||
| case Harness.Dev.Doctor.check(target) do | ||
| {:ok, data} -> json_response(conn, 200, %{ok: true, data: data}) | ||
| {:error, reason} -> json_response(conn, 400, %{ok: false, error: reason}) | ||
| end | ||
| end | ||
|
|
||
| defp route(%{request_path: "/api/dev/doctor"} = conn, _opts) do | ||
| json_response(conn, 200, %{ok: true, data: Harness.Dev.Doctor.full()}) | ||
| end | ||
|
|
||
| defp route(%{request_path: "/api/dev/session/" <> thread_id} = conn, _opts) | ||
| when thread_id != "" do | ||
| case Harness.Dev.Inspect.session(thread_id) do | ||
| {:ok, data} -> json_response(conn, 200, %{ok: true, data: data}) | ||
| {:error, reason} -> json_response(conn, 404, %{ok: false, error: reason}) | ||
| end | ||
| end | ||
|
|
||
| defp route(%{request_path: "/api/dev/sessions"} = conn, _opts) do | ||
| json_response(conn, 200, %{ok: true, data: Harness.Dev.Inspect.sessions()}) | ||
| end | ||
|
|
||
| defp route(%{request_path: "/api/dev/bridge"} = conn, _opts) do | ||
| json_response(conn, 200, %{ok: true, data: Harness.Dev.Inspect.bridge()}) |
There was a problem hiding this comment.
Gate the /api/dev/* surface explicitly.
These handlers expose session ids, binary paths, process info, snapshot state, and repo file paths, but nothing in this module limits them to localhost or authenticated callers. If this is intended to stay developer-only, please enforce that in code/config instead of relying on deployment topology.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/harness/lib/harness_web/endpoint.ex` around lines 74 - 109, The dev
endpoints under route/2 (paths matching "/api/dev/...") must be explicitly
gated; add a reusable check like dev_access_allowed?/0 (reads config e.g.
:harness, :dev_only or allowed IPs) and an ensure_dev_access(conn) helper that
returns a 403 response and halts if not allowed, then call
ensure_dev_access(conn) at the start of each route clause handling
"/api/dev/..." (or add a single catch-all clause that calls it before delegating
to Harness.Dev.* functions); implement using Plug.Conn.send_resp/3 and
Plug.Conn.halt/1 so unauthorized requests never reach
Harness.Dev.Explain.topic/1, Harness.Dev.Doctor.check/1,
Harness.Dev.Inspect.session/1, etc.
Comment on lines
+19
to
+48
| enriched = | ||
| Enum.map(raw_sessions, fn %{threadId: tid, provider: provider} -> | ||
| base = %{threadId: tid, provider: provider} | ||
|
|
||
| # Enrich with GenServer diagnostics (best-effort) | ||
| diag_fields = | ||
| case Harness.SessionManager.get_diagnostics(tid) do | ||
| {:ok, diag} -> | ||
| Map.take(diag, [ | ||
| :ready, | ||
| :port_alive, | ||
| :pending_count, | ||
| :binary_path, | ||
| :stopped, | ||
| :stopping | ||
| ]) | ||
|
|
||
| {:error, _} -> | ||
| %{} | ||
| end | ||
|
|
||
| # Enrich with snapshot status | ||
| snapshot_fields = | ||
| case Map.get(snapshot[:sessions] || %{}, tid) do | ||
| nil -> %{} | ||
| ss -> %{status: ss[:status], model: ss[:model]} | ||
| end | ||
|
|
||
| base |> Map.merge(diag_fields) |> Map.merge(snapshot_fields) | ||
| end) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the structure and read the relevant file
cd /tmp/repo 2>/dev/null || cd . && \
git ls-files apps/harness/lib/harness/dev/inspect.exRepository: Ranvier-Technologies/t3code-OTP
Length of output: 114
🏁 Script executed:
# Read the file to verify the code snippet
cat -n apps/harness/lib/harness/dev/inspect.ex | head -60Repository: Ranvier-Technologies/t3code-OTP
Length of output: 2284
🏁 Script executed:
# Check the get_diagnostics implementation to confirm the timeout
rg -A 10 "def get_diagnostics" apps/harness/lib/harness/session_manager.ex | head -20Repository: Ranvier-Technologies/t3code-OTP
Length of output: 444
🏁 Script executed:
# Verify if Task.async_stream is already imported or available in this file
rg "Task\." apps/harness/lib/harness/dev/inspect.exRepository: Ranvier-Technologies/t3code-OTP
Length of output: 57
Use Task.async_stream/3 to parallelize diagnostics calls across sessions.
Harness.SessionManager.get_diagnostics/1 uses a 5_000 ms GenServer.call/3 timeout. Running it serially inside Enum.map/2 makes /api/dev/sessions latency scale linearly with session count—a few slow providers stall the whole endpoint. Replace with Task.async_stream(raw_sessions, callback, timeout: :infinity) to fan out the calls with back-pressure, merging best-effort results per session.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/harness/lib/harness/dev/inspect.ex` around lines 19 - 48, The current
Enum.map over raw_sessions calls Harness.SessionManager.get_diagnostics/1
serially causing latency; replace the Enum.map with
Task.async_stream(raw_sessions, fn session -> ... end, timeout: :infinity) (or a
bounded :max_concurrency option) so diagnostics calls run concurrently with
back-pressure, then collect and map the async_stream results back into the
enriched list (preserving the same per-session merging logic for diag_fields and
snapshot_fields). Ensure you reference raw_sessions,
Harness.SessionManager.get_diagnostics/1, and the enriched mapping logic
(diag_fields, snapshot_fields, base |> Map.merge(...)) so each async result
merges best-effort diagnostics and snapshot info for that session.
Comment on lines
+316
to
+333
| diagnostics = %{ | ||
| thread_id: state.thread_id, | ||
| provider: state.provider, | ||
| ready: state.ready, | ||
| stopping: state.stopping, | ||
| codex_thread_id: state.codex_thread_id, | ||
| binary_path: state.binary_path, | ||
| codex_home: state.codex_home, | ||
| account: DH.sanitize_account(state.account), | ||
| port_alive: DH.port_alive?(state.port), | ||
| next_request_id: state.next_id, | ||
| pending_count: map_size(state.pending), | ||
| pending_methods: DH.pending_methods(state.pending), | ||
| collab_receiver_count: map_size(state.collab_receiver_turns), | ||
| buffer_bytes: byte_size(state.buffer || "") | ||
| } | ||
|
|
||
| {:reply, {:ok, diagnostics}, state} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find /api/dev route declarations =="
rg -n -C4 --type=elixir '"/api/dev"|/api/dev|dev/(sessions|session|bridge|doctor|explain)'
echo
echo "== Inspect router/endpoint guards around dev routes =="
fd 'router.ex|endpoint.ex' apps/harness | xargs -r rg -n -C8 'scope|pipeline|plug|/api/dev|Mix.env|compile_env|basic_auth|authorize|require_auth'Repository: Ranvier-Technologies/t3code-OTP
Length of output: 8116
Add strict authentication and environment guards to /api/dev/* routes.
Lines 322–323 expose filesystem paths (binary_path, codex_home) in the diagnostics payload, which is surfaced via unprotected /api/dev/session/<thread_id> endpoint (see apps/harness/lib/harness_web/endpoint.ex line 96). These routes currently lack authentication, authorization, and environment checks—they are unconditionally accessible in all environments. Either add Mix.env() guards to disable these routes in non-dev environments, or apply authentication middleware (e.g., basic auth or API key validation) to the /api/dev/* route group.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/harness/lib/harness/providers/codex_session.ex` around lines 316 - 333,
The diagnostics payload in CodexSession (the diagnostics map in the code
building diagnostics) currently exposes sensitive filesystem fields (binary_path
and codex_home) and is returned via the unprotected /api/dev/* endpoint; update
the code to prevent leaking these values by either (A) adding environment guards
around the dev endpoints so /api/dev/* is only registered when Mix.env() == :dev
(adjust the harness_web endpoint/router registration), or (B) add authentication
middleware to the /api/dev/* route group (e.g., basic auth or API key check)
and/or remove binary_path and codex_home from the diagnostics map in the
CodexSession module (remove the binary_path and codex_home keys from diagnostics
or replace with sanitized/boolean flags), and ensure the reply path that returns
diagnostics (the {:reply, {:ok, diagnostics}, state} flow in CodexSession) only
emits non-sensitive fields unless the request is authenticated and running in
dev.
- sanitize_turn_state: include :turn_id alongside :id and :started_at
- inspect bridge/0: wrap get_snapshot in try/catch for SnapshotServer down
- claude send_turn: return {:error, ...} when spawn fails (was {:ok, ...})
- claude exit_status: capture maybe_complete_turn state outside unless block
- codex is_user_input: guard is_binary(method) before String.downcase
- check_bridge: status based on all processes, not just SnapshotServer
- mix tasks: Mix.raise on invalid input, add "beam" to doctor docs
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pure whitespace/formatting changes from mix format. No logic changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/harness/lib/harness/providers/claude_session.ex (1)
437-442: Shell argument escaping looks reasonable but has a subtle edge case.The escaping
String.replace(to_string(arg), "'", "'\\''")}correctly handles single quotes within arguments. However, if an argument contains a newline character, it could break the shell command since the escaped string is passed to/bin/sh -c.This is low risk since prompts with embedded newlines are unlikely in normal usage, but worth noting for defensive coding.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/harness/lib/harness/providers/claude_session.ex` around lines 437 - 442, The current code builds a single shell command string (escaped_args and shell_cmd) which can be broken by arguments containing newlines; instead, stop composing a shell string and invoke the binary with an args list so the runtime handles quoting safely. Replace the Enum.map_join/escaped_args and shell_cmd construction and call the executable directly (e.g. use System.cmd(state.binary_path, args, into: ..., stderr_to_stdout: ..., env: ..., stdin: :null) or equivalent) so arguments (including newlines) are passed intact; update the call site that used shell_cmd to use the System.cmd-style invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/harness/lib/harness/dev/doctor.ex`:
- Around line 88-99: The case handling around Task.yield(task,
`@check_timeout_ms`) || Task.shutdown(task) must explicitly handle {:exit, reason}
and accurately reflect the real timeout; update the case in the function (the
block using Task.yield/2, Task.shutdown/2 and parse_version) to match an {:exit,
reason} tuple and return an {:error, "Exited: #{inspect(reason)}"} (or similar)
instead of letting a match error happen, and either call Task.shutdown(task, 0)
or pass an explicit shutdown timeout and update the timeout message to report
the combined yield+shutdown duration (use `@check_timeout_ms` plus the shutdown
timeout) so the error text is correct. Ensure parse_version handling remains for
{:ok, {output, 0}} and other {:ok, {output, code}} branches.
---
Nitpick comments:
In `@apps/harness/lib/harness/providers/claude_session.ex`:
- Around line 437-442: The current code builds a single shell command string
(escaped_args and shell_cmd) which can be broken by arguments containing
newlines; instead, stop composing a shell string and invoke the binary with an
args list so the runtime handles quoting safely. Replace the
Enum.map_join/escaped_args and shell_cmd construction and call the executable
directly (e.g. use System.cmd(state.binary_path, args, into: ...,
stderr_to_stdout: ..., env: ..., stdin: :null) or equivalent) so arguments
(including newlines) are passed intact; update the call site that used shell_cmd
to use the System.cmd-style invocation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c47c5dab-b4cc-4e82-b12c-dd1a27a51ab2
📒 Files selected for processing (7)
apps/harness/lib/harness/dev/diagnostics_helpers.exapps/harness/lib/harness/dev/doctor.exapps/harness/lib/harness/dev/inspect.exapps/harness/lib/harness/providers/claude_session.exapps/harness/lib/harness/providers/codex_session.exapps/harness/lib/mix/tasks/harness.doctor.exapps/harness/lib/mix/tasks/harness.explain.ex
✅ Files skipped from review due to trivial changes (1)
- apps/harness/lib/mix/tasks/harness.explain.ex
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/harness/lib/mix/tasks/harness.doctor.ex
- apps/harness/lib/harness/dev/inspect.ex
- apps/harness/lib/harness/providers/codex_session.ex
Comment on lines
+88
to
+99
| case Task.yield(task, @check_timeout_ms) || Task.shutdown(task) do | ||
| {:ok, {output, 0}} -> | ||
| version = parse_version(output) | ||
| {:ok, version || String.trim(output)} | ||
|
|
||
| {:ok, {output, code}} -> | ||
| {:error, "Exit code #{code}: #{String.slice(String.trim(output), 0, 200)}"} | ||
|
|
||
| nil -> | ||
| {:error, "Version check timed out after #{@check_timeout_ms}ms"} | ||
| end | ||
| rescue |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Elixir, what are the default timeout and return semantics of Task.shutdown/2, and can Task.yield(task, 4000) || Task.shutdown(task) wait longer than 4000ms?
💡 Result:
In Elixir, Task.shutdown/2 has a default timeout of 5000 milliseconds (5 seconds). It unlinks from the task process, sends a :shutdown exit signal, waits up to the timeout for the task to terminate (checking via monitor), and then brutally kills it if needed. Its return semantics match Task.yield/2: {:ok, term} if a reply is received, {:exit, reason} if the task exits, or nil if the timeout is reached without a reply or DOWN message. The expression Task.yield(task, 4000) || Task.shutdown(task) can wait longer than 4000ms. Task.yield/2 waits up to 4000ms. If it returns nil (no reply in time), Task.shutdown/2 (with default 5000ms timeout) is called next, which can block for up to another 5000ms waiting for shutdown completion, for a total potential wait of up to 9000ms. This pattern ensures late replies (after yield timeout but before shutdown starts) are captured, as documented.
Citations:
- 1: https://hexdocs.pm/elixir/Task.html
- 2: https://hexdocs.pm/elixir/1.12.3/Task.html
- 3: https://github.com/elixir-lang/elixir/blob/main/lib/elixir/lib/task.ex
Missing {:exit, reason} pattern and timeout can exceed 4000ms.
The expression Task.yield(task, @check_timeout_ms) || Task.shutdown(task) on line 88 doesn't explicitly handle {:exit, reason} outcomes from either Task.yield/2 or Task.shutdown/2, which causes a match error if a task exits. Additionally, the total timeout can reach 9000ms (4000ms from yield + 5000ms default shutdown timeout), not just 4000ms as the error message indicates.
💡 Suggested fix
- case Task.yield(task, `@check_timeout_ms`) || Task.shutdown(task) do
+ case Task.yield(task, `@check_timeout_ms`) do
{:ok, {output, 0}} ->
version = parse_version(output)
{:ok, version || String.trim(output)}
{:ok, {output, code}} ->
{:error, "Exit code #{code}: #{String.slice(String.trim(output), 0, 200)}"}
+ {:exit, reason} ->
+ {:error, "Version check failed: #{inspect(reason)}"}
+
nil ->
+ _ = Task.shutdown(task, :brutal_kill)
{:error, "Version check timed out after #{`@check_timeout_ms`}ms"}
end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case Task.yield(task, @check_timeout_ms) || Task.shutdown(task) do | |
| {:ok, {output, 0}} -> | |
| version = parse_version(output) | |
| {:ok, version || String.trim(output)} | |
| {:ok, {output, code}} -> | |
| {:error, "Exit code #{code}: #{String.slice(String.trim(output), 0, 200)}"} | |
| nil -> | |
| {:error, "Version check timed out after #{@check_timeout_ms}ms"} | |
| end | |
| rescue | |
| case Task.yield(task, `@check_timeout_ms`) do | |
| {:ok, {output, 0}} -> | |
| version = parse_version(output) | |
| {:ok, version || String.trim(output)} | |
| {:ok, {output, code}} -> | |
| {:error, "Exit code #{code}: #{String.slice(String.trim(output), 0, 200)}"} | |
| {:exit, reason} -> | |
| {:error, "Version check failed: #{inspect(reason)}"} | |
| nil -> | |
| _ = Task.shutdown(task, :brutal_kill) | |
| {:error, "Version check timed out after #{`@check_timeout_ms`}ms"} | |
| end | |
| rescue |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/harness/lib/harness/dev/doctor.ex` around lines 88 - 99, The case
handling around Task.yield(task, `@check_timeout_ms`) || Task.shutdown(task) must
explicitly handle {:exit, reason} and accurately reflect the real timeout;
update the case in the function (the block using Task.yield/2, Task.shutdown/2
and parse_version) to match an {:exit, reason} tuple and return an {:error,
"Exited: #{inspect(reason)}"} (or similar) instead of letting a match error
happen, and either call Task.shutdown(task, 0) or pass an explicit shutdown
timeout and update the timeout message to report the combined yield+shutdown
duration (use `@check_timeout_ms` plus the shutdown timeout) so the error text is
correct. Ensure parse_version handling remains for {:ok, {output, 0}} and other
{:ok, {output, code}} branches.
- serverLayers: move Codex from Node SDK to harness adapter when harnessPort is configured (HARNESS_PROVIDERS now includes "codex") - Remove unused BunPtyAdapterLive/NodePtyAdapterLive imports - Fix duplicate claudeBinaryPath in AppSettingsSchema - Add cursor/opencode entries to all Record<ProviderKind, ...> types - Remove dead "starting" status comparisons (not in SessionPhase) - Fix thread.create auto-dispatch with required fields - Use Partial<Record> for composerProviderRegistry (has defaultEntry fallback) - Fix ProviderModelOptions index type in composerDraftStore All 7 packages pass typecheck. oxfmt clean. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… from vitest - session-logic.test: expect all 4 providers available (was 3 with cursor: false) - vite.config: exclude e2e/ directory from vitest (Playwright files crash vitest) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- vite.config: import defineConfig from vitest/config (test field
not valid in vite's UserConfigExport)
- doctor: handle {:exit, reason} from Task.yield/Task.shutdown
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/components/Sidebar.logic.ts (1)
127-134:⚠️ Potential issue | 🟠 MajorKeep
"starting"threads in the Connecting pill.Because the next branch at Line 136 can still return
"Plan Ready", removing the"starting"fallback here lets a booting plan thread look actionable before the provider is actually connected.Suggested fix
- if (thread.session?.status === "connecting") { + if ( + thread.session?.status === "connecting" || + thread.session?.status === "starting" + ) { return { label: "Connecting",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/Sidebar.logic.ts` around lines 127 - 134, Update the status check that returns the "Connecting" pill to also treat a session status of "starting" as connecting: in the logic that inspects thread.session?.status (the branch that returns { label: "Connecting", colorClass: "...", dotClass: "...", pulse: true }), change the condition from only checking "connecting" to include "starting" so booting plan threads show the Connecting pill instead of falling through to the later "Plan Ready" branch.apps/web/src/composerDraftStore.ts (1)
190-197:⚠️ Potential issue | 🟠 MajorKeep the provider/options pairing type-safe.
ProviderModelOptions[keyof ProviderModelOptions]lets invalid pairs like("cursor", codexOptions)or("opencode", claudeOptions)type-check, even thoughProviderModelOptionsonly has keys forcodexandclaudeAgent. WhensetProviderModelOptionsorreplaceProviderModelOptionsreceives an invalid provider,normalizeProviderModelOptionssilently returnsnulland discards the options instead of failing at compile time. Constrain theproviderparameter toExtract<ProviderKind, keyof ProviderModelOptions>(or aProviderWithModelOptionsalias) and make it generic sonextProviderOptionsis indexed by the actual provider:Suggested fix
+type ProviderWithModelOptions = Extract<ProviderKind, keyof ProviderModelOptions>;- setProviderModelOptions: ( - threadId: ThreadId, - provider: ProviderKind, - nextProviderOptions: ProviderModelOptions[keyof ProviderModelOptions] | null | undefined, - options?: { - persistSticky?: boolean; - }, - ) => void; + setProviderModelOptions: <TProvider extends ProviderWithModelOptions>( + threadId: ThreadId, + provider: TProvider, + nextProviderOptions: ProviderModelOptions[TProvider] | null | undefined, + options?: { + persistSticky?: boolean; + }, + ) => void;-function replaceProviderModelOptions( +function replaceProviderModelOptions<TProvider extends ProviderWithModelOptions>( currentModelOptions: ProviderModelOptions | null | undefined, - provider: ProviderKind, - nextProviderOptions: ProviderModelOptions[keyof ProviderModelOptions] | null | undefined, + provider: TProvider, + nextProviderOptions: ProviderModelOptions[TProvider] | null | undefined, ): ProviderModelOptions | null {Also applies to: 420-437
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/composerDraftStore.ts` around lines 190 - 197, The parameter typing allows invalid provider/options pairs; make the functions type-safe by constraining the provider to only keys present in ProviderModelOptions and indexing options by that provider: change setProviderModelOptions and replaceProviderModelOptions to be generic over P extends Extract<ProviderKind, keyof ProviderModelOptions> (or introduce a ProviderWithModelOptions alias) and type nextProviderOptions as ProviderModelOptions[P] | null | undefined; update normalizeProviderModelOptions to accept the same constrained provider type so it returns the correctly indexed options type instead of silently returning null for mismatches.
🧹 Nitpick comments (1)
apps/server/src/serverLayers.ts (1)
95-99: Update the block comment abovemakeServerProviderLayer().The inline comment here matches the new routing, but the function docblock at Lines 63-72 still says Codex always uses the Node SDK and that harness only covers Cursor/OpenCode. Leaving both versions in the file makes the provider routing rules contradictory.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/serverLayers.ts` around lines 95 - 99, Docblock above makeServerProviderLayer() is outdated and contradicts the inline comment; update the function-level comment to reflect that harnessEnabled (serverConfig.harnessPort) enables the Elixir harness for HARNESS_PROVIDERS = ["codex","cursor","opencode"], while Claude always uses the Node SDK adapter (Agent SDK), and ensure the docblock's routing rules match the inline comment and variables harnessEnabled, HARNESS_PROVIDERS, and makeServerProviderLayer().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts`:
- Around line 888-895: The code in ProviderRuntimeIngestion that builds the
auto-created thread uses multiple `as any` casts and sets `projectId: ""`,
violating the TrimmedNonEmptyString/ProjectId contract; remove the `as any`
bypasses and stop using an empty-string placeholder. Fix by updating the thread
creation payload (the object passed to the `thread.create` command) so
`projectId` is either a valid ProjectId derived from event/context or explicitly
null, and update the `thread.create` command schema/handler to accept
`projectId: null | ProjectId` (or alternatively throw/fail early if no valid
project id can be derived). Ensure you adjust types of `title`, `model`, and
other fields to their proper types instead of `as any` (e.g., derive/cast
`model` safely) and update ProviderRuntimeIngestion and corresponding command
schema/handlers to preserve TypeScript safety and domain invariants.
In `@apps/web/src/session-logic.ts`:
- Around line 868-872: derivePhase currently maps session.status "connecting" to
"connecting" but treats "starting" as "ready", causing UI to unlock prematurely;
update derivePhase (used with ThreadSession and returning SessionPhase) to treat
both "starting" and "connecting" as the "connecting" phase (i.e., check for
session.status === "starting" || session.status === "connecting" and return
"connecting"), while keeping "running" -> "running" and "closed"/null ->
"disconnected".
---
Outside diff comments:
In `@apps/web/src/components/Sidebar.logic.ts`:
- Around line 127-134: Update the status check that returns the "Connecting"
pill to also treat a session status of "starting" as connecting: in the logic
that inspects thread.session?.status (the branch that returns { label:
"Connecting", colorClass: "...", dotClass: "...", pulse: true }), change the
condition from only checking "connecting" to include "starting" so booting plan
threads show the Connecting pill instead of falling through to the later "Plan
Ready" branch.
In `@apps/web/src/composerDraftStore.ts`:
- Around line 190-197: The parameter typing allows invalid provider/options
pairs; make the functions type-safe by constraining the provider to only keys
present in ProviderModelOptions and indexing options by that provider: change
setProviderModelOptions and replaceProviderModelOptions to be generic over P
extends Extract<ProviderKind, keyof ProviderModelOptions> (or introduce a
ProviderWithModelOptions alias) and type nextProviderOptions as
ProviderModelOptions[P] | null | undefined; update normalizeProviderModelOptions
to accept the same constrained provider type so it returns the correctly indexed
options type instead of silently returning null for mismatches.
---
Nitpick comments:
In `@apps/server/src/serverLayers.ts`:
- Around line 95-99: Docblock above makeServerProviderLayer() is outdated and
contradicts the inline comment; update the function-level comment to reflect
that harnessEnabled (serverConfig.harnessPort) enables the Elixir harness for
HARNESS_PROVIDERS = ["codex","cursor","opencode"], while Claude always uses the
Node SDK adapter (Agent SDK), and ensure the docblock's routing rules match the
inline comment and variables harnessEnabled, HARNESS_PROVIDERS, and
makeServerProviderLayer().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8ccb680-665a-4184-8ec3-ada3c5d589da
📒 Files selected for processing (23)
apps/harness/config/runtime.exsapps/harness/lib/harness/harness_event.exapps/harness/lib/harness/metrics.exapps/harness/lib/harness/model_discovery.exapps/harness/lib/harness/projector.exapps/harness/lib/harness/providers/mock_session.exapps/harness/lib/harness/snapshot.exapps/harness/lib/harness_web/harness_channel.exapps/harness/test/e2e_channel_test.exsapps/harness/test/harness/projector_test.exsapps/harness/test/harness/snapshot_server_test.exsapps/harness/test/live_e2e_test.exsapps/server/src/orchestration/Layers/ProviderRuntimeIngestion.tsapps/server/src/serverLayers.tsapps/web/src/appSettings.tsapps/web/src/components/Sidebar.logic.tsapps/web/src/components/chat/ProviderModelPicker.browser.tsxapps/web/src/components/chat/composerProviderRegistry.tsxapps/web/src/composerDraftStore.tsapps/web/src/routes/_chat.settings.tsxapps/web/src/session-logic.tsscripts/stress-test-memory-leak.tsscripts/stress-test-real-subagent.ts
💤 Files with no reviewable changes (1)
- apps/web/src/appSettings.ts
✅ Files skipped from review due to trivial changes (14)
- apps/harness/config/runtime.exs
- apps/harness/test/harness/snapshot_server_test.exs
- apps/harness/lib/harness/metrics.ex
- scripts/stress-test-real-subagent.ts
- scripts/stress-test-memory-leak.ts
- apps/harness/test/e2e_channel_test.exs
- apps/harness/test/live_e2e_test.exs
- apps/harness/lib/harness/projector.ex
- apps/harness/test/harness/projector_test.exs
- apps/harness/lib/harness/snapshot.ex
- apps/harness/lib/harness/model_discovery.ex
- apps/harness/lib/harness/harness_event.ex
- apps/harness/lib/harness/providers/mock_session.ex
- apps/harness/lib/harness_web/harness_channel.ex
Comment on lines
868
to
872
| export function derivePhase(session: ThreadSession | null): SessionPhase { | ||
| if (!session || session.status === "closed") return "disconnected"; | ||
| if (session.status === "connecting" || session.status === "starting") return "connecting"; | ||
| if (session.status === "connecting") return "connecting"; | ||
| if (session.status === "running") return "running"; | ||
| return "ready"; |
There was a problem hiding this comment.
Preserve "starting" as a connecting phase.
derivePhase() still feeds apps/web/src/components/ChatView.tsx, Line 625. With this condition narrowed, a session that is still booting now reports "ready" and can unlock the wrong UI before it is actually connected.
Suggested fix
- if (session.status === "connecting") return "connecting";
+ if (session.status === "connecting" || session.status === "starting") {
+ return "connecting";
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function derivePhase(session: ThreadSession | null): SessionPhase { | |
| if (!session || session.status === "closed") return "disconnected"; | |
| if (session.status === "connecting" || session.status === "starting") return "connecting"; | |
| if (session.status === "connecting") return "connecting"; | |
| if (session.status === "running") return "running"; | |
| return "ready"; | |
| export function derivePhase(session: ThreadSession | null): SessionPhase { | |
| if (!session || session.status === "closed") return "disconnected"; | |
| if (session.status === "connecting" || session.status === "starting") { | |
| return "connecting"; | |
| } | |
| if (session.status === "running") return "running"; | |
| return "ready"; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/session-logic.ts` around lines 868 - 872, derivePhase currently
maps session.status "connecting" to "connecting" but treats "starting" as
"ready", causing UI to unlock prematurely; update derivePhase (used with
ThreadSession and returning SessionPhase) to treat both "starting" and
"connecting" as the "connecting" phase (i.e., check for session.status ===
"starting" || session.status === "connecting" and return "connecting"), while
keeping "running" -> "running" and "closed"/null -> "disconnected".
…xEventBase - ProviderHealth: checkClaudeProviderStatus now runs `claude auth status` after version check, with JSON and text output parsing for auth state - ProviderHealth: add displayName option to checkBinaryProviderStatus for human-readable error messages - codexEventMapping: codexEventBase prefers event-level turnId/itemId/requestId (set by adapter manager) over msg-level ids (which may be child/sub-agent ids) Fixes 6 failing tests in ProviderHealth.test.ts and CodexAdapter.test.ts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- ProviderRuntimeIngestion: use ProjectId.makeUnsafe, TrimmedNonEmptyString.makeUnsafe instead of `as any` casts for thread.create dispatch - serverLayers: update docblock to reflect Codex routing through harness Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 28, 2026
ranvier2d2
added a commit
that referenced
this pull request
Mar 30, 2026
…handler - Guard against double subscription: lease_and_subscribe and subscribe_initial check if thread_id is already in subscribers before creating a new monitor, preventing ref count leak from duplicate monitors (#5). - Handle :runtime_sse_degraded in session: emit session/degraded event when runtime SSE reconnect is exhausted, instead of silently dropping the message (#8). - Add Logger.debug to event_relevant? catch-all so unrecognized SSE event shapes are logged instead of silently dropped (#10). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ranvier2d2
added a commit
that referenced
this pull request
Mar 30, 2026
…handler - Guard against double subscription: lease_and_subscribe and subscribe_initial check if thread_id is already in subscribers before creating a new monitor, preventing ref count leak from duplicate monitors (#5). - Handle :runtime_sse_degraded in session: emit session/degraded event when runtime SSE reconnect is exhausted, instead of silently dropping the message (#8). - Add Logger.debug to event_relevant? catch-all so unrecognized SSE event shapes are logged instead of silently dropped (#10). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ranvier2d2
added a commit
that referenced
this pull request
Mar 30, 2026
…handler - Guard against double subscription: lease_and_subscribe and subscribe_initial check if thread_id is already in subscribers before creating a new monitor, preventing ref count leak from duplicate monitors (#5). - Handle :runtime_sse_degraded in session: emit session/degraded event when runtime SSE reconnect is exhausted, instead of silently dropping the message (#8). - Add Logger.debug to event_relevant? catch-all so unrecognized SSE event shapes are logged instead of silently dropped (#10). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Harness.Dev.*) — transport-agnostic logic for inspect, doctor, and explain/api/dev/*endpoints — live runtime inspection viacurlfor coding agentsmix harness.explainandmix harness.doctorfor local-safe CLI diagnostics:get_diagnosticshandler on all 4 provider sessions exposing safe state subsetArchitecture
Logic lives in canonical modules. HTTP routes and Mix tasks are thin adapters:
Endpoints
GET /api/dev/sessionsGET /api/dev/session/:idGET /api/dev/bridgeGET /api/dev/doctorGET /api/dev/doctor/:targetGET /api/dev/explain/topicsGET /api/dev/explain/:topicTest plan
mix compile --warnings-as-errors— cleanmix test test/harness/dev/— 15 tests, 0 failuresmix credo— clean on new filescurl -s localhost:4321/api/dev/doctor | jq .against running harnesscurl -s localhost:4321/api/dev/session/<id> | jq .🤖 Generated with Claude Code
Summary by CodeRabbit