Skip to content

Socket.IO migration: address PR #39 review findings #40

@dittops

Description

@dittops

Context

Follow-up items from the code review of PR #39 (Socket.IO migration). These are non-blocking for the initial merge but should be addressed before the SSE deprecation cleanup.

Security

  • /api/auth/session-token hardening — This endpoint exposes the HttpOnly cookie value as JSON, effectively downgrading its protection. Add CSRF protection or rate limiting. Consider restricting to desktop mode only (check Tauri user agent header).

Error Handling

  • Gateway reconnect backoffgateway-server.ts: if connectGateway fails, every subsequent agent:execute retries the connection. Add exponential backoff or max retry count to avoid hammering the backend.
  • Socket.IO connect_error + auto-reconnect conflictgateway.ts: the first connect_error rejects the connect promise, but Socket.IO keeps reconnecting in the background. Consider disabling reconnection for the initial connect attempt, then enabling after first success.

Data Integrity

  • Partial state on _run_llm_turn exceptionagent_handler.py: if an exception occurs between persisting the assistant message and setting status to IDLE, partial DB state remains. Add state consistency check in the error recovery path.
  • Alembic migration revision IDa1b2c3d4e5f6 is a hardcoded placeholder instead of an auto-generated hash. Regenerate with alembic revision --autogenerate to avoid conflicts with other migrations on main.

Performance

  • Cache tool registry across reconnectsgateway.ts: createLocalToolRegistry() loads Playwright browser tools on every connect() call (~1-2s). Cache the registry instance and reuse across reconnects.
  • Cache session token in useAgentSocketgetSocketConfig() fetches /api/auth/session-token on every ensureConnected call. Cache the token and only refetch on 401.
  • Message loading optimizationagent_handler.py: load_messages_from_db on every tool:result could be slow for long sessions (500+ messages). Consider incremental loading.

Code Cleanup

  • Remove debug logging — Remove console.log("[LocalGateway]..." and console.log("[gateway-server]..." statements, or convert to a proper logger.
  • Remove redundant extraHeaders: { Cookie: ... }gateway.ts: the auth.token field is sufficient; the cookie header is redundant.
  • Move PAUSE_TOOLS to tool_definitions.pyagent_handler.py line 979 hardcodes {"ask_user_questions"}. Move alongside LOCAL_TOOLS and REMOTE_TOOLS.
  • Add payload validation for tool:result relaygateway-server.ts: validate the tool:result payload from browser before forwarding to cloud.
  • Add structured auth loggingsocketio_server.py: log which auth path (disabled/token/JWT/cookie) succeeded for debugging.

Testing

  • Integration tests for Socket.IO connection flow — Test connect, auth, disconnect, reconnect
  • Gateway relay server tests — Test event forwarding, tool:result relay, auth token extraction
  • _run_llm_turn streaming tests — Currently mocked entirely in unit tests; needs integration coverage
  • Tool approval flow E2E — Approve and deny paths
  • Stop during streaming E2E — Cancel mid-stream
  • User interruption E2E — New message mid-execution (stop-then-execute)

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions