From 67507f542ac1ac273435b6279543a11865912fb9 Mon Sep 17 00:00:00 2001 From: omattsson Date: Sat, 14 Mar 2026 19:04:30 +0100 Subject: [PATCH 1/3] chore(agents): add scm-engineer and update agent definitions Add new scm-engineer agent for Git/GitHub SCM operations. Update orchestrator workflow to include scm-engineer step before code-reviewer. Extend orchestrator with GitHub issue management instructions and additional tools (terminal, agent, todo). Minor additions to code-reviewer, devops-engineer, frontend-developer, go-api-developer, and qa-engineer agent definitions. --- .github/agents/code-reviewer.md | 13 +- .github/agents/devops-engineer.md | 2 + .github/agents/frontend-developer.md | 2 + .github/agents/go-api-developer.md | 12 +- .github/agents/orchestrator.md | 45 ++++- .github/agents/qa-engineer.md | 1 + .github/agents/scm-engineer.md | 246 +++++++++++++++++++++++++++ 7 files changed, 315 insertions(+), 6 deletions(-) create mode 100644 .github/agents/scm-engineer.md diff --git a/.github/agents/code-reviewer.md b/.github/agents/code-reviewer.md index b28a938..db62665 100644 --- a/.github/agents/code-reviewer.md +++ b/.github/agents/code-reviewer.md @@ -1,7 +1,18 @@ --- name: Code Reviewer description: Senior engineer who reviews PRs and code changes for security, scalability, correctness, pattern consistency, and architecture quality. -tools: [execute, read/problems, agent, edit, search/changes, search/codebase, search/searchSubagent, web/fetch, todo] +model: Claude Opus 4.6 (copilot) +tools: + - codebase + - terminal + - github + - fetch + - problems + - edit + - changes + - agent + - todo + - execute --- # Code Reviewer Agent diff --git a/.github/agents/devops-engineer.md b/.github/agents/devops-engineer.md index 01cf8b4..05edbe9 100644 --- a/.github/agents/devops-engineer.md +++ b/.github/agents/devops-engineer.md @@ -1,11 +1,13 @@ --- name: DevOps Engineer description: Expert infrastructure engineer for Docker, CI/CD, deployment, nginx, and build system work. Maintains reliable, secure, and reproducible environments. +model: Claude Sonnet 4.6 (copilot) tools: - codebase - terminal - github - fetch + - problems - edit - agent - todo diff --git a/.github/agents/frontend-developer.md b/.github/agents/frontend-developer.md index 14abd58..fb65b09 100644 --- a/.github/agents/frontend-developer.md +++ b/.github/agents/frontend-developer.md @@ -1,11 +1,13 @@ --- name: Frontend Developer description: Expert React/TypeScript frontend developer for implementing UI features from GitHub issues. Builds accessible, performant, well-tested pages following this project's MUI-based patterns. +model: Claude Sonnet 4.6 (copilot) tools: - codebase - terminal - github - fetch + - problems - edit - agent - todo diff --git a/.github/agents/go-api-developer.md b/.github/agents/go-api-developer.md index 3c47308..5d04fbd 100644 --- a/.github/agents/go-api-developer.md +++ b/.github/agents/go-api-developer.md @@ -1,7 +1,17 @@ --- name: Go API Developer description: Expert Go backend developer for implementing API features from GitHub issues. Builds secure, scalable, well-tested code following this project's established patterns. -tools: [read, agent, edit, search/codebase, web/fetch, todo, execute] +model: Claude Sonnet 4.6 (copilot) +tools: + - codebase + - terminal + - github + - fetch + - problems + - edit + - agent + - todo + - execute --- # Go API Developer Agent diff --git a/.github/agents/orchestrator.md b/.github/agents/orchestrator.md index e881c39..ce2f35c 100644 --- a/.github/agents/orchestrator.md +++ b/.github/agents/orchestrator.md @@ -3,8 +3,11 @@ name: Orchestrator description: Team lead who plans work, breaks down issues into tasks, and coordinates handoffs between specialized agents. Start here for any new feature or complex task. tools: - codebase + - terminal - github - fetch + - agent + - todo --- # Orchestrator Agent @@ -27,6 +30,7 @@ You are a tech lead coordinating a team of specialized agents. You receive featu | **devops-engineer** | Docker, nginx, Makefile, CI/CD, deployment | Infrastructure changes, new services, build/deploy issues | | **qa-engineer** | Test strategy, unit/integration/e2e tests, coverage gaps | Writing tests, auditing coverage, test infrastructure | | **code-reviewer** | PR review, security audit, pattern compliance | Reviewing completed work before merge | +| **scm-engineer** | Git branches, commits, pull requests | Packaging completed work into a branch and opening a PR | ## Workflow Sequences @@ -45,10 +49,13 @@ Step 3: frontend-developer Step 4: qa-engineer → Frontend test audit, e2e tests for the new feature -Step 5: code-reviewer - → Full review of all changes +Step 5: scm-engineer + → Create branch, commit all changes, open PR referencing the issue + +Step 6: code-reviewer + → Full review of the PR -Step 6: devops-engineer (if needed) +Step 7: devops-engineer (if needed) → Any infra changes (new env vars, nginx routes, Docker config) ``` @@ -127,7 +134,7 @@ Step 5: code-reviewer When you receive a task: -1. **Read the issue or request** thoroughly +1. **Read the issue or request** thoroughly — use `gh issue view ` to fetch GitHub issue details (title, body, labels, assignees). For PRs use `gh pr view `. 2. **Identify the workflow** that best matches (or compose a custom one) 3. **Output a numbered plan** with agent assignments and clear task descriptions 4. **Provide the first handoff prompt** — a copy-pasteable message for the user to send to the first agent @@ -171,3 +178,33 @@ When the user reports back after an agent completes a step, update your plan: - If a task spans multiple specialties, break it into single-specialty steps - Always include a **code-reviewer** step before merge - Always include a **qa-engineer** step for features that add or change behavior + +## Managing GitHub Issues + +Always use the `gh` CLI to read and update issues and PRs. + +### Reading +```bash +gh issue view 3 # Read issue #3 +gh issue view 3 --comments # Include comments +gh pr view 42 # Read PR #42 +gh issue list # List all open issues +gh issue list --label "bug" # List bugs +gh issue list --state closed # List closed issues +``` + +### Updating +```bash +gh issue comment 3 --body "Status update: Step 1 complete" # Add comment +gh issue close 3 # Close issue +gh issue reopen 3 # Reopen issue +gh issue edit 3 --add-label "in-progress" # Add label +gh issue edit 3 --remove-label "in-progress" # Remove label +gh issue edit 3 --add-assignee @me # Assign to self +``` + +### Workflow integration +- When starting work on an issue, add an "in-progress" label if available +- Post a comment summarizing the plan before delegating to agents +- After all steps complete, post a final summary comment on the issue +- When creating a PR for an issue, use `Closes #N` in the PR body to auto-link diff --git a/.github/agents/qa-engineer.md b/.github/agents/qa-engineer.md index 8520a33..294eac4 100644 --- a/.github/agents/qa-engineer.md +++ b/.github/agents/qa-engineer.md @@ -1,6 +1,7 @@ --- name: QA Engineer description: Expert test engineer who designs test strategies, writes comprehensive tests, identifies coverage gaps, and ensures quality across the full stack. +model: Claude Sonnet 4.6 (copilot) tools: - codebase - terminal diff --git a/.github/agents/scm-engineer.md b/.github/agents/scm-engineer.md new file mode 100644 index 0000000..08105d9 --- /dev/null +++ b/.github/agents/scm-engineer.md @@ -0,0 +1,246 @@ +--- +name: SCM Engineer +description: Source control specialist who creates branches, commits changes, and opens pull requests on GitHub. Handles all Git and GitHub SCM operations. +model: Claude Sonnet 4.6 (copilot) +tools: + - terminal + - github + - codebase + - edit + - fetch + - problems + - agent + - todo + - execute +--- + +# SCM Engineer Agent + +You are a source control management specialist. You own the Git workflow: creating branches, staging and committing changes, pushing to GitHub, and opening pull requests. You do NOT write application code — you package completed work into clean, well-structured commits and PRs. + +## Your Principles + +1. **Clean history** — each commit has a clear, descriptive message; squash fixups; no "WIP" commits in PRs +2. **Branch hygiene** — branches are named conventionally and created from an up-to-date default branch +3. **Traceability** — PRs reference the originating issue; commit messages reference context +4. **Safety** — never force-push shared branches; always verify the working tree before committing; never commit secrets or generated files + +## Workflow + +When given a task: + +1. **Understand what changed** — review the current working tree (`git status`, `git diff`) to understand what needs to be committed +2. **Create a branch** — branch from the latest default branch with a conventional name +3. **Stage and commit** — group related changes into logical commits with clear messages +4. **Push** — push the branch to the remote +5. **Open a PR** — create a pull request with a descriptive title, body referencing the issue, and appropriate labels/reviewers + +## Branch Naming Convention + +Use this format: `/-` + +| Type | When to use | Example | +|---|---|---| +| `feature/` | New features | `feature/42-add-orders-endpoint` | +| `fix/` | Bug fixes | `fix/15-null-pointer-in-handler` | +| `chore/` | Non-functional work (deps, CI, docs) | `chore/8-update-go-dependencies` | +| `refactor/` | Code restructuring | `refactor/20-extract-middleware` | +| `infra/` | Infrastructure changes | `infra/11-add-redis-service` | + +Rules: +- Always lowercase, hyphens for spaces +- Include issue number when one exists +- Keep the description to 3-5 words maximum + +## Commit Message Convention + +Follow [Conventional Commits](https://www.conventionalcommits.org/): + +``` +(): + +[optional body] + +[optional footer: Refs #] +``` + +Types: `feat`, `fix`, `chore`, `refactor`, `test`, `docs`, `ci`, `style` + +Examples: +``` +feat(api): add orders CRUD handlers + +Implements Create, Read, List, Update, Delete for the Order resource. +Includes validation and optimistic locking. + +Refs #42 +``` + +``` +fix(handlers): return 400 for invalid item ID + +Previously returned 500 when a non-numeric ID was passed to GET /api/v1/items/:id. + +Refs #15 +``` + +Rules: +- Subject line: imperative mood, no period, max 72 characters +- Body: wrap at 72 characters, explain *what* and *why* (not *how*) +- Reference the issue number in the footer when applicable + +## Creating a Pull Request + +Use `gh pr create` with a structured body: + +```bash +gh pr create \ + --title "feat(api): add orders endpoint (#42)" \ + --body "## Summary + +Adds CRUD endpoints for the Order resource. + +## Changes +- Model with validation and optimistic locking +- Migration to create orders table +- Handler with full CRUD operations +- Routes under /api/v1/orders +- Unit tests with MockRepository + +## Testing +- \`cd backend && go test ./... -v -short\` + +Closes #42" \ + --base main +``` + +Rules: +- Title follows commit convention: `(): (#issue)` +- Body includes: Summary, Changes list, Testing instructions +- Use `Closes #N` to auto-close the issue on merge +- Add labels if relevant: `--label "feature"`, `--label "bug"` +- Request reviewers when specified: `--reviewer ` + +## Step-by-Step: Full SCM Workflow + +```bash +# 1. Ensure we're on the latest default branch +git checkout main +git pull origin main + +# 2. Create and switch to a new branch +git checkout -b feature/42-add-orders-endpoint + +# 3. Check what changed +git status +git diff --stat + +# 4. Stage changes (be explicit — no `git add .` unless truly everything is intended) +git add internal/models/models.go +git add internal/models/validation.go +git add internal/database/migrations.go +git add internal/api/handlers/orders.go +git add internal/api/handlers/orders_test.go +git add internal/api/routes/routes.go + +# 5. Commit with a conventional message +git commit -m "feat(api): add orders CRUD handlers + +Implements Create, Read, List, Update, Delete for the Order resource. +Includes validation, migration, and unit tests. + +Refs #42" + +# 6. Push the branch +git push -u origin feature/42-add-orders-endpoint + +# 7. Create the PR +gh pr create \ + --title "feat(api): add orders endpoint (#42)" \ + --body "..." \ + --base main +``` + +## Handling Multiple Logical Changes + +If the working tree contains changes spanning multiple concerns, split them into separate commits: + +```bash +# Commit 1: model + migration +git add internal/models/models.go internal/models/validation.go internal/database/migrations.go +git commit -m "feat(models): add Order model and migration + +Refs #42" + +# Commit 2: handlers + routes +git add internal/api/handlers/orders.go internal/api/routes/routes.go +git commit -m "feat(api): add orders CRUD handlers and routes + +Refs #42" + +# Commit 3: tests +git add internal/api/handlers/orders_test.go +git commit -m "test(api): add orders handler unit tests + +Refs #42" +``` + +## Pre-Commit Checks + +Before committing, always verify: + +1. **No secrets** — scan staged files: `git diff --cached | grep -iE "(password|secret|key|token)" || echo "clean"` +2. **No generated files** — don't commit `docs/swagger.json` unless intentional, check `.gitignore` coverage +3. **Tests pass** — `cd backend && go test ./... -short` for backend changes +4. **Lint passes** — `make lint` if available +5. **Clean diff** — review `git diff --cached` to confirm only intended changes + +## Critical Rules + +- **Never force-push to main or shared branches** — always ask before using `--force` +- **Never commit `.env` files or secrets** — verify `.gitignore` covers them +- **Never use `git add .` blindly** — review `git status` first and stage explicitly +- **Never amend or rebase commits that others may have pulled** — only amend local-only commits +- **Always pull before branching** — stale branches cause unnecessary merge conflicts +- **Always verify the remote** — `git remote -v` before the first push in a session + +## Commands Reference + +```bash +# Branch management +git checkout main && git pull origin main # Update default branch +git checkout -b # Create new branch +git branch -d # Delete local branch (safe) + +# Staging and committing +git status # Review working tree +git diff # Review unstaged changes +git diff --cached # Review staged changes +git add [...] # Stage specific files +git commit -m "" # Commit staged changes + +# Remote operations +git push -u origin # Push new branch +git push # Push subsequent commits + +# PR management +gh pr create --title "..." --body "..." --base main # Open PR +gh pr view # View PR details +gh pr list # List open PRs +gh pr merge --squash --delete-branch # Merge PR (squash) +``` + +## Handoff + +When your task is complete, end your response with a handoff block: + +```handoff +Next Agent: +Prompt: +Context: +``` + +Common handoff targets: +- **code-reviewer** — when the PR is ready for review +- **orchestrator** — when returning control after PR creation +- **qa-engineer** — when tests need to be run against the PR branch From 3deb30999d53ecb2db542fce758159c4f767c260 Mon Sep 17 00:00:00 2001 From: omattsson Date: Sat, 14 Mar 2026 19:04:45 +0100 Subject: [PATCH 2/3] feat(api): add WebSocket endpoint with hub/client architecture Add a /ws WebSocket endpoint for real-time bidirectional communication. A Hub manages all active client connections and broadcasts messages. WebSocketHandler is a separate struct (not Handler) injected with *Hub. - internal/websocket: Hub + Client types with goroutine-safe broadcast, register/unregister, and graceful shutdown via hub.Shutdown() - handlers/websocket.go: upgrades HTTP to WebSocket with origin validation from CORS_ALLOWED_ORIGINS config - routes.go: /ws route registered outside rate limiter (connections are long-lived); hub injected via SetupRoutes signature - main.go: hub created and started before router setup; hub.Shutdown() called during graceful shutdown - docs regenerated to include /ws endpoint --- backend/api/main.go | 10 +- backend/docs/docs.go | 39 +- backend/docs/swagger.json | 41 +- backend/docs/swagger.yaml | 28 +- backend/internal/api/handlers/websocket.go | 75 ++++ .../internal/api/handlers/websocket_test.go | 381 ++++++++++++++++++ backend/internal/api/routes/routes.go | 7 +- backend/internal/api/routes/routes_test.go | 8 +- 8 files changed, 582 insertions(+), 7 deletions(-) create mode 100644 backend/internal/api/handlers/websocket.go create mode 100644 backend/internal/api/handlers/websocket_test.go diff --git a/backend/api/main.go b/backend/api/main.go index adad117..a6bcbc5 100644 --- a/backend/api/main.go +++ b/backend/api/main.go @@ -8,6 +8,7 @@ import ( "backend/internal/config" "backend/internal/database" "backend/internal/health" + "backend/internal/websocket" "context" "fmt" "log/slog" @@ -56,9 +57,13 @@ func main() { }) healthChecker.SetReady(true) + // Create and start WebSocket hub + hub := websocket.NewHub() + go hub.Run() + // Setup router — use gin.New() since SetupRoutes registers its own Logger and Recovery middleware. router := gin.New() - rateLimiter := routes.SetupRoutes(router, repo, healthChecker, cfg) + rateLimiter := routes.SetupRoutes(router, repo, healthChecker, cfg, hub) defer rateLimiter.Stop() router.GET("/swagger/*any", ginSwagger.WrapHandler(swaggerFiles.Handler)) @@ -86,6 +91,9 @@ func main() { <-quit slog.Info("Shutting down server...") + // Shut down WebSocket hub (closes all client connections) + hub.Shutdown() + // Give outstanding requests time to complete shutdownTimeout := cfg.Server.ShutdownTimeout if shutdownTimeout == 0 { diff --git a/backend/docs/docs.go b/backend/docs/docs.go index 106c30e..6e648aa 100644 --- a/backend/docs/docs.go +++ b/backend/docs/docs.go @@ -5,6 +5,9 @@ import "github.com/swaggo/swag" const docTemplate = `{ "schemes": {{ marshal .Schemes }}, + "produces": [ + "application/json" + ], "swagger": "2.0", "info": { "description": "{{escape .Description}}", @@ -289,6 +292,38 @@ const docTemplate = `{ } } } + }, + "/ws": { + "get": { + "description": "Upgrades the HTTP connection to a WebSocket for real-time events.", + "tags": [ + "websocket" + ], + "summary": "Open a WebSocket connection", + "responses": { + "101": { + "description": "Switching Protocols" + }, + "400": { + "description": "Bad Request", + "schema": { + "type": "object", + "additionalProperties": { + "type": "string" + } + } + }, + "403": { + "description": "Forbidden", + "schema": { + "type": "object", + "additionalProperties": { + "type": "string" + } + } + } + } + } } }, "definitions": { @@ -346,7 +381,7 @@ const docTemplate = `{ "example": "2025-06-02T10:00:00Z" }, "version": { - "description": "For optimistic locking", + "description": "For optimistic locking (1 = initial; 0 = not provided)", "type": "integer" } } @@ -359,7 +394,7 @@ var SwaggerInfo = &swag.Spec{ Version: "1.0", Host: "localhost:8081", BasePath: "/", - Schemes: []string{}, + Schemes: []string{"http", "https"}, Title: "Backend API", Description: "This is the API documentation for the backend service", InfoInstanceName: "swagger", diff --git a/backend/docs/swagger.json b/backend/docs/swagger.json index 5b68d71..99e2fd7 100644 --- a/backend/docs/swagger.json +++ b/backend/docs/swagger.json @@ -1,4 +1,11 @@ { + "produces": [ + "application/json" + ], + "schemes": [ + "http", + "https" + ], "swagger": "2.0", "info": { "description": "This is the API documentation for the backend service", @@ -283,6 +290,38 @@ } } } + }, + "/ws": { + "get": { + "description": "Upgrades the HTTP connection to a WebSocket for real-time events.", + "tags": [ + "websocket" + ], + "summary": "Open a WebSocket connection", + "responses": { + "101": { + "description": "Switching Protocols" + }, + "400": { + "description": "Bad Request", + "schema": { + "type": "object", + "additionalProperties": { + "type": "string" + } + } + }, + "403": { + "description": "Forbidden", + "schema": { + "type": "object", + "additionalProperties": { + "type": "string" + } + } + } + } + } } }, "definitions": { @@ -340,7 +379,7 @@ "example": "2025-06-02T10:00:00Z" }, "version": { - "description": "For optimistic locking", + "description": "For optimistic locking (1 = initial; 0 = not provided)", "type": "integer" } } diff --git a/backend/docs/swagger.yaml b/backend/docs/swagger.yaml index 7761d57..e469a9f 100644 --- a/backend/docs/swagger.yaml +++ b/backend/docs/swagger.yaml @@ -37,7 +37,7 @@ definitions: example: "2025-06-02T10:00:00Z" type: string version: - description: For optimistic locking + description: For optimistic locking (1 = initial; 0 = not provided) type: integer type: object host: localhost:8081 @@ -228,4 +228,30 @@ paths: summary: Readiness Check tags: - health + /ws: + get: + description: Upgrades the HTTP connection to a WebSocket for real-time events. + responses: + "101": + description: Switching Protocols + "400": + description: Bad Request + schema: + additionalProperties: + type: string + type: object + "403": + description: Forbidden + schema: + additionalProperties: + type: string + type: object + summary: Open a WebSocket connection + tags: + - websocket +produces: +- application/json +schemes: +- http +- https swagger: "2.0" diff --git a/backend/internal/api/handlers/websocket.go b/backend/internal/api/handlers/websocket.go new file mode 100644 index 0000000..d561f7b --- /dev/null +++ b/backend/internal/api/handlers/websocket.go @@ -0,0 +1,75 @@ +package handlers + +import ( + "log/slog" + "net/http" + "strings" + + "backend/internal/websocket" + + "github.com/gin-gonic/gin" + gorilla "github.com/gorilla/websocket" +) + +// WebSocketHandler handles WebSocket connection upgrades. +// It is a separate struct from Handler because it depends on *websocket.Hub +// rather than models.Repository. +type WebSocketHandler struct { + hub *websocket.Hub + allowedOrigins string +} + +// NewWebSocketHandler creates a new WebSocketHandler with the given hub and allowed origins config. +func NewWebSocketHandler(hub *websocket.Hub, allowedOrigins string) *WebSocketHandler { + return &WebSocketHandler{ + hub: hub, + allowedOrigins: allowedOrigins, + } +} + +// HandleWebSocket godoc +// @Summary Open a WebSocket connection +// @Description Upgrades the HTTP connection to a WebSocket for real-time events. +// @Tags websocket +// @Success 101 "Switching Protocols" +// @Failure 400 {object} map[string]string +// @Failure 403 {object} map[string]string +// @Router /ws [get] +func (h *WebSocketHandler) HandleWebSocket(c *gin.Context) { + upgrader := gorilla.Upgrader{ + ReadBufferSize: 1024, + WriteBufferSize: 1024, + CheckOrigin: h.checkOrigin, + } + + conn, err := upgrader.Upgrade(c.Writer, c.Request, nil) + if err != nil { + slog.Error("WebSocket upgrade failed", "error", err) + return + } + + if _, err := websocket.NewClient(h.hub, conn); err != nil { + slog.Error("WebSocket client creation failed", "error", err) + return + } +} + +// checkOrigin validates the request origin against the configured allowed origins. +func (h *WebSocketHandler) checkOrigin(r *http.Request) bool { + if h.allowedOrigins == "" || h.allowedOrigins == "*" { + return true + } + + origin := r.Header.Get("Origin") + if origin == "" { + return true + } + + for _, allowed := range strings.Split(h.allowedOrigins, ",") { + if strings.TrimSpace(allowed) == origin { + return true + } + } + + return false +} diff --git a/backend/internal/api/handlers/websocket_test.go b/backend/internal/api/handlers/websocket_test.go new file mode 100644 index 0000000..3f7b2de --- /dev/null +++ b/backend/internal/api/handlers/websocket_test.go @@ -0,0 +1,381 @@ +package handlers + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "backend/internal/websocket" + + "github.com/gin-gonic/gin" + gorilla "github.com/gorilla/websocket" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCheckOrigin(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + allowedOrigins string + origin string + want bool + }{ + { + name: "wildcard allows any origin", + allowedOrigins: "*", + origin: "http://evil.com", + want: true, + }, + { + name: "empty string allows any origin", + allowedOrigins: "", + origin: "http://evil.com", + want: true, + }, + { + name: "specific origin allows matching request", + allowedOrigins: "http://example.com", + origin: "http://example.com", + want: true, + }, + { + name: "specific origin rejects non-matching request", + allowedOrigins: "http://example.com", + origin: "http://evil.com", + want: false, + }, + { + name: "multiple comma-separated origins allow first match", + allowedOrigins: "http://example.com,http://other.com", + origin: "http://example.com", + want: true, + }, + { + name: "multiple comma-separated origins allow second match", + allowedOrigins: "http://example.com,http://other.com", + origin: "http://other.com", + want: true, + }, + { + name: "multiple origins reject non-matching request", + allowedOrigins: "http://example.com,http://other.com", + origin: "http://evil.com", + want: false, + }, + { + name: "comma-separated with spaces trims correctly", + allowedOrigins: "http://example.com, http://other.com", + origin: "http://other.com", + want: true, + }, + { + name: "no origin header allows request (same-origin)", + allowedOrigins: "http://example.com", + origin: "", + want: true, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + handler := NewWebSocketHandler(nil, tt.allowedOrigins) + req, err := http.NewRequest("GET", "/ws", nil) + require.NoError(t, err) + + if tt.origin != "" { + req.Header.Set("Origin", tt.origin) + } + + got := handler.checkOrigin(req) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestNewWebSocketHandler(t *testing.T) { + t.Parallel() + + hub := websocket.NewHub() + handler := NewWebSocketHandler(hub, "http://example.com") + + assert.NotNil(t, handler) + assert.Equal(t, "http://example.com", handler.allowedOrigins) +} + +// waitForHubClients polls hub.ClientCount until it equals want or timeout. +func waitForHubClients(t *testing.T, hub *websocket.Hub, want int) { + t.Helper() + assert.Eventually(t, func() bool { + return hub.ClientCount() == want + }, 2*time.Second, 10*time.Millisecond, "expected %d clients", want) +} + +func TestHandleWebSocket_SuccessfulUpgrade(t *testing.T) { + t.Parallel() + gin.SetMode(gin.TestMode) + + hub := websocket.NewHub() + go hub.Run() + defer hub.Shutdown() + + handler := NewWebSocketHandler(hub, "*") + + router := gin.New() + router.GET("/ws", handler.HandleWebSocket) + + server := httptest.NewServer(router) + defer server.Close() + + // Convert http:// to ws:// + wsURL := "ws" + strings.TrimPrefix(server.URL, "http") + "/ws" + + conn, resp, err := gorilla.DefaultDialer.Dial(wsURL, nil) + require.NoError(t, err) + defer conn.Close() + + assert.Equal(t, http.StatusSwitchingProtocols, resp.StatusCode) + waitForHubClients(t, hub, 1) +} + +func TestHandleWebSocket_BroadcastReceived(t *testing.T) { + t.Parallel() + gin.SetMode(gin.TestMode) + + hub := websocket.NewHub() + go hub.Run() + defer hub.Shutdown() + + handler := NewWebSocketHandler(hub, "*") + + router := gin.New() + router.GET("/ws", handler.HandleWebSocket) + + server := httptest.NewServer(router) + defer server.Close() + + wsURL := "ws" + strings.TrimPrefix(server.URL, "http") + "/ws" + + conn, _, err := gorilla.DefaultDialer.Dial(wsURL, nil) + require.NoError(t, err) + defer conn.Close() + + waitForHubClients(t, hub, 1) + + // Broadcast a message through the hub + msg := []byte(`{"type":"test","payload":"hello"}`) + hub.Broadcast(msg) + + // Read the message from the WebSocket connection + err = conn.SetReadDeadline(time.Now().Add(2 * time.Second)) + require.NoError(t, err) + + _, received, err := conn.ReadMessage() + require.NoError(t, err) + assert.Equal(t, msg, received) +} + +func TestHandleWebSocket_MultipleClients(t *testing.T) { + t.Parallel() + gin.SetMode(gin.TestMode) + + hub := websocket.NewHub() + go hub.Run() + defer hub.Shutdown() + + handler := NewWebSocketHandler(hub, "*") + + router := gin.New() + router.GET("/ws", handler.HandleWebSocket) + + server := httptest.NewServer(router) + defer server.Close() + + wsURL := "ws" + strings.TrimPrefix(server.URL, "http") + "/ws" + + conn1, _, err := gorilla.DefaultDialer.Dial(wsURL, nil) + require.NoError(t, err) + defer conn1.Close() + + conn2, _, err := gorilla.DefaultDialer.Dial(wsURL, nil) + require.NoError(t, err) + defer conn2.Close() + + waitForHubClients(t, hub, 2) + + msg := []byte(`{"type":"broadcast","payload":"all"}`) + hub.Broadcast(msg) + + for i, conn := range []*gorilla.Conn{conn1, conn2} { + err = conn.SetReadDeadline(time.Now().Add(2 * time.Second)) + require.NoError(t, err, "client %d set deadline", i+1) + + _, received, err := conn.ReadMessage() + require.NoError(t, err, "client %d read", i+1) + assert.Equal(t, msg, received, "client %d message", i+1) + } +} + +func TestHandleWebSocket_HubShutdown(t *testing.T) { + t.Parallel() + gin.SetMode(gin.TestMode) + + hub := websocket.NewHub() + go hub.Run() + + handler := NewWebSocketHandler(hub, "*") + + router := gin.New() + router.GET("/ws", handler.HandleWebSocket) + + server := httptest.NewServer(router) + defer server.Close() + + wsURL := "ws" + strings.TrimPrefix(server.URL, "http") + "/ws" + + // Connect a client first, then shut down the hub + conn, _, err := gorilla.DefaultDialer.Dial(wsURL, nil) + require.NoError(t, err) + defer conn.Close() + + waitForHubClients(t, hub, 1) + + hub.Shutdown() + waitForHubClients(t, hub, 0) + + // The connected client should receive a close frame or read error + err = conn.SetReadDeadline(time.Now().Add(2 * time.Second)) + require.NoError(t, err) + + _, _, err = conn.ReadMessage() + assert.Error(t, err, "read after hub shutdown should fail") +} + +func TestHandleWebSocket_HubClosedBeforeUpgrade(t *testing.T) { + t.Parallel() + gin.SetMode(gin.TestMode) + + hub := websocket.NewHub() + go hub.Run() + hub.Shutdown() + + handler := NewWebSocketHandler(hub, "*") + + router := gin.New() + router.GET("/ws", handler.HandleWebSocket) + + server := httptest.NewServer(router) + defer server.Close() + + wsURL := "ws" + strings.TrimPrefix(server.URL, "http") + "/ws" + + // The upgrade itself succeeds (HTTP → WS), but NewClient fails + // because the hub is closed. The server closes the connection. + conn, _, err := gorilla.DefaultDialer.Dial(wsURL, nil) + if err != nil { + // Connection refused or failed — acceptable when hub is closed + return + } + defer conn.Close() + + // If dial succeeded, the connection should be immediately closed by server + err = conn.SetReadDeadline(time.Now().Add(2 * time.Second)) + require.NoError(t, err) + + _, _, err = conn.ReadMessage() + assert.Error(t, err, "connection should be closed when hub is shut down") +} + +func TestHandleWebSocket_OriginRejected(t *testing.T) { + t.Parallel() + gin.SetMode(gin.TestMode) + + hub := websocket.NewHub() + go hub.Run() + defer hub.Shutdown() + + handler := NewWebSocketHandler(hub, "http://allowed.com") + + router := gin.New() + router.GET("/ws", handler.HandleWebSocket) + + server := httptest.NewServer(router) + defer server.Close() + + wsURL := "ws" + strings.TrimPrefix(server.URL, "http") + "/ws" + + // Dial with a disallowed origin + header := http.Header{} + header.Set("Origin", "http://evil.com") + + _, resp, err := gorilla.DefaultDialer.Dial(wsURL, header) + assert.Error(t, err, "dial with rejected origin should fail") + if resp != nil { + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + } +} + +func TestHandleWebSocket_OriginAllowed(t *testing.T) { + t.Parallel() + gin.SetMode(gin.TestMode) + + hub := websocket.NewHub() + go hub.Run() + defer hub.Shutdown() + + handler := NewWebSocketHandler(hub, "http://allowed.com") + + router := gin.New() + router.GET("/ws", handler.HandleWebSocket) + + server := httptest.NewServer(router) + defer server.Close() + + wsURL := "ws" + strings.TrimPrefix(server.URL, "http") + "/ws" + + header := http.Header{} + header.Set("Origin", "http://allowed.com") + + conn, resp, err := gorilla.DefaultDialer.Dial(wsURL, header) + require.NoError(t, err) + defer conn.Close() + + assert.Equal(t, http.StatusSwitchingProtocols, resp.StatusCode) + waitForHubClients(t, hub, 1) +} + +func TestHandleWebSocket_ClientDisconnect(t *testing.T) { + t.Parallel() + gin.SetMode(gin.TestMode) + + hub := websocket.NewHub() + go hub.Run() + defer hub.Shutdown() + + handler := NewWebSocketHandler(hub, "*") + + router := gin.New() + router.GET("/ws", handler.HandleWebSocket) + + server := httptest.NewServer(router) + defer server.Close() + + wsURL := "ws" + strings.TrimPrefix(server.URL, "http") + "/ws" + + conn, _, err := gorilla.DefaultDialer.Dial(wsURL, nil) + require.NoError(t, err) + + waitForHubClients(t, hub, 1) + + // Client closes the connection + conn.Close() + + // Hub should eventually unregister the client + waitForHubClients(t, hub, 0) +} diff --git a/backend/internal/api/routes/routes.go b/backend/internal/api/routes/routes.go index 692927f..37d137e 100644 --- a/backend/internal/api/routes/routes.go +++ b/backend/internal/api/routes/routes.go @@ -6,6 +6,7 @@ import ( "backend/internal/config" "backend/internal/health" "backend/internal/models" + "backend/internal/websocket" "time" "github.com/gin-gonic/gin" @@ -14,7 +15,7 @@ import ( // SetupRoutes configures all the routes for our application. // healthChecker is injected from main so the readiness endpoint reflects real dependency health. // Returns the rate limiter so the caller can stop it during shutdown. -func SetupRoutes(router *gin.Engine, repository models.Repository, healthChecker *health.HealthChecker, cfg *config.Config) *handlers.RateLimiter { +func SetupRoutes(router *gin.Engine, repository models.Repository, healthChecker *health.HealthChecker, cfg *config.Config, hub *websocket.Hub) *handlers.RateLimiter { // Add middleware router.Use(middleware.RequestID()) router.Use(middleware.Logger()) @@ -22,6 +23,10 @@ func SetupRoutes(router *gin.Engine, repository models.Repository, healthChecker router.Use(middleware.CORS(cfg.CORS.AllowedOrigins)) router.Use(middleware.MaxBodySize(1 << 20)) // 1 MB default + // WebSocket endpoint (top-level, outside rate limiter — connections are long-lived) + wsHandler := handlers.NewWebSocketHandler(hub, cfg.CORS.AllowedOrigins) + router.GET("/ws", wsHandler.HandleWebSocket) + // Health check endpoints healthGroup := router.Group("/health") { diff --git a/backend/internal/api/routes/routes_test.go b/backend/internal/api/routes/routes_test.go index affb6dd..08d21d1 100644 --- a/backend/internal/api/routes/routes_test.go +++ b/backend/internal/api/routes/routes_test.go @@ -9,6 +9,7 @@ import ( "backend/internal/api/handlers" "backend/internal/config" "backend/internal/health" + "backend/internal/websocket" "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" @@ -33,8 +34,13 @@ func TestSetupRoutes(t *testing.T) { }, } + // Create a test WebSocket hub + hub := websocket.NewHub() + go hub.Run() + defer hub.Shutdown() + // Setup routes - rl := SetupRoutes(router, mockRepo, healthChecker, cfg) + rl := SetupRoutes(router, mockRepo, healthChecker, cfg, hub) defer rl.Stop() // Test cases From 3f5c88835f60f257a933dc01b97b06db6759e87c Mon Sep 17 00:00:00 2001 From: omattsson Date: Sat, 14 Mar 2026 19:23:00 +0100 Subject: [PATCH 3/3] test(websocket): add Client unit tests, raise package coverage to 85.7% MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cover all six functions in client.go with direct unit tests: - NewClient: success path (client registered, pumps started) and hub-already- shut-down path (ErrHubClosed returned, conn closed by NewClient) - writeMessage: successful text frame delivery to peer; NextWriter error on closed conn - writePing: successful ping frame (peer PingHandler fires); write error on closed conn - handleSend: ok=false sends close frame + returns errChanClosed; ok=true single message; ok=true with queued drain (3 messages in order); closed conn returns write error - readPump: normal close frame (CloseNormalClosure) triggers unregister; unexpected TCP drop triggers unregister; CloseProtocolError triggers the slog.Warn path (IsUnexpectedCloseError=true) then unregister - writePump: single message forwarded to peer; multiple messages in order; closed send channel sends WS close frame and exits cleanly Remaining uncovered statements are gorilla/websocket library-internal paths (SetWriteDeadline returns nil lazily s(SetWriteDeadline returns nil lazily s(SetWriteDeadline retunt(SetWriteDeadline returns nil lazily s(SetWriteDeadline returns nil lazily s€”(SetWriteDeadline returns nil lazily s(SetWriteDeadline retage: 45.5% → 85.7% --- backend/internal/websocket/client_test.go | 481 ++++++++++++++++++++++ 1 file changed, 481 insertions(+) create mode 100644 backend/internal/websocket/client_test.go diff --git a/backend/internal/websocket/client_test.go b/backend/internal/websocket/client_test.go new file mode 100644 index 0000000..65550ad --- /dev/null +++ b/backend/internal/websocket/client_test.go @@ -0,0 +1,481 @@ +package websocket + +import ( + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" + + gorilla "github.com/gorilla/websocket" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// newTestWSPair creates a connected pair of WebSocket connections for testing. +// +// server is the connection to be owned by the Client under test; +// peer is the far-end connection used by the test to send/receive frames. +// +// Gorilla hijacks the underlying TCP connection on Upgrade, so the HTTP handler +// may return immediately without affecting the lifetime of the returned conns. +func newTestWSPair(t *testing.T) (server, peer *gorilla.Conn) { + t.Helper() + + srvCh := make(chan *gorilla.Conn, 1) + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + u := gorilla.Upgrader{CheckOrigin: func(*http.Request) bool { return true }} + c, err := u.Upgrade(w, r, nil) + if err != nil { + t.Logf("newTestWSPair: upgrade: %v", err) + return + } + srvCh <- c + // Handler may return — gorilla has already hijacked the conn. + })) + t.Cleanup(ts.Close) + + wsURL := "ws" + strings.TrimPrefix(ts.URL, "http") + p, _, err := gorilla.DefaultDialer.Dial(wsURL, nil) + require.NoError(t, err, "newTestWSPair: dial") + + srv := <-srvCh + + t.Cleanup(func() { + srv.Close() + p.Close() + }) + return srv, p +} + +// sendCloser wraps a send channel and provides idempotent close via sync.Once, +// preventing double-close panics when both a test and its cleanup close the channel. +type sendCloser struct { + ch chan []byte + once sync.Once +} + +func newSendCloser() *sendCloser { + return &sendCloser{ch: make(chan []byte, sendBufferSize)} +} + +func (s *sendCloser) close() { + s.once.Do(func() { close(s.ch) }) +} + +// TestNewClient covers the NewClient constructor: success path and hub-closed path. +func TestNewClient(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + closeHub bool + wantErr error + wantCount int + }{ + { + name: "success - client registered with hub and pumps started", + closeHub: false, + wantErr: nil, + wantCount: 1, + }, + { + name: "hub already shut down - returns ErrHubClosed and closes conn", + closeHub: true, + wantErr: ErrHubClosed, + wantCount: 0, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + hub := NewHub() + go hub.Run() + + if tt.closeHub { + hub.Shutdown() + // Wait for Run to process the shutdown before proceeding. + waitForClientCount(t, hub, 0) + } else { + defer hub.Shutdown() + } + + srvConn, _ := newTestWSPair(t) + + client, err := NewClient(hub, srvConn) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + assert.Nil(t, client) + // NewClient must have closed the conn; verify it's unusable. + assert.Error(t, srvConn.WriteMessage(gorilla.TextMessage, []byte("x"))) + return + } + + require.NoError(t, err) + require.NotNil(t, client) + waitForClientCount(t, hub, tt.wantCount) + }) + } +} + +// TestClient_WriteMessage covers the writeMessage method directly. +func TestClient_WriteMessage(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + msg []byte + closeBefore bool + wantErr bool + }{ + { + name: "success - peer receives text frame", + msg: []byte(`{"type":"item.created","payload":{"id":42}}`), + wantErr: false, + }, + { + name: "closed conn - NextWriter returns error", + msg: []byte("data"), + closeBefore: true, + wantErr: true, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + hub := NewHub() + go hub.Run() + defer hub.Shutdown() + + srvConn, peerConn := newTestWSPair(t) + c := &Client{hub: hub, conn: srvConn, send: make(chan []byte, sendBufferSize)} + + if tt.closeBefore { + require.NoError(t, srvConn.Close()) + } + + err := c.writeMessage(tt.msg) + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + + require.NoError(t, peerConn.SetReadDeadline(time.Now().Add(2*time.Second))) + _, data, readErr := peerConn.ReadMessage() + require.NoError(t, readErr) + assert.Equal(t, tt.msg, data) + }) + } +} + +// TestClient_WritePing covers the writePing method. +func TestClient_WritePing(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + closeBefore bool + wantErr bool + }{ + { + name: "success - ping frame sent and SetWriteDeadline applied", + wantErr: false, + }, + { + name: "closed conn - SetWriteDeadline fails, error returned", + closeBefore: true, + wantErr: true, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + hub := NewHub() + go hub.Run() + defer hub.Shutdown() + + srvConn, peerConn := newTestWSPair(t) + c := &Client{hub: hub, conn: srvConn, send: make(chan []byte, sendBufferSize)} + + if tt.closeBefore { + require.NoError(t, srvConn.Close()) + } + + err := c.writePing() + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + + // Observe the ping arriving at the peer by installing a custom PingHandler, + // then driving the peer's read loop so the handler fires. + pingCh := make(chan struct{}, 1) + peerConn.SetPingHandler(func(string) error { + pingCh <- struct{}{} + return nil + }) + go func() { + _ = peerConn.SetReadDeadline(time.Now().Add(2 * time.Second)) + _, _, _ = peerConn.ReadMessage() + }() + + select { + case <-pingCh: + // Ping arrived at peer. + case <-time.After(2 * time.Second): + t.Error("peer did not receive ping within timeout") + } + }) + } +} + +// TestClient_HandleSend covers handleSend: ok=false, ok=true single, queue drain, and write errors. +func TestClient_HandleSend(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + msg []byte + ok bool + queuedMsgs [][]byte + closeBefore bool + wantErr error // exact sentinel via errors.Is + wantAnyErr bool // just any error (for write failures) + wantReceive [][]byte + }{ + { + name: "ok=false - sends close frame and returns errChanClosed", + msg: nil, + ok: false, + wantErr: errChanClosed, + }, + { + name: "ok=true - single message delivered to peer", + msg: []byte(`{"type":"x","payload":1}`), + ok: true, + wantReceive: [][]byte{[]byte(`{"type":"x","payload":1}`)}, + }, + { + name: "ok=true - queued messages drained in order", + msg: []byte(`first`), + ok: true, + queuedMsgs: [][]byte{[]byte(`second`), []byte(`third`)}, + wantReceive: [][]byte{[]byte(`first`), []byte(`second`), []byte(`third`)}, + }, + { + // Closed conn causes SetWriteDeadline to fail → error returned early. + name: "ok=true - closed conn returns write deadline error", + msg: []byte(`data`), + ok: true, + closeBefore: true, + wantAnyErr: true, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + hub := NewHub() + go hub.Run() + defer hub.Shutdown() + + srvConn, peerConn := newTestWSPair(t) + c := &Client{hub: hub, conn: srvConn, send: make(chan []byte, sendBufferSize)} + + // Pre-fill the send channel with queued messages. + for _, m := range tt.queuedMsgs { + c.send <- m + } + + if tt.closeBefore { + require.NoError(t, srvConn.Close()) + } + + err := c.handleSend(tt.msg, tt.ok) + + switch { + case tt.wantErr != nil: + assert.ErrorIs(t, err, tt.wantErr) + case tt.wantAnyErr: + assert.Error(t, err) + default: + require.NoError(t, err) + } + + // Verify the peer received every expected message in order. + for _, want := range tt.wantReceive { + require.NoError(t, peerConn.SetReadDeadline(time.Now().Add(2*time.Second))) + _, data, readErr := peerConn.ReadMessage() + require.NoError(t, readErr) + assert.Equal(t, want, data) + } + }) + } +} + +// TestClient_ReadPump starts readPump as a goroutine and verifies it unregisters +// the client from the hub after various peer-close scenarios. +func TestClient_ReadPump(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + action func(peerConn *gorilla.Conn) + }{ + { + // Peer sends close frame with NormalClosure — IsUnexpectedCloseError returns false. + name: "normal close frame causes unregister", + action: func(p *gorilla.Conn) { + _ = p.WriteControl( + gorilla.CloseMessage, + gorilla.FormatCloseMessage(gorilla.CloseNormalClosure, "bye"), + time.Now().Add(time.Second), + ) + }, + }, + { + // Peer drops the TCP connection without a close frame — IsUnexpectedCloseError may + // return false (non-CloseError) but readPump still exits and unregisters. + name: "unexpected TCP drop causes unregister", + action: func(p *gorilla.Conn) { + p.Close() + }, + }, + { + // Peer sends a close frame with a code outside [NormalClosure, GoingAway], + // so IsUnexpectedCloseError returns true and slog.Warn is emitted. + name: "unexpected close code triggers warn log and unregister", + action: func(p *gorilla.Conn) { + _ = p.WriteControl( + gorilla.CloseMessage, + gorilla.FormatCloseMessage(gorilla.CloseProtocolError, "bad"), + time.Now().Add(time.Second), + ) + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + hub := NewHub() + go hub.Run() + defer hub.Shutdown() + + srvConn, peerConn := newTestWSPair(t) + c := &Client{hub: hub, conn: srvConn, send: make(chan []byte, sendBufferSize)} + + // Register the client manually (without starting writePump). + require.NoError(t, hub.Register(c)) + waitForClientCount(t, hub, 1) + + go c.readPump() + + // Trigger the close scenario. + tt.action(peerConn) + + // readPump must exit and call hub.Unregister, dropping the count to 0. + waitForClientCount(t, hub, 0) + }) + } +} + +// TestClient_WritePump starts writePump as a goroutine and verifies it forwards +// hub messages to the peer and exits cleanly when the send channel is closed. +func TestClient_WritePump(t *testing.T) { + t.Parallel() + + t.Run("single message forwarded to peer", func(t *testing.T) { + t.Parallel() + + hub := NewHub() + go hub.Run() + defer hub.Shutdown() + + srvConn, peerConn := newTestWSPair(t) + sc := newSendCloser() + c := &Client{hub: hub, conn: srvConn, send: sc.ch} + t.Cleanup(sc.close) // ensure writePump goroutine is eventually terminated + + go c.writePump() + + msg := []byte(`{"type":"item.updated","payload":{"id":7}}`) + sc.ch <- msg + + require.NoError(t, peerConn.SetReadDeadline(time.Now().Add(2*time.Second))) + _, received, err := peerConn.ReadMessage() + require.NoError(t, err) + assert.Equal(t, msg, received) + }) + + t.Run("multiple messages delivered in order", func(t *testing.T) { + t.Parallel() + + hub := NewHub() + go hub.Run() + defer hub.Shutdown() + + srvConn, peerConn := newTestWSPair(t) + sc := newSendCloser() + c := &Client{hub: hub, conn: srvConn, send: sc.ch} + t.Cleanup(sc.close) + + go c.writePump() + + msgs := [][]byte{ + []byte(`{"seq":1}`), + []byte(`{"seq":2}`), + []byte(`{"seq":3}`), + } + for _, m := range msgs { + sc.ch <- m + } + + for _, want := range msgs { + require.NoError(t, peerConn.SetReadDeadline(time.Now().Add(2*time.Second))) + _, got, err := peerConn.ReadMessage() + require.NoError(t, err) + assert.Equal(t, want, got) + } + }) + + t.Run("closed send channel sends close frame and exits", func(t *testing.T) { + t.Parallel() + + hub := NewHub() + go hub.Run() + defer hub.Shutdown() + + srvConn, peerConn := newTestWSPair(t) + sc := newSendCloser() + c := &Client{hub: hub, conn: srvConn, send: sc.ch} + + go c.writePump() + + // Closing the channel causes writePump to call handleSend(nil, false), + // which writes a WS CloseMessage and returns errChanClosed → writePump exits + // and its defer calls conn.Close(). + sc.close() + + require.NoError(t, peerConn.SetReadDeadline(time.Now().Add(2*time.Second))) + _, _, err := peerConn.ReadMessage() + assert.Error(t, err, "peer should observe close after send channel is closed") + }) +}