spec: update api consistency
Align optimistic locking conflict payload, update websocket docs, and adjust tests.
This commit is contained in:
@@ -407,10 +407,13 @@ async def update_task(
|
|||||||
if task_data.version != task.version:
|
if task_data.version != task.version:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=status.HTTP_409_CONFLICT,
|
status_code=status.HTTP_409_CONFLICT,
|
||||||
detail=(
|
detail={
|
||||||
f"Version conflict: current_version={task.version}, "
|
"error": "conflict",
|
||||||
f"provided_version={task_data.version}"
|
"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
|
# Capture old values for audit and triggers
|
||||||
|
|||||||
@@ -168,7 +168,6 @@ async def websocket_notifications(
|
|||||||
- Connect with: ws://host/ws/notifications?token=<jwt_token>
|
- Connect with: ws://host/ws/notifications?token=<jwt_token>
|
||||||
|
|
||||||
Messages sent by server:
|
Messages sent by server:
|
||||||
- {"type": "auth_required"} - Sent when waiting for auth message
|
|
||||||
- {"type": "connected", "data": {"user_id": "...", "message": "..."}} - Connection success
|
- {"type": "connected", "data": {"user_id": "...", "message": "..."}} - Connection success
|
||||||
- {"type": "unread_sync", "data": {"notifications": [...], "unread_count": N}} - All unread on connect
|
- {"type": "unread_sync", "data": {"notifications": [...], "unread_count": N}} - All unread on connect
|
||||||
- {"type": "notification", "data": {...}} - New notification
|
- {"type": "notification", "data": {...}} - New notification
|
||||||
@@ -356,7 +355,6 @@ async def websocket_project_sync(
|
|||||||
- Connect with: ws://host/ws/projects/{project_id}?token=<jwt_token>
|
- Connect with: ws://host/ws/projects/{project_id}?token=<jwt_token>
|
||||||
|
|
||||||
Messages sent by server:
|
Messages sent by server:
|
||||||
- {"type": "auth_required"} - Sent when waiting for auth message
|
|
||||||
- {"type": "connected", "data": {"project_id": "...", "user_id": "..."}}
|
- {"type": "connected", "data": {"project_id": "...", "user_id": "..."}}
|
||||||
- {"type": "task_created", "data": {...}, "triggered_by": "..."}
|
- {"type": "task_created", "data": {...}, "triggered_by": "..."}
|
||||||
- {"type": "task_updated", "data": {...}, "triggered_by": "..."}
|
- {"type": "task_updated", "data": {...}, "triggered_by": "..."}
|
||||||
|
|||||||
@@ -88,7 +88,11 @@ class TestOptimisticLocking:
|
|||||||
)
|
)
|
||||||
|
|
||||||
assert response.status_code == 409
|
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):
|
def test_update_without_version_succeeds(self, client, admin_token, db):
|
||||||
"""Test that update without version (for backward compatibility) still works."""
|
"""Test that update without version (for backward compatibility) still works."""
|
||||||
|
|||||||
30
openspec/changes/update-api-consistency/design.md
Normal file
30
openspec/changes/update-api-consistency/design.md
Normal file
@@ -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?
|
||||||
14
openspec/changes/update-api-consistency/proposal.md
Normal file
14
openspec/changes/update-api-consistency/proposal.md
Normal file
@@ -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.
|
||||||
@@ -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
|
||||||
@@ -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
|
||||||
@@ -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
|
||||||
6
openspec/changes/update-api-consistency/tasks.md
Normal file
6
openspec/changes/update-api-consistency/tasks.md
Normal file
@@ -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
|
||||||
Reference in New Issue
Block a user