mirror of
https://github.com/leonvanzyl/autocoder.git
synced 2026-03-16 18:33:08 +00:00
fix: accept WebSocket before validation to prevent opaque 403 errors
All 5 WebSocket endpoints (expand, spec, assistant, terminal, project) were closing the connection before calling accept() when validation failed. Starlette converts pre-accept close into an HTTP 403, giving clients no meaningful error information. Server changes: - Move websocket.accept() before all validation checks in every WS handler - Send JSON error message before closing so clients get actionable errors - Fix validate_project_name usage (raises HTTPException, not returns bool) - ConnectionManager.connect() no longer calls accept() (caller's job) Client changes: - All 3 WS hooks (useWebSocket, useExpandChat, useSpecChat) skip reconnection on 4xxx close codes (application errors won't self-resolve) - Gate expand button, keyboard shortcut, and modal on hasSpec - Add hasSpec to useEffect dependency array to prevent stale closure - Update keyboard shortcuts help text for E key context Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
0
bin/autoforge.js
Normal file → Executable file
0
bin/autoforge.js
Normal file → Executable file
@@ -217,20 +217,26 @@ async def assistant_chat_websocket(websocket: WebSocket, project_name: str):
|
|||||||
- {"type": "error", "content": "..."} - Error message
|
- {"type": "error", "content": "..."} - Error message
|
||||||
- {"type": "pong"} - Keep-alive pong
|
- {"type": "pong"} - Keep-alive pong
|
||||||
"""
|
"""
|
||||||
if not validate_project_name(project_name):
|
# Always accept WebSocket first to avoid opaque 403 errors
|
||||||
|
await websocket.accept()
|
||||||
|
|
||||||
|
try:
|
||||||
|
project_name = validate_project_name(project_name)
|
||||||
|
except HTTPException:
|
||||||
|
await websocket.send_json({"type": "error", "content": "Invalid project name"})
|
||||||
await websocket.close(code=4000, reason="Invalid project name")
|
await websocket.close(code=4000, reason="Invalid project name")
|
||||||
return
|
return
|
||||||
|
|
||||||
project_dir = _get_project_path(project_name)
|
project_dir = _get_project_path(project_name)
|
||||||
if not project_dir:
|
if not project_dir:
|
||||||
|
await websocket.send_json({"type": "error", "content": "Project not found in registry"})
|
||||||
await websocket.close(code=4004, reason="Project not found in registry")
|
await websocket.close(code=4004, reason="Project not found in registry")
|
||||||
return
|
return
|
||||||
|
|
||||||
if not project_dir.exists():
|
if not project_dir.exists():
|
||||||
|
await websocket.send_json({"type": "error", "content": "Project directory not found"})
|
||||||
await websocket.close(code=4004, reason="Project directory not found")
|
await websocket.close(code=4004, reason="Project directory not found")
|
||||||
return
|
return
|
||||||
|
|
||||||
await websocket.accept()
|
|
||||||
logger.info(f"Assistant WebSocket connected for project: {project_name}")
|
logger.info(f"Assistant WebSocket connected for project: {project_name}")
|
||||||
|
|
||||||
session: Optional[AssistantChatSession] = None
|
session: Optional[AssistantChatSession] = None
|
||||||
|
|||||||
@@ -104,19 +104,26 @@ async def expand_project_websocket(websocket: WebSocket, project_name: str):
|
|||||||
- {"type": "error", "content": "..."} - Error message
|
- {"type": "error", "content": "..."} - Error message
|
||||||
- {"type": "pong"} - Keep-alive pong
|
- {"type": "pong"} - Keep-alive pong
|
||||||
"""
|
"""
|
||||||
|
# Always accept the WebSocket first to avoid opaque 403 errors.
|
||||||
|
# Starlette returns 403 if we close before accepting.
|
||||||
|
await websocket.accept()
|
||||||
|
|
||||||
try:
|
try:
|
||||||
project_name = validate_project_name(project_name)
|
project_name = validate_project_name(project_name)
|
||||||
except HTTPException:
|
except HTTPException:
|
||||||
|
await websocket.send_json({"type": "error", "content": "Invalid project name"})
|
||||||
await websocket.close(code=4000, reason="Invalid project name")
|
await websocket.close(code=4000, reason="Invalid project name")
|
||||||
return
|
return
|
||||||
|
|
||||||
# Look up project directory from registry
|
# Look up project directory from registry
|
||||||
project_dir = _get_project_path(project_name)
|
project_dir = _get_project_path(project_name)
|
||||||
if not project_dir:
|
if not project_dir:
|
||||||
|
await websocket.send_json({"type": "error", "content": "Project not found in registry"})
|
||||||
await websocket.close(code=4004, reason="Project not found in registry")
|
await websocket.close(code=4004, reason="Project not found in registry")
|
||||||
return
|
return
|
||||||
|
|
||||||
if not project_dir.exists():
|
if not project_dir.exists():
|
||||||
|
await websocket.send_json({"type": "error", "content": "Project directory not found"})
|
||||||
await websocket.close(code=4004, reason="Project directory not found")
|
await websocket.close(code=4004, reason="Project directory not found")
|
||||||
return
|
return
|
||||||
|
|
||||||
@@ -124,11 +131,10 @@ async def expand_project_websocket(websocket: WebSocket, project_name: str):
|
|||||||
from autoforge_paths import get_prompts_dir
|
from autoforge_paths import get_prompts_dir
|
||||||
spec_path = get_prompts_dir(project_dir) / "app_spec.txt"
|
spec_path = get_prompts_dir(project_dir) / "app_spec.txt"
|
||||||
if not spec_path.exists():
|
if not spec_path.exists():
|
||||||
|
await websocket.send_json({"type": "error", "content": "Project has no spec. Create a spec first before expanding."})
|
||||||
await websocket.close(code=4004, reason="Project has no spec. Create spec first.")
|
await websocket.close(code=4004, reason="Project has no spec. Create spec first.")
|
||||||
return
|
return
|
||||||
|
|
||||||
await websocket.accept()
|
|
||||||
|
|
||||||
session: Optional[ExpandChatSession] = None
|
session: Optional[ExpandChatSession] = None
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -166,22 +166,28 @@ async def spec_chat_websocket(websocket: WebSocket, project_name: str):
|
|||||||
- {"type": "error", "content": "..."} - Error message
|
- {"type": "error", "content": "..."} - Error message
|
||||||
- {"type": "pong"} - Keep-alive pong
|
- {"type": "pong"} - Keep-alive pong
|
||||||
"""
|
"""
|
||||||
if not validate_project_name(project_name):
|
# Always accept WebSocket first to avoid opaque 403 errors
|
||||||
|
await websocket.accept()
|
||||||
|
|
||||||
|
try:
|
||||||
|
project_name = validate_project_name(project_name)
|
||||||
|
except HTTPException:
|
||||||
|
await websocket.send_json({"type": "error", "content": "Invalid project name"})
|
||||||
await websocket.close(code=4000, reason="Invalid project name")
|
await websocket.close(code=4000, reason="Invalid project name")
|
||||||
return
|
return
|
||||||
|
|
||||||
# Look up project directory from registry
|
# Look up project directory from registry
|
||||||
project_dir = _get_project_path(project_name)
|
project_dir = _get_project_path(project_name)
|
||||||
if not project_dir:
|
if not project_dir:
|
||||||
|
await websocket.send_json({"type": "error", "content": "Project not found in registry"})
|
||||||
await websocket.close(code=4004, reason="Project not found in registry")
|
await websocket.close(code=4004, reason="Project not found in registry")
|
||||||
return
|
return
|
||||||
|
|
||||||
if not project_dir.exists():
|
if not project_dir.exists():
|
||||||
|
await websocket.send_json({"type": "error", "content": "Project directory not found"})
|
||||||
await websocket.close(code=4004, reason="Project directory not found")
|
await websocket.close(code=4004, reason="Project directory not found")
|
||||||
return
|
return
|
||||||
|
|
||||||
await websocket.accept()
|
|
||||||
|
|
||||||
session: Optional[SpecChatSession] = None
|
session: Optional[SpecChatSession] = None
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -221,8 +221,14 @@ async def terminal_websocket(websocket: WebSocket, project_name: str, terminal_i
|
|||||||
- {"type": "pong"} - Keep-alive response
|
- {"type": "pong"} - Keep-alive response
|
||||||
- {"type": "error", "message": "..."} - Error message
|
- {"type": "error", "message": "..."} - Error message
|
||||||
"""
|
"""
|
||||||
|
# Always accept WebSocket first to avoid opaque 403 errors
|
||||||
|
await websocket.accept()
|
||||||
|
|
||||||
# Validate project name
|
# Validate project name
|
||||||
if not validate_project_name(project_name):
|
try:
|
||||||
|
project_name = validate_project_name(project_name)
|
||||||
|
except Exception:
|
||||||
|
await websocket.send_json({"type": "error", "message": "Invalid project name"})
|
||||||
await websocket.close(
|
await websocket.close(
|
||||||
code=TerminalCloseCode.INVALID_PROJECT_NAME, reason="Invalid project name"
|
code=TerminalCloseCode.INVALID_PROJECT_NAME, reason="Invalid project name"
|
||||||
)
|
)
|
||||||
@@ -230,6 +236,7 @@ async def terminal_websocket(websocket: WebSocket, project_name: str, terminal_i
|
|||||||
|
|
||||||
# Validate terminal ID
|
# Validate terminal ID
|
||||||
if not validate_terminal_id(terminal_id):
|
if not validate_terminal_id(terminal_id):
|
||||||
|
await websocket.send_json({"type": "error", "message": "Invalid terminal ID"})
|
||||||
await websocket.close(
|
await websocket.close(
|
||||||
code=TerminalCloseCode.INVALID_PROJECT_NAME, reason="Invalid terminal ID"
|
code=TerminalCloseCode.INVALID_PROJECT_NAME, reason="Invalid terminal ID"
|
||||||
)
|
)
|
||||||
@@ -238,6 +245,7 @@ async def terminal_websocket(websocket: WebSocket, project_name: str, terminal_i
|
|||||||
# Look up project directory from registry
|
# Look up project directory from registry
|
||||||
project_dir = _get_project_path(project_name)
|
project_dir = _get_project_path(project_name)
|
||||||
if not project_dir:
|
if not project_dir:
|
||||||
|
await websocket.send_json({"type": "error", "message": "Project not found in registry"})
|
||||||
await websocket.close(
|
await websocket.close(
|
||||||
code=TerminalCloseCode.PROJECT_NOT_FOUND,
|
code=TerminalCloseCode.PROJECT_NOT_FOUND,
|
||||||
reason="Project not found in registry",
|
reason="Project not found in registry",
|
||||||
@@ -245,6 +253,7 @@ async def terminal_websocket(websocket: WebSocket, project_name: str, terminal_i
|
|||||||
return
|
return
|
||||||
|
|
||||||
if not project_dir.exists():
|
if not project_dir.exists():
|
||||||
|
await websocket.send_json({"type": "error", "message": "Project directory not found"})
|
||||||
await websocket.close(
|
await websocket.close(
|
||||||
code=TerminalCloseCode.PROJECT_NOT_FOUND,
|
code=TerminalCloseCode.PROJECT_NOT_FOUND,
|
||||||
reason="Project directory not found",
|
reason="Project directory not found",
|
||||||
@@ -254,14 +263,13 @@ async def terminal_websocket(websocket: WebSocket, project_name: str, terminal_i
|
|||||||
# Verify terminal exists in metadata
|
# Verify terminal exists in metadata
|
||||||
terminal_info = get_terminal_info(project_name, terminal_id)
|
terminal_info = get_terminal_info(project_name, terminal_id)
|
||||||
if not terminal_info:
|
if not terminal_info:
|
||||||
|
await websocket.send_json({"type": "error", "message": "Terminal not found"})
|
||||||
await websocket.close(
|
await websocket.close(
|
||||||
code=TerminalCloseCode.PROJECT_NOT_FOUND,
|
code=TerminalCloseCode.PROJECT_NOT_FOUND,
|
||||||
reason="Terminal not found",
|
reason="Terminal not found",
|
||||||
)
|
)
|
||||||
return
|
return
|
||||||
|
|
||||||
await websocket.accept()
|
|
||||||
|
|
||||||
# Get or create terminal session for this project/terminal
|
# Get or create terminal session for this project/terminal
|
||||||
session = get_terminal_session(project_name, project_dir, terminal_id)
|
session = get_terminal_session(project_name, project_dir, terminal_id)
|
||||||
|
|
||||||
|
|||||||
@@ -640,9 +640,7 @@ class ConnectionManager:
|
|||||||
self._lock = asyncio.Lock()
|
self._lock = asyncio.Lock()
|
||||||
|
|
||||||
async def connect(self, websocket: WebSocket, project_name: str):
|
async def connect(self, websocket: WebSocket, project_name: str):
|
||||||
"""Accept a WebSocket connection for a project."""
|
"""Register a WebSocket connection for a project (must already be accepted)."""
|
||||||
await websocket.accept()
|
|
||||||
|
|
||||||
async with self._lock:
|
async with self._lock:
|
||||||
if project_name not in self.active_connections:
|
if project_name not in self.active_connections:
|
||||||
self.active_connections[project_name] = set()
|
self.active_connections[project_name] = set()
|
||||||
@@ -727,16 +725,24 @@ async def project_websocket(websocket: WebSocket, project_name: str):
|
|||||||
- Agent status changes
|
- Agent status changes
|
||||||
- Agent stdout/stderr lines
|
- Agent stdout/stderr lines
|
||||||
"""
|
"""
|
||||||
if not validate_project_name(project_name):
|
# Always accept WebSocket first to avoid opaque 403 errors
|
||||||
|
await websocket.accept()
|
||||||
|
|
||||||
|
try:
|
||||||
|
project_name = validate_project_name(project_name)
|
||||||
|
except Exception:
|
||||||
|
await websocket.send_json({"type": "error", "content": "Invalid project name"})
|
||||||
await websocket.close(code=4000, reason="Invalid project name")
|
await websocket.close(code=4000, reason="Invalid project name")
|
||||||
return
|
return
|
||||||
|
|
||||||
project_dir = _get_project_path(project_name)
|
project_dir = _get_project_path(project_name)
|
||||||
if not project_dir:
|
if not project_dir:
|
||||||
|
await websocket.send_json({"type": "error", "content": "Project not found in registry"})
|
||||||
await websocket.close(code=4004, reason="Project not found in registry")
|
await websocket.close(code=4004, reason="Project not found in registry")
|
||||||
return
|
return
|
||||||
|
|
||||||
if not project_dir.exists():
|
if not project_dir.exists():
|
||||||
|
await websocket.send_json({"type": "error", "content": "Project directory not found"})
|
||||||
await websocket.close(code=4004, reason="Project directory not found")
|
await websocket.close(code=4004, reason="Project directory not found")
|
||||||
return
|
return
|
||||||
|
|
||||||
|
|||||||
@@ -178,8 +178,8 @@ function App() {
|
|||||||
setShowAddFeature(true)
|
setShowAddFeature(true)
|
||||||
}
|
}
|
||||||
|
|
||||||
// E : Expand project with AI (when project selected and has features)
|
// E : Expand project with AI (when project selected, has spec and has features)
|
||||||
if ((e.key === 'e' || e.key === 'E') && selectedProject && features &&
|
if ((e.key === 'e' || e.key === 'E') && selectedProject && hasSpec && features &&
|
||||||
(features.pending.length + features.in_progress.length + features.done.length) > 0) {
|
(features.pending.length + features.in_progress.length + features.done.length) > 0) {
|
||||||
e.preventDefault()
|
e.preventDefault()
|
||||||
setShowExpandProject(true)
|
setShowExpandProject(true)
|
||||||
@@ -239,7 +239,7 @@ function App() {
|
|||||||
|
|
||||||
window.addEventListener('keydown', handleKeyDown)
|
window.addEventListener('keydown', handleKeyDown)
|
||||||
return () => window.removeEventListener('keydown', handleKeyDown)
|
return () => window.removeEventListener('keydown', handleKeyDown)
|
||||||
}, [selectedProject, showAddFeature, showExpandProject, selectedFeature, debugOpen, debugActiveTab, assistantOpen, features, showSettings, showKeyboardHelp, isSpecCreating, viewMode, showResetModal, wsState.agentStatus])
|
}, [selectedProject, showAddFeature, showExpandProject, selectedFeature, debugOpen, debugActiveTab, assistantOpen, features, showSettings, showKeyboardHelp, isSpecCreating, viewMode, showResetModal, wsState.agentStatus, hasSpec])
|
||||||
|
|
||||||
// Combine WebSocket progress with feature data
|
// Combine WebSocket progress with feature data
|
||||||
const progress = wsState.progress.total > 0 ? wsState.progress : {
|
const progress = wsState.progress.total > 0 ? wsState.progress : {
|
||||||
@@ -490,7 +490,7 @@ function App() {
|
|||||||
)}
|
)}
|
||||||
|
|
||||||
{/* Expand Project Modal - AI-powered bulk feature creation */}
|
{/* Expand Project Modal - AI-powered bulk feature creation */}
|
||||||
{showExpandProject && selectedProject && (
|
{showExpandProject && selectedProject && hasSpec && (
|
||||||
<ExpandProjectModal
|
<ExpandProjectModal
|
||||||
isOpen={showExpandProject}
|
isOpen={showExpandProject}
|
||||||
projectName={selectedProject}
|
projectName={selectedProject}
|
||||||
|
|||||||
@@ -51,7 +51,7 @@ export function KanbanBoard({ features, onFeatureClick, onAddFeature, onExpandPr
|
|||||||
onFeatureClick={onFeatureClick}
|
onFeatureClick={onFeatureClick}
|
||||||
onAddFeature={onAddFeature}
|
onAddFeature={onAddFeature}
|
||||||
onExpandProject={onExpandProject}
|
onExpandProject={onExpandProject}
|
||||||
showExpandButton={hasFeatures}
|
showExpandButton={hasFeatures && hasSpec}
|
||||||
onCreateSpec={onCreateSpec}
|
onCreateSpec={onCreateSpec}
|
||||||
showCreateSpec={!hasSpec && !hasFeatures}
|
showCreateSpec={!hasSpec && !hasFeatures}
|
||||||
/>
|
/>
|
||||||
|
|||||||
@@ -19,7 +19,7 @@ const shortcuts: Shortcut[] = [
|
|||||||
{ key: 'D', description: 'Toggle debug panel' },
|
{ key: 'D', description: 'Toggle debug panel' },
|
||||||
{ key: 'T', description: 'Toggle terminal tab' },
|
{ key: 'T', description: 'Toggle terminal tab' },
|
||||||
{ key: 'N', description: 'Add new feature', context: 'with project' },
|
{ key: 'N', description: 'Add new feature', context: 'with project' },
|
||||||
{ key: 'E', description: 'Expand project with AI', context: 'with features' },
|
{ key: 'E', description: 'Expand project with AI', context: 'with spec & features' },
|
||||||
{ key: 'A', description: 'Toggle AI assistant', context: 'with project' },
|
{ key: 'A', description: 'Toggle AI assistant', context: 'with project' },
|
||||||
{ key: 'G', description: 'Toggle Kanban/Graph view', context: 'with project' },
|
{ key: 'G', description: 'Toggle Kanban/Graph view', context: 'with project' },
|
||||||
{ key: ',', description: 'Open settings' },
|
{ key: ',', description: 'Open settings' },
|
||||||
|
|||||||
@@ -107,16 +107,20 @@ export function useExpandChat({
|
|||||||
}, 30000)
|
}, 30000)
|
||||||
}
|
}
|
||||||
|
|
||||||
ws.onclose = () => {
|
ws.onclose = (event) => {
|
||||||
setConnectionStatus('disconnected')
|
setConnectionStatus('disconnected')
|
||||||
if (pingIntervalRef.current) {
|
if (pingIntervalRef.current) {
|
||||||
clearInterval(pingIntervalRef.current)
|
clearInterval(pingIntervalRef.current)
|
||||||
pingIntervalRef.current = null
|
pingIntervalRef.current = null
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Don't retry on application-level errors (4xxx codes won't resolve on retry)
|
||||||
|
const isAppError = event.code >= 4000 && event.code <= 4999
|
||||||
|
|
||||||
// Attempt reconnection if not intentionally closed
|
// Attempt reconnection if not intentionally closed
|
||||||
if (
|
if (
|
||||||
!manuallyDisconnectedRef.current &&
|
!manuallyDisconnectedRef.current &&
|
||||||
|
!isAppError &&
|
||||||
reconnectAttempts.current < maxReconnectAttempts &&
|
reconnectAttempts.current < maxReconnectAttempts &&
|
||||||
!isCompleteRef.current
|
!isCompleteRef.current
|
||||||
) {
|
) {
|
||||||
|
|||||||
@@ -157,15 +157,18 @@ export function useSpecChat({
|
|||||||
}, 30000)
|
}, 30000)
|
||||||
}
|
}
|
||||||
|
|
||||||
ws.onclose = () => {
|
ws.onclose = (event) => {
|
||||||
setConnectionStatus('disconnected')
|
setConnectionStatus('disconnected')
|
||||||
if (pingIntervalRef.current) {
|
if (pingIntervalRef.current) {
|
||||||
clearInterval(pingIntervalRef.current)
|
clearInterval(pingIntervalRef.current)
|
||||||
pingIntervalRef.current = null
|
pingIntervalRef.current = null
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Don't retry on application-level errors (4xxx codes won't resolve on retry)
|
||||||
|
const isAppError = event.code >= 4000 && event.code <= 4999
|
||||||
|
|
||||||
// Attempt reconnection if not intentionally closed
|
// Attempt reconnection if not intentionally closed
|
||||||
if (reconnectAttempts.current < maxReconnectAttempts && !isCompleteRef.current) {
|
if (!isAppError && reconnectAttempts.current < maxReconnectAttempts && !isCompleteRef.current) {
|
||||||
reconnectAttempts.current++
|
reconnectAttempts.current++
|
||||||
const delay = Math.min(1000 * Math.pow(2, reconnectAttempts.current), 10000)
|
const delay = Math.min(1000 * Math.pow(2, reconnectAttempts.current), 10000)
|
||||||
reconnectTimeoutRef.current = window.setTimeout(connect, delay)
|
reconnectTimeoutRef.current = window.setTimeout(connect, delay)
|
||||||
|
|||||||
@@ -335,10 +335,14 @@ export function useProjectWebSocket(projectName: string | null) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
ws.onclose = () => {
|
ws.onclose = (event) => {
|
||||||
setState(prev => ({ ...prev, isConnected: false }))
|
setState(prev => ({ ...prev, isConnected: false }))
|
||||||
wsRef.current = null
|
wsRef.current = null
|
||||||
|
|
||||||
|
// Don't retry on application-level errors (4xxx codes won't resolve on retry)
|
||||||
|
const isAppError = event.code >= 4000 && event.code <= 4999
|
||||||
|
if (isAppError) return
|
||||||
|
|
||||||
// Exponential backoff reconnection
|
// Exponential backoff reconnection
|
||||||
const delay = Math.min(1000 * Math.pow(2, reconnectAttempts.current), 30000)
|
const delay = Math.min(1000 * Math.pow(2, reconnectAttempts.current), 30000)
|
||||||
reconnectAttempts.current++
|
reconnectAttempts.current++
|
||||||
|
|||||||
Reference in New Issue
Block a user