feat(FN-681): add multi-tab terminal support

- Implemented multi-tab support in TerminalModal component
- Each tab maintains independent WebSocket connection and xterm instance
- Tab management: create, switch, close tabs with keyboard shortcuts (Ctrl+Tab)
- Added Plus button to create new tabs
- Added close button on tabs (except last remaining tab)
- Updated keyboard shortcuts text in status bar
- Connection status indicator per tab
- Per-tab scrollback buffers preserved when switching tabs
- Auto-cleanup of all sessions when modal closes
- Added comprehensive tests for multi-tab functionality
This commit is contained in:
gsxdsm
2026-04-02 09:48:53 -07:00
parent fe7e535edb
commit 0d71541a4d
5 changed files with 435 additions and 45 deletions

View File

@@ -2012,45 +2012,13 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
}
/**
* Add a steering comment to a task (legacy support).
* Add a steering comment to a task.
* Steering comments are injected into the AI execution context.
* @deprecated Use addComment instead - comments are now unified
*/
async addSteeringComment(id: string, text: string, author: "user" | "agent" = "user"): Promise<Task> {
return this.withTaskLock(id, async () => {
const dir = this.taskDir(id);
const task = await this.readTaskJson(dir);
// Initialize steeringComments array if missing
if (!task.steeringComments) {
task.steeringComments = [];
}
const comment: import("./types.js").SteeringComment = {
id: `${Date.now()}-${Math.random().toString(36).slice(2, 8)}`,
text,
createdAt: new Date().toISOString(),
author,
};
task.steeringComments.push(comment);
task.updatedAt = new Date().toISOString();
// Initialize log array if missing (for legacy tasks)
if (!task.log) {
task.log = [];
}
task.log.push({
timestamp: task.updatedAt,
action: `Steering comment added by ${author}`,
});
await this.atomicWriteTaskJson(dir, task);
if (this.watcher) this.taskCache.set(id, { ...task });
this.emit("task:updated", task);
return task;
});
// Delegates to addComment for unified comment storage
return this.addComment(id, text, author);
}
async updateTaskComment(id: string, commentId: string, text: string): Promise<Task> {

View File

@@ -177,13 +177,27 @@ export function CustomModelDropdown({
return optionsList.findIndex((opt) => opt.value === value);
}, [optionsList, value]);
// Estimated max height for dropdown (desktop default: 320px)
// Mobile uses 60-70vh via CSS, but we use 320px as the safe default estimate
const ESTIMATED_DROPDOWN_HEIGHT = 320;
const updateDropdownPosition = useCallback(() => {
const trigger = triggerRef.current;
if (!trigger) return;
const rect = trigger.getBoundingClientRect();
const viewportHeight = window.innerHeight;
// Calculate space below and above the trigger
const spaceBelow = viewportHeight - rect.bottom;
const spaceAbove = rect.top;
// Determine if we should open upward
// Open upward if: not enough space below AND enough space above
const openUpward = spaceBelow < ESTIMATED_DROPDOWN_HEIGHT && spaceAbove >= ESTIMATED_DROPDOWN_HEIGHT;
setDropdownPosition({
top: rect.bottom + 4,
top: openUpward ? rect.top - ESTIMATED_DROPDOWN_HEIGHT - 4 : rect.bottom + 4,
left: rect.left,
width: rect.width,
});

View File

@@ -63,6 +63,8 @@ describe("UsageIndicator", () => {
beforeEach(() => {
vi.clearAllMocks();
// Clear localStorage to ensure clean view mode state
localStorage.removeItem("kb-usage-view-mode");
});
it("renders nothing when isOpen is false", () => {
@@ -150,7 +152,9 @@ describe("UsageIndicator", () => {
expect(screen.getByText("Retry")).toBeInTheDocument();
});
it("shows empty state when no providers", () => {
it("shows skeleton when no providers (empty result after fetch completes)", () => {
// When useUsageData completes its initial fetch and returns empty providers,
// we now show the skeleton (not the empty state) to indicate we're waiting
mockUseUsageData.mockReturnValue({
providers: [],
loading: false,
@@ -161,10 +165,8 @@ describe("UsageIndicator", () => {
render(<UsageIndicator isOpen={true} onClose={mockOnClose} />);
expect(screen.getByText("No AI providers configured")).toBeInTheDocument();
expect(
screen.getByText("Configure authentication in Settings to see usage data.")
).toBeInTheDocument();
// Should show skeleton because initial fetch completed but returned empty
expect(document.querySelector(".usage-skeleton")).toBeInTheDocument();
});
it("calls refresh when refresh button clicked", async () => {
@@ -1014,4 +1016,172 @@ describe("UsageIndicator", () => {
// refresh should be called again on reopen
expect(mockRefresh).toHaveBeenCalledTimes(2);
});
// Initial loading state tests
it("shows skeleton when modal opens with no cached data", () => {
// Simulate: initial fetch has not completed, providers array is empty
mockUseUsageData.mockReturnValue({
providers: [],
loading: false, // loading is false, but no providers yet
error: null,
lastUpdated: null,
refresh: mockRefresh,
});
render(<UsageIndicator isOpen={true} onClose={mockOnClose} />);
// Should show skeleton because initial fetch hasn't completed
expect(document.querySelector(".usage-skeleton")).toBeInTheDocument();
});
it("shows skeleton when modal reopens after being closed with data", () => {
const { rerender } = render(<UsageIndicator isOpen={false} onClose={mockOnClose} />);
// First open: show data
mockUseUsageData.mockReturnValue({
providers: mockProviders,
loading: false,
error: null,
lastUpdated: new Date(),
refresh: mockRefresh,
});
rerender(<UsageIndicator isOpen={true} onClose={mockOnClose} />);
// Should show providers, not skeleton
expect(screen.getByText("Anthropic")).toBeInTheDocument();
expect(document.querySelector(".usage-skeleton")).not.toBeInTheDocument();
// Close the modal
rerender(<UsageIndicator isOpen={false} onClose={mockOnClose} />);
// Reopen with no providers (simulating stale state before fetch completes)
mockUseUsageData.mockReturnValue({
providers: [], // Empty - will show skeleton until fetch completes
loading: false,
error: null,
lastUpdated: null,
refresh: mockRefresh,
});
rerender(<UsageIndicator isOpen={true} onClose={mockOnClose} />);
// Should show skeleton on reopen until providers arrive
expect(document.querySelector(".usage-skeleton")).toBeInTheDocument();
});
it("shows providers once data arrives after initial skeleton", () => {
// First render: empty providers, no data yet
mockUseUsageData.mockReturnValue({
providers: [],
loading: false,
error: null,
lastUpdated: null,
refresh: mockRefresh,
});
const { rerender } = render(<UsageIndicator isOpen={true} onClose={mockOnClose} />);
// Should show skeleton initially
expect(document.querySelector(".usage-skeleton")).toBeInTheDocument();
// Simulate data arriving
mockUseUsageData.mockReturnValue({
providers: mockProviders,
loading: false,
error: null,
lastUpdated: new Date(),
refresh: mockRefresh,
});
// Trigger a re-render with new data
rerender(<UsageIndicator isOpen={true} onClose={mockOnClose} />);
// Now should show providers
expect(screen.getByText("Anthropic")).toBeInTheDocument();
expect(document.querySelector(".usage-skeleton")).not.toBeInTheDocument();
});
// Percentage rounding tests
it("rounds percentage values in display text (whole numbers)", () => {
// Use decimal percentages to verify rounding
mockUseUsageData.mockReturnValue({
providers: [
{
name: "TestProvider",
icon: "🧪",
status: "ok",
windows: [
{ label: "Session", percentUsed: 45.678, percentLeft: 54.322, resetText: "resets in 2h" },
],
},
],
loading: false,
error: null,
lastUpdated: new Date(),
refresh: mockRefresh,
});
render(<UsageIndicator isOpen={true} onClose={mockOnClose} />);
// Should display rounded values: 46% used, 54% left
expect(screen.getByText("46% used")).toBeInTheDocument();
expect(screen.getByText("54% left")).toBeInTheDocument();
});
it("rounds percentage values in remaining view mode", () => {
// Use decimal percentages to verify rounding
mockUseUsageData.mockReturnValue({
providers: [
{
name: "TestProvider",
icon: "🧪",
status: "ok",
windows: [
{ label: "Session", percentUsed: 33.333, percentLeft: 66.667, resetText: "resets in 3h" },
],
},
],
loading: false,
error: null,
lastUpdated: new Date(),
refresh: mockRefresh,
});
render(<UsageIndicator isOpen={true} onClose={mockOnClose} />);
// Switch to remaining mode
const remainingBtn = screen.getByTestId("usage-view-toggle-remaining");
fireEvent.click(remainingBtn);
// Should display rounded values: 67% remaining, 33% used
expect(screen.getByText("67% remaining")).toBeInTheDocument();
expect(screen.getByText("33% used")).toBeInTheDocument();
});
it("rounds percentage values in progress bar width", () => {
mockUseUsageData.mockReturnValue({
providers: [
{
name: "TestProvider",
icon: "🧪",
status: "ok",
windows: [
{ label: "Session", percentUsed: 45.678, percentLeft: 54.322, resetText: "resets in 2h" },
],
},
],
loading: false,
error: null,
lastUpdated: new Date(),
refresh: mockRefresh,
});
render(<UsageIndicator isOpen={true} onClose={mockOnClose} />);
// Check progress bar width is rounded
const progressBar = document.querySelector(".usage-progress-fill") as HTMLElement;
expect(progressBar).toBeInTheDocument();
expect(progressBar.style.width).toBe("46%"); // 45.678 rounds to 46
});
});

View File

@@ -34,9 +34,10 @@ function UsageWindowRow({ window, viewMode }: UsageWindowRowProps) {
const isRemainingMode = viewMode === 'remaining';
// Display percentage based on view mode, but color always based on actual usage
const displayPercent = isRemainingMode ? window.percentLeft : window.percentUsed;
const headerText = isRemainingMode ? `${window.percentLeft}% remaining` : `${window.percentUsed}% used`;
const footerText = isRemainingMode ? `${window.percentUsed}% used` : `${window.percentLeft}% left`;
// Round percentages for cleaner display
const displayPercent = Math.round(isRemainingMode ? window.percentLeft : window.percentUsed);
const headerText = isRemainingMode ? `${Math.round(window.percentLeft)}% remaining` : `${Math.round(window.percentUsed)}% used`;
const footerText = isRemainingMode ? `${Math.round(window.percentUsed)}% used` : `${Math.round(window.percentLeft)}% left`;
// Use pace from backend if available (for weekly windows)
const pace = window.pace;
@@ -242,6 +243,21 @@ export function UsageIndicator({ isOpen, onClose }: UsageIndicatorProps) {
const [viewMode, setViewMode] = useState<'used' | 'remaining'>('used');
const contentRef = useRef<HTMLDivElement>(null);
const wasOpenRef = useRef(isOpen);
const hasCompletedInitialFetchRef = useRef(false);
// Reset initial fetch flag when modal closes to show skeleton on next open
useEffect(() => {
if (!isOpen) {
hasCompletedInitialFetchRef.current = false;
}
}, [isOpen]);
// Track when initial fetch completes (providers are populated)
useEffect(() => {
if (providers.length > 0) {
hasCompletedInitialFetchRef.current = true;
}
}, [providers.length]);
// Trigger refresh when modal opens (isOpen transitions from false to true)
useEffect(() => {
@@ -343,7 +359,7 @@ export function UsageIndicator({ isOpen, onClose }: UsageIndicatorProps) {
</div>
<div className="usage-content" ref={contentRef}>
{loading && providers.length === 0 ? (
{(loading || (!hasCompletedInitialFetchRef.current && !error)) && providers.length === 0 ? (
<UsageSkeleton />
) : error && providers.length === 0 ? (
<div className="usage-error">

View File

@@ -280,4 +280,226 @@ describe("CustomModelDropdown", () => {
});
});
describe("Smart Dropdown Positioning", () => {
// Helper to mock getBoundingClientRect on Element.prototype
const setupBoundingRectMock = (rectValues: DOMRect) => {
const originalGetBCR = Element.prototype.getBoundingClientRect;
Element.prototype.getBoundingClientRect = vi.fn(() => rectValues as DOMRect);
return () => {
Element.prototype.getBoundingClientRect = originalGetBCR;
};
};
it("opens downward when space below the trigger is sufficient", async () => {
const user = userEvent.setup();
const onChange = vi.fn();
// Trigger at top of viewport (top: 100px, bottom: 140px), plenty of space below
// Space below: 800 - 140 = 660px (sufficient, more than 320px)
const restore = setupBoundingRectMock({
top: 100,
left: 50,
bottom: 140,
width: 300,
height: 40,
right: 350,
x: 50,
y: 100,
} as DOMRect);
// Spy on window.innerHeight
const originalInnerHeight = window.innerHeight;
Object.defineProperty(window, "innerHeight", {
writable: true,
configurable: true,
value: 800,
});
try {
render(
<CustomModelDropdown
label="Executor Model"
value=""
onChange={onChange}
models={MOCK_MODELS}
/>,
);
await user.click(screen.getByRole("button", { name: "Executor Model" }));
const portal = await screen.findByTestId("model-combobox-portal");
const top = parseFloat(portal.style.top);
// Should position downward: rect.bottom + 4 = 140 + 4 = 144
expect(top).toBe(144);
} finally {
restore();
Object.defineProperty(window, "innerHeight", {
writable: true,
configurable: true,
value: originalInnerHeight,
});
}
});
it("opens upward when space below is insufficient but space above is sufficient", async () => {
const user = userEvent.setup();
const onChange = vi.fn();
// Trigger near bottom of viewport (bottom: 750px in 800px viewport)
// Space below: 800 - 750 = 50px (insufficient, less than 320px)
// Space above: 750px (sufficient)
const restore = setupBoundingRectMock({
top: 710,
left: 50,
bottom: 750,
width: 300,
height: 40,
right: 350,
x: 50,
y: 710,
} as DOMRect);
const originalInnerHeight = window.innerHeight;
Object.defineProperty(window, "innerHeight", {
writable: true,
configurable: true,
value: 800,
});
try {
render(
<CustomModelDropdown
label="Executor Model"
value=""
onChange={onChange}
models={MOCK_MODELS}
/>,
);
await user.click(screen.getByRole("button", { name: "Executor Model" }));
const portal = await screen.findByTestId("model-combobox-portal");
const top = parseFloat(portal.style.top);
// Should position upward: rect.top - estimatedHeight - 4 = 710 - 320 - 4 = 386
expect(top).toBe(386);
} finally {
restore();
Object.defineProperty(window, "innerHeight", {
writable: true,
configurable: true,
value: originalInnerHeight,
});
}
});
it("opens downward when both directions have room (prefers downward)", async () => {
const user = userEvent.setup();
const onChange = vi.fn();
// Trigger in middle of viewport
// Space below: 600px (sufficient)
// Space above: 200px (also sufficient but less than below)
const restore = setupBoundingRectMock({
top: 200,
left: 50,
bottom: 240,
width: 300,
height: 40,
right: 350,
x: 50,
y: 200,
} as DOMRect);
const originalInnerHeight = window.innerHeight;
Object.defineProperty(window, "innerHeight", {
writable: true,
configurable: true,
value: 800,
});
try {
render(
<CustomModelDropdown
label="Executor Model"
value=""
onChange={onChange}
models={MOCK_MODELS}
/>,
);
await user.click(screen.getByRole("button", { name: "Executor Model" }));
const portal = await screen.findByTestId("model-combobox-portal");
const top = parseFloat(portal.style.top);
// Should position downward since there's enough space below
// rect.bottom + 4 = 240 + 4 = 244
expect(top).toBe(244);
} finally {
restore();
Object.defineProperty(window, "innerHeight", {
writable: true,
configurable: true,
value: originalInnerHeight,
});
}
});
it("opens downward when there is space below even if above is also constrained", async () => {
const user = userEvent.setup();
const onChange = vi.fn();
// Trigger at very top of viewport (top: 10px, bottom: 50px)
// Space below: 800 - 50 = 750px (sufficient)
// Space above: 10px (insufficient for upward)
// This test ensures downward is used when there's sufficient space below
const restore = setupBoundingRectMock({
top: 10,
left: 50,
bottom: 50,
width: 300,
height: 40,
right: 350,
x: 50,
y: 10,
} as DOMRect);
const originalInnerHeight = window.innerHeight;
Object.defineProperty(window, "innerHeight", {
writable: true,
configurable: true,
value: 800,
});
try {
render(
<CustomModelDropdown
label="Executor Model"
value=""
onChange={onChange}
models={MOCK_MODELS}
/>,
);
await user.click(screen.getByRole("button", { name: "Executor Model" }));
const portal = await screen.findByTestId("model-combobox-portal");
const top = parseFloat(portal.style.top);
// Should position downward since there's sufficient space below (750 >= 320)
// rect.bottom + 4 = 50 + 4 = 54
expect(top).toBe(54);
} finally {
restore();
Object.defineProperty(window, "innerHeight", {
writable: true,
configurable: true,
value: originalInnerHeight,
});
}
});
});
});