Skip to content

Add runpane panes focus command (#401) - #461

Open
ilijachrchev wants to merge 5 commits into
dcouple:mainfrom
ilijachrchev:feature/panes-focus-command
Open

Add runpane panes focus command (#401)#461
ilijachrchev wants to merge 5 commits into
dcouple:mainfrom
ilijachrchev:feature/panes-focus-command

Conversation

@ilijachrchev

@ilijachrchev ilijachrchev commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Description

Adds runpane panes focus command. Raises the Pane window, selects a pane, and optionally selects a specific panel within it.

Closes #401.

Follows the same pattern as panes rename / panes archive: contract.json -> generated artifacts -> CLI dispatch -> daemon handler -> renderer event.

New pieces: window-raising plumbing (show/restore/focus on BrowserWindow) and a pane:focus-requested IPC event that tells the renderer to call setActiveSession and setActivePanel.

Usage:

runpane panes focus --pane [--panel ] --source user|agent --yes [--json]

Requires --yes. Accepts --source agent|user. Archived/missing panes and mismatched panels return clear errors, never falls back to focusing something else. Doctrine text added to contract: agents invoke this only on explicit user request, never proactively.

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Documentation update

Checklist

  • I have read the CONTRIBUTING.md guidelines
  • My code follows the code style of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have run pnpm typecheck and pnpm lint locally
  • I have tested the Electron app locally with pnpm electron-dev

Critical Areas Modified

  • State management/IPC events

Screenshots (if applicable)

N/A. CLI command with no UI changes. The renderer-side change is a new IPC listener that calls existing store actions (setActiveSession, setActivePanel).

Additional Notes

5 new tests in runpane.test.ts: happy path, minimized window restore + panel selection, archived pane rejection, unknown pane rejection, panel-not-belonging rejection. All 64 existing tests pass (1 pre-existing skip).

Contract artifacts regenerated via pnpm run generate-runpane-contract. The 6 generated files were not hand-edited.

The handler sends pane:focus-requested via window.webContents.send() directly rather than through sendRendererEvent in events.ts, since the handler already holds the raised window reference. This matches how window:focus-changed is dispatched in index.ts.

Automated QA

Status: passed on head cbfcab8d using an isolated Pane data directory.

  • Started the Electron development app and verified the local daemon advertised runpane:panes:focus.
  • Focused a disposable pane without --panel, then focused its second terminal panel explicitly.
  • Verified through panels list that the requested second panel became the sole persisted active panel.
  • Archived the disposable pane and verified a later focus request failed with exit code 1 and the expected archived-pane error.
  • Archived the test pane, removed its managed worktree, and removed the two QA-only local branches/worktrees.
  • Ran the cross-language RunPane contract suite and npm/pip package smoke tests, including Python parser parity for panes focus.

No screenshot is included because this command adds no new visual surface. Final human review can confirm native macOS window raising if desired.

@parsakhaz parsakhaz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Verdict: Request changes - the branch fails typecheck and optional panel focus can be overwritten by the persisted active panel.
Counts: Must Fix: 2 (security: 0) · Should Fix: 0 · pass 1/3

Must Fix

  • MF-1 - The PR fails typecheck in every touched TypeScript package · frontend/src/hooks/useIPCEvents.ts:220, main/src/ipc/runpane.ts:370, main/src/ipc/runpane.ts:2148, packages/runpane/src/localControl.ts:889 · restore the existing deletion-event contract, keep daemon request values as PaneCommandValue, and decode the focus response with a boundary schema · violates the PR checklist and repository typecheck requirement

    • Evidence: pnpm --filter frontend typecheck reports TS2552 for undefined SessionDeletedEventData; pnpm --filter main typecheck reports TS2345 twice because unknown is passed to PaneCommandValue helpers; pnpm --filter runpane typecheck reports TS2353 because the options object is passed where BoundarySchema<PaneFocusResult> is required.
    • Failure scenario: CI cannot build or typecheck the branch, so the command cannot ship.
  • MF-2 - Selecting --panel updates only renderer memory and can be immediately reverted · main/src/ipc/runpane.ts:338, frontend/src/hooks/useIPCEvents.ts:212 · persist the validated selection through panelManager.setActivePanel before emitting the renderer focus request, and cover that call in the focus tests · violates the stated intent to select the requested panel exactly like clicking it in the UI

    • Evidence: the normal click path calls panelApi.setActivePanel in frontend/src/components/SessionView.tsx:575, which reaches panelManager.setActivePanel and updates the database. The new handler only calls the Zustand setter. On a pane switch, SessionView then reads panelApi.getActivePanel at frontend/src/components/SessionView.tsx:247 and writes that persisted value back into the store.
    • Failure scenario: focusing a different pane with --panel briefly sets the requested tab, then the pane-load effect restores the previously persisted tab, so the command reports success without leaving the requested panel selected.

Praise

  • The main handler rejects archived or missing panes and cross-pane panel IDs before raising the window, and the new unit tests prove those negative paths.

Cannot verify

  • The configured code-reviewer skill could not run because its required .claude/agents/code-reviewer.md instruction file is absent locally, absent from this PR, and absent from Parsa's upstream skill folder. This review follows the skill's required report format and evidence standard, but does not claim the missing role instructions were applied.

@parsakhaz
parsakhaz force-pushed the feature/panes-focus-command branch from 914006e to 3aa3ff6 Compare August 23, 2026 20:03

@parsakhaz parsakhaz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Verdict: Approve - both Must-Fix findings are resolved and the final branch passes the required checks.
Counts: Must Fix: 0 (security: 0) · Should Fix: 0 · pass 2/3

Resolved

  • MF-1 fixed - root typecheck now passes across frontend, main, runpane, and shared. The focus response has boundary decoding, request parsers retain PaneCommandValue, and the unrelated broken session-deletion typing was restored.
  • MF-2 fixed - --panel now persists through panelManager.setActivePanel before the renderer event. Split-layout activation is also aligned through one tested pure layout operation.

Checks

  • pnpm typecheck
  • pnpm lint
  • pnpm --filter main exec vitest run src/ipc/runpane.test.ts (67 passed)
  • pnpm --filter frontend exec vitest run src/utils/panelLayout.test.ts (55 passed)
  • pnpm run check:runpane-contract
  • Isolated Electron and local-wrapper QA for pane focus, panel focus with persisted readback, and archived-pane rejection

@parsakhaz

parsakhaz commented Aug 23, 2026

Copy link
Copy Markdown
Member

Completed all three passes and pushed head cbfcab8d to feature/panes-focus-command.

Pass results

  • REVIEW: rebased the PR onto current origin/main; fixed three package-level typecheck failures; added boundary decoding for the daemon response; restored the existing session-deletion contract; persisted requested panel selection before reporting focus success; added missing Python parser and execution parity found by the full CI contract suite. Commits: 0e5624d7, cbfcab8d.
  • SIMPLIFY: consolidated repeated test fixture assertions and removed an unreachable success branch. Root lint improved from 8 blocking findings before this pass to 0 after it. Commit: abe06d7e.
  • REFACTOR: added one pure layout activation operation used by normal clicks and persisted focus loading, so --panel also selects the correct tab/group in split layouts; shared the focus-event payload type across main, preload, and renderer. Commit: 3aa3ff6f.

Verification

  • pnpm typecheck: passed
  • pnpm lint: passed, including Knip and boundary conformance
  • RunPane IPC tests: 67 passed
  • Panel layout tests: 55 passed
  • Contract generation check: passed
  • Full RunPane contract tests: passed
  • npm and pip package smoke tests: passed
  • Python wrapper bytecode compilation and parser parity: passed
  • Isolated Electron QA: pane-only focus passed; explicit second-panel focus passed; panels list confirmed it was the sole persisted active panel; archived-pane focus failed with exit code 1 as expected

The disposable QA pane was archived, its managed worktree was removed, and its QA-only local branches/worktrees were deleted. The long temporary path caused an unrelated optional permission-socket warning, but the Pane daemon and focus flow remained healthy.

Follow-ups

  • Consider routing pane:focus-requested through a typed renderer-only event-sink policy instead of direct webContents.send. This needs focused remote-forwarding tests before changing the boundary.
  • Consider generating published wrapper request/result types from the RunPane contract so packages/runpane does not maintain package-local interface copies.

Left for parsa

  • Review and merge when CI is green. I did not merge.
  • Optional manual check: confirm native macOS window raising from a minimized/background window. Automated tests prove restore, show, and focus calls, while the live QA proved the full CLI and persisted panel-selection flow.

@parsakhaz parsakhaz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Verdict: Approve - both Must-Fix findings are resolved and the final branch passes the required checks.
Counts: Must Fix: 0 (security: 0) · Should Fix: 0 · pass 2/3

Resolved

  • MF-1 fixed - root typecheck now passes across frontend, main, runpane, and shared. The focus response has boundary decoding, request parsers retain PaneCommandValue, and the unrelated broken session-deletion typing was restored.
  • MF-2 fixed - --panel now persists through panelManager.setActivePanel before the renderer event. Split-layout activation is also aligned through one tested pure layout operation.
  • CI parity fixed - the Python wrapper now parses, dispatches, confirms, and invokes panes focus with the same local-control behavior as the npm wrapper.

Checks

  • pnpm typecheck
  • pnpm lint
  • pnpm --filter main exec vitest run src/ipc/runpane.test.ts (67 passed)
  • pnpm --filter frontend exec vitest run src/utils/panelLayout.test.ts (55 passed)
  • pnpm run check:runpane-contract
  • node scripts/test-runpane-contract.js
  • node scripts/test-runpane-package-smoke.js
  • Isolated Electron and local-wrapper QA for pane focus, panel focus with persisted readback, and archived-pane rejection

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.

CLI: focus/open a pane on request, so the orchestrator can navigate for the user

2 participants