diff --git a/backend/app/api/tasks/router.py b/backend/app/api/tasks/router.py index 41874d5..f437caa 100644 --- a/backend/app/api/tasks/router.py +++ b/backend/app/api/tasks/router.py @@ -407,10 +407,13 @@ async def update_task( if task_data.version != task.version: raise HTTPException( status_code=status.HTTP_409_CONFLICT, - detail=( - f"Version conflict: current_version={task.version}, " - f"provided_version={task_data.version}" - ), + detail={ + "error": "conflict", + "message": "Task has been modified by another user. Please refresh and retry.", + "current_version": task.version, + "provided_version": task_data.version, + "your_version": task_data.version, + }, ) # Capture old values for audit and triggers diff --git a/backend/app/api/websocket/router.py b/backend/app/api/websocket/router.py index 453bcfa..e5401ed 100644 --- a/backend/app/api/websocket/router.py +++ b/backend/app/api/websocket/router.py @@ -168,7 +168,6 @@ async def websocket_notifications( - Connect with: ws://host/ws/notifications?token= Messages sent by server: - - {"type": "auth_required"} - Sent when waiting for auth message - {"type": "connected", "data": {"user_id": "...", "message": "..."}} - Connection success - {"type": "unread_sync", "data": {"notifications": [...], "unread_count": N}} - All unread on connect - {"type": "notification", "data": {...}} - New notification @@ -356,7 +355,6 @@ async def websocket_project_sync( - Connect with: ws://host/ws/projects/{project_id}?token= Messages sent by server: - - {"type": "auth_required"} - Sent when waiting for auth message - {"type": "connected", "data": {"project_id": "...", "user_id": "..."}} - {"type": "task_created", "data": {...}, "triggered_by": "..."} - {"type": "task_updated", "data": {...}, "triggered_by": "..."} diff --git a/backend/tests/test_concurrency_reliability.py b/backend/tests/test_concurrency_reliability.py index de23fe2..1d7680b 100644 --- a/backend/tests/test_concurrency_reliability.py +++ b/backend/tests/test_concurrency_reliability.py @@ -88,7 +88,11 @@ class TestOptimisticLocking: ) assert response.status_code == 409 - assert "conflict" in response.json().get("detail", "").lower() or "version" in response.json().get("detail", "").lower() + detail = response.json().get("detail") + assert isinstance(detail, dict) + assert detail.get("error") == "conflict" + assert detail.get("current_version") == 5 + assert detail.get("provided_version") == 1 def test_update_without_version_succeeds(self, client, admin_token, db): """Test that update without version (for backward compatibility) still works.""" diff --git a/openspec/changes/update-api-consistency/design.md b/openspec/changes/update-api-consistency/design.md new file mode 100644 index 0000000..e0a8587 --- /dev/null +++ b/openspec/changes/update-api-consistency/design.md @@ -0,0 +1,30 @@ +## Context +The current API behavior is functionally correct but has ambiguities that cause client confusion: WebSocket handshake messaging differs from inline docs, 409 conflict responses are inconsistent for clients, and workload heatmap defaults vary based on calendar edge cases (Sunday). + +## Goals / Non-Goals +- Goals: + - Make WebSocket auth expectations explicit and consistent. + - Provide a stable, machine-readable conflict response for optimistic locking. + - Define predictable workload heatmap defaults and edge-case handling. +- Non-Goals: + - Full API version migration (/api to /api/v1). + - Redesign of workload UI or heatmap visualization. + +## Decisions +- WebSocket auth handshake: keep first-message authentication; do not require a pre-auth banner. Add explicit invalid-token error message prior to close. +- Optimistic locking: return a standardized 409 payload containing a human-readable message plus current/provided version fields. +- Workload defaults: show all accessible users by default (including zero workload); support `hide_empty=true` to exclude them. When `week_start` is omitted and today is Sunday, extend the default week window to include the upcoming week to avoid empty default views. +- Caching: include `hide_empty` in cache key and cache the default path to keep latency stable. + +## Risks / Trade-offs +- Returning more users by default may increase response sizes; cache should offset this. +- Sunday window extension changes semantics of "current week"; clients must understand the rule. +- Conflict payload change may require frontend updates if it expects a string-only detail. + +## Migration Plan +- Update specs first, then implement backend changes. +- Update frontend only if new defaults or conflict payloads require parsing changes. +- Add regression tests for WebSocket auth invalid token, conflict payload shape, and heatmap defaults. + +## Open Questions +- Should we add an explicit `auth_required` message before waiting for the auth payload (client UX improvement), or keep the minimal handshake? diff --git a/openspec/changes/update-api-consistency/proposal.md b/openspec/changes/update-api-consistency/proposal.md new file mode 100644 index 0000000..6049866 --- /dev/null +++ b/openspec/changes/update-api-consistency/proposal.md @@ -0,0 +1,14 @@ +# Change: Update API Consistency for WebSocket Auth, Workload Defaults, and Task Locking + +## Why +Several API behaviors are currently ambiguous or inconsistent (WebSocket auth handshake messaging, optimistic locking conflict payloads, and workload heatmap defaults). These gaps can confuse clients and reduce product completeness. + +## What Changes +- Clarify WebSocket authentication error handling and client expectations. +- Standardize optimistic locking conflict responses for task updates. +- Define workload heatmap default inclusion rules and Sunday week-window behavior. +- Align workload heatmap `hide_empty` defaults and caching behavior. + +## Impact +- Affected specs: user-auth, task-management, resource-management +- Affected code: backend WebSocket router, task update endpoint, workload service/router, workload cache; frontend workload views may require adjustments depending on default behavior. diff --git a/openspec/changes/update-api-consistency/specs/resource-management/spec.md b/openspec/changes/update-api-consistency/specs/resource-management/spec.md new file mode 100644 index 0000000..ed46780 --- /dev/null +++ b/openspec/changes/update-api-consistency/specs/resource-management/spec.md @@ -0,0 +1,35 @@ +## MODIFIED Requirements +### Requirement: Workload Heatmap UI +The system SHALL provide a visual workload heatmap interface for managers. + +#### Scenario: View workload heatmap +- **GIVEN** user is logged in as manager or admin +- **WHEN** user navigates to /workload page without filters +- **THEN** system displays a heatmap showing all accessible users' workload +- **AND** users with zero workload are included by default +- **AND** each user cell is color-coded by load level (green/yellow/red) + +#### Scenario: Hide empty workloads +- **GIVEN** user is viewing the workload page +- **WHEN** user enables the hide_empty filter +- **THEN** the heatmap excludes users with zero workload + +#### Scenario: Navigate between weeks +- **GIVEN** user is viewing the workload page +- **WHEN** user clicks previous/next week buttons +- **THEN** the heatmap updates to show that week's workload data + +#### Scenario: Default week window on Sunday +- **GIVEN** today is Sunday and user opens workload page without selecting week_start +- **THEN** the default heatmap window includes tasks due in the upcoming week + +#### Scenario: View user workload details +- **GIVEN** user is viewing the workload heatmap +- **WHEN** user clicks on a specific user's cell +- **THEN** a modal/drawer opens showing that user's task breakdown +- **AND** tasks show title, project, time estimate, and due date + +#### Scenario: Filter by department +- **GIVEN** user is a system admin +- **WHEN** user selects a department from the filter +- **THEN** the heatmap shows only users from that department diff --git a/openspec/changes/update-api-consistency/specs/task-management/spec.md b/openspec/changes/update-api-consistency/specs/task-management/spec.md new file mode 100644 index 0000000..f9439e8 --- /dev/null +++ b/openspec/changes/update-api-consistency/specs/task-management/spec.md @@ -0,0 +1,16 @@ +## MODIFIED Requirements +### Requirement: Optimistic Locking for Task Updates +The system SHALL use optimistic locking to prevent concurrent update conflicts on tasks. + +#### Scenario: Concurrent update detected +- **WHEN** user A and user B both load task at version 1 +- **WHEN** user A saves changes, incrementing version to 2 +- **WHEN** user B attempts to save with version 1 +- **THEN** system returns 409 Conflict error +- **AND** response includes a human-readable message instructing refresh and retry +- **AND** response includes the current and provided version numbers + +#### Scenario: Sequential updates succeed +- **WHEN** user loads task at version N +- **WHEN** user saves changes with correct version N +- **THEN** system accepts update and increments version to N+1 diff --git a/openspec/changes/update-api-consistency/specs/user-auth/spec.md b/openspec/changes/update-api-consistency/specs/user-auth/spec.md new file mode 100644 index 0000000..d77faa2 --- /dev/null +++ b/openspec/changes/update-api-consistency/specs/user-auth/spec.md @@ -0,0 +1,18 @@ +## MODIFIED Requirements +### Requirement: Secure WebSocket Authentication +The system SHALL authenticate WebSocket connections without exposing tokens in URL query parameters. + +#### Scenario: WebSocket connection with token in first message +- **WHEN** client connects to WebSocket endpoint without a query token +- **THEN** server waits for authentication message containing JWT token +- **THEN** server validates token before accepting further messages +- **THEN** server sends an authentication acknowledgment message + +#### Scenario: WebSocket connection with invalid token +- **WHEN** client sends an invalid or expired token +- **THEN** server sends an error message indicating invalid or expired token +- **THEN** server closes the connection with an authentication error code + +#### Scenario: WebSocket connection timeout without authentication +- **WHEN** client connects but does not send authentication within 10 seconds +- **THEN** server closes the connection with appropriate error code diff --git a/openspec/changes/update-api-consistency/tasks.md b/openspec/changes/update-api-consistency/tasks.md new file mode 100644 index 0000000..01f1b3b --- /dev/null +++ b/openspec/changes/update-api-consistency/tasks.md @@ -0,0 +1,6 @@ +## 1. Implementation +- [ ] 1.1 Update WebSocket auth spec and align server handshake/error messaging with the agreed behavior +- [ ] 1.2 Update optimistic locking conflict response spec and implement standardized payload +- [ ] 1.3 Update workload heatmap defaults (hide_empty, week window) and cache behavior +- [ ] 1.4 Update frontend workload views and API handling if required by new defaults +- [ ] 1.5 Add/adjust tests for WebSocket auth, conflict responses, and workload heatmap defaults