Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions backend/openmlr/routes/terminal.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
Security:
- Minimal environment (no server secrets leaked)
- Workspace path validated against WORKSPACES_ROOT
- Shell spawned via subprocess (not os.fork) to avoid async corruption
- Shell spawned via asyncio.create_subprocess_exec (not os.fork) to avoid async corruption
- --norc --noprofile to prevent .bashrc injection
- Proper zombie process cleanup with SIGKILL escalation
"""
Expand All @@ -19,7 +19,6 @@
import pty
import signal
import struct
import subprocess
import termios
from pathlib import Path

Expand Down Expand Up @@ -185,15 +184,17 @@ async def terminal_websocket(

await websocket.accept()

# Spawn PTY using subprocess instead of os.fork() to avoid
# Spawn PTY using asyncio.create_subprocess_exec instead of os.fork() to avoid
# corrupting the async event loop and leaking file descriptors.
master_fd, slave_fd = pty.openpty()
env = _build_safe_env(workspace_path)
shell = "/bin/bash"

try:
proc = subprocess.Popen(
[shell, "--norc", "--noprofile"],
proc = await asyncio.create_subprocess_exec(
shell,
"--norc",
"--noprofile",
stdin=slave_fd,
stdout=slave_fd,
stderr=slave_fd,
Expand Down
2 changes: 1 addition & 1 deletion backend/tests/test_sandbox_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def test_failure(self):
r = ExecutionResult(output="error", success=False, exit_code=1, duration_seconds=2.5)
assert r.success is False
assert r.exit_code == 1
assert r.duration_seconds == 2.5
assert r.duration_seconds == pytest.approx(2.5)

def test_truncation_handled_by_caller(self):
# Truncation is done by the tools, not the dataclass
Expand Down
11 changes: 8 additions & 3 deletions backend/tests/test_tools_workspace.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Tests for workspace agent tools."""

import asyncio
import os
import tempfile

Expand Down Expand Up @@ -177,10 +178,14 @@ async def test_recent_failures_empty(self, workspace_dir):
assert "No recent" in result

async def test_search_operation(self, workspace_dir):
# Create a test file
# Create a test file using async-compatible file write
test_file = os.path.join(workspace_dir, "code", "test.py")
with open(test_file, "w") as f:
f.write("import torch\nmodel = TransformerModel()")

def _write_file():
with open(test_file, "w") as f:
f.write("import torch\nmodel = TransformerModel()")

await asyncio.to_thread(_write_file)

result, success = await _handle_workspace(
operation="search",
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/__tests__/ConfirmDialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ describe('ConfirmDialog', () => {
onCancel={onCancel}
/>
);
fireEvent.keyDown(window, { key: 'Escape' });
// The dialog handles escape via the native cancel event
const dialog = document.querySelector('dialog');
fireEvent(dialog!, new Event('cancel'));
expect(onCancel).toHaveBeenCalled();
});

Expand Down
11 changes: 6 additions & 5 deletions frontend/src/__tests__/ModelModal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,9 @@ describe('ModelModal', () => {

fireEvent.click(screen.getByText('Close'));

const dialog = document.querySelector('dialog');
await waitFor(() => {
expect(screen.queryByText('GPT-4o Mini')).not.toBeInTheDocument();
expect(dialog).not.toHaveAttribute('open');
});
});

Expand Down Expand Up @@ -205,12 +206,12 @@ describe('ModelModal', () => {
expect(screen.getAllByText('GPT-4o').length).toBeGreaterThanOrEqual(1);
});

// Find the overlay (the fixed div with bg-black/60)
const overlay = document.querySelector('.fixed.inset-0');
fireEvent.click(overlay!);
// Click on the dialog element itself (backdrop area) to close
const dialog = document.querySelector('dialog');
fireEvent.click(dialog!);

await waitFor(() => {
expect(screen.queryByText('GPT-4o Mini')).not.toBeInTheDocument();
expect(dialog).not.toHaveAttribute('open');
});
});

Expand Down
3 changes: 2 additions & 1 deletion frontend/src/__tests__/Sidebar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ describe('Sidebar', () => {
<Sidebar {...defaultProps} onSwitch={onSwitch} />
</MemoryRouter>
);
fireEvent.click(screen.getByText('First conversation'));
// The conversation item uses an overlay button with aria-label
fireEvent.click(screen.getByRole('button', { name: /Switch to conversation: First conversation/i }));
expect(onSwitch).toHaveBeenCalledWith('conv-1');
});

Expand Down
9 changes: 9 additions & 0 deletions frontend/src/__tests__/setup.ts
Original file line number Diff line number Diff line change
@@ -1 +1,10 @@
import '@testing-library/jest-dom/vitest';

