feat(tags): implement tag management API & UI (#89)#101
Conversation
- Add tag service with CRUD operations: - listTags(): fetch all tags sorted alphabetically - createTag(): validate name (1-50 chars, unique case-insensitive), color format (#RRGGBB) - updateTag(): partial update with validation, duplicate name check - deleteTag(): cascade removes from work items via FK - Add tag routes: - GET /api/tags: list all tags - POST /api/tags: create tag (201 Created) - PATCH /api/tags/:id: update tag (200 OK) - DELETE /api/tags/:id: delete tag (204 No Content) - All endpoints require authentication (both admin and member roles) - JSON schema validation for request bodies - Error handling: 400 VALIDATION_ERROR, 404 NOT_FOUND, 409 CONFLICT Fixes #89 Co-Authored-By: Claude backend-developer (Sonnet 4.5) <noreply@anthropic.com>
Implements the frontend components for tag management (Story 3.3): - API client functions in `tagsApi.ts` for CRUD operations on tags - TagPill component: displays tags with configurable colors and optional remove button - TagPicker component: multi-select dropdown with search, inline tag creation, and color picker - TagManagementPage: full CRUD interface for managing tags (create, edit, delete) - Added "Tags" navigation link to sidebar - Updated Sidebar tests to account for new navigation link Key features: - Color contrast calculation ensures readable text on colored backgrounds - Responsive design works on desktop, tablet, and mobile - Delete confirmation modal warns about cascade removal from work items - Inline editing for existing tags - Preview of tag appearance before creation - Empty, loading, and error states handled Co-Authored-By: Claude frontend-developer (Sonnet 4.5) <noreply@anthropic.com>
- Service unit tests (40 tests): listTags, getTagById, createTag, updateTag, deleteTag - Integration tests (31 tests): GET/POST/PATCH/DELETE /api/tags endpoints - UAT coverage: All 24 API scenarios (UAT-3.3-01 to UAT-3.3-42) - Edge cases: case-insensitive uniqueness, hex color validation, name trimming, cascade delete - RBAC: member users can manage tags (UAT-3.3-35 to UAT-3.3-37) - 95%+ test coverage on tagService and tag routes Test count: 811 total (38 suites) — all pass Quality gates: lint, format, typecheck — all pass Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
steilerDev
left a comment
There was a problem hiding this comment.
[product-owner]
PR #101 Review: Story 3.3 — Tag Management API & UI (#89)
Acceptance Criteria Verification
| # | Criterion | Verdict | Evidence |
|---|---|---|---|
| 1 | POST /api/tags creates tag (201), required name, optional color (hex), 409 CONFLICT on duplicate (case-insensitive) |
PASS | tagRoutes POST handler with createTagSchema (AJV validation), tagService.createTag() with LOWER() duplicate check. Integration tests UAT-3.3-01, 02, 06 verify. |
| 2 | GET /api/tags returns all tags sorted alphabetically (200), no pagination |
PASS | tagService.listTags() uses orderBy(sql\LOWER(name)`), returns { tags: [...] }`. Integration tests UAT-3.3-08, 09, 10 verify sorting, empty array, and 100-tag no-pagination. |
| 3 | PATCH /api/tags/:id updates name/color (200), 404 NOT_FOUND, 409 CONFLICT on rename |
PASS | tagService.updateTag() validates existence, checks case-insensitive conflict excluding self, supports color: null. Integration tests UAT-3.3-12, 13, 14, 15, 16, 42 verify. |
| 4 | DELETE /api/tags/:id removes tag (204), cascading deletes from work items, 404 NOT_FOUND |
PASS | tagService.deleteTag() checks existence then deletes (FK cascade handles work_item_tags). Integration test UAT-3.3-18 verifies cascade with work item association preserved and junction row removed. |
| 5 | Validation errors return 400 VALIDATION_ERROR for: empty name, name > 50 chars, invalid color format | PASS | Both AJV schema-level validation (minLength: 1, maxLength: 50, pattern: '^#[0-9A-Fa-f]{6}$') and service-level validation (trim + re-check). Integration tests UAT-3.3-03, 04, 05, 41 verify. |
| 6 | All endpoints require auth (401 UNAUTHORIZED), both admin and member can manage tags | PASS | All 4 route handlers check request.user and throw UnauthorizedError(). Integration tests UAT-3.3-07, 11, 17, 20 verify 401 without auth. Tests UAT-3.3-35, 36, 37 verify member access. |
| 7 | Client provides tag management interface accessible from work items area (create, edit, delete tags) | PASS | TagManagementPage at /tags with sidebar nav link "Tags". Create form (name + color picker + preview), inline edit (name + color), delete with confirmation modal. tagsApi.ts typed API client. |
| 8 | Tags displayed as colored pills/badges throughout UI | PASS | TagPill component with configurable name and color props. Used in TagManagementPage tag list, TagPicker selected tags, and TagPicker dropdown options. |
| 9 | When creating/editing work items, users can select multiple tags or create inline | PASS | TagPicker component: multi-select with search input, dropdown of unselected tags, inline "Create new tag" form with color picker when search term has no exact match. onCreateTag callback handles POST. |
| 10 | Tag color visually represented with sufficient contrast for readability | PASS | TagPill.isLightColor() uses WCAG luminance formula (0.299*R + 0.587*G + 0.114*B) / 255 with threshold 0.5. Dark text (#111827) on light backgrounds, white (#ffffff) on dark. Default gray (#e5e7eb) for null color. |
All 10 acceptance criteria: PASS
Agent Responsibilities
| Agent | Responsibility | Fulfilled |
|---|---|---|
| backend-developer | Tag service + route implementation | Yes — commit feat(tags): implement tag management API endpoints by backend-developer (Sonnet 4.5) |
| frontend-developer | TagManagementPage, TagPill, TagPicker, tagsApi, sidebar update | Yes — commit feat(tags): implement tag management UI and tag picker component by frontend-developer (Sonnet 4.5) |
| qa-integration-tester | 95%+ test coverage on new code | Yes — commit test(tags): add comprehensive tag management tests by qa-integration-tester (Sonnet 4.5). 40 service unit tests + 31 integration tests = 71 total. |
| product-architect | Architecture review | Pending — no review comment found on this PR yet |
| security-engineer | Security review | Pending — noted as happening in parallel |
UAT Scenario Coverage
Cross-referencing the 42 UAT scenarios posted by uat-validator on issue #89:
- API scenarios (UAT-3.3-01 through UAT-3.3-20): All covered by integration tests. Each test is labeled with its UAT scenario ID (e.g.,
(UAT-3.3-01)). - UI scenarios (UAT-3.3-21 through UAT-3.3-26): Implementation covers all: tag management page accessible via sidebar, lists tags with colors, create/edit/delete with confirmation, validation errors displayed inline.
- Tag display (UAT-3.3-27 through UAT-3.3-29): TagPill with color backgrounds, default gray for null, auto-contrast calculation.
- Inline tag assignment (UAT-3.3-30 through UAT-3.3-34): TagPicker component covers multi-select, search, inline creation with color picker.
- RBAC (UAT-3.3-35 through UAT-3.3-37): Covered by member role integration tests.
- Edge cases (UAT-3.3-38 through UAT-3.3-42): Delete confirmation modal (3.3-38 - partial, see observation below), name trimming (3.3-39), hex case acceptance (3.3-40), short hex rejection (3.3-41), same-name update (3.3-42) all covered.
CI Status
- Quality Gates: PASS
- Docker: PASS
- E2E: Skipped (expected — E2E tests owned by e2e-test-engineer in separate cycle)
Observations (non-blocking)
-
Missing
:focus-visibleon several TagManagementPage buttons: The.editButton,.deleteButton,.saveButton,.cancelButton, and.confirmDeleteButtonCSS classes lack:focus-visiblestyles. The.button,.tagOption,.createButton, and.removeButtoncorrectly have them. This is a WCAG 2.1 AA gap — keyboard users navigating the tag list or delete confirmation modal will not see a focus indicator on these buttons. Recommend adding in refinement. -
Delete confirmation does not show affected work item count (UAT-3.3-38): The modal says "This tag will be removed from all work items that reference it" but does not display the actual number of affected work items. The UAT scenario specifies "the dialog indicates the number of work items affected." This is a minor UX gap — the current generic warning is functional but less informative. Recommend addressing in refinement.
-
TagPicker keyboard navigation: The TagPicker supports focus-visible on dropdown options and has search, but full keyboard navigation (arrow keys to browse, Enter to select, Escape to close) per UAT-3.3-34 would benefit from explicit
onKeyDownhandling. The current implementation relies on Tab-based navigation through buttons, which is functional but not the streamlined arrow-key experience described. Consider enhancing in refinement. -
Sidebar link position: "Tags" is placed after "Documents" in the sidebar. This is a reasonable position within the work items area context, consistent with AC #7's requirement that it be "accessible from the work items area."
Verdict
APPROVED (conditional on product-architect and security-engineer completing their reviews).
All 10 acceptance criteria are met. Tests are correctly authored by the qa-integration-tester agent. UAT scenarios are comprehensively addressed. CI passes. The three non-blocking observations above (focus styles, delete count, keyboard nav) should be captured for the EPIC-03 refinement task.
steilerDev
left a comment
There was a problem hiding this comment.
[product-architect] PR #101 Review -- Story 3.3 Tag Management API & UI
Verdict: APPROVED
I have reviewed this PR against the Wiki API Contract, Wiki Schema, CLAUDE.md conventions, and established codebase patterns. The implementation is solid and well-aligned.
Architecture Compliance
Passes. The PR follows all established patterns:
- Plugin/route registration order in
server/src/app.tsis correct -- tag routes registered after work item routes, before health checks. - Service layer pattern:
tagService.tscontains all business logic; route handlers intags.tsonly do auth checks, JSON schema validation, and delegation. This matches theworkItemService/workItemspattern. - Route file structure: Default export async function, Fastify generic type parameters, AJV schema objects. Consistent with
workItems.tsandusers.ts. - Auth guard pattern:
if (!request.user) throw new UnauthorizedError()on every handler. Matches all other non-public routes. - Error handling: Uses
AppErrorsubclasses (NotFoundError,ValidationError,ConflictError) which the global error handler plugin maps to correct HTTP responses. - Client-side patterns: Lazy-loaded page in
App.tsx, NavLink inSidebar.tsx, API module inlib/tagsApi.tsusing the sharedapiClienthelpers. All consistent with existing pages.
API Contract Alignment
Passes. All 4 endpoints match the Wiki API Contract specification exactly:
| Endpoint | Wiki Spec | Implementation | Status |
|---|---|---|---|
GET /api/tags |
200: { tags: [...] }, no pagination |
Returns { tags } sorted by LOWER(name), no pagination metadata |
Match |
POST /api/tags |
201, name required (1-50), color optional hex, 400/409 | AJV schema validates name/color, service validates uniqueness | Match |
PATCH /api/tags/:id |
200, at least one field, 400/404/409 | minProperties: 1 in schema, service checks existence + uniqueness |
Match |
DELETE /api/tags/:id |
204, 404 | Returns 204 empty body, checks existence, cascade via FK | Match |
- Error codes:
VALIDATION_ERROR(400),UNAUTHORIZED(401),NOT_FOUND(404),CONFLICT(409) -- all match the contract. - Case-insensitive name uniqueness enforced at application layer via
LOWER()SQL comparison, as documented in the Schema wiki. - Name trimming before validation and storage -- good defensive practice.
Schema Consistency
Passes. The service operates on the tags table defined in migration 0002. No new migrations needed -- this story only adds API/UI on top of the existing schema from Story 3.1.
Shared Types
Passes. The implementation uses TagResponse, CreateTagRequest, UpdateTagRequest, and TagListResponse from @cornerstone/shared, all of which were defined in Story 3.1. The toTagResponse() mapper in the service correctly maps all fields.
One minor note: TagResponse.createdAt is typed as optional (createdAt?: string) in the shared type, but the service always returns it. This is fine -- the optional typing allows flexibility for embedded contexts (e.g., tags within work item detail responses) where createdAt might be omitted.
Code Quality
Good overall.
- Naming conventions: All correct -- camelCase for TS, kebab-case API routes (
/api/tags), PascalCase for React components (TagPill,TagPicker,TagManagementPage). - TypeScript usage: Proper
typeimports throughout, noanytypes, generics on Fastify route handlers. - AJV schemas: Well-defined with
additionalProperties: falseto prevent unexpected fields. - Color validation: Dual layer -- AJV pattern regex at route level, plus
isValidHexColor()in service. Belt-and-suspenders approach is correct since the service may be called from other contexts. - UUID generation: Uses
randomUUID()fromnode:crypto-- consistent with existing patterns. - CSS Modules: Proper
.module.cssfiles with responsive breakpoints at 767px.
Observations (non-blocking, suitable for refinement):
-
TagPill contrast calculation (line ~469 in
TagPill.tsx): TheisLightColorfunction uses the older ITU-R BT.601 luminance formula (0.299*R + 0.587*G + 0.114*B). This is functional and widely used. For WCAG 2.1 compliance, the sRGB relative luminance formula with gamma correction would be more accurate, but this is a cosmetic concern and not a correctness issue. -
TagPicker
id="tagColor"collision risk: If multipleTagPickercomponents are rendered on the same page, the<label htmlFor="tagColor">/<input id="tagColor">pair could collide. Consider using a unique ID (e.g.,useId()from React 18+) in a refinement pass if this component will be reused in multiple places simultaneously. -
Frontend
console.errorin TagPicker (line ~314): ThehandleCreateTagcatch block hasconsole.error('Failed to create tag:', error). This is acceptable during development but should be removed or replaced with proper error state in production refinement.
Test Coverage
Adequate. 71 new tests (40 service unit + 31 integration) covering:
- All CRUD operations (happy path + error cases)
- Case-insensitive name uniqueness (create + update)
- Hex color validation (valid, invalid, uppercase, lowercase, short codes)
- Name trimming
- Cascade delete to
work_item_tags - RBAC: member users can perform all tag operations
- Auth: 401 for unauthenticated requests on all endpoints
- Edge cases: empty payload on PATCH, 100 tags without pagination
The tests are well-structured with Given/When/Then comments and UAT scenario references. The 95%+ coverage target appears met based on the test breadth.
Summary
This is a clean, well-structured implementation that faithfully follows the API contract, schema design, and established codebase patterns. The three observations above are non-blocking refinement candidates, not merge blockers. Approved.
steilerDev
left a comment
There was a problem hiding this comment.
[security-engineer]
Security Review - PR #101 (Story 3.3: Tag Management API & UI)
Review Date: 2026-02-17
Status: ✅ APPROVED — No security vulnerabilities identified
I have completed a comprehensive security review of the Tag Management feature implementation covering 71 tests (40 service + 31 integration). This review examined:
- OWASP Top 10 vulnerability classes
- Authentication and authorization enforcement
- Input validation and injection protection
- XSS prevention patterns
- Sensitive data handling
- SQL injection risks
Security Findings
✅ Authentication & Authorization (OWASP A01, A07)
Status: SECURE
- All 4 endpoints (
GET,POST,PATCH,DELETE /api/tags) properly enforce authentication viarequest.usercheck - Returns
401 UNAUTHORIZEDwhen session token missing or invalid - Tests confirm both admin and member roles can access all tag endpoints (appropriate for this feature)
- Authorization tests present: UAT-3.3-11, UAT-3.3-07, UAT-3.3-17, UAT-3.3-20, UAT-3.3-35, UAT-3.3-36, UAT-3.3-37
✅ SQL Injection Protection (OWASP A03)
Status: SECURE
Backend uses Drizzle ORM parameterized queries throughout:
-
eq(tags.id, id)— parameterized ID lookups -
sqlLOWER(${tags.name}) = LOWER(${trimmedName})`` — Drizzle's tagged template parameterization -
db.insert(tags).values({...})— parameterized inserts -
db.update(tags).set(updates)— parameterized updates -
db.delete(tags).where(eq(tags.id, id))— parameterized deletes
No raw SQL string concatenation found. All user input is passed through Drizzle's query builder which uses bound parameters.
Verified in: server/src/services/tagService.ts (lines 33-174)
✅ XSS Prevention (OWASP A03)
Status: SECURE
Frontend rendering is safe:
- ✅ Zero instances of
dangerouslySetInnerHTML,innerHTML, oreval()in client codebase - ✅ Tag names rendered via
{name}JSX expression (React auto-escapes) - ✅ Tag colors used in inline styles via React style objects (not string templates):
style={{ backgroundColor, color: textColor }}in TagPill.tsx- React sanitizes style object properties — no CSS injection possible
- ✅ HTML5
<input type="color">constrains input to#RRGGBBformat natively
Verified in:
client/src/components/TagPill/TagPill.tsx(lines 28-52)client/src/pages/TagManagementPage/TagManagementPage.tsx(lines 258, 328, 367)client/src/components/TagPicker/TagPicker.tsx(lines 93-99, 123)
✅ Input Validation (OWASP A03)
Status: SECURE
Multi-layer validation implemented:
Backend JSON Schema Validation (Fastify AJV)
- Tag name:
minLength: 1,maxLength: 50(lines 12-13, 24-25 in routes/tags.ts) - Color:
pattern: '^#[0-9A-Fa-f]{6}$'regex (lines 13, 25) additionalProperties: falseprevents extra fields
Service Layer Validation (tagService.ts)
- Name trimming before validation (lines 61, 126)
- Empty/whitespace-only name rejection (lines 62-64, 127-129)
- Hex color format validation via
isValidHexColor()function (lines 26-28, 67-69, 147-149) - Case-insensitive duplicate detection (lines 72-76, 132-136)
Frontend Validation
- Client-side
maxLength={50}on text inputs - HTML5
type="color"picker enforces#RRGGBBformat - Trimming and length checks before API calls
Test coverage: UAT-3.3-03, UAT-3.3-04, UAT-3.3-05, UAT-3.3-39, UAT-3.3-40, UAT-3.3-41
✅ Sensitive Data Exposure (OWASP A02)
Status: SECURE
- No sensitive data in tag responses (
TagResponsecontains only:id,name,color,createdAt) - Tag names and colors are intentionally user-visible — no PII or secrets
- All fields appropriate for client-side rendering
✅ CSRF Protection (OWASP A01)
Status: SECURE (inherited)
- Session cookies use
SameSite=strictflag (set in Story #32) - No token-based CSRF needed — cookie-based protection sufficient
- All endpoints are state-changing POST/PATCH/DELETE and properly protected
✅ Database Schema Security
Status: SECURE
Schema design verified in server/src/db/schema.ts:
tags.namehas UNIQUE constraint (prevents duplicates at DB level)- Foreign key cascade delete on
work_item_tags.tag_id(prevents orphaned references) createdAttimestamp for audit trail- No sensitive fields in tags table
✅ Error Message Security (OWASP A05)
Status: SECURE
- Generic
UNAUTHORIZEDerror (no session enumeration) NOT_FOUNDonly when tag doesn't exist (acceptable information disclosure)CONFLICTreveals duplicate tag name (acceptable — users need to know why creation failed)VALIDATION_ERRORwith specific message (necessary for UX, no security risk)- No stack traces or internal details leaked
✅ Test Coverage
Excellent security test coverage:
- ✅ Authentication bypass tests for all 4 endpoints (401 without session)
- ✅ Input validation tests (empty names, whitespace, invalid colors, long strings)
- ✅ Injection protection via parameterized queries (implicit in ORM usage)
- ✅ Case-insensitive duplicate detection tests
- ✅ Trimming/normalization tests
- ✅ RBAC tests (member role access)
71 tests pass covering all security-relevant scenarios.
Recommendations (Low Priority)
These are defense-in-depth suggestions — no blocking issues:
-
Rate Limiting (Low): Consider adding rate limits to tag creation endpoints to prevent abuse (e.g., 100 tags per minute). Not critical for 1-5 user deployments.
-
Audit Logging (Informational): Future enhancement could log tag create/update/delete operations for compliance auditing.
-
Tag Usage Tracking (Informational): Consider adding
usageCountor preventing deletion of tags in use (current cascade delete is acceptable but may surprise users).
Final Assessment
Security Posture: STRONG ✅
This implementation demonstrates secure coding practices:
- Consistent authentication enforcement
- Parameterized queries throughout (ORM best practice)
- Multi-layer input validation
- Safe React rendering (no XSS vectors)
- Appropriate error handling
- Comprehensive test coverage of security scenarios
No critical, high, or medium severity vulnerabilities identified.
Approval: ✅ This PR meets security standards for production deployment.
Reviewed by: security-engineer (Claude Sonnet 4.5)
Next Steps: Awaiting product-owner and product-architect reviews before merge.
|
🎉 This PR is included in version 1.8.0-beta.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.8.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Implements Story 3.3 (#89) — Tag Management API & UI for EPIC-03.
Backend
/api/tagsFrontend
/tags): Full CRUD with inline editing, color picker, delete confirmationTests (71 new, 811 total)
Test plan
Fixes #89
🤖 Generated with Claude Code