Skip to content

fix(sidebar): an OmO DAG viewer pane adds no tab strip or row - #447

Merged
devswha merged 3 commits into
devswha:mainfrom
nahwan-kim:fix/hide-dag-pane
Oct 5, 2026
Merged

devswha merged 3 commits into
devswha:mainfrom
nahwan-kim:fix/hide-dag-pane

Conversation

@nahwan-kim

Copy link
Copy Markdown
Contributor

Change

omo-herdr-dag splits a DAG viewer pane to the right of an OmO pane (labelled DAG · <session>, its viewer titles itself OmO DAG). In the web UI that pane made the OmO workspace look like a two-pane tab: a tab strip with a single tab ("Tab 1") and a pane picker appeared above the OmO pane, and the folder header counted the extra pane. The viewer draws the same workflow the OmO pane's background-task list already shows, so in the app it is only clutter.

  • src/lib/dagPane.ts: isDagViewerPane tells the viewer the way the plugin finds its own panes (its controller's isDagViewerPane: the DAG · label or the OmO DAG title). rosterPanes leaves a viewer out while its tab has another pane.
  • The sidebar rows, folder counts and the tab strip use rosterPanes. An OmO workspace with its viewer now shows as one pane, as it does without one.
  • The viewer is not hidden when it is the open pane (opened from the palette, or herdr's focus) or when it is all its tab has (its OmO pane closed), so it can still be reached and closed. The command palette still lists it; the workspace menu's close still counts and closes every pane.
  • Client only: no HTTP/WS contract change.

Validation

  • src/lib/dagPane.test.ts: the viewer is told by its label or title only; it is left out beside its OmO pane, kept while open, kept when alone in its tab, and other panes are untouched.
  • Fast checks: generate:types --check, typecheck, build pass; test:unit 1151 pass, 0 fail.
  • Real browser, isolated herdr 0.9.3 session (herdr-web-ui-test-dagqa): one workspace with an OmO stand-in pane and, split beside it, a pane labelled DAG · 01a10998 running the real omo-herdr-dag viewer (title OmO DAG). Same session served by upstream/main and by this branch:
    • before: tab strip [Tab 1] with a pane picker over the OmO pane;
    • after: no tab strip; the sidebar rows are the same (~, omo).
Before After
before after

Not verified: a phone width (the change is width-independent logic) and Linux/Windows CI (fork CI waits for approval).

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cd5ed242-c3c8-47cd-b243-2171c2c959f4
📥 Commits

Reviewing files that changed from the base of the PR and between 586ec28 and acfa051.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • src/App.tsx
  • src/lib/dagPane.test.ts
  • src/lib/dagPane.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • CHANGELOG.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • When the DAG viewer shares a tab with an OmO pane, it no longer appears separately in the sidebar or tab strip; the workspace presents as a single pane.
    • The viewer remains available through the command palette and appears in the pane list when open alone or selected.
    • Adjacent-pane navigation follows the panes shown in the workspace, preventing navigation to a hidden DAG viewer.

Walkthrough

The change identifies DAG viewer panes and filters them from sidebar, tab-strip, and adjacent-pane navigation rosters when another non-DAG pane shares their tab. Selected viewers and viewers alone on their tab remain in the roster.

Changes

DAG viewer pane roster

Layer / File(s) Summary
Identify and filter DAG viewer panes
src/lib/dagPane.ts, src/lib/dagPane.test.ts
isDagViewerPane recognizes viewer labels and terminal titles. rosterPanes excludes a viewer beside another non-DAG pane, while retaining selected and standalone viewers. Tests cover these cases.
Use filtered rosters across pane navigation
src/components/Sidebar.tsx, src/components/TabStrip.tsx, src/App.tsx, CHANGELOG.md
Sidebar grouping and workspace/worktree rows, TabStrip pane ordering, and adjacent-pane navigation use filtered panes. The changelog notes the display change and command-palette access.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to acfa0

The DAG viewer is omitted from the sidebar and tab controls beside its OmO pane without leaving an extra tab strip. No identified issue prevents merging after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 586ec

A hidden active pane may no longer trigger the busy-agent confirmation, although closing its tab still affects it. This is a localized failure-containment concern requiring an existing tab-close action, not demonstrated access escalation.

Retained concerns

  • Medium · reliability · observed: The busy-agent close safeguard now examines a presentation-only subset, while closure remains tab-wide. A hidden working or blocked pane is excluded from confirmation unless another visible pane is busy or this is the last tab. This weakens containment of unintended active-task termination; actual runtime occurrence remains unverified.
Security review details

Security Blast Radius

  • inferred — The identified consequence is bounded to the existing close operation on one tab and its panes. Confirmation bypass requires another workspace tab, a hidden busy pane, and no busy visible sibling. It still requires the user to initiate closure; no cross-machine, cross-tenant, or privilege-gaining path was demonstrated.

Trust Boundaries and Controls

  • observed — Presentation metadata now influences a destructive-action confirmation predicate. The close request itself continues through the existing machine API client and carries the mutation header; the filter does not create a new endpoint or add close authority. Server-side enforcement was not independently verified.

Resilience and Maintainability Implications

  • observed — Selected and standalone viewers remain represented, and last-tab closure still requires confirmation. Sidebar's menu actions use raw snapshot panes for counts and mutation targets. These are meaningful containment controls, but they do not cover TabStrip's hidden-busy-pane case.

Hardening Proposals

  • proposed — Separate display rosters from lifecycle checks: evaluate close confirmation against all snapshot panes affected by the tab-close contract, while retaining roster filtering for presentation and navigation.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: preventing an OmO DAG viewer pane from adding a tab strip or sidebar row.
Description check ✅ Passed The description covers the problem and resulting behavior under Change. Under Validation, it lists tests and build checks, provides a real-browser comparison for the UI change, and notes unverified pl…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

…ver the viewer

Maintainer follow-up to the review of devswha#447.

- A pane left out of the roster also dropped out of the checks made on that
  list: a tab's close did not ask about it when it was working or waiting, and
  the workspace row's status did not count it. isDagViewerPane now refuses a
  pane that has an agent or that works or waits for input, so a hidden pane is
  never one of those, whatever it is called.
- The label is matched as the plugin's prefix and the title as its exact words,
  not either in both fields.
- Mod+Shift+Up/Down steps through the panes the sidebar lists, so it no longer
  lands on a viewer the sidebar leaves out.
@devswha
devswha merged commit eab319c into devswha:main Oct 5, 2026
4 checks passed
@devswha

devswha commented Oct 5, 2026

Copy link
Copy Markdown
Owner

Merged, thank you. One commit was added on the branch before the merge (main was merged in, nothing was rebased):

  • acfa051: a pane left out of the roster also dropped out of the checks made on that list. A tab's close did not ask about it when it was working or waiting (busy in TabStrip), and the workspace row's status did not count it. isDagViewerPane now refuses a pane that has an agent, or that works or waits for input, so a hidden pane is never one of those, whatever it is called.
  • The label is matched as the plugin's prefix and the title as its exact words, not either in both fields.
  • Mod+Shift+Up/Down steps through the panes the sidebar lists, so it no longer lands on a viewer the sidebar leaves out. The command palette still lists the viewer, as you described.

The two added tests fail with the matcher as it was and pass now. The changelog entry links this PR.

devswha added a commit that referenced this pull request Oct 5, 2026
Bumps package.json and herdr-plugin.toml to 0.3.50 and moves the
Unreleased notes under [0.3.50] - 2026-10-06, with the compare link.
Adds the PR link and credit the #471 entry was missing; every other
entry already carries them. Raises the remote PC bundle to 17: #470
changes the prompt answer route on the bridge, so a remote PC on bundle
16 keeps the old behaviour until its bridge is updated. Push the
`remote-v17` tag and let the bundle workflow publish before running
release.yml.

Known limits carried into the notes: Japanese and Chinese text uses the
device's fonts beside Pretendard; #469 stays open for what #470 does not
cover (answers after the first committing key, identical twin prompts,
screens no reader knows).

Carries #422 #423 #424 #425 #426 #435 #437 #439 #446 #447 #448 #452 #454
#455 #456 #457 #458 #459 #460 #461 #462 #463 #465 #466 #467 #468 #471
#473 #474 #475

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants