feat(opencode): live attached-UI viewer in the dashboard (show_attached_ui) - #307
feat(opencode): live attached-UI viewer in the dashboard (show_attached_ui)#307Wrenbjor wants to merge 4 commits into
Conversation
New seam methods the attached-UI viewer needs: window_exists, capture_pane (was missing -t, capturing whatever pane was current), set_session_environment (credentials without ps-visible argv), and resize_window; the parked-window exit banner becomes a shared PARKED_EXIT_BANNER constant so observers can detect a died command. Also fixes parked windows on Debian-family hosts: `read -r` with no variable is a bashism dash rejects, so every parked window closed the instant its command exited instead of holding the exit status. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With `[adapter] show_attached_ui = true`, the opencode-http adapter runs `opencode attach` in a detached, parked tmux window alongside the headless server, so the live session is observable like the tmux adapters' panes. - authenticated via the tmux session environment (the per-session OPENCODE_SERVER_PASSWORD never reaches argv or viewer.json) - verified live before viewer.json persists; an immediately-exiting viewer is reported (exit status + output) in viewer-error.log plus a viewer-create-failed journal event, never recorded as a live target - idempotent across retries/resume; teardown closes viewer first - engine control-flow exceptions (RunStopped/RunPaused) landing inside creation propagate after cleanup instead of breaking the stop path - covered by unit tests and real-tmux integration tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Log tab shows the viewer's pane capture (rendered through pyte with LNM set — capture rows are bare-\n separated, without it every row starts where the previous ended) and falls back to the logfile when no viewer is recorded. The viewer window is kept resized to the displayed pane's content area so the attached TUI redraws at the size actually shown. `attach` and the TUI `a` action prefer the viewer window. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WalkthroughAdds an optional ChangesViewer contracts and launch
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bmad_loop/tui/screens/dashboard.py (1)
793-817: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winJournal "jump to log position" silently stalls forever once a viewer is active.
snap.log_task(and thereforeself._displayed_log_task) is only assigned inside theif not snap.viewer_available:fallback (line 852); while a viewer is showing,_displayed_log_taskstaysNoneevery tick._scroll_log_to()bails wheneverjump[0] != self._displayed_log_task(dashboard.py line ~397, not shown in this diff), so a journal-entry click that sets_pending_jumpwhile the viewer is active can never match and is silently re-attempted on every poll indefinitely — with no notification to the user, unlike the "no log position recorded" case which doesself.notify(...).Consider surfacing feedback (e.g.
self.notify("log position not available while the live viewer is showing", severity="warning")) when a jump is requested but the Log tab is showing a viewer, instead of leaving it in silent limbo.Also applies to: 943-943
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bmad_loop/tui/screens/dashboard.py` around lines 793 - 817, Handle pending journal log jumps while a live viewer is active instead of leaving them retrying indefinitely. In the dashboard update flow around ViewerView selection and the pending-jump handling near _scroll_log_to, detect when the Log tab is showing a viewer and notify the user with a warning that the log position is unavailable; clear or otherwise resolve the pending jump so it is not re-attempted every poll, while preserving the existing behavior for normal logfile scrolling and missing log positions.
🧹 Nitpick comments (10)
src/bmad_loop/adapters/viewer_launch.py (2)
248-257: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
_is_control_flowis annotatedExceptionbut the caller passes aBaseException.Line 230 calls it from
except BaseException as exc. Widen the annotation toBaseExceptionso the type matches the reachable call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bmad_loop/adapters/viewer_launch.py` around lines 248 - 257, Update the _is_control_flow function signature to accept BaseException instead of Exception, matching the BaseException value passed by its caller while preserving its existing lazy import and isinstance behavior.
8-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocstring sentence is missing a conjunction and quotes two different window-name forms.
"Idempotency: a viewer already exists for the given session name + window name (
<session_name>-viewer,bmad-loop-<run_id>-viewer)" — add "when"/"if", and keep only the actual name produced by_viewer_window_name.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bmad_loop/adapters/viewer_launch.py` around lines 8 - 11, Update the idempotency sentence in the viewer launch docstring to add “when” or “if” for grammatical clarity, and document only the window-name format actually produced by _viewer_window_name, removing the alternate form.src/bmad_loop/tui/data.py (1)
775-796: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant except tuple and a stale comment.
except (MultiplexerError, OSError, Exception)is justexcept Exception— the first two are subsumed. And the comment on Lines 788-790 says the screen isreset()before feeding, but the code builds a freshHistoryScreeneach refresh instead.♻️ Proposed cleanup
try: raw = self._mux.capture_pane(self._target, start_line=self._start_line) - except (MultiplexerError, OSError, Exception): + except Exception: # Pane may have been killed or mux transport failed. return None @@ - # Full-screen viewer: each capture is a complete snapshot, so - # reset() the screen before feeding the new data so the output - # replaces the previous frame rather than appending. + # Full-screen viewer: each capture is a complete snapshot, so a fresh + # screen is built per refresh — the output replaces the previous frame + # rather than appending.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bmad_loop/tui/data.py` around lines 775 - 796, Clean up the exception handler around _mux.capture_pane by replacing the redundant exception tuple with except Exception. Update the nearby full-screen viewer comment to state that a fresh pyte.HistoryScreen is created for each capture, rather than claiming reset() is called.tests/test_viewer_integration.py (1)
54-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSocket dir created with
mkdtempis never removed.Each test leaves a
blvt-*directory under the system temp dir. Clean it up after the server is killed.♻️ Proposed cleanup
+ import shutil sock_dir = Path(tempfile.mkdtemp(prefix="blvt-")) monkeypatch.setenv("TMUX_TMPDIR", str(sock_dir)) monkeypatch.delenv("TMUX", raising=False) get_multiplexer.cache_clear() yield subprocess.run(["tmux", "kill-server"], capture_output=True) get_multiplexer.cache_clear() + shutil.rmtree(sock_dir, ignore_errors=True)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_viewer_integration.py` around lines 54 - 60, Update the fixture cleanup around the tmux server teardown to remove the temporary sock_dir created by tempfile.mkdtemp after killing the server. Preserve the existing cache clearing and ensure cleanup occurs even if teardown commands encounter an error.tests/test_viewer.py (1)
177-200: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe policy/profile/run-dir setup block is copy-pasted across ~8 tests.
Extract a fixture (e.g.
opencode_adapter(sandbox)) returning a configuredOpencodeHttpAdapter; each test then keeps only what it varies.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_viewer.py` around lines 177 - 200, Extract the repeated policy, profile, run-directory, and OpencodeHttpAdapter construction into a shared pytest fixture such as opencode_adapter, using the existing sandbox/tmp_path setup. Update the affected tests to depend on this fixture and retain only test-specific variations, preserving the current adapter configuration and directory initialization.src/bmad_loop/engine.py (1)
2288-2292: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueJournal entry repeats per session for a viewer created once.
viewer_configpersists on the adapter across tasks, soviewer-createdis appended after everyadapter.run(...)with the same window. Consider journaling only on transition (track the last-journalled window).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bmad_loop/engine.py` around lines 2288 - 2292, Update the post-run viewer journaling logic near adapter.run to record viewer-created only when the viewer_window differs from the previously journaled window. Track the last-journaled window across sessions, preserve journaling for a newly created or changed non-empty window, and avoid duplicate entries when the adapter reuses the same viewer_config.src/bmad_loop/adapters/opencode_http.py (1)
1054-1076: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid duplicating
viewer.jsonparsing in the adapter.
load_viewer_configis only defined here, whiletui/data.read_viewer_configalready readsviewer.json(adapter-filtered or any window). Use the shared TUI reader or move the parsing intoviewer_launch/adapter utilities instead of adding a second implementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bmad_loop/adapters/opencode_http.py` around lines 1054 - 1076, The load_viewer_config method duplicates viewer.json parsing already provided by tui.data.read_viewer_config. Replace its local file-reading and entry-selection logic with the shared reader, preserving adapter filtering and the existing ViewerConfig | None behavior; otherwise relocate the parsing to the established viewer_launch/adapter utility rather than maintaining a second implementation.src/bmad_loop/adapters/multiplexer.py (2)
229-244: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
capture_pane's default contradicts its own docstring.The docstring says the base returns
"""when the pane is missing or the backend is unavailable," but the actual default body unconditionallyraise NotImplementedError. Current callers happen to tolerate this (viewer_launch.pycatchesNotImplementedErrorexplicitly;ViewerView.refreshcatches a bareException), so there's no live bug today — but a future backend author reading only this docstring could reasonably assume the seam default already degrades gracefully to"".♻️ Suggested fix: align default body with the documented contract
def capture_pane(self, target: str, start_line: int = -100, end_line: int | None = None) -> str: """Capture the contents of a pane as a text string. Equivalent to ``tmux capture-pane -p -e -J -S <start> -E <end> -t <target>``. Args: target: pane/window session target (e.g. ``=bmad-loop-abc:viewer``). start_line: first line to capture (negative = lines above the visible pane; ``-100`` ≈ 100 lines of history). end_line: last line to capture (``None`` = current line). Returns the captured text (including escape sequences) or empty string when the pane is missing or the backend is unavailable. Callers parse the text through pyte to produce a styled render.""" - raise NotImplementedError + return ""🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bmad_loop/adapters/multiplexer.py` around lines 229 - 244, Update the base adapter’s capture_pane method to return an empty string by default instead of raising NotImplementedError, matching its documented unavailable-backend contract. Preserve the method signature and existing backend overrides.
215-222: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
set_viewer_option's docstring doesn't match how it's actually called.The docstring states
window_id"is the backend-native id returned by :meth:TerminalMultiplexer.new_window," butviewer_launch.pycalls it with the seam-canonical=session:windowtarget token (the native id fromnew_parked_windowis never captured). It works because tmux's-taccepts both forms, but the doc is misleading for anyone implementing a new backend.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bmad_loop/adapters/multiplexer.py` around lines 215 - 222, The set_viewer_option docstring incorrectly describes window_id as the native identifier from TerminalMultiplexer.new_window. Update the docstring to state that callers provide the seam-canonical =session:window target token used by viewer_launch.py, and remove the misleading new_window reference while preserving the best-effort behavior description.src/bmad_loop/tui/screens/dashboard.py (1)
818-841: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winViewer
capture_pane(a realtmux capture-panesubprocess call) runs every poll tick regardless of Log-tab visibility.Only the
resize_windowcall is gated bydims(which is(0, 0)while the Log tab is hidden);ctx.viewer_view.refresh()itself is unconditional wheneverctx.viewer_view is not None, so a subprocess exec fires roughly once per second even while the user is on the Journal/Attention tab. The prior logfile-fallback path only did a cheap file stat/read each tick — this introduces a recurring process spawn that isn't gated the same way.Since
_log_pane_dimsis already threaded from_apply(UI thread) to the poll worker, consider also publishing whether the Log tab is currently active and skippingrefresh()/resize_windowwhen it isn't.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bmad_loop/tui/screens/dashboard.py` around lines 818 - 841, Gate the viewer update path in the dashboard poll flow on whether the Log tab is currently active, not merely whether ctx.viewer_view exists. Publish the active-tab state alongside _log_pane_dims from _apply to the poll worker, and in the viewer block skip ctx.viewer_view.refresh() and mux.resize_window when inactive while preserving the existing snapshot behavior for visible and unavailable viewers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/bmad_loop/adapters/base.py`:
- Around line 186-188: Align the viewer-close API between the base adapter
method close_viewer_window and OpencodeHttpAdapter.close_viewer_window: use the
same parameter shape and type in both declarations, ensuring callers through the
base contract do not pass a ViewerConfig to a string-typed parameter. Update the
implementation and any related call sites consistently, while preserving the
existing teardown behavior.
In `@src/bmad_loop/adapters/opencode_http.py`:
- Line 1019: Update _make_viewer_config so it stores the generated configuration
in the same _viewer_config attribute consumed by _close_viewer, while preserving
the public viewer_config assignment if required by existing callers. Ensure
direct callers of _make_viewer_config leave teardown with the correct
configuration without relying on start_session.
- Around line 1037-1040: The adapter never persists the current task ID because
_current_task_id is not initialized or set. In
src/bmad_loop/adapters/opencode_http.py lines 1037-1040, initialize
_current_task_id alongside _viewer_config in __init__ and add
_set_current_task_id(self, task_id: str) to store it; in src/bmad_loop/engine.py
lines 2284-2286, remove the silent getattr fallback or enforce the setter
contract so missing or renamed setters fail loudly.
- Around line 1088-1113: Update ensure_viewer_window to use the same internal
viewer-creation path as _make_viewer_config, ensuring the resulting ViewerConfig
is assigned to self.viewer_config and passed to _persist_viewer_config. Preserve
the existing session password lookup, control-flow exception handling, and empty
ViewerConfig fallback while routing creation through the shared path so
teardown, dashboard state, and journaling remain synchronized.
In `@src/bmad_loop/adapters/viewer_launch.py`:
- Around line 146-148: The existing-viewer reuse check in _ensure_viewer
incorrectly reuses windows keyed only by run_id across tasks. Update
_find_existing_viewer or the surrounding _ensure_viewer logic to key viewer
windows by session/task identity, or validate that the found window’s recorded
session matches the current session before returning _success(); otherwise
create a new viewer attached to the current server and session.
In `@src/bmad_loop/tui/launch.py`:
- Around line 30-46: Replace the hardcoded viewer window name and duplicated
lookup logic in _find_view_window with the shared public helper from
viewer_launch. Promote _viewer_window_name to a public helper if needed, import
it at module scope, and reuse viewer_launch.viewer_window_exists so attach
routing follows the producer’s naming behavior.
- Around line 194-197: Update the viewer-window branch in the attach routing
logic around _find_view_window to return the viewer target only when no pending
interactive decision exists; otherwise allow execution to continue to the
existing pending-decision ctl route. Preserve the current viewer behavior when
the sweep is not blocked.
In `@tests/test_viewer.py`:
- Around line 362-371: Replace or remove test_viewer_log_path_format so it
validates the actual log path produced by the viewer adapter rather than
constructing expected_log_path locally. Exercise the production viewer flow and
assert the adapter’s written path matches the task log format; remove the unused
sandbox fixture if it is no longer needed.
- Around line 373-419: Update test_viewer_config_stored_to_json to assert that
ensure_viewer_window persists viewer.json: after calling
OpencodeHttpAdapter.ensure_viewer_window, verify the file exists in run_dir and
its parsed contents include the expected session identifier and viewer URL. Also
update ensure_viewer_window to invoke _persist_viewer_config so the test
exercises the promised persistence behavior.
---
Outside diff comments:
In `@src/bmad_loop/tui/screens/dashboard.py`:
- Around line 793-817: Handle pending journal log jumps while a live viewer is
active instead of leaving them retrying indefinitely. In the dashboard update
flow around ViewerView selection and the pending-jump handling near
_scroll_log_to, detect when the Log tab is showing a viewer and notify the user
with a warning that the log position is unavailable; clear or otherwise resolve
the pending jump so it is not re-attempted every poll, while preserving the
existing behavior for normal logfile scrolling and missing log positions.
---
Nitpick comments:
In `@src/bmad_loop/adapters/multiplexer.py`:
- Around line 229-244: Update the base adapter’s capture_pane method to return
an empty string by default instead of raising NotImplementedError, matching its
documented unavailable-backend contract. Preserve the method signature and
existing backend overrides.
- Around line 215-222: The set_viewer_option docstring incorrectly describes
window_id as the native identifier from TerminalMultiplexer.new_window. Update
the docstring to state that callers provide the seam-canonical =session:window
target token used by viewer_launch.py, and remove the misleading new_window
reference while preserving the best-effort behavior description.
In `@src/bmad_loop/adapters/opencode_http.py`:
- Around line 1054-1076: The load_viewer_config method duplicates viewer.json
parsing already provided by tui.data.read_viewer_config. Replace its local
file-reading and entry-selection logic with the shared reader, preserving
adapter filtering and the existing ViewerConfig | None behavior; otherwise
relocate the parsing to the established viewer_launch/adapter utility rather
than maintaining a second implementation.
In `@src/bmad_loop/adapters/viewer_launch.py`:
- Around line 248-257: Update the _is_control_flow function signature to accept
BaseException instead of Exception, matching the BaseException value passed by
its caller while preserving its existing lazy import and isinstance behavior.
- Around line 8-11: Update the idempotency sentence in the viewer launch
docstring to add “when” or “if” for grammatical clarity, and document only the
window-name format actually produced by _viewer_window_name, removing the
alternate form.
In `@src/bmad_loop/engine.py`:
- Around line 2288-2292: Update the post-run viewer journaling logic near
adapter.run to record viewer-created only when the viewer_window differs from
the previously journaled window. Track the last-journaled window across
sessions, preserve journaling for a newly created or changed non-empty window,
and avoid duplicate entries when the adapter reuses the same viewer_config.
In `@src/bmad_loop/tui/data.py`:
- Around line 775-796: Clean up the exception handler around _mux.capture_pane
by replacing the redundant exception tuple with except Exception. Update the
nearby full-screen viewer comment to state that a fresh pyte.HistoryScreen is
created for each capture, rather than claiming reset() is called.
In `@src/bmad_loop/tui/screens/dashboard.py`:
- Around line 818-841: Gate the viewer update path in the dashboard poll flow on
whether the Log tab is currently active, not merely whether ctx.viewer_view
exists. Publish the active-tab state alongside _log_pane_dims from _apply to the
poll worker, and in the viewer block skip ctx.viewer_view.refresh() and
mux.resize_window when inactive while preserving the existing snapshot behavior
for visible and unavailable viewers.
In `@tests/test_viewer_integration.py`:
- Around line 54-60: Update the fixture cleanup around the tmux server teardown
to remove the temporary sock_dir created by tempfile.mkdtemp after killing the
server. Preserve the existing cache clearing and ensure cleanup occurs even if
teardown commands encounter an error.
In `@tests/test_viewer.py`:
- Around line 177-200: Extract the repeated policy, profile, run-directory, and
OpencodeHttpAdapter construction into a shared pytest fixture such as
opencode_adapter, using the existing sandbox/tmp_path setup. Update the affected
tests to depend on this fixture and retain only test-specific variations,
preserving the current adapter configuration and directory initialization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 51afa8ea-20d1-4ebf-9e9c-10ee0ad1d7d1
📒 Files selected for processing (17)
CHANGELOG.mdREADME.mdsrc/bmad_loop/adapters/base.pysrc/bmad_loop/adapters/multiplexer.pysrc/bmad_loop/adapters/opencode_http.pysrc/bmad_loop/adapters/tmux_base.pysrc/bmad_loop/adapters/viewer_launch.pysrc/bmad_loop/data/settings/core.tomlsrc/bmad_loop/engine.pysrc/bmad_loop/policy.pysrc/bmad_loop/tui/data.pysrc/bmad_loop/tui/launch.pysrc/bmad_loop/tui/screens/dashboard.pytests/test_multiplexer.pytests/test_tui_data.pytests/test_viewer.pytests/test_viewer_integration.py
| def close_viewer_window(self, config: ViewerConfig) -> None: | ||
| """Teardown the viewer window; a no-op when window is None.""" | ||
| pass |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Base close_viewer_window signature does not match the implementing adapter.
The base declares close_viewer_window(self, config: ViewerConfig), but OpencodeHttpAdapter.close_viewer_window(self, target: str | None = None) (see src/bmad_loop/adapters/opencode_http.py Lines 1084-1086) takes an optional string and ignores it. Any future caller written against the base contract would pass a ViewerConfig into a parameter typed str | None. Align on one shape — either both take ViewerConfig, or both take no argument and read the adapter's own state.
♻️ Suggested alignment
- def close_viewer_window(self, config: ViewerConfig) -> None:
- """Teardown the viewer window; a no-op when window is None."""
- pass
+ def close_viewer_window(self, config: ViewerConfig | None = None) -> None:
+ """Teardown the viewer window; a no-op when window is None.
+
+ ``config`` defaults to the adapter's own recorded ``viewer_config``."""
+ return None📝 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.
| def close_viewer_window(self, config: ViewerConfig) -> None: | |
| """Teardown the viewer window; a no-op when window is None.""" | |
| pass | |
| def close_viewer_window(self, config: ViewerConfig | None = None) -> None: | |
| """Teardown the viewer window; a no-op when window is None. | |
| ``config`` defaults to the adapter's own recorded ``viewer_config``.""" | |
| return None |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 186-188: CodingCLIAdapter.close_viewer_window is an empty method in an abstract base class, but has no abstract decorator
(B027)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/bmad_loop/adapters/base.py` around lines 186 - 188, Align the
viewer-close API between the base adapter method close_viewer_window and
OpencodeHttpAdapter.close_viewer_window: use the same parameter shape and type
in both declarations, ensuring callers through the base contract do not pass a
ViewerConfig to a string-typed parameter. Update the implementation and any
related call sites consistently, while preserving the existing teardown
behavior.
| if _viewer_control_flow(exc): | ||
| raise # the engine's stop/pause landed here — never swallow it | ||
| vc = ViewerConfig() | ||
| self.viewer_config = vc |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
_make_viewer_config writes the public viewer_config but teardown reads _viewer_config.
Line 1019 sets self.viewer_config; _close_viewer (Line 1081) reads self._viewer_config. Today this only works because start_session (Line 448) happens to assign the return value to _viewer_config as well. Set both here (or collapse to a single attribute) so teardown can't silently miss the window when another caller uses this method.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/bmad_loop/adapters/opencode_http.py` at line 1019, Update
_make_viewer_config so it stores the generated configuration in the same
_viewer_config attribute consumed by _close_viewer, while preserving the public
viewer_config assignment if required by existing callers. Ensure direct callers
of _make_viewer_config leave teardown with the correct configuration without
relying on start_session.
| entries = [e for e in entries if e.get("adapter") != self.name] + [ | ||
| { | ||
| "adapter": self.name, | ||
| "task_id": getattr(self, "_current_task_id", ""), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
_current_task_id is read but never written: no adapter implements _set_current_task_id. The engine invokes the setter through getattr(..., lambda tid: None), which silently no-ops when the method is absent, and the adapter's persist path then falls back to "" — so every viewer.json entry records an empty task_id.
src/bmad_loop/adapters/opencode_http.py#L1037-L1040: add a real_set_current_task_id(self, task_id: str)that storesself._current_task_id, and initialize it in__init__alongside_viewer_config.src/bmad_loop/engine.py#L2284-L2286: once the adapter defines the method, drop thegetattrfallback (or keep it but assert the contract in a test) so a future rename fails loudly instead of silently writing empty ids.
#!/bin/bash
rg -n '_set_current_task_id|_current_task_id' -C2 src tests📍 Affects 2 files
src/bmad_loop/adapters/opencode_http.py#L1037-L1040(this comment)src/bmad_loop/engine.py#L2284-L2286
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/bmad_loop/adapters/opencode_http.py` around lines 1037 - 1040, The
adapter never persists the current task ID because _current_task_id is not
initialized or set. In src/bmad_loop/adapters/opencode_http.py lines 1037-1040,
initialize _current_task_id alongside _viewer_config in __init__ and add
_set_current_task_id(self, task_id: str) to store it; in src/bmad_loop/engine.py
lines 2284-2286, remove the silent getattr fallback or enforce the setter
contract so missing or renamed setters fail loudly.
| def ensure_viewer_window( | ||
| self, spec: SessionSpec, session_id: str, base_url: str | ||
| ) -> ViewerConfig: | ||
| """Adapter-seam ensure — delegates to the internal path.""" | ||
| run_id = self.run_dir.name | ||
| # The viewer must authenticate against the per-session serve password; | ||
| # find the live session this id belongs to (absent for a foreign id — | ||
| # the viewer then runs unauthenticated and shows the server's 401). | ||
| password = next( | ||
| (s.password for s in self._sessions.values() if s.session_id == session_id), | ||
| None, | ||
| ) | ||
| try: | ||
| return _ensure_viewer( | ||
| run_id=run_id, | ||
| server_url=base_url, | ||
| session_id=session_id, | ||
| working_directory=spec.cwd, | ||
| attach_binary=shutil.which("opencode") or "opencode", | ||
| run_dir=self.run_dir, | ||
| attach_env=({"OPENCODE_SERVER_PASSWORD": password} if password else None), | ||
| ) | ||
| except Exception as exc: # nosec B110 - best effort | ||
| if _viewer_control_flow(exc): | ||
| raise # the engine's stop/pause landed here — never swallow it | ||
| return ViewerConfig() |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The public ensure_viewer_window seam diverges from _make_viewer_config: no state, no persistence.
_make_viewer_config stores self.viewer_config = vc and calls _persist_viewer_config(vc); this seam does neither. A viewer created through the adapter seam is therefore invisible to _close_viewer (teardown leaks the window), to viewer.json (the dashboard Log tab and attach routing can't find it), and to the engine's viewer-created journal entry. Route the seam through the same internal path.
♻️ Proposed fix
try:
- return _ensure_viewer(
+ vc = _ensure_viewer(
run_id=run_id,
server_url=base_url,
session_id=session_id,
working_directory=spec.cwd,
attach_binary=shutil.which("opencode") or "opencode",
run_dir=self.run_dir,
attach_env=({"OPENCODE_SERVER_PASSWORD": password} if password else None),
)
except Exception as exc: # nosec B110 - best effort
if _viewer_control_flow(exc):
raise # the engine's stop/pause landed here — never swallow it
- return ViewerConfig()
+ vc = ViewerConfig()
+ self.viewer_config = vc
+ self._viewer_config = vc
+ self._persist_viewer_config(vc)
+ return vc📝 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.
| def ensure_viewer_window( | |
| self, spec: SessionSpec, session_id: str, base_url: str | |
| ) -> ViewerConfig: | |
| """Adapter-seam ensure — delegates to the internal path.""" | |
| run_id = self.run_dir.name | |
| # The viewer must authenticate against the per-session serve password; | |
| # find the live session this id belongs to (absent for a foreign id — | |
| # the viewer then runs unauthenticated and shows the server's 401). | |
| password = next( | |
| (s.password for s in self._sessions.values() if s.session_id == session_id), | |
| None, | |
| ) | |
| try: | |
| return _ensure_viewer( | |
| run_id=run_id, | |
| server_url=base_url, | |
| session_id=session_id, | |
| working_directory=spec.cwd, | |
| attach_binary=shutil.which("opencode") or "opencode", | |
| run_dir=self.run_dir, | |
| attach_env=({"OPENCODE_SERVER_PASSWORD": password} if password else None), | |
| ) | |
| except Exception as exc: # nosec B110 - best effort | |
| if _viewer_control_flow(exc): | |
| raise # the engine's stop/pause landed here — never swallow it | |
| return ViewerConfig() | |
| def ensure_viewer_window( | |
| self, spec: SessionSpec, session_id: str, base_url: str | |
| ) -> ViewerConfig: | |
| """Adapter-seam ensure — delegates to the internal path.""" | |
| run_id = self.run_dir.name | |
| # The viewer must authenticate against the per-session serve password; | |
| # find the live session this id belongs to (absent for a foreign id — | |
| # the viewer then runs unauthenticated and shows the server's 401). | |
| password = next( | |
| (s.password for s in self._sessions.values() if s.session_id == session_id), | |
| None, | |
| ) | |
| try: | |
| vc = _ensure_viewer( | |
| run_id=run_id, | |
| server_url=base_url, | |
| session_id=session_id, | |
| working_directory=spec.cwd, | |
| attach_binary=shutil.which("opencode") or "opencode", | |
| run_dir=self.run_dir, | |
| attach_env=({"OPENCODE_SERVER_PASSWORD": password} if password else None), | |
| ) | |
| except Exception as exc: # nosec B110 - best effort | |
| if _viewer_control_flow(exc): | |
| raise # the engine's stop/pause landed here — never swallow it | |
| vc = ViewerConfig() | |
| self.viewer_config = vc | |
| self._viewer_config = vc | |
| self._persist_viewer_config(vc) | |
| return vc |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/bmad_loop/adapters/opencode_http.py` around lines 1088 - 1113, Update
ensure_viewer_window to use the same internal viewer-creation path as
_make_viewer_config, ensuring the resulting ViewerConfig is assigned to
self.viewer_config and passed to _persist_viewer_config. Preserve the existing
session password lookup, control-flow exception handling, and empty ViewerConfig
fallback while routing creation through the shared path so teardown, dashboard
state, and journaling remain synchronized.
| # Idempotency: check for existing viewer window BEFORE trying to create. | ||
| if _find_existing_viewer(run_id, session_name) is not None: | ||
| return _success() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Reuse is keyed on run_id only, so a second task in the same run gets the previous session's viewer.
_find_existing_viewer matches bmad-loop-<run_id>-viewer, but the window it finds is running opencode attach … --session <old session_id>. OpencodeHttpAdapter calls _ensure_viewer once per start_session (a fresh server + session id per task), so from task 2 onward this returns _success() — a ViewerConfig advertising the new viewer_url/viewer_session while the live window is still attached to the dead task-1 server. The dashboard then renders (and attach lands on) a stale/disconnected pane.
Either key the window name on the session (or task) id, or verify the existing window's recorded session matches before reusing and recreate otherwise.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/bmad_loop/adapters/viewer_launch.py` around lines 146 - 148, The
existing-viewer reuse check in _ensure_viewer incorrectly reuses windows keyed
only by run_id across tasks. Update _find_existing_viewer or the surrounding
_ensure_viewer logic to key viewer windows by session/task identity, or validate
that the found window’s recorded session matches the current session before
returning _success(); otherwise create a new viewer attached to the current
server and session.
| def _find_view_window(run_id: str) -> str | None: | ||
| """Find a viewer window for this run's session (set by opencode-http | ||
| adapters when show_attached_ui is enabled). Returns the mux target | ||
| token, or None.""" | ||
| if not mux_available(): | ||
| return None | ||
| session_name = runs.session_name(run_id) | ||
| viewer_name = f"bmad-loop-{run_id}-viewer" | ||
| mux = get_multiplexer() | ||
| try: | ||
| rows = mux.list_windows(session_name, ["window_name"]) | ||
| except MultiplexerError: | ||
| return None | ||
| for (wn,) in rows: | ||
| if wn == viewer_name: | ||
| return mux.target(session_name, wn) | ||
| return None |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Window name is hardcoded here and computed in viewer_launch.
f"bmad-loop-{run_id}-viewer" duplicates viewer_launch._viewer_window_name, and this whole function duplicates viewer_launch.viewer_window_exists. A rename on the producer side would silently break attach routing with no test failure. Import the shared helper instead.
♻️ Proposed fix
- viewer_name = f"bmad-loop-{run_id}-viewer"
+ from ..adapters.viewer_launch import _viewer_window_name
+
+ viewer_name = _viewer_window_name(run_id)(Better: promote _viewer_window_name to a public helper in viewer_launch and import it at module scope.)
📝 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.
| def _find_view_window(run_id: str) -> str | None: | |
| """Find a viewer window for this run's session (set by opencode-http | |
| adapters when show_attached_ui is enabled). Returns the mux target | |
| token, or None.""" | |
| if not mux_available(): | |
| return None | |
| session_name = runs.session_name(run_id) | |
| viewer_name = f"bmad-loop-{run_id}-viewer" | |
| mux = get_multiplexer() | |
| try: | |
| rows = mux.list_windows(session_name, ["window_name"]) | |
| except MultiplexerError: | |
| return None | |
| for (wn,) in rows: | |
| if wn == viewer_name: | |
| return mux.target(session_name, wn) | |
| return None | |
| def _find_view_window(run_id: str) -> str | None: | |
| """Find a viewer window for this run's session (set by opencode-http | |
| adapters when show_attached_ui is enabled). Returns the mux target | |
| token, or None.""" | |
| if not mux_available(): | |
| return None | |
| session_name = runs.session_name(run_id) | |
| from ..adapters.viewer_launch import _viewer_window_name | |
| viewer_name = _viewer_window_name(run_id) | |
| mux = get_multiplexer() | |
| try: | |
| rows = mux.list_windows(session_name, ["window_name"]) | |
| except MultiplexerError: | |
| return None | |
| for (wn,) in rows: | |
| if wn == viewer_name: | |
| return mux.target(session_name, wn) | |
| return None |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/bmad_loop/tui/launch.py` around lines 30 - 46, Replace the hardcoded
viewer window name and duplicated lookup logic in _find_view_window with the
shared public helper from viewer_launch. Promote _viewer_window_name to a public
helper if needed, import it at module scope, and reuse
viewer_launch.viewer_window_exists so attach routing follows the producer’s
naming behavior.
| session = runs.session_name(run_id) | ||
| viewer_window = _find_view_window(run_id) | ||
| if viewer_window: | ||
| return runs.attach_target_argv(viewer_window), None |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Viewer preference now shadows the pending-decision ctl route.
Previously, when a sweep was blocked on an interactive decision, attach landed on the orchestrator's ctl window so the operator could answer the prompt. With a viewer window present, that branch (Lines 198-204) is unreachable — the operator is dropped into a read-only attached UI and has no way to reach the blocking prompt. Gate the viewer preference on there being no pending decision.
🐛 Proposed fix
session = runs.session_name(run_id)
- viewer_window = _find_view_window(run_id)
- if viewer_window:
- return runs.attach_target_argv(viewer_window), None
window = ctl_window(run_id)
agent_live = session_exists(session)
+ pending = decision_pending(runs.run_dir_for(project, run_id))
+ viewer_window = _find_view_window(run_id)
+ if viewer_window and not pending:
+ return runs.attach_target_argv(viewer_window), None
- if window is not None and (
- decision_pending(runs.run_dir_for(project, run_id)) or not agent_live
- ):
+ if window is not None and (pending or not agent_live):
select_ctl_window(window)
return runs.attach_target_argv(ctl_target()), ctl_target(window)📝 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.
| session = runs.session_name(run_id) | |
| viewer_window = _find_view_window(run_id) | |
| if viewer_window: | |
| return runs.attach_target_argv(viewer_window), None | |
| session = runs.session_name(run_id) | |
| window = ctl_window(run_id) | |
| agent_live = session_exists(session) | |
| pending = decision_pending(runs.run_dir_for(project, run_id)) | |
| viewer_window = _find_view_window(run_id) | |
| if viewer_window and not pending: | |
| return runs.attach_target_argv(viewer_window), None | |
| if window is not None and (pending or not agent_live): | |
| select_ctl_window(window) | |
| return runs.attach_target_argv(ctl_target()), ctl_target(window) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/bmad_loop/tui/launch.py` around lines 194 - 197, Update the viewer-window
branch in the attach routing logic around _find_view_window to return the viewer
target only when no pending interactive decision exists; otherwise allow
execution to continue to the existing pending-decision ctl route. Preserve the
current viewer behavior when the sweep is not blocked.
| def test_viewer_log_path_format(self, sandbox): | ||
| """Viewer output logs to <run>.log which is the standard task log format.""" | ||
| from bmad_loop.journal import LOGS_DIR | ||
|
|
||
| task_id = "task-abc123-def" | ||
| expected_log_path = Path(str(LOGS_DIR)) / f"{task_id}.log" | ||
|
|
||
| expected_name = expected_log_path.name # e.g. "task-abc123-def.log" | ||
| expected_pattern = re.compile(r"^task-[a-f0-9-]+\.log$") | ||
| assert expected_pattern.match(expected_name) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
This test exercises no production code.
It builds expected_log_path locally and then asserts a regex against the name it just built — it passes regardless of the viewer implementation. Either assert against the path the adapter actually writes, or drop it. (The unused sandbox fixture argument is another sign.)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_viewer.py` around lines 362 - 371, Replace or remove
test_viewer_log_path_format so it validates the actual log path produced by the
viewer adapter rather than constructing expected_log_path locally. Exercise the
production viewer flow and assert the adapter’s written path matches the task
log format; remove the unused sandbox fixture if it is no longer needed.
| def test_viewer_config_stored_to_json(self, sandbox): | ||
| """Viewer metadata persisted to viewer.json in the run dir.""" | ||
| from bmad_loop.adapters.base import SessionSpec | ||
| from bmad_loop.adapters.opencode_http import OpencodeHttpAdapter | ||
|
|
||
| tmp_path, mux = sandbox | ||
| mux.available.return_value = True | ||
| mux.list_windows.return_value = [] | ||
|
|
||
| policy_patch = MagicMock() | ||
| policy_patch.adapter = AdapterPolicy(show_attached_ui=True) | ||
| policy_patch.limits.stop_without_result_nudges = 0 | ||
| policy_patch.limits.max_tokens_per_session = 4_000_000 | ||
| policy_patch.limits.session_budget_mode = "off" | ||
| policy_patch.limits.cache_read_weight = 0.1 | ||
|
|
||
| profile = MagicMock() | ||
| profile.name = "opencode" | ||
| profile.binary = "opencode" | ||
| profile.skill_tree = "skills" | ||
| profile.env_fault_patterns = [] | ||
| profile.hookless = True | ||
|
|
||
| run_dir = tmp_path / "run1" | ||
| run_dir.mkdir() | ||
| (run_dir / "tasks").mkdir(exist_ok=True) | ||
| (run_dir / "logs").mkdir(exist_ok=True) | ||
|
|
||
| adapter = OpencodeHttpAdapter( | ||
| run_dir=run_dir, | ||
| policy=policy_patch, | ||
| profile=profile, | ||
| ) | ||
|
|
||
| spec = SessionSpec( | ||
| task_id="task-abc", | ||
| role="dev", | ||
| prompt="test", | ||
| cwd=run_dir, | ||
| ) | ||
| # ensure_viewer_window won't actually create a tmux window | ||
| # (since we're mocking the mux), but it should still persist | ||
| # and call _persist_viewer_config | ||
| result_vc = adapter.ensure_viewer_window(spec, "sess-123", "http://127.0.0.1:9999") | ||
|
|
||
| # The viewer_config was saved on the adapter | ||
| assert result_vc.viewer_session == "sess-123" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Name and docstring promise viewer.json persistence that is never asserted.
The test only checks result_vc.viewer_session. As written it would still pass if nothing were written — and the seam it calls (OpencodeHttpAdapter.ensure_viewer_window) does not in fact call _persist_viewer_config (see the comment on src/bmad_loop/adapters/opencode_http.py Lines 1088-1113). Assert the file contents once the seam is fixed.
💚 Suggested assertion
result_vc = adapter.ensure_viewer_window(spec, "sess-123", "http://127.0.0.1:9999")
- # The viewer_config was saved on the adapter
assert result_vc.viewer_session == "sess-123"
+ entries = json.loads((run_dir / "viewer.json").read_text(encoding="utf-8"))
+ assert any(e["viewer_session"] == "sess-123" for e in entries)📝 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.
| def test_viewer_config_stored_to_json(self, sandbox): | |
| """Viewer metadata persisted to viewer.json in the run dir.""" | |
| from bmad_loop.adapters.base import SessionSpec | |
| from bmad_loop.adapters.opencode_http import OpencodeHttpAdapter | |
| tmp_path, mux = sandbox | |
| mux.available.return_value = True | |
| mux.list_windows.return_value = [] | |
| policy_patch = MagicMock() | |
| policy_patch.adapter = AdapterPolicy(show_attached_ui=True) | |
| policy_patch.limits.stop_without_result_nudges = 0 | |
| policy_patch.limits.max_tokens_per_session = 4_000_000 | |
| policy_patch.limits.session_budget_mode = "off" | |
| policy_patch.limits.cache_read_weight = 0.1 | |
| profile = MagicMock() | |
| profile.name = "opencode" | |
| profile.binary = "opencode" | |
| profile.skill_tree = "skills" | |
| profile.env_fault_patterns = [] | |
| profile.hookless = True | |
| run_dir = tmp_path / "run1" | |
| run_dir.mkdir() | |
| (run_dir / "tasks").mkdir(exist_ok=True) | |
| (run_dir / "logs").mkdir(exist_ok=True) | |
| adapter = OpencodeHttpAdapter( | |
| run_dir=run_dir, | |
| policy=policy_patch, | |
| profile=profile, | |
| ) | |
| spec = SessionSpec( | |
| task_id="task-abc", | |
| role="dev", | |
| prompt="test", | |
| cwd=run_dir, | |
| ) | |
| # ensure_viewer_window won't actually create a tmux window | |
| # (since we're mocking the mux), but it should still persist | |
| # and call _persist_viewer_config | |
| result_vc = adapter.ensure_viewer_window(spec, "sess-123", "http://127.0.0.1:9999") | |
| # The viewer_config was saved on the adapter | |
| assert result_vc.viewer_session == "sess-123" | |
| def test_viewer_config_stored_to_json(self, sandbox): | |
| """Viewer metadata persisted to viewer.json in the run dir.""" | |
| from bmad_loop.adapters.base import SessionSpec | |
| from bmad_loop.adapters.opencode_http import OpencodeHttpAdapter | |
| tmp_path, mux = sandbox | |
| mux.available.return_value = True | |
| mux.list_windows.return_value = [] | |
| policy_patch = MagicMock() | |
| policy_patch.adapter = AdapterPolicy(show_attached_ui=True) | |
| policy_patch.limits.stop_without_result_nudges = 0 | |
| policy_patch.limits.max_tokens_per_session = 4_000_000 | |
| policy_patch.limits.session_budget_mode = "off" | |
| policy_patch.limits.cache_read_weight = 0.1 | |
| profile = MagicMock() | |
| profile.name = "opencode" | |
| profile.binary = "opencode" | |
| profile.skill_tree = "skills" | |
| profile.env_fault_patterns = [] | |
| profile.hookless = True | |
| run_dir = tmp_path / "run1" | |
| run_dir.mkdir() | |
| (run_dir / "tasks").mkdir(exist_ok=True) | |
| (run_dir / "logs").mkdir(exist_ok=True) | |
| adapter = OpencodeHttpAdapter( | |
| run_dir=run_dir, | |
| policy=policy_patch, | |
| profile=profile, | |
| ) | |
| spec = SessionSpec( | |
| task_id="task-abc", | |
| role="dev", | |
| prompt="test", | |
| cwd=run_dir, | |
| ) | |
| # ensure_viewer_window won't actually create a tmux window | |
| # (since we're mocking the mux), but it should still persist | |
| # and call _persist_viewer_config | |
| result_vc = adapter.ensure_viewer_window(spec, "sess-123", "http://127.0.0.1:9999") | |
| assert result_vc.viewer_session == "sess-123" | |
| entries = json.loads((run_dir / "viewer.json").read_text(encoding="utf-8")) | |
| assert any(e["viewer_session"] == "sess-123" for e in entries) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_viewer.py` around lines 373 - 419, Update
test_viewer_config_stored_to_json to assert that ensure_viewer_window persists
viewer.json: after calling OpencodeHttpAdapter.ensure_viewer_window, verify the
file exists in run_dir and its parsed contents include the expected session
identifier and viewer URL. Also update ensure_viewer_window to invoke
_persist_viewer_config so the test exercises the promised persistence behavior.
|
I do appreciate the contribution but at this time I'm going to have to decline the PR as is. We recently merged PR #279 as our first iteration of logging in the run and in the TUI. We may modify and edit it some more as testing continues and encourage contributions against that. I do also want to recommend bringing discussions in regards to PR's this large into the discord first before you spend any time working on it to prevent duplication of work or perhaps design problems we may run into working in a vacuum. We always want to keep the PR scoped to one thing, the blast radius of your actual PR is quite large and adjust things not actually relevant to it. We also built the opencode adapter as an http adapter solution so It doesn't have the user require tmux in its usage so it would be an unexpected requirement if we add it in now as well. Nor is the log a live viewer, its actually a pipe of the output written and emulated in pyte, it just looks live :) I do look forward to reviewing any of your other contributions in the future. |
What
[adapter] show_attached_ui = truegives the opencode-http adapter a live viewer: a detached tmux window runningopencode attach, rendered in the dashboard Log tab — so an OpenCode run is watchable like the tmux adapters' sessions.Why
opencode-http drives sessions over HTTP with no pane to observe; the Log tab could only show the raw server log, leaving runs effectively headless.
How
viewer_launchmodule: parked, authenticated (password via tmux session environment, never argv), verified-live viewer window; failures land inviewer-error.log+ aviewer-create-failedjournal event, andviewer.jsonnever records a dead targetcapture_pane -t,set_session_environment,resize_window,window_exists); parked windows fixed for dash (read -r _park)attach/aprefer itTesting
Unit + real-tmux integration tests (2990 passed, isolated tmux server); verified end-to-end in the live TUI against a real run (
opencode serve+ vllm).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
bmad-autotobmad-loopmigration compatibility.