// Mock HTMLDialogElement methods not implemented in jsdom
HTMLDialogElement.prototype.showModal = function() {
this.setAttribute('open', '');
};

HTMLDialogElement.prototype.close = function() {
this.removeAttribute('open');
};
88 changes: 52 additions & 36 deletions frontend/src/components/ConfirmDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,54 +19,70 @@
onCancel,
danger = false,
}: Props) {
const dialogRef = useRef<HTMLDialogElement>(null);
const cancelRef = useRef<HTMLButtonElement>(null);

useEffect(() => {
const dialog = dialogRef.current;
if (dialog && !dialog.open) {
dialog.showModal();
}

// Focus cancel button by default for safety
cancelRef.current?.focus();

// Handle escape key
const handleKeyDown = (e: KeyboardEvent) => {
if (e.key === 'Escape') {
onCancel();
}
// Handle escape key via dialog's native cancel event
const handleCancel = (e: Event) => {
e.preventDefault();
onCancel();
};
window.addEventListener('keydown', handleKeyDown);
return () => window.removeEventListener('keydown', handleKeyDown);

dialog?.addEventListener('cancel', handleCancel);
return () => dialog?.removeEventListener('cancel', handleCancel);
}, [onCancel]);

const handleBackdropClick = (e: React.MouseEvent<HTMLDialogElement>) => {
// Close when clicking the backdrop (outside the dialog content)
if (e.target === dialogRef.current) {
onCancel();
}
};

return (
<div
className="fixed inset-0 bg-black/60 flex items-center justify-center z-50 p-4"
onClick={onCancel}
<dialog
ref={dialogRef}
className="fixed bg-transparent p-4 m-0 max-w-none max-h-none w-full h-full backdrop:bg-black/60"
onClick={handleBackdropClick}
aria-labelledby="confirm-dialog-title"
>

Check warning on line 57 in frontend/src/components/ConfirmDialog.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Non-interactive elements should not be assigned mouse or keyboard event listeners.

See more on https://sonarcloud.io/project/issues?id=xprilion_OpenMLR&issues=AZ3aTkIOph0VCrK0BHbd&open=AZ3aTkIOph0VCrK0BHbd&pullRequest=32

Check warning on line 57 in frontend/src/components/ConfirmDialog.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Visible, non-interactive elements with click handlers must have at least one keyboard listener.

See more on https://sonarcloud.io/project/issues?id=xprilion_OpenMLR&issues=AZ3aTkIOph0VCrK0BHbe&open=AZ3aTkIOph0VCrK0BHbe&pullRequest=32
<div
className="bg-surface rounded-xl border border-border p-6 max-w-md w-full shadow-xl animate-slide-up"
onClick={(e) => e.stopPropagation()}
>
<h3 className="text-lg font-semibold text-text mb-3">{title}</h3>
<p className="text-text-dim mb-6 leading-relaxed">{message}</p>

<div className="flex gap-3">
<button
ref={cancelRef}
className="flex-1 py-2.5 px-4 rounded-lg border border-border text-text-dim hover:bg-surface-hover hover:text-text transition-colors"
onClick={onCancel}
>
{cancelLabel}
</button>
<button
className={`flex-1 py-2.5 px-4 rounded-lg font-medium text-white transition-colors ${
danger
? 'bg-error hover:opacity-90'
: 'bg-primary hover:bg-primary-hover'
}`}
onClick={onConfirm}
>
{confirmLabel}
</button>
<div className="flex items-center justify-center min-h-full">
<div
className="bg-surface rounded-xl border border-border p-6 max-w-md w-full shadow-xl animate-slide-up"
>
<h3 id="confirm-dialog-title" className="text-lg font-semibold text-text mb-3">{title}</h3>
<p className="text-text-dim mb-6 leading-relaxed">{message}</p>

<div className="flex gap-3">
<button
ref={cancelRef}
className="flex-1 py-2.5 px-4 rounded-lg border border-border text-text-dim hover:bg-surface-hover hover:text-text transition-colors"
onClick={onCancel}
>
{cancelLabel}
</button>
<button
className={`flex-1 py-2.5 px-4 rounded-lg font-medium text-white transition-colors ${
danger
? 'bg-error hover:opacity-90'
: 'bg-primary hover:bg-primary-hover'
}`}
onClick={onConfirm}
>
{confirmLabel}
</button>
</div>
</div>
</div>
</div>
</dialog>
);
}
2 changes: 1 addition & 1 deletion frontend/src/components/FileTree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ export function FileTree({ projectUuid, refreshKey, onFileSelect }: Props) {
return (data.entries || []).map((entry: FileNode) => ({
...entry,
expanded: false,
children: entry.is_dir ? undefined : undefined,
children: undefined,
}));
} catch (err: any) {
setError(err.message);
Expand Down
41 changes: 33 additions & 8 deletions frontend/src/components/ModelModal.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@


import { useState, useEffect, useMemo, useCallback } from 'react';
import { useState, useEffect, useMemo, useCallback, useRef } from 'react';
import { Search, ChevronDown, Check, X, Filter, Save } from 'lucide-react';
import { api } from '../api';
import { useNavigate } from 'react-router-dom';
Expand Down Expand Up @@ -89,6 +89,7 @@

export function ModelModal({ currentModel, onModelChange }: Props) {
const navigate = useNavigate();
const dialogRef = useRef<HTMLDialogElement>(null);
const [open, setOpen] = useState(false);
const [tab, setTab] = useState<Tab>('models');
const [providers, setProviders] = useState<Provider[]>([]);
Expand Down Expand Up @@ -123,6 +124,26 @@
loadData();
}, [open, loadData]);

// Handle dialog open/close
useEffect(() => {
const dialog = dialogRef.current;
if (!dialog) return;

if (open && !dialog.open) {
dialog.showModal();
} else if (!open && dialog.open) {
dialog.close();
}

const handleCancel = (e: Event) => {
e.preventDefault();
setOpen(false);
};

dialog.addEventListener('cancel', handleCancel);
return () => dialog.removeEventListener('cancel', handleCancel);
}, [open]);

// Get only configured providers that are in the models category
const configuredModelProviders = useMemo(() => {
return providers.filter((p) => p.configured && p.categories?.includes('models'));
Expand Down Expand Up @@ -248,20 +269,24 @@
<ChevronDown size={14} className="shrink-0" />
</button>

{open && (
<div
className="fixed inset-0 bg-black/60 flex items-center justify-center z-50 p-4"
onClick={() => setOpen(false)}
>
<dialog
ref={dialogRef}
className="fixed bg-transparent p-4 m-0 max-w-none max-h-none w-full h-full backdrop:bg-black/60"
onClick={(e) => {
if (e.target === dialogRef.current) setOpen(false);
}}
aria-labelledby="model-modal-title"
>

Check warning on line 279 in frontend/src/components/ModelModal.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Visible, non-interactive elements with click handlers must have at least one keyboard listener.

See more on https://sonarcloud.io/project/issues?id=xprilion_OpenMLR&issues=AZ3aTkKIph0VCrK0BHbk&open=AZ3aTkKIph0VCrK0BHbk&pullRequest=32

Check warning on line 279 in frontend/src/components/ModelModal.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Non-interactive elements should not be assigned mouse or keyboard event listeners.

See more on https://sonarcloud.io/project/issues?id=xprilion_OpenMLR&issues=AZ3aTkKIph0VCrK0BHbj&open=AZ3aTkKIph0VCrK0BHbj&pullRequest=32
<div className="flex items-center justify-center min-h-full">
{/* Fixed-size modal — never changes dimensions between loading and loaded */}
<div
className="bg-surface rounded-xl border border-border w-full max-w-lg flex flex-col shadow-xl"
style={{ height: 'min(80vh, 600px)' }}
onClick={(e) => e.stopPropagation()}
>
{/* Tabs */}
<div className="flex border-b border-border shrink-0">
<button
id="model-modal-title"
className={`flex-1 py-3 px-4 text-sm font-medium transition-colors ${
tab === 'models' ? 'text-primary border-b-2 border-primary' : 'text-text-dim hover:text-text'
}`}
Expand Down Expand Up @@ -453,7 +478,7 @@
</div>
</div>
</div>
)}
</dialog>
</>
);
}
6 changes: 4 additions & 2 deletions frontend/src/components/OnboardingModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,11 @@ export function OnboardingModal({ onComplete }: Props) {
with files, knowledge graph, and conversation history.
</p>
<div>
<label className="block text-sm font-medium text-text mb-1.5">
<label className="block text-sm font-medium text-text mb-1.5" htmlFor="project-name">
Project Name <span className="text-error">*</span>
</label>
<input
id="project-name"
type="text"
className="w-full bg-bg border border-border rounded-lg px-4 py-3 text-text placeholder-text-dim focus:border-primary focus:outline-none transition-colors"
placeholder="e.g., Transformer Efficiency Survey"
Expand All @@ -294,10 +295,11 @@ export function OnboardingModal({ onComplete }: Props) {
/>
</div>
<div>
<label className="block text-sm font-medium text-text mb-1.5">
<label className="block text-sm font-medium text-text mb-1.5" htmlFor="project-desc">
Description <span className="text-text-dim">(optional)</span>
</label>
<textarea
id="project-desc"
className="w-full bg-bg border border-border rounded-lg px-4 py-3 text-text placeholder-text-dim focus:border-primary focus:outline-none transition-colors resize-none"
rows={3}
placeholder="What is this project about?"
Expand Down
Loading
Loading