Skip to content

Commit dffa65b

Browse files
authored
Fix worktree switch tab order, focus restoration, and remap Cmd+J to editor terminal (#81)
* Fix worktree switch tab order and focus restoration Preserve the original interleaved editor/terminal tab order when switching between worktrees. Previously, Phase 2 terminal restore appended terminals at the end (e.g. [E1, E2, T1, T2] instead of [E1, T1, T2, E2]) and the last terminal stole focus. - Add tabIndex to ITerminalOwnership; snapshot in Phase 1 alongside groupIndex so each terminal's tab position is recorded before backgrounding - After Phase 2 shows terminals, reorder them to their saved tabIndex via moveEditor to restore original interleaved positions - Save per-group active editor URI in Phase 1; restore focus in Phase 2 after terminals are re-shown to prevent focus theft - Replace console.warn debug leftovers with logService.trace - Skip duplicate Stop notifications (self-transition guard) - Fix test mocks: add shellLaunchConfig, capabilities, getSessionState * Bump version to v0.2.8 * Remap Cmd+J to open terminal in editor area instead of toggling bottom panel
1 parent 3942ea8 commit dffa65b

5 files changed

Lines changed: 517 additions & 18 deletions

File tree

apps/desktop/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "code-oss-dev",
3-
"version": "0.2.7",
3+
"version": "0.2.8",
44
"distro": "d48ce3e61fa56a702d13bb450ceb3532620dbd46",
55
"author": {
66
"name": "Workstream Labs"

apps/desktop/src/vs/workbench/browser/parts/panel/panelActions.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
import './media/panelpart.css';
77
import { localize, localize2 } from '../../../../nls.js';
8-
import { KeyMod, KeyCode } from '../../../../base/common/keyCodes.js';
98
import { MenuId, MenuRegistry, registerAction2, Action2, IAction2Options } from '../../../../platform/actions/common/actions.js';
109
import { Categories } from '../../../../platform/action/common/actionCommonCategories.js';
1110
import { isHorizontal, IWorkbenchLayoutService, PanelAlignment, Parts, Position, positionToString } from '../../../services/layout/browser/layoutService.js';
@@ -19,7 +18,6 @@ import { IViewsService } from '../../../services/views/common/viewsService.js';
1918
import { IPaneCompositePartService } from '../../../services/panecomposite/browser/panecomposite.js';
2019
import { INotificationService } from '../../../../platform/notification/common/notification.js';
2120
import { ICommandActionTitle } from '../../../../platform/action/common/action.js';
22-
import { KeybindingWeight } from '../../../../platform/keybinding/common/keybindingsRegistry.js';
2321
import { SwitchCompositeViewAction } from '../compositeBarActions.js';
2422

2523
const maximizeIcon = registerIcon('panel-maximize', Codicon.screenFull, localize('maximizeIcon', 'Icon to maximize a panel.'));
@@ -46,7 +44,7 @@ export class TogglePanelAction extends Action2 {
4644
metadata: {
4745
description: localize('openAndClosePanel', 'Open/Show and Close/Hide Panel'),
4846
},
49-
keybinding: { primary: KeyMod.CtrlCmd | KeyCode.KeyJ, weight: KeybindingWeight.WorkbenchContrib },
47+
// keybinding removed — Cmd+J now opens terminal in editor area
5048
menu: [
5149
{
5250
id: MenuId.MenubarAppearanceMenu,

apps/desktop/src/vs/workbench/contrib/orchestrator/browser/orchestratorTerminalContribution.ts

Lines changed: 117 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6+
import { URI } from '../../../../base/common/uri.js';
67
import { Disposable } from '../../../../base/common/lifecycle.js';
78
import { RunOnceScheduler } from '../../../../base/common/async.js';
89
import { ILogService } from '../../../../platform/log/common/log.js';
@@ -27,6 +28,8 @@ interface ITerminalOwnership {
2728
readonly worktreeKey: string;
2829
/** Editor group index (GRID_APPEARANCE order) before backgrounding. */
2930
groupIndex: number;
31+
/** Tab index within the editor group before backgrounding. */
32+
tabIndex: number;
3033
}
3134

3235
/**
@@ -57,6 +60,14 @@ export class OrchestratorTerminalContribution extends Disposable {
5760
*/
5861
private readonly _ownership = new Map<number, ITerminalOwnership>();
5962

63+
/**
64+
* Per-worktree snapshot of each group's active editor URI, captured in
65+
* Phase 1 before backgrounding so Phase 2 can restore focus after
66+
* terminal editors are re-shown.
67+
* Key: worktreeKey, Value: Map<groupIndex, activeEditorResource>
68+
*/
69+
private readonly _savedActiveEditors = new Map<string, Map<number, URI>>();
70+
6071
/**
6172
* Heartbeat timers per worktree. If no hook event arrives within the
6273
* timeout period while a worktree is in Working state, the state is
@@ -84,7 +95,7 @@ export class OrchestratorTerminalContribution extends Disposable {
8495
* from the main-process HTTP server and updates session state.
8596
*/
8697
this._register(this._hookNotificationService.onDidReceiveNotification(event => {
87-
console.warn(`[LIFECYCLE DEBUG] Hook event: ${event.eventType} for "${event.worktreePath}" | current state: ${this._findWorktreeSessionState(event.worktreePath) ?? 'undefined'}`);
98+
this._logService.trace(`${TAG} Hook event: ${event.eventType} for "${event.worktreePath}" | current state: ${this._findWorktreeSessionState(event.worktreePath) ?? 'undefined'}`);
8899
this._logService.info(`${TAG} Hook notification: ${event.eventType} for "${event.worktreePath}"`);
89100

90101
const worktreeName = this._resolveWorktreeName(event.worktreePath);
@@ -108,9 +119,17 @@ export class OrchestratorTerminalContribution extends Disposable {
108119
}
109120

110121
const isActive = this._orchestratorService.activeWorktree?.path === event.worktreePath;
122+
const targetState = isActive ? WorktreeSessionState.Idle : WorktreeSessionState.Review;
123+
124+
// Skip notification for self-transitions (duplicate Stop events)
125+
if (currentState === targetState) {
126+
this._logService.trace(`${TAG} Duplicate Stop for "${event.worktreePath}" (already ${currentState}) — skipping`);
127+
break;
128+
}
129+
111130
const accepted = this._orchestratorService.setSessionState(
112131
event.worktreePath,
113-
isActive ? WorktreeSessionState.Idle : WorktreeSessionState.Review
132+
targetState
114133
);
115134

116135
if (accepted) {
@@ -165,7 +184,7 @@ export class OrchestratorTerminalContribution extends Disposable {
165184
this._logService.trace(`${TAG} Moved panel terminal ${instance.instanceId} → editor`);
166185
}
167186
if (this._activeKey) {
168-
this._ownership.set(instance.instanceId, { worktreeKey: this._activeKey, groupIndex: 0 });
187+
this._ownership.set(instance.instanceId, { worktreeKey: this._activeKey, groupIndex: 0, tabIndex: -1 });
169188
this._logService.trace(`${TAG} Claimed terminal ${instance.instanceId} → "${this._activeKey}"`);
170189

171190
// Inject worktree path so Claude hooks can identify which
@@ -195,7 +214,7 @@ export class OrchestratorTerminalContribution extends Disposable {
195214
this._register(this._terminalService.onDidDisposeInstance(instance => {
196215
const info = this._ownership.get(instance.instanceId);
197216
if (info) {
198-
console.warn(`[LIFECYCLE DEBUG] onDidDisposeInstance: terminal ${instance.instanceId} owner="${info.worktreeKey}"`);
217+
this._logService.trace(`${TAG} onDidDisposeInstance: terminal ${instance.instanceId} owner="${info.worktreeKey}"`);
199218
this._logService.trace(`${TAG} onDidDisposeInstance: ${instance.instanceId} owner="${info.worktreeKey}" — removing from ownership`);
200219
this._ownership.delete(instance.instanceId);
201220

@@ -261,7 +280,7 @@ export class OrchestratorTerminalContribution extends Disposable {
261280
*/
262281
private _listenForCommandFinished(instance: ITerminalInstance): void {
263282
const handleCommandFinished = () => {
264-
console.warn(`[LIFECYCLE DEBUG] onCommandFinished: terminal ${instance.instanceId}`);
283+
this._logService.trace(`${TAG} onCommandFinished: terminal ${instance.instanceId}`);
265284
this._orchestratorService.scheduleRefresh();
266285

267286
// Crash recovery: if a command finished in a terminal whose
@@ -319,6 +338,24 @@ export class OrchestratorTerminalContribution extends Disposable {
319338
return 0;
320339
}
321340

341+
/**
342+
* Find the tab index (position within the editor group's tab list)
343+
* for a terminal. Returns -1 if not found.
344+
*/
345+
private _findTabIndex(instance: ITerminalInstance): number {
346+
if (instance.target !== TerminalLocation.Editor) {
347+
return -1;
348+
}
349+
const groups = this._editorGroupsService.getGroups(GroupsOrder.GRID_APPEARANCE);
350+
for (const group of groups) {
351+
const editors = group.findEditors(instance.resource);
352+
if (editors.length > 0) {
353+
return group.getIndexOfEditor(editors[0]);
354+
}
355+
}
356+
return -1;
357+
}
358+
322359
/**
323360
* Phase 1: Fires BEFORE saveWorkingSet/applyWorkingSet.
324361
* Snapshots each terminal's group position, then backgrounds all managed
@@ -349,20 +386,36 @@ export class OrchestratorTerminalContribution extends Disposable {
349386
if (previousKey) {
350387
for (const inst of this._terminalService.foregroundInstances) {
351388
if (!this._ownership.has(inst.instanceId)) {
352-
this._ownership.set(inst.instanceId, { worktreeKey: previousKey, groupIndex: 0 });
389+
this._ownership.set(inst.instanceId, { worktreeKey: previousKey, groupIndex: 0, tabIndex: -1 });
353390
this._logService.trace(`${TAG} Adopted unclaimed terminal ${inst.instanceId} → "${previousKey}"`);
354391
}
355392
}
356393
}
357394

358395
/**
359-
* Snapshot group positions BEFORE backgrounding (detach removes from group).
396+
* Snapshot the active editor per group for focus restoration in Phase 2.
397+
*/
398+
if (previousKey) {
399+
const activeEditors = new Map<number, URI>();
400+
const groups = this._editorGroupsService.getGroups(GroupsOrder.GRID_APPEARANCE);
401+
for (let i = 0; i < groups.length; i++) {
402+
const active = groups[i].activeEditor;
403+
if (active?.resource) {
404+
activeEditors.set(i, active.resource);
405+
}
406+
}
407+
this._savedActiveEditors.set(previousKey, activeEditors);
408+
}
409+
410+
/**
411+
* Snapshot group + tab positions BEFORE backgrounding (detach removes from group).
360412
*/
361413
for (const instance of this._terminalService.foregroundInstances) {
362414
const info = this._ownership.get(instance.instanceId);
363415
if (info) {
364416
info.groupIndex = this._findGroupIndex(instance);
365-
this._logService.trace(`${TAG} Snapshotted terminal ${instance.instanceId} → groupIndex=${info.groupIndex}`);
417+
info.tabIndex = this._findTabIndex(instance);
418+
this._logService.trace(`${TAG} Snapshotted terminal ${instance.instanceId} → groupIndex=${info.groupIndex} tabIndex=${info.tabIndex}`);
366419
}
367420
}
368421

@@ -407,14 +460,14 @@ export class OrchestratorTerminalContribution extends Disposable {
407460
/**
408461
* Collect background terminals owned by the new worktree.
409462
*/
410-
const toShow: { instance: ITerminalInstance; groupIndex: number }[] = [];
463+
const toShow: { instance: ITerminalInstance; groupIndex: number; tabIndex: number }[] = [];
411464
for (const instance of [...this._terminalService.instances]) {
412465
const isFg = this._terminalService.foregroundInstances.includes(instance);
413466
const info = this._ownership.get(instance.instanceId);
414467
if (!isFg && info?.worktreeKey === newKey) {
415468
const current = this._terminalService.getInstanceFromId(instance.instanceId);
416469
if (current && !current.isDisposed) {
417-
toShow.push({ instance: current, groupIndex: info.groupIndex });
470+
toShow.push({ instance: current, groupIndex: info.groupIndex, tabIndex: info.tabIndex });
418471
}
419472
} else if (!isFg && info) {
420473
this._logService.trace(`${TAG} Not showing bg terminal ${instance.instanceId}: owner="${info.worktreeKey}" wanted="${newKey}"`);
@@ -471,6 +524,59 @@ export class OrchestratorTerminalContribution extends Disposable {
471524
}
472525
}
473526

527+
/**
528+
* Reorder terminal tabs to their original positions within each group.
529+
* After showBackgroundTerminal + openEditor, terminals are appended
530+
* at the end. Move each one to its saved tabIndex to restore the
531+
* original interleaved order (e.g. [E1, T1, T2, E2] not [E1, E2, T1, T2]).
532+
*/
533+
const groupsAfter = this._editorGroupsService.getGroups(GroupsOrder.GRID_APPEARANCE);
534+
const terminalsByGroup = new Map<number, { instance: ITerminalInstance; tabIndex: number }[]>();
535+
for (const { instance, groupIndex, tabIndex } of toShow) {
536+
if (tabIndex < 0) { continue; }
537+
let list = terminalsByGroup.get(groupIndex);
538+
if (!list) {
539+
list = [];
540+
terminalsByGroup.set(groupIndex, list);
541+
}
542+
list.push({ instance, tabIndex });
543+
}
544+
545+
for (const [groupIndex, terminals] of terminalsByGroup) {
546+
const group = groupsAfter[groupIndex] ?? groupsAfter[0];
547+
if (!group) { continue; }
548+
// Sort by tabIndex ascending so earlier moves don't shift later targets
549+
terminals.sort((a, b) => a.tabIndex - b.tabIndex);
550+
for (const { instance, tabIndex } of terminals) {
551+
const editors = group.findEditors(instance.resource);
552+
if (editors.length > 0) {
553+
const targetIdx = Math.min(tabIndex, group.editors.length - 1);
554+
group.moveEditor(editors[0], group, { index: targetIdx });
555+
this._logService.trace(`${TAG} Reordered terminal ${instance.instanceId} → tabIndex=${targetIdx}`);
556+
}
557+
}
558+
}
559+
560+
/**
561+
* Restore per-group active editor focus. Phase 1 saved which editor
562+
* was active in each group before terminals were backgrounded.
563+
* After terminals are re-shown, the last openEditor call will have
564+
* stolen focus — restore it to the original active editor.
565+
*/
566+
const savedActive = this._savedActiveEditors.get(newKey);
567+
if (savedActive) {
568+
for (const [groupIndex, activeResource] of savedActive) {
569+
const group = groupsAfter[groupIndex] ?? groupsAfter[0];
570+
if (!group) { continue; }
571+
const editors = group.findEditors(activeResource);
572+
if (editors.length > 0) {
573+
await group.openEditor(editors[0], { preserveFocus: true });
574+
this._logService.trace(`${TAG} Restored active editor in group ${groupIndex}: ${activeResource.path}`);
575+
}
576+
}
577+
this._savedActiveEditors.delete(newKey);
578+
}
579+
474580
this._dumpState('phase2-done');
475581
this._logService.trace(`${TAG} ===== PHASE 2 DONE: showed=${toShow.length} fg=${this._terminalService.foregroundInstances.length} =====`);
476582
}
@@ -523,7 +629,7 @@ export class OrchestratorTerminalContribution extends Disposable {
523629
scheduler = new RunOnceScheduler(() => {
524630
const state = this._findWorktreeSessionState(worktreePath);
525631
if (state === WorktreeSessionState.Working) {
526-
console.warn(`[LIFECYCLE DEBUG] Heartbeat timeout: "${worktreePath}" state=${state} → Idle`);
632+
this._logService.trace(`${TAG} Heartbeat timeout: "${worktreePath}" state=${state} → Idle`);
527633
this._logService.warn(`${TAG} Heartbeat timeout for "${worktreePath}" — resetting to Idle`);
528634
this._orchestratorService.setSessionState(worktreePath, WorktreeSessionState.Idle);
529635
const worktreeName = this._resolveWorktreeName(worktreePath);

0 commit comments

Comments
 (0)