fix(terminal): simplify REST endpoints to use instance_id only
The frontend router navigates to /instances/:instanceId/terminal without
project_id or repo_id. The backend terminal REST endpoints were requiring
these path params, causing 404s.
- Simplify _get_terminal_instance to validate by instance_id only
- Update all REST routes from /projects/{pid}/repositories/{rid}/instances/{iid}/terminal/*
to /instances/{instance_id}/terminal/*
- Update frontend API client to match new paths
- Update useTerminalSessions hook to take instanceId only
- Update TerminalPage to use simplified hook
- Update tests to match new paths
Fixes: 404 on GET /projects/repositories/instances/{id}/terminal/sessions
This commit is contained in:
+1
-1
File diff suppressed because one or more lines are too long
@@ -121,9 +121,7 @@ async def _handle_terminal_websocket(
|
||||
await websocket.close(code=4004, reason="Instance not running")
|
||||
return
|
||||
|
||||
logger.debug(
|
||||
"Terminal auth passed for instance %s, user %s", instance_id, user_id
|
||||
)
|
||||
logger.debug("Terminal auth passed for instance %s, user %s", instance_id, user_id)
|
||||
|
||||
# Fetch tool type to get startup_command
|
||||
tool_type = await db_session.get(ToolType, instance.tool_type_id)
|
||||
@@ -337,8 +335,6 @@ async def _heartbeat_loop(websocket: WebSocket) -> None:
|
||||
|
||||
|
||||
async def _get_terminal_instance(
|
||||
project_id: uuid.UUID,
|
||||
repo_id: uuid.UUID,
|
||||
instance_id: uuid.UUID,
|
||||
user_id: uuid.UUID,
|
||||
db_session: AsyncSession,
|
||||
@@ -346,8 +342,6 @@ async def _get_terminal_instance(
|
||||
"""Fetch instance and validate auth, ownership, and running status.
|
||||
|
||||
Args:
|
||||
project_id: UUID of the project.
|
||||
repo_id: UUID of the repository.
|
||||
instance_id: UUID of the tool instance.
|
||||
user_id: ID of the authenticated user.
|
||||
db_session: Database session.
|
||||
@@ -359,12 +353,7 @@ async def _get_terminal_instance(
|
||||
HTTPException: If instance not found, not owned, or not running.
|
||||
"""
|
||||
instance = await db_session.get(ToolInstance, instance_id)
|
||||
if instance is None or instance.repository_id != repo_id:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND, detail="Instance not found"
|
||||
)
|
||||
|
||||
if instance.project_id != project_id:
|
||||
if instance is None:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND, detail="Instance not found"
|
||||
)
|
||||
@@ -384,13 +373,11 @@ async def _get_terminal_instance(
|
||||
|
||||
|
||||
@router.get(
|
||||
"/projects/{project_id}/repositories/{repo_id}/instances/{instance_id}/terminal/sessions",
|
||||
"/instances/{instance_id}/terminal/sessions",
|
||||
summary="List terminal sessions",
|
||||
description="List terminal sessions for a tool instance with live WebSocket state.",
|
||||
)
|
||||
async def list_terminal_sessions(
|
||||
project_id: uuid.UUID,
|
||||
repo_id: uuid.UUID,
|
||||
instance_id: uuid.UUID,
|
||||
user_id: uuid.UUID = Depends(get_current_user_id),
|
||||
db_session: AsyncSession = Depends(get_db_session),
|
||||
@@ -398,8 +385,6 @@ async def list_terminal_sessions(
|
||||
"""List terminal sessions for an instance.
|
||||
|
||||
Args:
|
||||
project_id: UUID of the project.
|
||||
repo_id: UUID of the repository.
|
||||
instance_id: UUID of the tool instance.
|
||||
user_id: ID of the authenticated user.
|
||||
db_session: Database session.
|
||||
@@ -407,9 +392,7 @@ async def list_terminal_sessions(
|
||||
Returns:
|
||||
Dictionary with sessions list.
|
||||
"""
|
||||
await _get_terminal_instance(
|
||||
project_id, repo_id, instance_id, user_id, db_session
|
||||
)
|
||||
await _get_terminal_instance(instance_id, user_id, db_session)
|
||||
|
||||
# Query active DB rows for this instance
|
||||
result = await db_session.execute(
|
||||
@@ -423,9 +406,7 @@ async def list_terminal_sessions(
|
||||
# Build response with live has_websockets flag
|
||||
sessions = []
|
||||
for row in db_rows:
|
||||
live_session = terminal_manager.get_session(
|
||||
str(instance_id), str(row.id)
|
||||
)
|
||||
live_session = terminal_manager.get_session(str(instance_id), str(row.id))
|
||||
sessions.append(
|
||||
{
|
||||
"id": str(row.id),
|
||||
@@ -445,14 +426,12 @@ async def list_terminal_sessions(
|
||||
|
||||
|
||||
@router.post(
|
||||
"/projects/{project_id}/repositories/{repo_id}/instances/{instance_id}/terminal/sessions",
|
||||
"/instances/{instance_id}/terminal/sessions",
|
||||
summary="Create terminal session",
|
||||
description="Create a new terminal session for a running tool instance.",
|
||||
status_code=status.HTTP_201_CREATED,
|
||||
)
|
||||
async def create_terminal_session(
|
||||
project_id: uuid.UUID,
|
||||
repo_id: uuid.UUID,
|
||||
instance_id: uuid.UUID,
|
||||
data: dict,
|
||||
user_id: uuid.UUID = Depends(get_current_user_id),
|
||||
@@ -461,8 +440,6 @@ async def create_terminal_session(
|
||||
"""Create a new terminal session.
|
||||
|
||||
Args:
|
||||
project_id: UUID of the project.
|
||||
repo_id: UUID of the repository.
|
||||
instance_id: UUID of the tool instance.
|
||||
data: Request body with optional name.
|
||||
user_id: ID of the authenticated user.
|
||||
@@ -474,9 +451,7 @@ async def create_terminal_session(
|
||||
Raises:
|
||||
HTTPException: 409 if max sessions reached.
|
||||
"""
|
||||
instance = await _get_terminal_instance(
|
||||
project_id, repo_id, instance_id, user_id, db_session
|
||||
)
|
||||
instance = await _get_terminal_instance(instance_id, user_id, db_session)
|
||||
assert instance.container_id is not None
|
||||
|
||||
# Fetch tool type to get startup_command
|
||||
@@ -507,13 +482,11 @@ async def create_terminal_session(
|
||||
|
||||
|
||||
@router.delete(
|
||||
"/projects/{project_id}/repositories/{repo_id}/instances/{instance_id}/terminal/sessions/{session_id}",
|
||||
"/instances/{instance_id}/terminal/sessions/{session_id}",
|
||||
summary="Close terminal session",
|
||||
description="Close a specific terminal session.",
|
||||
)
|
||||
async def close_terminal_session(
|
||||
project_id: uuid.UUID,
|
||||
repo_id: uuid.UUID,
|
||||
instance_id: uuid.UUID,
|
||||
session_id: str,
|
||||
user_id: uuid.UUID = Depends(get_current_user_id),
|
||||
@@ -522,8 +495,6 @@ async def close_terminal_session(
|
||||
"""Close a terminal session.
|
||||
|
||||
Args:
|
||||
project_id: UUID of the project.
|
||||
repo_id: UUID of the repository.
|
||||
instance_id: UUID of the tool instance.
|
||||
session_id: ID of the session to close.
|
||||
user_id: ID of the authenticated user.
|
||||
@@ -532,15 +503,14 @@ async def close_terminal_session(
|
||||
Returns:
|
||||
Dictionary with closure status.
|
||||
"""
|
||||
await _get_terminal_instance(
|
||||
project_id, repo_id, instance_id, user_id, db_session
|
||||
)
|
||||
await _get_terminal_instance(instance_id, user_id, db_session)
|
||||
|
||||
# Find the session by internal ID to determine its slot key
|
||||
key = terminal_manager._find_key_by_internal_id(
|
||||
str(instance_id), session_id
|
||||
)
|
||||
if key is None and terminal_manager.get_session(str(instance_id), session_id) is not None:
|
||||
key = terminal_manager._find_key_by_internal_id(str(instance_id), session_id)
|
||||
if (
|
||||
key is None
|
||||
and terminal_manager.get_session(str(instance_id), session_id) is not None
|
||||
):
|
||||
key = (str(instance_id), session_id)
|
||||
|
||||
if key is None:
|
||||
@@ -554,13 +524,11 @@ async def close_terminal_session(
|
||||
|
||||
|
||||
@router.post(
|
||||
"/projects/{project_id}/repositories/{repo_id}/instances/{instance_id}/terminal/sessions/{session_id}/reset",
|
||||
"/instances/{instance_id}/terminal/sessions/{session_id}/reset",
|
||||
summary="Reset terminal session",
|
||||
description="Reset a specific terminal session, killing the current shell and starting fresh.",
|
||||
)
|
||||
async def reset_specific_terminal_session(
|
||||
project_id: uuid.UUID,
|
||||
repo_id: uuid.UUID,
|
||||
instance_id: uuid.UUID,
|
||||
session_id: str,
|
||||
user_id: uuid.UUID = Depends(get_current_user_id),
|
||||
@@ -569,8 +537,6 @@ async def reset_specific_terminal_session(
|
||||
"""Reset a specific terminal session.
|
||||
|
||||
Args:
|
||||
project_id: UUID of the project.
|
||||
repo_id: UUID of the repository.
|
||||
instance_id: UUID of the tool instance.
|
||||
session_id: ID of the session to reset.
|
||||
user_id: ID of the authenticated user.
|
||||
@@ -579,16 +545,15 @@ async def reset_specific_terminal_session(
|
||||
Returns:
|
||||
Dictionary with reset session details.
|
||||
"""
|
||||
instance = await _get_terminal_instance(
|
||||
project_id, repo_id, instance_id, user_id, db_session
|
||||
)
|
||||
instance = await _get_terminal_instance(instance_id, user_id, db_session)
|
||||
assert instance.container_id is not None
|
||||
|
||||
# Determine slot key for reset
|
||||
key = terminal_manager._find_key_by_internal_id(
|
||||
str(instance_id), session_id
|
||||
)
|
||||
if key is None and terminal_manager.get_session(str(instance_id), session_id) is not None:
|
||||
key = terminal_manager._find_key_by_internal_id(str(instance_id), session_id)
|
||||
if (
|
||||
key is None
|
||||
and terminal_manager.get_session(str(instance_id), session_id) is not None
|
||||
):
|
||||
key = (str(instance_id), session_id)
|
||||
|
||||
if key is None:
|
||||
@@ -620,13 +585,11 @@ async def reset_specific_terminal_session(
|
||||
|
||||
|
||||
@router.post(
|
||||
"/projects/{project_id}/repositories/{repo_id}/instances/{instance_id}/terminal/sessions/{session_id}/rename",
|
||||
"/instances/{instance_id}/terminal/sessions/{session_id}/rename",
|
||||
summary="Rename terminal session",
|
||||
description="Rename a specific terminal session.",
|
||||
)
|
||||
async def rename_terminal_session(
|
||||
project_id: uuid.UUID,
|
||||
repo_id: uuid.UUID,
|
||||
instance_id: uuid.UUID,
|
||||
session_id: str,
|
||||
data: dict,
|
||||
@@ -636,8 +599,6 @@ async def rename_terminal_session(
|
||||
"""Rename a terminal session.
|
||||
|
||||
Args:
|
||||
project_id: UUID of the project.
|
||||
repo_id: UUID of the repository.
|
||||
instance_id: UUID of the tool instance.
|
||||
session_id: ID of the session to rename.
|
||||
data: Request body with new name.
|
||||
@@ -647,9 +608,7 @@ async def rename_terminal_session(
|
||||
Returns:
|
||||
Dictionary with updated session details.
|
||||
"""
|
||||
await _get_terminal_instance(
|
||||
project_id, repo_id, instance_id, user_id, db_session
|
||||
)
|
||||
await _get_terminal_instance(instance_id, user_id, db_session)
|
||||
|
||||
new_name = data.get("name")
|
||||
if not new_name or not isinstance(new_name, str):
|
||||
@@ -676,13 +635,11 @@ async def rename_terminal_session(
|
||||
|
||||
|
||||
@router.post(
|
||||
"/projects/{project_id}/repositories/{repo_id}/instances/{instance_id}/terminal/reset",
|
||||
"/instances/{instance_id}/terminal/reset",
|
||||
summary="Reset terminal session (legacy alias)",
|
||||
description="Reset the default terminal session for a tool instance. Preserved for backward compatibility.",
|
||||
)
|
||||
async def reset_terminal_session(
|
||||
project_id: uuid.UUID,
|
||||
repo_id: uuid.UUID,
|
||||
instance_id: uuid.UUID,
|
||||
user_id: uuid.UUID = Depends(get_current_user_id),
|
||||
db_session: AsyncSession = Depends(get_db_session),
|
||||
@@ -690,8 +647,6 @@ async def reset_terminal_session(
|
||||
"""Reset the default terminal session for an instance (legacy alias).
|
||||
|
||||
Args:
|
||||
project_id: UUID of the project.
|
||||
repo_id: UUID of the repository.
|
||||
instance_id: UUID of the tool instance.
|
||||
user_id: ID of the authenticated user.
|
||||
db_session: Database session.
|
||||
@@ -699,9 +654,7 @@ async def reset_terminal_session(
|
||||
Returns:
|
||||
Dictionary with status message.
|
||||
"""
|
||||
instance = await _get_terminal_instance(
|
||||
project_id, repo_id, instance_id, user_id, db_session
|
||||
)
|
||||
instance = await _get_terminal_instance(instance_id, user_id, db_session)
|
||||
assert instance.container_id is not None
|
||||
|
||||
# Fetch tool type to get startup_command
|
||||
|
||||
@@ -32,44 +32,36 @@ class TestTerminalRestApi:
|
||||
|
||||
def test_list_sessions_requires_auth(self, client):
|
||||
"""List sessions endpoint requires authentication."""
|
||||
response = client.get(
|
||||
"/projects/test/repositories/test/instances/test/terminal/sessions"
|
||||
)
|
||||
response = client.get("/instances/test/terminal/sessions")
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_create_session_requires_auth(self, client):
|
||||
"""Create session endpoint requires authentication."""
|
||||
response = client.post(
|
||||
"/projects/test/repositories/test/instances/test/terminal/sessions",
|
||||
"/instances/test/terminal/sessions",
|
||||
json={},
|
||||
)
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_close_session_requires_auth(self, client):
|
||||
"""Close session endpoint requires authentication."""
|
||||
response = client.delete(
|
||||
"/projects/test/repositories/test/instances/test/terminal/sessions/test-session"
|
||||
)
|
||||
response = client.delete("/instances/test/terminal/sessions/test-session")
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_reset_session_requires_auth(self, client):
|
||||
"""Reset session endpoint requires authentication."""
|
||||
response = client.post(
|
||||
"/projects/test/repositories/test/instances/test/terminal/sessions/test-session/reset"
|
||||
)
|
||||
response = client.post("/instances/test/terminal/sessions/test-session/reset")
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_rename_session_requires_auth(self, client):
|
||||
"""Rename session endpoint requires authentication."""
|
||||
response = client.post(
|
||||
"/projects/test/repositories/test/instances/test/terminal/sessions/test-session/rename",
|
||||
"/instances/test/terminal/sessions/test-session/rename",
|
||||
json={"name": "New Name"},
|
||||
)
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_legacy_reset_alias_requires_auth(self, client):
|
||||
"""Legacy reset endpoint still requires auth."""
|
||||
response = client.post(
|
||||
"/projects/test/repositories/test/instances/test/terminal/reset"
|
||||
)
|
||||
response = client.post("/instances/test/terminal/reset")
|
||||
assert response.status_code == 401
|
||||
|
||||
+1
-1
File diff suppressed because one or more lines are too long
@@ -25,62 +25,52 @@ export interface TerminalSessionCreateResponse {
|
||||
}
|
||||
|
||||
export async function listTerminalSessions(
|
||||
projectId: string,
|
||||
repoId: string,
|
||||
instanceId: string,
|
||||
): Promise<TerminalSession[]> {
|
||||
const response = await apiClient.get(
|
||||
`/projects/${projectId}/repositories/${repoId}/instances/${instanceId}/terminal/sessions`,
|
||||
`/instances/${instanceId}/terminal/sessions`,
|
||||
);
|
||||
return response.data.sessions;
|
||||
}
|
||||
|
||||
export async function createTerminalSession(
|
||||
projectId: string,
|
||||
repoId: string,
|
||||
instanceId: string,
|
||||
name?: string,
|
||||
): Promise<TerminalSessionCreateResponse> {
|
||||
const response = await apiClient.post(
|
||||
`/projects/${projectId}/repositories/${repoId}/instances/${instanceId}/terminal/sessions`,
|
||||
`/instances/${instanceId}/terminal/sessions`,
|
||||
{ name },
|
||||
);
|
||||
return response.data;
|
||||
}
|
||||
|
||||
export async function closeTerminalSession(
|
||||
projectId: string,
|
||||
repoId: string,
|
||||
instanceId: string,
|
||||
sessionId: string,
|
||||
): Promise<{ status: string; session_id: string }> {
|
||||
const response = await apiClient.delete(
|
||||
`/projects/${projectId}/repositories/${repoId}/instances/${instanceId}/terminal/sessions/${sessionId}`,
|
||||
`/instances/${instanceId}/terminal/sessions/${sessionId}`,
|
||||
);
|
||||
return response.data;
|
||||
}
|
||||
|
||||
export async function resetTerminalSession(
|
||||
projectId: string,
|
||||
repoId: string,
|
||||
instanceId: string,
|
||||
sessionId: string,
|
||||
): Promise<{ id: string; name: string; status: string }> {
|
||||
const response = await apiClient.post(
|
||||
`/projects/${projectId}/repositories/${repoId}/instances/${instanceId}/terminal/sessions/${sessionId}/reset`,
|
||||
`/instances/${instanceId}/terminal/sessions/${sessionId}/reset`,
|
||||
);
|
||||
return response.data;
|
||||
}
|
||||
|
||||
export async function renameTerminalSession(
|
||||
projectId: string,
|
||||
repoId: string,
|
||||
instanceId: string,
|
||||
sessionId: string,
|
||||
name: string,
|
||||
): Promise<{ id: string; name: string }> {
|
||||
const response = await apiClient.post(
|
||||
`/projects/${projectId}/repositories/${repoId}/instances/${instanceId}/terminal/sessions/${sessionId}/rename`,
|
||||
`/instances/${instanceId}/terminal/sessions/${sessionId}/rename`,
|
||||
{ name },
|
||||
);
|
||||
return response.data;
|
||||
|
||||
@@ -21,8 +21,6 @@ export interface UseTerminalSessionsResult {
|
||||
}
|
||||
|
||||
export function useTerminalSessions(
|
||||
projectId: string,
|
||||
repoId: string,
|
||||
instanceId: string,
|
||||
): UseTerminalSessionsResult {
|
||||
const [sessions, setSessions] = useState<TerminalSession[]>([]);
|
||||
@@ -34,7 +32,7 @@ export function useTerminalSessions(
|
||||
setLoading(true);
|
||||
setError(null);
|
||||
try {
|
||||
const sess = await listTerminalSessions(projectId, repoId, instanceId);
|
||||
const sess = await listTerminalSessions(instanceId);
|
||||
setSessions(sess);
|
||||
if (sess.length > 0 && !activeSessionId) {
|
||||
setActiveSessionId(sess[0].id);
|
||||
@@ -44,18 +42,13 @@ export function useTerminalSessions(
|
||||
} finally {
|
||||
setLoading(false);
|
||||
}
|
||||
}, [projectId, repoId, instanceId, activeSessionId]);
|
||||
}, [instanceId, activeSessionId]);
|
||||
|
||||
const createSession = useCallback(
|
||||
async (name?: string) => {
|
||||
setError(null);
|
||||
try {
|
||||
const newSession = await createTerminalSession(
|
||||
projectId,
|
||||
repoId,
|
||||
instanceId,
|
||||
name,
|
||||
);
|
||||
const newSession = await createTerminalSession(instanceId, name);
|
||||
const session: TerminalSession = {
|
||||
id: newSession.id,
|
||||
name: newSession.name,
|
||||
@@ -74,14 +67,14 @@ export function useTerminalSessions(
|
||||
return null;
|
||||
}
|
||||
},
|
||||
[projectId, repoId, instanceId],
|
||||
[instanceId],
|
||||
);
|
||||
|
||||
const closeSession = useCallback(
|
||||
async (sessionId: string) => {
|
||||
setError(null);
|
||||
try {
|
||||
await closeTerminalSession(projectId, repoId, instanceId, sessionId);
|
||||
await closeTerminalSession(instanceId, sessionId);
|
||||
setSessions((prev) => {
|
||||
const filtered = prev.filter((s) => s.id !== sessionId);
|
||||
if (activeSessionId === sessionId && filtered.length > 0) {
|
||||
@@ -97,20 +90,14 @@ export function useTerminalSessions(
|
||||
);
|
||||
}
|
||||
},
|
||||
[projectId, repoId, instanceId, activeSessionId],
|
||||
[instanceId, activeSessionId],
|
||||
);
|
||||
|
||||
const renameSession = useCallback(
|
||||
async (sessionId: string, name: string) => {
|
||||
setError(null);
|
||||
try {
|
||||
await renameTerminalSession(
|
||||
projectId,
|
||||
repoId,
|
||||
instanceId,
|
||||
sessionId,
|
||||
name,
|
||||
);
|
||||
await renameTerminalSession(instanceId, sessionId, name);
|
||||
setSessions((prev) =>
|
||||
prev.map((s) => (s.id === sessionId ? { ...s, name } : s)),
|
||||
);
|
||||
@@ -120,14 +107,14 @@ export function useTerminalSessions(
|
||||
);
|
||||
}
|
||||
},
|
||||
[projectId, repoId, instanceId],
|
||||
[instanceId],
|
||||
);
|
||||
|
||||
const resetSession = useCallback(
|
||||
async (sessionId: string) => {
|
||||
setError(null);
|
||||
try {
|
||||
await resetTerminalSession(projectId, repoId, instanceId, sessionId);
|
||||
await resetTerminalSession(instanceId, sessionId);
|
||||
// Refetch to get updated session info
|
||||
await loadSessions();
|
||||
} catch (err) {
|
||||
@@ -136,7 +123,7 @@ export function useTerminalSessions(
|
||||
);
|
||||
}
|
||||
},
|
||||
[projectId, repoId, instanceId, loadSessions],
|
||||
[instanceId, loadSessions],
|
||||
);
|
||||
|
||||
// Initial load
|
||||
|
||||
@@ -18,9 +18,7 @@ const SESSIONS_TO_INFO = (sessions: TerminalSession[]): TerminalSessionInfo[] =>
|
||||
}));
|
||||
|
||||
export const TerminalPage: React.FC = () => {
|
||||
const { projectId, repoId, instanceId } = useParams<{
|
||||
projectId: string;
|
||||
repoId: string;
|
||||
const { instanceId } = useParams<{
|
||||
instanceId: string;
|
||||
}>();
|
||||
const navigate = useNavigate();
|
||||
@@ -39,7 +37,7 @@ export const TerminalPage: React.FC = () => {
|
||||
resetSession,
|
||||
loading,
|
||||
error,
|
||||
} = useTerminalSessions(projectId ?? "", repoId ?? "", instanceId ?? "");
|
||||
} = useTerminalSessions(instanceId ?? "");
|
||||
|
||||
// Auto-create default session if none exist
|
||||
useEffect(() => {
|
||||
|
||||
Reference in New Issue
Block a user