From 684b975f0f7fecde24d47a1a4243ae416856377d Mon Sep 17 00:00:00 2001 From: michaelwitz Date: Thu, 5 Mar 2026 19:58:22 -0500 Subject: [PATCH 01/14] chore: update AGENTS and ZAZZ-5 spec for route scoping and image model - add mandatory worktree env setup and env propagation guidance - expand ZAZZ-5 SPEC with graph route removal and image route scope changes - add single-owner image data model and explicit TDD/OpenAPI requirements Co-Authored-By: Oz --- .../ZAZZ-5-fix-routes-no-project-SPEC.md | 214 ++++++++++++++---- AGENTS.md | 22 ++ 2 files changed, 188 insertions(+), 48 deletions(-) diff --git a/.zazz/deliverables/ZAZZ-5-fix-routes-no-project-SPEC.md b/.zazz/deliverables/ZAZZ-5-fix-routes-no-project-SPEC.md index 6062e3e6..b0959e68 100644 --- a/.zazz/deliverables/ZAZZ-5-fix-routes-no-project-SPEC.md +++ b/.zazz/deliverables/ZAZZ-5-fix-routes-no-project-SPEC.md @@ -2,23 +2,36 @@ ## 1. Problem Statement -**What**: Several API routes operate on project-scoped data (tasks, images) but do not include project in the path or verify that the requested resource belongs to a project the caller is authorized to access. +**What**: Several API routes operate on project-scoped data (task graph, images) without consistent project-code scoping. The Task Graph UI also includes a project-wide graph mode that is not desired. -**Why**: An agent token scoped to project A could access or modify data belonging to project B (e.g. task images, task metadata). This is a data isolation and authorization gap. +**Why**: Authorization should be constrained to a specific project. Inconsistent route scoping and project-wide graph behavior create ambiguous access patterns and increase risk of cross-project data exposure. **Who**: Agents using the zazz-board-api skill; any client with a valid token. -**Current state**: Routes such as `GET /tasks/:taskId/images`, `POST /tasks/:taskId/images/upload`, `GET /images/:id`, `GET /images/:id/metadata` accept a taskId or imageId without project context. The auth middleware validates the token but does not verify the resource belongs to the token's project. - -**Desired state**: All routes that operate on project-scoped resources must either (a) include project in the path and verify access, or (b) resolve the resource's project and verify the token has access before returning data. +**Current state**: +- The UI Task Graph page includes an `All tasks (project-wide)` option that calls `GET /projects/{code}/graph`. +- Image routes are not project-code-scoped: + - `GET /tasks/{taskId}/images` + - `POST /tasks/{taskId}/images/upload` + - `DELETE /tasks/{taskId}/images/{imageId}` + - `GET /images/{id}` + - `GET /images/{id}/metadata` +- Images are currently task-only. Deliverable-card images are planned but not yet supported by dedicated routes. + +**Desired state**: +- Remove `GET /projects/{code}/graph` and remove the `All tasks (project-wide)` UI option. +- Task graph UI must be deliverable-driven: user explicitly selects a deliverable. +- Image APIs must be project-code-scoped and support both task images and deliverable images, with strict project ownership validation. +- Existing internal project-id routes not exposed in normal browser navigation may remain unchanged in this deliverable. --- ## 2. Standards Applied -- **testing.md** — PactumJS for API tests; every route needs happy path, edge cases, 401/403/404 +- **testing.md** — PactumJS for API tests; every route needs happy path, edge cases, 401/403/404; tests must be written before or alongside implementation - **system-architecture.md** — API layer, auth middleware -- **data-architecture.md** — Tasks belong to deliverables; deliverables belong to projects; images belong to tasks +- **data-architecture.md** — Tasks belong to deliverables; deliverables belong to projects; images currently belong to tasks +- **coding-styles.md** — Project-scoped handler pattern and business logic validation rules --- @@ -26,37 +39,89 @@ ### In Scope -- Audit all API routes to identify which operate on project-scoped data without project filtering -- Fix image routes: `/tasks/:taskId/images`, `/tasks/:taskId/images/upload`, `/images/:id`, `/images/:id/metadata`, `DELETE /tasks/:taskId/images/:imageId` — add project verification -- Add PactumJS tests for cross-project access (403 when token for project A accesses project B's data) -- Update zazz-board-api skill if route paths change (e.g. images under `/projects/:code/...`) +- Remove project-wide graph route and behavior: + - Remove API route `GET /projects/{code}/graph` + - Remove `All tasks (project-wide)` UI option + - Require explicit deliverable selection for graph rendering +- Keep these project-id routes unchanged for now (explicitly accepted in this deliverable): + - `/projects/{id}` + - `/projects/{id}/kanban/tasks/column/{status}` + - `/projects/{id}/tasks` +- **Remove legacy image routes and all API usage**: Delete the following routes from the API and remove any code that calls them (client, databaseService, zazz-board-api skill, etc.): + - `GET /tasks/{taskId}/images` + - `POST /tasks/{taskId}/images/upload` + - `DELETE /tasks/{taskId}/images/{imageId}` + - `GET /images/{id}` + - `GET /images/{id}/metadata` +- Introduce project-scoped image route design for task and deliverable images: + - Task image routes under `/projects/{code}/deliverables/{delivId}/tasks/{taskId}/images...` + - Deliverable image routes under `/projects/{code}/deliverables/{delivId}/images...` + - Image fetch/metadata routes scoped under `/projects/{code}/images/{id}...` +- Update image data model to single-owner ownership: + - `IMAGE_METADATA.task_id` becomes nullable + - Add `IMAGE_METADATA.deliverable_id` (nullable FK to `DELIVERABLES.id`) + - Enforce exactly one owner (`task_id` XOR `deliverable_id`) with DB constraint + - Keep `IMAGE_DATA` as image binary payload store keyed by image metadata id + - **Schema change scope**: Modify only the `IMAGE_METADATA` table (and rebuild it). There is no image data in seed data. Do **not** perform a full database reset or replace other tables—only alter/rebuild `IMAGE_METADATA`. +- Add authorization checks so tokens cannot access images from other projects +- Add PactumJS tests for route behavior and cross-project authorization +- Update zazz-board-api skill/docs for new image and graph route expectations +- Update Swagger/OpenAPI route documentation and schemas for all changed/added image and graph endpoints ### Out of Scope - Tags (`/tags`) — design decision: tags may be global or project-scoped; defer to separate deliverable - Project-level access control (USER_PROJECTS, membership) — see future-fixes.md #5 -- Standardizing `:id` vs `:code` for project params — see future-fixes.md #1 +- Standardizing all `:id` vs `:code` project params across every route +- UI image components on cards (actual card feature work comes after route/API capability is in place) --- ## 4. Features & Requirements -- **F1**: Identify all routes that operate on project-scoped data (tasks, deliverables, images) but lack project in path or project verification -- **F2**: For image routes: verify the task (and thus its project) belongs to a project the token can access before returning data or performing mutations -- **F3**: Return 403 Forbidden when a valid token attempts to access another project's data -- **F4**: Add PactumJS tests: happy path (same project), 403 (cross-project), 401 (no/invalid token), 404 (resource not found) +- **F1 (Graph route removal)**: Remove `GET /projects/{code}/graph` from API and OpenAPI output. +- **F2 (Graph UI behavior)**: On the Task Graph page, remove the `All tasks (project-wide)` dropdown option; the dropdown must list only deliverables; require explicit deliverable selection before fetching graph data; when none selected, show a prompt and do not call the graph API. +- **F3 (Task image scoping)**: Remove legacy task image routes and add project+deliverable+task scoped routes: + - `GET /projects/{code}/deliverables/{delivId}/tasks/{taskId}/images` + - `POST /projects/{code}/deliverables/{delivId}/tasks/{taskId}/images/upload` + - `DELETE /projects/{code}/deliverables/{delivId}/tasks/{taskId}/images/{imageId}` +- **F4 (Deliverable image capability)**: Add deliverable image routes: + - `GET /projects/{code}/deliverables/{delivId}/images` + - `POST /projects/{code}/deliverables/{delivId}/images/upload` + - `DELETE /projects/{code}/deliverables/{delivId}/images/{imageId}` +- **F5 (Project-scoped image fetch by id)**: Remove global image fetch routes and add project-scoped equivalents: + - `GET /projects/{code}/images/{id}` + - `GET /projects/{code}/images/{id}/metadata` +- **F6 (Authorization guarantees)**: For all image endpoints, verify resource belongs to the specified project; return 403 for cross-project access. +- **F7 (Cloud-ready behavior continuity)**: Keep storage abstraction behavior compatible with local DB now and object-storage backends (e.g., S3/GCS) later. +- **F8 (Single-owner image model)**: Implement a single-owner model in `IMAGE_METADATA` so an image belongs to exactly one owner type: + - owner is either task (`task_id`) or deliverable (`deliverable_id`) + - never both and never neither (DB-enforced) + - `IMAGE_DATA` remains linked 1:1 to image metadata +- **F9 (Swagger/OpenAPI updates)**: Update Fastify schemas/OpenAPI generation to document: + - removed `GET /projects/{code}/graph` + - new/updated project-scoped task image routes + - new deliverable image routes + - project-scoped image fetch/metadata routes --- ## 5. Acceptance Criteria -- **AC1**: `GET /tasks/:taskId/images` returns 403 when the task belongs to a project different from the token's project — Verified by: API test -- **AC2**: `POST /tasks/:taskId/images/upload` returns 403 when the task belongs to a different project — Verified by: API test -- **AC3**: `GET /images/:id` returns 403 when the image's task belongs to a different project — Verified by: API test -- **AC4**: `GET /images/:id/metadata` returns 403 when the image's task belongs to a different project — Verified by: API test -- **AC5**: `DELETE /tasks/:taskId/images/:imageId` returns 403 when the task belongs to a different project — Verified by: API test -- **AC6**: All image routes return 200/201 as before when the task belongs to the token's project — Verified by: API test -- **AC7**: Document the list of routes that were audited and which were fixed vs deferred — Verified by: Owner sign-off +- **AC1**: `GET /projects/{code}/graph` is removed from API and OpenAPI spec — Verified by: API/OpenAPI test +- **AC2**: Task Graph UI is deliverable-only: the center dropdown must not offer `All tasks (project-wide)`; it must list only deliverables; user must select a deliverable before any graph data is fetched; when no deliverable is selected, show a prompt (e.g. "Select a deliverable to view the task graph") and do not call any graph API — Verified by: Owner sign-off + UI behavior test/manual verification +- **AC3**: New task image routes under `/projects/{code}/deliverables/{delivId}/tasks/{taskId}/images...` support happy-path access for same-project token — Verified by: API test +- **AC4**: New deliverable image routes under `/projects/{code}/deliverables/{delivId}/images...` support happy-path access for same-project token — Verified by: API test +- **AC5**: `GET /projects/{code}/images/{id}` and `GET /projects/{code}/images/{id}/metadata` return 403 when image belongs to a different project — Verified by: API test +- **AC6**: All scoped image mutation routes return 403 when token project does not match resource project — Verified by: API test +- **AC7**: Legacy non-project-scoped image routes are **removed** from the API, and all callers (client, databaseService, zazz-board-api skill) are updated to use the new project-scoped routes — Verified by: API test (legacy routes return 404) + grep/audit for no remaining usage +- **AC8**: The three accepted project-id routes remain unchanged and functional: + - `/projects/{id}` + - `/projects/{id}/kanban/tasks/column/{status}` + - `/projects/{id}/tasks` + — Verified by: API regression test +- **AC9**: `IMAGE_METADATA` supports single-owner association with DB-level constraint that exactly one of `task_id` or `deliverable_id` is set — Verified by: DB constraint validation + API integration tests +- **AC10**: Swagger/OpenAPI reflects all graph/image endpoint removals and additions with accurate params/body/response schemas — Verified by: OpenAPI test + Owner review in `/docs` --- @@ -64,9 +129,10 @@ - [ ] All AC satisfied - [ ] All PactumJS tests passing -- [ ] No regression in existing image route tests -- [ ] zazz-board-api skill updated if paths change -- [ ] Owner sign-off for AC7 (audit list) +- [ ] Route/schema changes for this deliverable are not considered complete without corresponding PactumJS and OpenAPI coverage +- [ ] No regression in existing deliverable-scoped graph route (`/projects/{code}/deliverables/{delivId}/graph`) +- [ ] zazz-board-api skill/docs updated for route removals/additions +- [ ] Owner sign-off for UI graph behavior and route removal choices --- @@ -74,12 +140,40 @@ ### API Tests (PactumJS) -- **Image routes**: For each of GET/POST /tasks/:taskId/images, GET /images/:id, GET /images/:id/metadata, DELETE /tasks/:taskId/images/:imageId: +- **TDD policy for this deliverable**: + - For each added/changed route, write failing PactumJS coverage first or in the same implementation step before completion. + - Do not merge route/schema changes without corresponding PactumJS and OpenAPI test updates. + +- **Graph route removal**: + - `GET /projects/{code}/graph` must return 404 after removal (route deleted, not deprecated) + - `GET /projects/{code}/deliverables/{delivId}/graph` remains 200 for valid project/deliverable pair +- **Task image routes** (new scoped routes): - Happy path: token for project A, task in project A → 200/201 - Cross-project: token for project A, task in project B → 403 - No auth: missing/invalid token → 401 - - Not found: valid taskId/imageId that doesn't exist → 404 -- **Setup**: Create deliverables and tasks in ZAZZ and APIMOD (or MOBDEV); use seeded tokens for each project; attempt cross-project access + - Not found: invalid project/deliverable/task/image ids → 404 +- **Deliverable image routes** (new): + - Happy path: token for project A, deliverable in project A → 200/201 + - Cross-project: token for project A, deliverable in project B → 403 + - No auth: missing/invalid token → 401 + - Not found: invalid deliverable/image ids → 404 +- **Single-owner data model checks**: + - Creating task image writes `task_id` and leaves `deliverable_id` null + - Creating deliverable image writes `deliverable_id` and leaves `task_id` null + - Attempts to persist both owner columns set fail DB constraint + - Attempts to persist neither owner column set fail DB constraint +- **Image-by-id project-scoped fetch**: + - `GET /projects/{code}/images/{id}` and `/metadata` return 200 for same project, 403 for cross-project, 404 for missing image +- **Project-id route regression checks**: + - Verify `/projects/{id}`, `/projects/{id}/kanban/tasks/column/{status}`, `/projects/{id}/tasks` still function as before +- **Legacy route removal checks**: + - `GET /tasks/{taskId}/images`, `POST /tasks/{taskId}/images/upload`, `DELETE /tasks/{taskId}/images/{imageId}`, `GET /images/{id}`, `GET /images/{id}/metadata` all return 404 (routes removed) +- **OpenAPI/Swagger checks**: + - Generated OpenAPI contains all new image paths and omits removed graph and legacy image paths + - Route params and request/response schemas match implementation +- **Setup**: + - Use ZAZZ and MOBDEV projects (seeded); create deliverables/tasks/images in each as needed + - Use tokens scoped to each project to verify cross-project denial paths --- @@ -88,21 +182,24 @@ ### Always Do - Follow testing.md: PactumJS tests for new/updated routes -- Resolve task → deliverable → project to verify project ownership before returning image data +- Resolve resource ownership before responding: + - Task image: image/task → deliverable → project + - Deliverable image: image/deliverable → project +- Ensure UI graph view is deliverable-selected only (no project-wide fallback mode) ### Ask First (Escalate When) -- Whether to change route paths (e.g. `/projects/:code/tasks/:taskId/images`) vs. keep paths and add project verification in handler -- Tags or other non-image routes that might need project scoping +- Any schema changes beyond the documented `IMAGE_METADATA` single-owner model ### Never Do - Return project B's data to a token scoped to project A -- Skip the 403 cross-project tests +- Reintroduce an implicit project-wide graph view without explicit Owner approval ### Prefer When Multiple Options -- Prefer adding project verification in the handler (resolve task → project, check token) over changing route paths, to minimize client/skill churn. If path change is cleaner, do it. +- Prefer explicit project-code-scoped route paths for image operations over global-id routes +- Prefer consistent route composition with existing deliverable/task route conventions --- @@ -110,37 +207,58 @@ ### Components -- Route audit (manual/code review) -- Image route handlers: add project verification -- PactumJS tests for cross-project 403 -- zazz-board-api skill update (if paths change) +- Remove project-wide graph API route and related schema/docs references +- Update Task Graph UI to deliverable-only selection mode +- Implement scoped task and deliverable image routes +- Add/adjust image ownership validation logic +- PactumJS coverage for new scoped routes and removed routes +- Update zazz-board-api skill/docs ### Break Patterns for Planner -- Phase 1: Audit and document routes; implement project verification for image routes -- Phase 2: Add PactumJS tests; update skill if needed +- Phase 1: Graph route/UI cleanup (`/projects/{code}/graph` removal + dropdown behavior) +- Phase 2: Image route expansion and project-scoped authorization for task + deliverable images +- Phase 3: Tests, OpenAPI updates, docs/skill updates --- ## 10. Evaluation -- **Functional**: All image routes return 403 for cross-project access; existing same-project flows unchanged +- **Functional**: + - Project-wide graph endpoint is removed + - Graph UI requires explicit deliverable selection + - Image routes are project-scoped and enforce project authorization - **Quality**: Tests pass; no new lint issues - **Completeness**: DoD checklist satisfied -- **Owner verification**: Audit list (AC7) reviewed +- **Owner verification**: Graph UX matches expectation; route scope decisions accepted --- ## 11. Technical Context -- **Integration**: Auth middleware provides `request.user` and project context from token. Image routes need to resolve task → project and compare with token's project. -- **Modified**: `api/src/routes/images.js`; possibly `api/src/middleware/authMiddleware.js` if shared helper for project verification -- **Dependencies**: None +- **Integration**: + - Task Graph UI currently calls `/projects/{code}/graph` when no deliverable is selected (via `useTaskGraph(projectCode, null)`); this behavior will be removed. `useTaskGraph` must not fetch when `deliverableId` is null—only call `/projects/{code}/deliverables/{delivId}/graph` when a deliverable is selected. + - Existing graph endpoint `/projects/{code}/deliverables/{delivId}/graph` remains the supported path. + - Image handling must remain compatible with DB storage now and object storage backends later. +- **Likely modified**: + - `api/lib/db/schema.js` + - `api/src/services/databaseService.js` + - `api/src/routes/taskGraph.js` + - `api/src/routes/images.js` + - `api/src/schemas/images.js` and OpenAPI-related tests + - `client/src/hooks/useTaskGraph.js` + - `client/src/App.jsx` and `client/src/pages/TaskGraphPage.jsx` + - `.agents/skills/zazz-board-api/SKILL.md` +- **Dependencies**: + - Schema change for `IMAGE_METADATA` only: add `deliverable_id`, make `task_id` nullable, add XOR constraint. Rebuild that table; no full DB reset. Seed data has no images. --- ## 12. Edge Cases & Constraints -- **Task not found**: Return 404 before 403 (don't leak existence of tasks in other projects) -- **Image not found**: Return 404; if we resolve image → task → project for 403, ensure we don't leak image existence across projects -- **Performance**: Resolving task → project adds one DB lookup per request; acceptable for image routes +- **Task/deliverable not found**: Return 404 +- **Cross-project resource**: Return 403 for valid resource in another project when authorization context differs +- **Image ownership checks**: Ensure image lookup path includes ownership validation before returning binary/metadata +- **Ownership model**: Single-owner invariant must be enforced in DB and respected in service layer logic +- **Legacy routes**: Removed entirely; no backward-compatible aliases. All callers must use new project-scoped routes. +- **Performance**: Additional ownership lookups per image request are acceptable for this deliverable diff --git a/AGENTS.md b/AGENTS.md index d293b6f3..1c222888 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,6 +9,28 @@ Reference for AI agents and developers. **Legacy**: If you see "Task Blaster" or - **Flow:** Work in feature worktree → push branch to GitHub → merge on GitHub → pull main locally. - **Never merge into main locally.** Main must reflect GitHub after pull. +### New worktree setup (MANDATORY) + +When creating a new feature worktree, always do all of the following: + +1. Create the worktree from `main`: + - `git worktree add -b ../ main` +2. Copy root env file from main: + - `cp ../main/.env ./.env` +3. Copy API env file from main: + - `cp ../main/api/.env ./api/.env` +4. Verify both files match main: + - `cmp -s ../main/.env ./.env` + - `cmp -s ../main/api/.env ./api/.env` + +### Env changes made in a feature worktree (MANDATORY) + +If any branch/worktree adds or changes settings in `.env` or `api/.env`: + +- The agent must explicitly ask the user whether those env changes should also be applied to the `main` worktree. +- Do not assume automatic propagation without user confirmation. +- If the user confirms, copy the updated env files into `main` and verify parity with `cmp -s`. + --- ## Standards From fa3f090c6c240e5d04dd5f6cea050ecf9393938c Mon Sep 17 00:00:00 2001 From: michaelwitz Date: Thu, 5 Mar 2026 20:36:10 -0500 Subject: [PATCH 02/14] Add ZAZZ-5 plan and update planning/API skills Co-Authored-By: Oz --- .agents/skills/planner-agent/SKILL.md | 176 +++++++------ .agents/skills/zazz-board-api/SKILL.md | 155 ++++++----- .../ZAZZ-5-fix-routes-no-project-PLAN.md | 245 ++++++++++++++++++ .zazz/deliverables/index.yaml | 1 + 4 files changed, 423 insertions(+), 154 deletions(-) create mode 100644 .zazz/deliverables/ZAZZ-5-fix-routes-no-project-PLAN.md diff --git a/.agents/skills/planner-agent/SKILL.md b/.agents/skills/planner-agent/SKILL.md index be63aca6..33a3f0dc 100644 --- a/.agents/skills/planner-agent/SKILL.md +++ b/.agents/skills/planner-agent/SKILL.md @@ -1,82 +1,110 @@ -# Planner Agent Skill - -**Role**: One-shot decomposition of SPEC into a detailed PLAN. Phases work, assigns files to tasks, identifies parallel sequences that avoid file conflicts. Does not participate in execution. - -**Agents Using This Skill**: Planner (invoked when Owner requests a plan) - -**TDD emphasis**: Every task must have explicit test requirements—what tests to create, what tests to run. Map SPEC AC and test requirements to each task. - --- - -## System Prompt - -You are the Planner Agent for the Zazz multi-agent deliverable framework. Your role is to perform a **one-shot, full decomposition** of an approved SPEC into a detailed Implementation Plan (PLAN). You do not participate in execution—the Coordinator takes over once the plan is approved. - -You must: - -1. **Decompose into manageable chunks** — Break the SPEC into phases and steps with clear task boundaries -2. **Phase the work** — Organize tasks into logical phases (e.g., setup, core feature, tests, integration) -3. **Assign files to tasks** — Use file names, conventions, and project structure to assign specific files to each task. This is critical for parallelization. -4. **Identify parallel sequences** — Find sequences where tasks can run in parallel without impacting the same files. Tasks that touch different files can proceed concurrently; tasks that share files must use DEPENDS_ON. -5. **Minimize file conflicts** — Design the plan so that when work is combined in the shared worktree, conflicts are rare. All agents work in the same worktree. -6. **Map AC and test requirements** — Each task gets derived acceptance criteria and explicit test requirements (what tests to create, what tests to run) from the SPEC -7. **Define dependencies** — Use DEPENDS_ON when a task requires output from another; use COORDINATES_WITH when tasks must complete before a dependent can start (merge points) - ---- - -## Output - -**Output**: `.zazz/deliverables/{deliverable-name}-PLAN.md` - -The PLAN document must include: -- Phases and steps -- Per-task: description, objectives, acceptance criteria, test requirements, file assignments -- Dependencies (DEPENDS_ON, COORDINATES_WITH) -- Parallelization notes: which task groups can run concurrently - +name: planner-agent +description: Creates a generic, execution-ready implementation PLAN from an approved SPEC for any deliverable. Use when an Owner asks to generate or update a phased plan with dependencies, file assignments, testing, and parallelization. --- -## Trigger - -**When invoked**: The Owner requests a plan (e.g., after SPEC approval, during the Planning phase). You run once, produce the PLAN, and exit. The Owner reviews and approves; the deliverable moves to Ready. The Coordinator then takes over to create tasks and begin execution. - -**Vendor-native planning**: When available, leverage vendor-native planning features (e.g., Warp Plan, Claude Code plan mode) to enhance decomposition—planning is a natural fit for these tools. +# Planner Agent Skill ---- +## Role +Perform a one-shot decomposition of an approved SPEC into a PLAN document. The PLAN is for execution by Coordinator/Worker/QA agents and must minimize file conflicts in a shared worktree. + +You are a planner only. Do not implement code in this step. + +## Framework Context +- Zazz is spec-driven and test-driven. +- The SPEC defines intent (`what`); the PLAN defines execution (`how work is broken down`). +- The SPEC is read-only during planning. +- The Coordinator executes and maintains the PLAN during implementation. + +## Required Inputs +Before generating a PLAN, confirm these values are known: +- Project code (e.g. `ZAZZ`) +- Deliverable code (e.g. `ZAZZ-5`) +- Deliverable numeric ID (integer, e.g. `8`) +- SPEC file path + +If any are missing, ask the Owner. + +## PLAN Naming + Location (Generic Rule) +- Store all plans in `.zazz/deliverables/`. +- Derive PLAN name from SPEC name by replacing `-SPEC.md` with `-PLAN.md`. +- Enforce hyphen-delimited filenames. +- Update `.zazz/deliverables/index.yaml` when generating/updating a PLAN: + - if deliverable entry exists, add or update its `plan` field + - if entry does not exist, add a new deliverable record with `id`, `name`, `spec`, and `plan` + +Example: +- SPEC: `.zazz/deliverables/ZAZZ-5-fix-routes-no-project-SPEC.md` +- PLAN: `.zazz/deliverables/ZAZZ-5-fix-routes-no-project-PLAN.md` + +## Output Requirements +Write one markdown PLAN file that includes: +1. Header metadata: + - Project Code + - Deliverable Code + - Deliverable ID (integer) + - SPEC Reference +2. Current-state summary from repository reality (not guesses) +3. Impacted files by subsystem +4. Phased decomposition with numbered phases (`1`, `2`, `3`, ...) +5. Numbered tasks/steps within each phase (`1.1`, `1.2`, `1.3`, ...) +6. Explicit dependencies (`DEPENDS_ON`, optional `COORDINATES_WITH`) +7. Parallelization notes driven by file overlap +8. Testing and validation tasks (unit/API/E2E/manual as applicable) + +## Planning Workflow +1. Read SPEC completely and extract AC + test requirements. +2. Inspect repository structure and identify actual files likely to change. +3. Group work into dependency-safe phases. +4. Split phases into concrete steps with file ownership. +5. Identify safe parallel streams (disjoint file sets). +6. Add validation steps (tests, lint/type checks, OpenAPI/doc checks, manual sign-off where needed). +7. Write PLAN file to `.zazz/deliverables/`. +8. Update `.zazz/deliverables/index.yaml` so the deliverable points to the generated PLAN file. ## Decomposition Rules - -1. **File-first thinking** — Before defining tasks, map SPEC requirements to files. Group tasks by file ownership to maximize parallelization. -2. **No same-file parallelism** — Tasks that modify the same file(s) must be sequential (DEPENDS_ON). Tasks that touch disjoint file sets can run in parallel. -3. **Convention awareness** — Use .zazz/standards/, .zazz/project.md, project structure, and naming conventions to infer file locations and task boundaries. -4. **Test tasks** — Plan test creation tasks (unit, API, E2E) per SPEC. These may precede or accompany feature tasks. -5. **Task sizing** — Each task should be self-contained and completable within a reasonable context window. Avoid monolithic tasks. - ---- - -## Key Responsibilities - -- [ ] Read SPEC completely -- [ ] Extract AC and test requirements -- [ ] Identify phases and natural task boundaries -- [ ] Map files to tasks using project structure -- [ ] Identify parallel sequences (disjoint file sets) -- [ ] Define DEPENDS_ON and COORDINATES_WITH -- [ ] Produce .zazz/deliverables/{deliverable-name}-PLAN.md with all task definitions - ---- - -## Best Practices - -1. **Maximize parallelism** — The more tasks that can run without file overlap, the faster the deliverable completes -2. **Explicit file assignments** — Every task should list the files it will create or modify -3. **Clear dependencies** — No circular dependencies; all relations declared upfront -4. **TDD per task** — Each task knows exactly what tests to create and run - ---- - -## Environment Variables Required - +1. **File-first decomposition**: every step lists affected files. +2. **No same-file parallelism**: if steps touch same file(s), they must be sequential via `DEPENDS_ON`. +3. **Test-first planning**: every AC must map to one or more test activities. +4. **Small, finishable steps**: avoid oversized tasks; each step has a clear completion signal. +5. **No circular dependencies**. + +## Step Format (Use for every step) +For each numbered step (`1.1`, `1.2`, ...), include: +- Objective +- Files affected +- Deliverables/output +- DEPENDS_ON +- Parallelizable with +- Test requirements +- Completion signal + +## Parallelization Guidance +- Maximize concurrency across disjoint files/subsystems. +- Call out merge points where parallel streams converge. +- Serialize around high-conflict files (route registries, schema barrels, shared configs). +- Prefer planning streams such as: + - API route stream + - data/schema stream + - client/UI stream + - tests/docs stream + +## Warp-Specific Planning Capabilities (When Available) +Use Warp capabilities to improve decomposition quality: +- Semantic code search for likely impact areas +- Exact symbol search for routes/functions +- TODO decomposition for task sequencing +- Native planning/doc tools for structured phase generation + +## Quality Bar +A PLAN is complete only if it: +- Uses correct `-PLAN.md` naming derived from SPEC +- Includes project/deliverable identifiers (including numeric deliverable ID) +- Uses phased numbering (`1`, `2`, `3`) and step numbering (`1.1`, `1.2`) +- Includes development + testing + validation work +- Explicitly documents dependencies and parallelizable groups + +## Environment Variables ```bash export ZAZZ_API_BASE_URL="http://localhost:3000" export ZAZZ_API_TOKEN="your-api-token" @@ -84,5 +112,3 @@ export AGENT_ID="planner" export ZAZZ_WORKSPACE="/path/to/project" export ZAZZ_STATE_DIR="${ZAZZ_WORKSPACE}/.zazz" ``` - -Notes - in the top of the plan we should call out the Project Code and the Deliverable code and the deliverable Id (integer) so the planner agent does not need to look up these values \ No newline at end of file diff --git a/.agents/skills/zazz-board-api/SKILL.md b/.agents/skills/zazz-board-api/SKILL.md index fe5f0aea..48930293 100644 --- a/.agents/skills/zazz-board-api/SKILL.md +++ b/.agents/skills/zazz-board-api/SKILL.md @@ -1,123 +1,120 @@ --- name: "Zazz Board API" type: "rule" -description: "Required API skill for agents to create and manage deliverables and tasks. Uses live OpenAPI spec; only agent-relevant routes are needed." +description: "Required API skill for agents to create and manage deliverables/tasks using live OpenAPI. OpenAPI is source of truth; resolve routes by capability instead of brittle hardcoded full path lists." required_for: ["planner", "coordinator", "worker", "qa", "spec-builder"] --- # Zazz Board API (Agent Routes) - -**Purpose**: Agents use this API to create deliverables, create tasks, update content, and change statuses. Projects and users are pre-configured; agents do not create them. +## Purpose +Agents use this API to create/manage deliverables and tasks, update statuses, append notes, and inspect task graph/readiness. Projects and users are pre-configured; agents do not create them. --- ## Authentication - All API requests (except `/openapi.json`, `/health`, `/`, `/db-test`, `/token-info`) require: -- **Header**: `TB_TOKEN: ` or `Authorization: Bearer ` -- **Token**: `ZAZZ_API_TOKEN` or fallback `550e8400-e29b-41d4-a716-446655440000` +- Header: `TB_TOKEN: ` or `Authorization: Bearer ` +- Token: `ZAZZ_API_TOKEN` or fallback `550e8400-e29b-41d4-a716-446655440000` --- ## Environment variables - -| Variable | Fallback | Purpose | -|----------|----------|---------| -| `ZAZZ_API_BASE_URL` | `http://localhost:3030` | API base; spec at `{base}/openapi.json` | -| `ZAZZ_API_TOKEN` | `550e8400-e29b-41d4-a716-446655440000` | Auth token | -| `ZAZZ_PROJECT_CODE` | `ZAZZ` | Default project code | +- `ZAZZ_API_BASE_URL` (fallback: `http://localhost:3030`) +- `ZAZZ_API_TOKEN` (fallback: `550e8400-e29b-41d4-a716-446655440000`) +- `ZAZZ_PROJECT_CODE` (fallback: `ZAZZ`) --- -## Source of truth: OpenAPI spec - -**Fetch the live spec for request/response schemas.** The spec is at `{ZAZZ_API_BASE_URL}/openapi.json`. No auth required for the spec. Parse as JSON; use `paths` for routes. - -**Agent routes only** — When using the spec, extract only these paths. Ignore projects, users, tags, and other non-agent routes. - -| Capability | Method | Path | -|------------|--------|------| -| Create deliverable | POST | `/projects/{projectCode}/deliverables` | -| Get deliverable | GET | `/projects/{projectCode}/deliverables/{id}` | -| Update deliverable (add spec path, plan path, git worktree, etc.) | PUT | `/projects/{projectCode}/deliverables/{id}` | -| Change deliverable status | PATCH | `/projects/{projectCode}/deliverables/{id}/status` | -| Approve deliverable (required before creating tasks) | PATCH | `/projects/{projectCode}/deliverables/{id}/approve` | -| List deliverables | GET | `/projects/{projectCode}/deliverables` | -| Create task | POST | `/projects/{code}/deliverables/{delivId}/tasks` | -| Get task | GET | `/projects/{code}/deliverables/{delivId}/tasks/{taskId}` | -| Update task | PUT | `/projects/{code}/deliverables/{delivId}/tasks/{taskId}` | -| Change task status | PATCH | `/projects/{code}/deliverables/{delivId}/tasks/{taskId}/status` | -| Append note to task | PATCH | `/projects/{code}/deliverables/{delivId}/tasks/{taskId}/notes` | -| List deliverable tasks | GET | `/projects/{projectCode}/deliverables/{id}/tasks` | -| Get deliverable graph | GET | `/projects/{code}/deliverables/{delivId}/graph` | -| Check task readiness | GET | `/projects/{code}/tasks/{taskId}/readiness` | -| Get deliverable statuses | GET | `/projects/{code}/deliverable-statuses` | -| List task images | GET | `/tasks/{taskId}/images` | -| Upload task images | POST | `/tasks/{taskId}/images/upload` | -| Get image binary | GET | `/images/{id}` | -| Get image metadata | GET | `/images/{id}/metadata` | +## Source of truth: OpenAPI +Always fetch the live spec from: +`{ZAZZ_API_BASE_URL}/openapi.json` ---- +Rules: +- Parse `paths` + operation metadata (`tags`, `summary`, `description`, params, requestBody, responses). +- Do not trust stale hardcoded route lists when OpenAPI differs. +- Do not invent routes; derive from live spec. -## Fetch spec +--- -**URL**: `{ZAZZ_API_BASE_URL}/openapi.json` (see Environment variables for base URL) +## Capability-first routing model (hybrid) +Use capability names as the stable contract, then resolve concrete routes from OpenAPI. -Filter `spec.paths` to the agent paths in the table above before reading schemas. +Core capabilities: +- Create/list/get/update/approve/status-change deliverable +- Create/list/get/update/delete/status-change task (deliverable-scoped) +- Append notes to task +- Get deliverable graph +- Check task readiness +- Get deliverable status workflow +- Image operations (list/upload/delete/fetch/metadata), preferring project-scoped contracts when available --- -## Create deliverable - -**POST** `/projects/{projectCode}/deliverables` - -- **Required body**: `name` (string, 1–30 chars), `type` (enum: `FEATURE`, `BUG_FIX`, `REFACTOR`, `ENHANCEMENT`, `CHORE`, `DOCUMENTATION`). -- **Optional body**: `description`, `dedFilePath`, `planFilePath`, `prdFilePath`, `gitWorktree`, `gitBranch`, `pullRequestUrl`. -- **Response (201)**: `id` (numeric, for API paths), `deliverableId` (e.g. `ZAZZ-4`, for display). Return `deliverableId` to the user. +## Deterministic route resolution rules +For each capability: +1. Filter operations by tags relevant to agent workflows: `deliverables`, `projects`, `task-graph`, `images`. +2. Match method + intent keywords in `summary`/`description`. +3. Prefer project/deliverable-scoped routes over global/legacy routes. +4. If multiple matches remain, choose the most specific path (more scoped params). +5. Read request/response schemas from OpenAPI before constructing requests. +6. If no match is found, stop and report missing capability + method + candidates. -**Before creating:** Ensure you have `name`, `type`, and `projectCode` (path; from user or `PROJECT_CODE` env). If any are missing, ask the human. Do not infer or invent. +Image routing policy: +- Prefer project-scoped image routes when present in OpenAPI. +- Fallback to legacy image routes only if project-scoped routes are absent. --- -## Create task +## Minimal critical assertions (guardrails) +These capabilities must resolve for normal agent workflows: +- Create deliverable +- Update deliverable +- Change deliverable status +- Approve deliverable +- Create task in deliverable +- Change task status in deliverable +- Get deliverable graph +- Check task readiness -**POST** `/projects/{code}/deliverables/{delivId}/tasks` - -- **Path params**: `code` = project code, `delivId` = numeric deliverable id from create deliverable response. -- **Required body**: `title` (string, 1–255 chars). -- **Optional body**: `description`, `status`, `priority`, `agentName`, `storyPoints`, `position`, `phase`, `phaseTaskId`, `prompt`, `dependencies`, `gitWorktree`. -- **Prerequisite**: Deliverable must be approved (PATCH `.../approve`) before creating tasks. -- **Response (201)**: `id` (numeric task id). Return `id` to the user. - -**Before creating:** Ensure you have `code`, `delivId`, and `title`. If any are missing, ask the human. Do not infer or invent. +If a critical capability cannot be resolved, stop and surface the mismatch. --- -## Missing data — do not invent - -If required fields are missing, **do not make up values**. Ask the human (or surface through the agent so the human can provide them). Example: "To create the deliverable, I need the type. Please choose: FEATURE, BUG_FIX, REFACTOR, ENHANCEMENT, CHORE, DOCUMENTATION." +## Request construction rules +- Never infer body fields from memory; derive from OpenAPI schema. +- Never invent required user inputs; ask the human for missing data. +- Use numeric IDs where path schema expects numeric IDs (`id`, `delivId`, `taskId`). +- Treat `deliverableId` (e.g., `ZAZZ-4`) as display-only unless schema says otherwise. --- -## Key conventions - -- **`id` / `delivId`**: Numeric ids for API paths. `deliverableId` (e.g. ZAZZ-4) is display-only. -- **Update deliverable**: PUT to add `dedFilePath`, `planFilePath`, `gitWorktree`, `gitBranch`, `pullRequestUrl` after creation. -- **Append note**: PATCH `.../tasks/{taskId}/notes` — body `{ "note": "...", "agentName": "..." }`. -- **Claim task**: Include `agentName` when PATCHing status to IN_PROGRESS. -- **Upload images**: POST `/tasks/{taskId}/images/upload` — body `{ images: [{ originalName, contentType, fileSize, base64Data }] }`, `contentType` must be `image/*`. +## Practical workflow +1. Fetch OpenAPI spec. +2. Resolve routes for required capabilities using deterministic rules. +3. Validate required path/query/body schema for each operation. +4. Execute request with `TB_TOKEN`. +5. On errors, report capability + path + status + API error payload. --- -## Workflow - -1. Fetch spec from `/openapi.json`. -2. Extract only the agent paths listed above from `spec.paths`. -3. For each operation, read `requestBody.content.application/json.schema` and `responses.201` (or 200) from the spec. -4. Make requests to `{ZAZZ_API_BASE_URL}{path}` with `TB_TOKEN` header and JSON body per spec. +## Capability-specific guidance +- Create deliverable: + - Required inputs: `projectCode`, `name`, `type` + - Return both numeric `id` and display `deliverableId` +- Create task: + - Required inputs: `code`, `delivId`, `title` + - Respect deliverable approval prerequisites +- Append note: + - Include `note` and optional `agentName` +- Status changes: + - Validate allowed values with workflow/status endpoints when needed +- Images: + - Prefer project-scoped routes when available + - Validate upload payload schema + content type from OpenAPI --- ## Error handling - -200 OK, 201 Created, 400 Bad Request, 401 Unauthorized, 404 Not Found, 409 Conflict, 500 Internal Server Error. Response bodies may include `error` field. +Expected statuses: `200`, `201`, `400`, `401`, `403`, `404`, `409`, `500`. +- Include API `error` payload when present. +- Do not retry with guessed alternate routes; re-resolve from OpenAPI first. diff --git a/.zazz/deliverables/ZAZZ-5-fix-routes-no-project-PLAN.md b/.zazz/deliverables/ZAZZ-5-fix-routes-no-project-PLAN.md new file mode 100644 index 00000000..4a5dc433 --- /dev/null +++ b/.zazz/deliverables/ZAZZ-5-fix-routes-no-project-PLAN.md @@ -0,0 +1,245 @@ +# Implementation Plan: Fix Routes No Project +Project Code: `ZAZZ` +Deliverable Code: `ZAZZ-5` +Deliverable ID (integer): `8` +SPEC Reference: `.zazz/deliverables/ZAZZ-5-fix-routes-no-project-SPEC.md` + +## 1. Current State and Repository Impact +The current codebase still supports project-wide graph access and legacy non-project-scoped image routes. +- `api/src/routes/taskGraph.js` still exposes `GET /projects/:code/graph`. +- `client/src/App.jsx` and `client/src/hooks/useTaskGraph.js` still allow an “All tasks (project-wide)” graph path. +- `api/src/routes/images.js`, `api/src/schemas/images.js`, and image methods in `api/src/services/databaseService.js` are task/global-id scoped, not project-code scoped. +- `api/lib/db/schema.js` models images as task-only ownership (`IMAGE_METADATA.task_id` required). +- `.agents/skills/zazz-board-api/SKILL.md` still documents legacy image routes. +- OpenAPI coverage in `api/__tests__/routes/openapi.test.mjs` does not yet assert this deliverable’s graph/image route removals/additions. + +Primary files likely affected: +- API/data model: + - `api/lib/db/schema.js` + - `api/src/services/databaseService.js` + - `api/src/routes/images.js` + - `api/src/routes/taskGraph.js` + - `api/src/schemas/images.js` + - `api/src/schemas/taskGraph.js` +- Client: + - `client/src/App.jsx` + - `client/src/hooks/useTaskGraph.js` + - `client/src/pages/TaskGraphPage.jsx` +- Tests: + - `api/__tests__/helpers/testDatabase.js` + - `api/__tests__/routes/openapi.test.mjs` + - `api/__tests__/routes/task-graph-scoping.test.mjs` (new) + - `api/__tests__/routes/images-scoping.test.mjs` (new) +- Skill/docs: + - `.agents/skills/zazz-board-api/SKILL.md` + +## 2. Dependency and Parallelization Strategy +Critical path: +1. Image ownership schema change +2. Database service ownership/scoping logic +3. Route/schema refactor to new image endpoints +4. Route-level tests + OpenAPI assertions + +Parallelization opportunities: +- Graph cleanup and client graph UX work can proceed in parallel with image ownership/service refactor because they touch mostly different files. +- OpenAPI assertion updates can proceed in parallel with route tests once route contracts stabilize. +- API skill documentation update can run in parallel with tests after final endpoint naming is confirmed. + +Conflict-prone files (serialize changes): +- `api/src/services/databaseService.js` +- `api/lib/db/schema.js` +- `api/src/routes/images.js` +- `api/src/schemas/images.js` +- `client/src/App.jsx` + +## 3. Phased Plan and Task List +### Phase 1 — Remove project-wide graph scope and enforce deliverable-first graph UX +#### Step 1.1 +Objective: Remove project-wide graph endpoint and keep only deliverable-scoped graph retrieval. +Files affected: +- `api/src/routes/taskGraph.js` +- `api/src/schemas/taskGraph.js` +Deliverables/output: +- `GET /projects/:code/graph` removed +- deliverable graph endpoint remains and is documented +DEPENDS_ON: none +Parallelizable with: Step `1.2` (different files) +Test requirements: +- Add route test for removed endpoint returning 404 +- Add route test for existing deliverable graph endpoint success path +Completion signal: +- Project-wide graph endpoint unreachable and deliverable endpoint intact + +#### Step 1.2 +Objective: Remove “All tasks (project-wide)” graph mode and require explicit deliverable selection. +Files affected: +- `client/src/App.jsx` +- `client/src/hooks/useTaskGraph.js` +- `client/src/pages/TaskGraphPage.jsx` +Deliverables/output: +- No “All tasks (project-wide)” selector option +- Graph fetch is disabled when no deliverable is selected +- Clear prompt shown when deliverable is not selected +DEPENDS_ON: Step `1.1` +Parallelizable with: Step `2.1` +Test requirements: +- Manual validation of graph page behavior +- Verify no API call to removed project-wide endpoint in this state +Completion signal: +- UI cannot trigger project-wide graph fetch path + +#### Step 1.3 +Objective: Add/adjust graph scope regression tests and OpenAPI assertions. +Files affected: +- `api/__tests__/routes/openapi.test.mjs` +- `api/__tests__/routes/task-graph-scoping.test.mjs` (new) +Deliverables/output: +- OpenAPI asserts removed project-wide graph path +- Route tests cover 404 removed path + success on deliverable-scoped path +DEPENDS_ON: Step `1.1` +Parallelizable with: Step `2.4` +Test requirements: +- Run targeted route tests and openapi tests +Completion signal: +- Graph scoping behavior is test-enforced + +### Phase 2 — Migrate image model and endpoints to project-scoped ownership-aware routes +#### Step 2.1 +Objective: Introduce single-owner image model (`task_id XOR deliverable_id`) in image metadata. +Files affected: +- `api/lib/db/schema.js` +- `api/__tests__/helpers/testDatabase.js` (if helper cleanup/setup requires updates) +Deliverables/output: +- `IMAGE_METADATA.task_id` nullable +- `IMAGE_METADATA.deliverable_id` added +- DB-level XOR constraint enforces exactly one owner +DEPENDS_ON: none +Parallelizable with: Step `1.2` +Test requirements: +- Constraint-level validation via integration tests (both-set and neither-set failure) +Completion signal: +- Schema supports task and deliverable image ownership with invariant enforced + +#### Step 2.2 +Objective: Refactor image service methods for project ownership validation and scoped URL semantics. +Files affected: +- `api/src/services/databaseService.js` +Deliverables/output: +- New/updated service methods for: + - scoped task image operations + - scoped deliverable image operations + - project-scoped image fetch/metadata +- ownership checks resolve project via task/deliverable relationship before read/delete +DEPENDS_ON: Step `2.1` +Parallelizable with: none (same high-conflict service file) +Test requirements: +- Integration tests assert 403 on cross-project resources and 404 on missing entities +Completion signal: +- Service layer can back all new project-scoped image endpoints + +#### Step 2.3 +Objective: Replace legacy image routes with project/deliverable/task-scoped routes and update schemas. +Files affected: +- `api/src/routes/images.js` +- `api/src/schemas/images.js` +- `api/src/schemas/validation.js` (if exports need adjustment) +Deliverables/output: +- Legacy routes removed: + - `GET /tasks/:taskId/images` + - `POST /tasks/:taskId/images/upload` + - `DELETE /tasks/:taskId/images/:imageId` + - `GET /images/:id` + - `GET /images/:id/metadata` +- New routes added: + - task scoped: `/projects/:code/deliverables/:delivId/tasks/:taskId/images...` + - deliverable scoped: `/projects/:code/deliverables/:delivId/images...` + - project image fetch: `/projects/:code/images/:id` and `/metadata` +DEPENDS_ON: Step `2.2` +Parallelizable with: none (shared route/schema files) +Test requirements: +- Route tests for happy path, 401, 403, 404 +Completion signal: +- API exposes only project-scoped image routes + +#### Step 2.4 +Objective: Update skill/docs references from legacy image routes to new project-scoped routes. +Files affected: +- `.agents/skills/zazz-board-api/SKILL.md` +- `docs/swagger-for-agent-enhancement.md` (if route examples must match current API) +Deliverables/output: +- Agent-facing route table aligns with new image + graph contracts +DEPENDS_ON: Steps `1.1`, `2.3` +Parallelizable with: Step `3.1` (different files) +Test requirements: +- Documentation review against generated OpenAPI +Completion signal: +- No legacy image or project-wide graph route references remain in API skill docs + +### Phase 3 — Verification, regression safety, and readiness for execution +#### Step 3.1 +Objective: Add integration tests for image route scoping and ownership model constraints. +Files affected: +- `api/__tests__/routes/images-scoping.test.mjs` (new) +- `api/__tests__/helpers/testDatabase.js` (if helper methods needed for image setup) +Deliverables/output: +- Coverage for: + - same-project success paths + - cross-project 403 paths + - 401 unauthorized + - 404 not found + - single-owner DB invariant behavior +DEPENDS_ON: Steps `2.1`, `2.3` +Parallelizable with: Step `3.2` +Test requirements: +- PactumJS integration tests with seeded projects/tokens +Completion signal: +- Image scope + ownership behavior is enforced by tests + +#### Step 3.2 +Objective: Strengthen OpenAPI tests for route removals/additions and schema correctness. +Files affected: +- `api/__tests__/routes/openapi.test.mjs` +Deliverables/output: +- Assertions verify: + - removed `GET /projects/{code}/graph` + - removed legacy image paths + - added project-scoped image paths + - expected summaries/request schemas for new routes +DEPENDS_ON: Steps `1.1`, `2.3` +Parallelizable with: Step `3.1` +Test requirements: +- Run OpenAPI schema validator test suite +Completion signal: +- API contract changes are protected by documentation tests + +#### Step 3.3 +Objective: Run full validation commands and confirm release readiness for this deliverable. +Files affected: +- No code files; execution/verification commands +Deliverables/output: +- Green test and quality signals for API and client behavior +DEPENDS_ON: Steps `1.3`, `2.4`, `3.1`, `3.2` +Parallelizable with: none (final convergence) +Test requirements: +- API tests: + - `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test` +- OpenAPI tests included in API test run +- Client lint: + - `cd client && npm run lint` +- Manual graph UI smoke check: + - no project-wide option + - deliverable selection required before graph fetch +Completion signal: +- All required automated checks pass and manual graph behavior matches SPEC + +## 4. AC-to-Phase Coverage Mapping +- AC1/AC2 covered by Phase 1 and Phase 3. +- AC3/AC4/AC5/AC6/AC7 covered by Phase 2 and Phase 3. +- AC8 regression covered in Phase 3 integration tests. +- AC9 covered by Step `2.1` and Step `3.1`. +- AC10 covered by Step `3.2`. + +## 5. Execution Notes for Coordinator +- Create tasks using `phase.step` IDs aligned to this PLAN (`1.1`, `1.2`, ...). +- Run Phase 1 and Phase 2 streams with parallelization noted above, but serialize high-conflict files. +- Preserve SPEC as read-only during execution unless Owner-approved change mechanism is invoked. diff --git a/.zazz/deliverables/index.yaml b/.zazz/deliverables/index.yaml index 12a5c31d..99f7047f 100644 --- a/.zazz/deliverables/index.yaml +++ b/.zazz/deliverables/index.yaml @@ -9,6 +9,7 @@ deliverables: - id: ZAZZ-5 name: fix-routes-no-project spec: ZAZZ-5-fix-routes-no-project-SPEC.md + plan: ZAZZ-5-fix-routes-no-project-PLAN.md - id: ZAZZ-6 name: multiple-agent-tokens-feature spec: ZAZZ-6-multiple-agent-tokens-feature-SPEC.md From 13fc50e74cee3a29f9e2f38cb5fede27d5b0ed9e Mon Sep 17 00:00:00 2001 From: michaelwitz Date: Fri, 6 Mar 2026 20:13:48 -0500 Subject: [PATCH 03/14] feat(api): scope graph and image routes to deliverables and projects --- .agents/skills/planner-agent/SKILL.md | 56 ++- .agents/skills/zazz-board-api/SKILL.md | 13 +- ...ZAZZ-5-fix-routes-no-project-CODEX-PLAN.md | 476 ++++++++++++++++++ api/__tests__/helpers/testDatabase.js | 7 +- .../helpers/testServerWithSwagger.js | 11 +- api/__tests__/routes/image-scoping.test.mjs | 204 ++++++++ api/__tests__/routes/openapi.test.mjs | 21 +- .../project-id-routes-regression.test.mjs | 61 +++ .../routes/task-graph-scoping.test.mjs | 56 +++ api/lib/db/schema.js | 17 +- api/src/routes/images.js | 472 +++++++++++------ api/src/routes/index.js | 2 +- api/src/routes/statusDefinitions.js | 2 +- api/src/routes/taskGraph.js | 26 - api/src/schemas/images.js | 266 ++++++---- api/src/schemas/taskGraph.js | 22 - api/src/server.js | 3 +- api/src/services/databaseService.js | 88 +++- client/src/App.jsx | 12 +- client/src/hooks/useTaskGraph.js | 24 +- client/src/pages/TaskGraphPage.jsx | 11 + docs/swagger-for-agent-enhancement.md | 8 +- 22 files changed, 1518 insertions(+), 340 deletions(-) create mode 100644 .zazz/deliverables/ZAZZ-5-fix-routes-no-project-CODEX-PLAN.md create mode 100644 api/__tests__/routes/image-scoping.test.mjs create mode 100644 api/__tests__/routes/project-id-routes-regression.test.mjs create mode 100644 api/__tests__/routes/task-graph-scoping.test.mjs diff --git a/.agents/skills/planner-agent/SKILL.md b/.agents/skills/planner-agent/SKILL.md index 33a3f0dc..60b8fae6 100644 --- a/.agents/skills/planner-agent/SKILL.md +++ b/.agents/skills/planner-agent/SKILL.md @@ -6,7 +6,7 @@ description: Creates a generic, execution-ready implementation PLAN from an appr # Planner Agent Skill ## Role -Perform a one-shot decomposition of an approved SPEC into a PLAN document. The PLAN is for execution by Coordinator/Worker/QA agents and must minimize file conflicts in a shared worktree. +Perform a one-shot decomposition of an approved SPEC into an execution-ready PLAN document. The PLAN is for Coordinator/Worker/QA execution in a shared worktree and must minimize file conflicts. You are a planner only. Do not implement code in this step. @@ -16,6 +16,10 @@ You are a planner only. Do not implement code in this step. - The SPEC is read-only during planning. - The Coordinator executes and maintains the PLAN during implementation. +## Companion Skill Requirement +- Load and follow `.agents/skills/zazz-board-api/SKILL.md` when planning API work. +- Treat live OpenAPI as route truth when available; do not rely on stale hardcoded route assumptions. + ## Required Inputs Before generating a PLAN, confirm these values are known: - Project code (e.g. `ZAZZ`) @@ -29,9 +33,10 @@ If any are missing, ask the Owner. - Store all plans in `.zazz/deliverables/`. - Derive PLAN name from SPEC name by replacing `-SPEC.md` with `-PLAN.md`. - Enforce hyphen-delimited filenames. -- Update `.zazz/deliverables/index.yaml` when generating/updating a PLAN: +- Update `.zazz/deliverables/index.yaml` when generating/updating the canonical PLAN: - if deliverable entry exists, add or update its `plan` field - if entry does not exist, add a new deliverable record with `id`, `name`, `spec`, and `plan` +- If the Owner explicitly asks for an alternate planning draft (for example `-CODEX-PLAN.md`), create it without replacing canonical `-PLAN.md` unless asked. Example: - SPEC: `.zazz/deliverables/ZAZZ-5-fix-routes-no-project-SPEC.md` @@ -44,30 +49,37 @@ Write one markdown PLAN file that includes: - Deliverable Code - Deliverable ID (integer) - SPEC Reference -2. Current-state summary from repository reality (not guesses) -3. Impacted files by subsystem -4. Phased decomposition with numbered phases (`1`, `2`, `3`, ...) -5. Numbered tasks/steps within each phase (`1.1`, `1.2`, `1.3`, ...) -6. Explicit dependencies (`DEPENDS_ON`, optional `COORDINATES_WITH`) -7. Parallelization notes driven by file overlap -8. Testing and validation tasks (unit/API/E2E/manual as applicable) +2. Scope guardrails (in-scope, out-of-scope, explicit non-goals from SPEC) +3. Current-state summary from repository reality (not guesses), with evidence references +4. Impacted files by subsystem (API, DB, client, tests, docs/skills) +5. API/contract delta table for changed and removed endpoints (if applicable) +6. AC traceability matrix (`AC -> implementation steps -> tests`) +7. Phased decomposition with numbered phases (`1`, `2`, `3`, ...) +8. Numbered tasks/steps within each phase (`1.1`, `1.2`, `1.3`, ...) +9. Explicit dependencies (`DEPENDS_ON`, optional `COORDINATES_WITH`) +10. Parallelization notes driven by file overlap +11. Testing and validation tasks (unit/API/E2E/manual as applicable) +12. Final verification command set and owner-signoff checkpoints where applicable ## Planning Workflow 1. Read SPEC completely and extract AC + test requirements. -2. Inspect repository structure and identify actual files likely to change. -3. Group work into dependency-safe phases. -4. Split phases into concrete steps with file ownership. -5. Identify safe parallel streams (disjoint file sets). -6. Add validation steps (tests, lint/type checks, OpenAPI/doc checks, manual sign-off where needed). -7. Write PLAN file to `.zazz/deliverables/`. -8. Update `.zazz/deliverables/index.yaml` so the deliverable points to the generated PLAN file. +2. Read relevant standards (`testing.md`, `coding-styles.md`, architecture/data docs). +3. Inspect repository structure and identify actual files likely to change. +4. For API work, resolve target routes/capabilities from OpenAPI (or document if unavailable). +5. Group work into dependency-safe phases and explicit parallel streams. +6. Split phases into concrete steps with file ownership. +7. Add validation steps (tests, lint/type checks, OpenAPI/doc checks, manual sign-off where needed). +8. Write PLAN file to `.zazz/deliverables/`. +9. Update `.zazz/deliverables/index.yaml` only when canonical plan target changes. ## Decomposition Rules 1. **File-first decomposition**: every step lists affected files. 2. **No same-file parallelism**: if steps touch same file(s), they must be sequential via `DEPENDS_ON`. 3. **Test-first planning**: every AC must map to one or more test activities. -4. **Small, finishable steps**: avoid oversized tasks; each step has a clear completion signal. -5. **No circular dependencies**. +4. **TDD gates per step**: include both tests-to-write-first and tests-to-run-to-complete. +5. **Small, finishable steps**: avoid oversized tasks; each step has a clear completion signal. +6. **No circular dependencies**. +7. **Reality over assumptions**: do not cite files/tests that do not exist without marking them as new files. ## Step Format (Use for every step) For each numbered step (`1.1`, `1.2`, ...), include: @@ -75,8 +87,11 @@ For each numbered step (`1.1`, `1.2`, ...), include: - Files affected - Deliverables/output - DEPENDS_ON +- COORDINATES_WITH (optional) - Parallelizable with -- Test requirements +- TDD: tests to write first +- TDD: tests to run for completion +- Acceptance criteria mapped - Completion signal ## Parallelization Guidance @@ -102,7 +117,10 @@ A PLAN is complete only if it: - Includes project/deliverable identifiers (including numeric deliverable ID) - Uses phased numbering (`1`, `2`, `3`) and step numbering (`1.1`, `1.2`) - Includes development + testing + validation work +- Includes AC traceability and test traceability - Explicitly documents dependencies and parallelizable groups +- Includes concrete commands for required verification runs +- Avoids speculative routes/files and aligns to repository reality ## Environment Variables ```bash diff --git a/.agents/skills/zazz-board-api/SKILL.md b/.agents/skills/zazz-board-api/SKILL.md index 48930293..bc6713f3 100644 --- a/.agents/skills/zazz-board-api/SKILL.md +++ b/.agents/skills/zazz-board-api/SKILL.md @@ -44,9 +44,10 @@ Core capabilities: - Create/list/get/update/delete/status-change task (deliverable-scoped) - Append notes to task - Get deliverable graph +- Create task relations (`DEPENDS_ON`, `COORDINATES_WITH`) - Check task readiness - Get deliverable status workflow -- Image operations (list/upload/delete/fetch/metadata), preferring project-scoped contracts when available +- Image operations (list/upload/delete/fetch/metadata) using project-scoped routes --- @@ -59,9 +60,10 @@ For each capability: 5. Read request/response schemas from OpenAPI before constructing requests. 6. If no match is found, stop and report missing capability + method + candidates. -Image routing policy: -- Prefer project-scoped image routes when present in OpenAPI. -- Fallback to legacy image routes only if project-scoped routes are absent. +Image/graph routing policy: +- Use deliverable graph route (`/projects/{code}/deliverables/{delivId}/graph`). +- Do not use project-wide graph route (`/projects/{code}/graph`) if absent in OpenAPI. +- Use only project-scoped image routes; do not fallback to legacy global/task-only image routes. --- @@ -104,12 +106,13 @@ If a critical capability cannot be resolved, stop and surface the mismatch. - Create task: - Required inputs: `code`, `delivId`, `title` - Respect deliverable approval prerequisites + - If dependencies are required, create relation edges explicitly via task-relation endpoint after task creation - Append note: - Include `note` and optional `agentName` - Status changes: - Validate allowed values with workflow/status endpoints when needed - Images: - - Prefer project-scoped routes when available + - Use project-scoped routes only - Validate upload payload schema + content type from OpenAPI --- diff --git a/.zazz/deliverables/ZAZZ-5-fix-routes-no-project-CODEX-PLAN.md b/.zazz/deliverables/ZAZZ-5-fix-routes-no-project-CODEX-PLAN.md new file mode 100644 index 00000000..0420157e --- /dev/null +++ b/.zazz/deliverables/ZAZZ-5-fix-routes-no-project-CODEX-PLAN.md @@ -0,0 +1,476 @@ +# CODEX Implementation Plan: Fix Routes No Project +Project Code: `ZAZZ` +Deliverable Code: `ZAZZ-5` +Deliverable ID (integer): `8` +SPEC Reference: `.zazz/deliverables/ZAZZ-5-fix-routes-no-project-SPEC.md` +Status: `DRAFT_FOR_OWNER_APPROVAL` +Planning basis: repository audit + standards (`testing.md`, `coding-styles.md`, `system-architecture.md`, `data-architecture.md`) + API skill constraints from `.agents/skills/zazz-board-api/SKILL.md` + +## 1. Scope Guardrails +In scope: +- Remove project-wide graph route and project-wide graph UI mode. +- Replace all legacy image routes with project-code-scoped routes supporting task and deliverable images. +- Introduce single-owner image model (`task_id XOR deliverable_id`) in `IMAGE_METADATA`. +- Add TDD-first API/OpenAPI regression coverage for all route contract changes. +- Update API skill/docs to reflect the final route contract. + +Out of scope (must not be pulled into this plan): +- SPEC changes. +- Project membership/access-control redesign (`USER_PROJECTS` style token-project mapping). +- Global normalization of every `:id` vs `:code` route. +- Building card-image UI components. + +## 2. Verified Current State (Repository Reality) +- `api/src/routes/taskGraph.js` currently exposes `GET /projects/:code/graph` and `GET /projects/:code/deliverables/:delivId/graph`. +- `client/src/App.jsx` currently renders `All tasks (project-wide)` in Task Graph center selector and passes `null` deliverable. +- `client/src/hooks/useTaskGraph.js` currently falls back to `/projects/{code}/graph` when `deliverableId` is null. +- `api/src/routes/images.js` currently exposes only legacy non-project-scoped routes: + - `/tasks/:taskId/images` + - `/tasks/:taskId/images/upload` + - `/tasks/:taskId/images/:imageId` + - `/images/:id` + - `/images/:id/metadata` +- `api/lib/db/schema.js` currently enforces task-only image ownership (`IMAGE_METADATA.task_id` is `NOT NULL`; no `deliverable_id`). +- `api/src/services/databaseService.js` currently has task/global image methods only (`getTaskImages`, `storeTaskImage`, `getImageWithData`, `getImageMetadata`, `deleteImage`) and URL generation tied to `/images/{id}`. +- Route-test reality: there are no dedicated image route tests in `api/__tests__/routes/`; graph behavior is covered indirectly in `agent-workflow.test.mjs`. +- `api/__tests__/helpers/testDatabase.js` does not clear image tables; image tests would currently leak state across tests. +- `api/__tests__/routes/openapi.test.mjs` does not assert removal of `/projects/{code}/graph` or removal/addition of legacy/new image paths. +- `.agents/skills/zazz-board-api/SKILL.md` still allows legacy image fallback if project-scoped routes are absent. + +## 3. Contract Delta (Target API) +| Capability | Current | Target | +| --- | --- | --- | +| Project-wide graph | `GET /projects/{code}/graph` | Removed (404) | +| Deliverable graph | `GET /projects/{code}/deliverables/{delivId}/graph` | Unchanged | +| Task image list/upload/delete | `/tasks/{taskId}/images...` | `/projects/{code}/deliverables/{delivId}/tasks/{taskId}/images...` | +| Deliverable image list/upload/delete | Not available | `/projects/{code}/deliverables/{delivId}/images...` | +| Image binary + metadata | `/images/{id}`, `/images/{id}/metadata` | `/projects/{code}/images/{id}`, `/projects/{code}/images/{id}/metadata` | +| Legacy image routes | Active | Removed (404) | + +Behavior requirements: +- Cross-project resource access for image routes returns `403`. +- Missing project/resource returns `404`. +- Missing/invalid token returns `401`. + +## 4. Parallelization Strategy (Shared Worktree Safe) +Parallel streams: +- Stream A: Graph API + Graph UI (`taskGraph.js`, client graph files, graph tests). +- Stream B: Image schema + service + routes (`schema.js`, `databaseService.js`, `images.js`, `schemas/images.js`). +- Stream C: Test harness + regression tests + OpenAPI assertions (new route test files + `openapi.test.mjs`). +- Stream D: Skill/docs updates (`.agents/skills/zazz-board-api/SKILL.md`, optional docs note). + +Serialization hotspots: +- `api/lib/db/schema.js` +- `api/src/services/databaseService.js` +- `api/src/routes/images.js` +- `api/src/schemas/images.js` +- `api/__tests__/routes/openapi.test.mjs` + +Merge points: +- Stream C depends on final contracts from Stream A + Stream B. +- Stream D should run after Stream A/B contracts stabilize and OpenAPI assertions are green. + +## 5. AC Traceability Matrix +| AC | Implementation steps | Tests/evidence | +| --- | --- | --- | +| AC1 | 1.2, 3.4 | `task-graph-scoping.test.mjs` + `openapi.test.mjs` path absence assertion | +| AC2 | 1.3, 3.5 | Manual owner sign-off checklist + client behavior verification (no fetch on null selection) | +| AC3 | 2.3, 3.1 | `image-scoping.test.mjs` task route happy path | +| AC4 | 2.3, 3.1 | `image-scoping.test.mjs` deliverable route happy path | +| AC5 | 2.2, 2.3, 3.1 | `image-scoping.test.mjs` binary/metadata cross-project 403 assertions | +| AC6 | 2.2, 2.3, 3.1 | `image-scoping.test.mjs` mutation route 403 assertions | +| AC7 | 2.3, 3.1, 3.4 | Legacy route 404 tests + grep/audit + skill/doc updates | +| AC8 | 3.2 | `project-id-routes-regression.test.mjs` | +| AC9 | 2.1, 3.1 | DB constraint insert-fail tests (both-set and neither-set) | +| AC10 | 2.3, 3.4 | OpenAPI path add/remove + schema assertions | + +## 6. Phased Execution Plan +### Phase 1 - Graph De-Scope + Test Harness Foundation +#### Step 1.1 +Objective: Prepare test harness for image-route work and deterministic cleanup. + +Files affected: +- `api/__tests__/helpers/testDatabase.js` +- `api/lib/db/schema.js` (imports used by helper only, if needed) + +Deliverables/output: +- `clearTaskData()` (or new helper) also clears `IMAGE_DATA` and `IMAGE_METADATA` to prevent image-state bleed. +- Add helper utilities for image test setup (task image + deliverable image seed helpers) if needed. + +DEPENDS_ON: none +COORDINATES_WITH: none +Parallelizable with: Step `1.2` + +TDD: tests to write first: +- Add a basic image fixture cleanup assertion in new `image-scoping.test.mjs` setup section (failing until helper is updated). + +TDD: tests to run for completion: +- `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test -- image-scoping.test.mjs` + +Acceptance criteria mapped: +- AC3, AC4, AC5, AC6, AC9 (test infrastructure prerequisite) + +Completion signal: +- Image test suite can create/delete fixture images across tests without cross-test contamination. + +#### Step 1.2 +Objective: Remove project-wide graph endpoint and enforce API contract at test level. + +Files affected: +- `api/src/routes/taskGraph.js` +- `api/src/schemas/taskGraph.js` +- `api/__tests__/routes/task-graph-scoping.test.mjs` (new) + +Deliverables/output: +- Remove `GET /projects/:code/graph` route and schema export usage. +- Add focused tests: + - removed route returns `404` + - `GET /projects/:code/deliverables/:delivId/graph` remains `200` + - invalid deliverable/project combinations return expected `404` + +DEPENDS_ON: none +COORDINATES_WITH: Step `1.3` +Parallelizable with: Step `1.1` + +TDD: tests to write first: +- Create `task-graph-scoping.test.mjs` with failing assertions for removed route and preserved deliverable route. + +TDD: tests to run for completion: +- `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test -- task-graph-scoping.test.mjs` + +Acceptance criteria mapped: +- AC1 + +Completion signal: +- Removed graph route is unreachable, and deliverable route remains stable under tests. + +#### Step 1.3 +Objective: Make Task Graph UI strictly deliverable-driven (no project-wide fallback call path). + +Files affected: +- `client/src/App.jsx` +- `client/src/hooks/useTaskGraph.js` +- `client/src/pages/TaskGraphPage.jsx` (prompt behavior if no deliverable selected) + +Deliverables/output: +- Remove `All tasks (project-wide)` option from selector. +- Selector contains only deliverables. +- `useTaskGraph` does not fetch when `deliverableId` is `null`. +- Task Graph page shows explicit prompt when no deliverable is selected. + +DEPENDS_ON: Step `1.2` +COORDINATES_WITH: none +Parallelizable with: Step `2.1` + +TDD: tests to write first: +- If client test harness is available, add hook/component tests for null-selection no-fetch behavior. +- If no client test harness exists, add a manual verification checklist artifact in this step. + +TDD: tests to run for completion: +- `cd client && npm run lint` +- Manual check in running UI: + - no project-wide selector option + - no graph API call before deliverable selection + - clear guidance text shown + +Acceptance criteria mapped: +- AC2 + +Completion signal: +- UI cannot trigger removed project-wide graph API path. + +### Phase 2 - Image Ownership Model + Scoped Route Migration +#### Step 2.1 +Objective: Implement `IMAGE_METADATA` single-owner model (`task_id XOR deliverable_id`) with DB enforcement. + +Files affected: +- `api/lib/db/schema.js` + +Deliverables/output: +- Add nullable `deliverable_id` FK to `DELIVERABLES.id`. +- Make `task_id` nullable. +- Add DB check constraint enforcing exactly one owner (`task_id IS NOT NULL` xor `deliverable_id IS NOT NULL`). +- Keep `IMAGE_DATA` unchanged. + +DEPENDS_ON: Step `1.1` +COORDINATES_WITH: Step `2.2` +Parallelizable with: Step `1.3` + +TDD: tests to write first: +- In `image-scoping.test.mjs`, add failing direct DB write cases for: + - both owner columns set + - neither owner column set + +TDD: tests to run for completion: +- `cd api && npm run db:push` +- `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test -- image-scoping.test.mjs` + +Acceptance criteria mapped: +- AC9 + +Completion signal: +- Constraint violations are test-proven; valid task/deliverable owner rows insert successfully. + +#### Step 2.2 +Objective: Refactor image service layer to explicit project-scoped resolution and ownership checks. + +Files affected: +- `api/src/services/databaseService.js` + +Deliverables/output: +- Replace legacy generic methods with scoped methods for: + - task image list/upload/delete under project+deliverable+task context + - deliverable image list/upload/delete under project+deliverable context + - project-scoped image binary + metadata fetch +- Add project ownership resolution for each operation. +- Standardize error semantics for service consumers (`not found` vs `forbidden` conditions). + +DEPENDS_ON: Step `2.1` +COORDINATES_WITH: Step `2.3` +Parallelizable with: none (high-conflict file) + +TDD: tests to write first: +- Add failing assertions in `image-scoping.test.mjs` for cross-project `403` and missing-resource `404`. + +TDD: tests to run for completion: +- `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test -- image-scoping.test.mjs` + +Acceptance criteria mapped: +- AC5, AC6, AC9 + +Completion signal: +- Service APIs provide all scoped operations needed by routes with correct auth/error behavior. + +#### Step 2.3 +Objective: Replace legacy image routes/schemas with project-code-scoped contracts and deliverable image support. + +Files affected: +- `api/src/routes/images.js` +- `api/src/schemas/images.js` +- `api/src/schemas/validation.js` (if export surface changes) + +Deliverables/output: +- Remove: + - `GET /tasks/:taskId/images` + - `POST /tasks/:taskId/images/upload` + - `DELETE /tasks/:taskId/images/:imageId` + - `GET /images/:id` + - `GET /images/:id/metadata` +- Add: + - `GET /projects/:code/deliverables/:delivId/tasks/:taskId/images` + - `POST /projects/:code/deliverables/:delivId/tasks/:taskId/images/upload` + - `DELETE /projects/:code/deliverables/:delivId/tasks/:taskId/images/:imageId` + - `GET /projects/:code/deliverables/:delivId/images` + - `POST /projects/:code/deliverables/:delivId/images/upload` + - `DELETE /projects/:code/deliverables/:delivId/images/:imageId` + - `GET /projects/:code/images/:id` + - `GET /projects/:code/images/:id/metadata` +- Route handlers enforce project-resource ownership (`403` on cross-project). + +DEPENDS_ON: Step `2.2` +COORDINATES_WITH: none +Parallelizable with: none (shared route/schema files) + +TDD: tests to write first: +- Build full failing endpoint matrix in `image-scoping.test.mjs` for `200/201`, `401`, `403`, `404`, and legacy-route `404`. + +TDD: tests to run for completion: +- `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test -- image-scoping.test.mjs` + +Acceptance criteria mapped: +- AC3, AC4, AC5, AC6, AC7 + +Completion signal: +- Only scoped image routes remain exposed; legacy routes are absent. + +#### Step 2.4 +Objective: Align core API metadata text with new image contract language. + +Files affected: +- `api/src/routes/index.js` +- `api/src/server.js` (tag/description text only, if needed) + +Deliverables/output: +- Root endpoint docs/list no longer imply legacy `/images` global usage. +- Tag descriptions reflect scoped task/deliverable image operations. + +DEPENDS_ON: Step `2.3` +COORDINATES_WITH: Step `3.4` +Parallelizable with: Step `3.2` + +TDD: tests to write first: +- Add/adjust OpenAPI assertions first (Step `3.4`) for updated descriptions when practical. + +TDD: tests to run for completion: +- `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test -- openapi.test.mjs` + +Acceptance criteria mapped: +- AC10 + +Completion signal: +- Generated OpenAPI/service metadata no longer references legacy image behavior. + +### Phase 3 - Regression Net, OpenAPI Contract, and Skill/Docs +#### Step 3.1 +Objective: Add comprehensive image scoping integration tests. + +Files affected: +- `api/__tests__/routes/image-scoping.test.mjs` (new) +- `api/__tests__/helpers/testDatabase.js` (if additional helpers are needed) + +Deliverables/output: +- Pactum tests cover: + - task image routes (`200/201`, `401`, `403`, `404`) + - deliverable image routes (`200/201`, `401`, `403`, `404`) + - project-scoped image fetch + metadata (`200`, `403`, `404`) + - legacy route removal (`404`) + - DB ownership constraint behavior + +DEPENDS_ON: Steps `2.1`, `2.3` +COORDINATES_WITH: Step `3.4` +Parallelizable with: Step `3.2` + +TDD: tests to write first: +- This step is test-authoring itself; create exhaustive failing suite before final route/service edits close. + +TDD: tests to run for completion: +- `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test -- image-scoping.test.mjs` + +Acceptance criteria mapped: +- AC3, AC4, AC5, AC6, AC7, AC9 + +Completion signal: +- Image route behavior and ownership invariants are fully test-enforced. + +#### Step 3.2 +Objective: Lock regression behavior for accepted unchanged project-id routes. + +Files affected: +- `api/__tests__/routes/project-id-routes-regression.test.mjs` (new) + +Deliverables/output: +- Regression tests for: + - `GET /projects/:id` + - `GET /projects/:id/kanban/tasks/column/:status` + - `GET /projects/:id/tasks` + +DEPENDS_ON: none +COORDINATES_WITH: none +Parallelizable with: Step `3.1` + +TDD: tests to write first: +- Add failing assertions (if route behavior drifted during refactor). + +TDD: tests to run for completion: +- `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test -- project-id-routes-regression.test.mjs` + +Acceptance criteria mapped: +- AC8 + +Completion signal: +- These three routes are protected against accidental refactor regression. + +#### Step 3.3 +Objective: Finalize graph removal regression tests. + +Files affected: +- `api/__tests__/routes/task-graph-scoping.test.mjs` + +Deliverables/output: +- Confirm removed project graph route remains absent while deliverable graph path remains healthy. + +DEPENDS_ON: Step `1.2` +COORDINATES_WITH: Step `3.4` +Parallelizable with: Step `3.2` + +TDD: tests to write first: +- Done in Step `1.2`; extend edge-case matrix here if needed. + +TDD: tests to run for completion: +- `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test -- task-graph-scoping.test.mjs` + +Acceptance criteria mapped: +- AC1 + +Completion signal: +- Graph contract regression is permanently enforced. + +#### Step 3.4 +Objective: Harden OpenAPI tests for all route removals/additions and schema shape. + +Files affected: +- `api/__tests__/routes/openapi.test.mjs` + +Deliverables/output: +- Add assertions that OpenAPI: + - omits `/projects/{code}/graph` + - omits all legacy image paths + - includes all new scoped task/deliverable/project image paths + - includes expected params/body/response schema references for new routes + +DEPENDS_ON: Steps `1.2`, `2.3` +COORDINATES_WITH: Step `2.4` +Parallelizable with: Step `3.3` + +TDD: tests to write first: +- Add failing path presence/absence assertions before route/schema edits finalize. + +TDD: tests to run for completion: +- `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test -- openapi.test.mjs` + +Acceptance criteria mapped: +- AC1, AC7, AC10 + +Completion signal: +- OpenAPI contract and docs are test-locked to final intended route set. + +#### Step 3.5 +Objective: Update API skill/docs and perform final verification gate. + +Files affected: +- `.agents/skills/zazz-board-api/SKILL.md` +- `docs/swagger-for-agent-enhancement.md` (if examples reference removed routes) + +Deliverables/output: +- Remove/replace legacy image route guidance and project-wide graph references. +- Route resolution guidance prefers only scoped image routes for this repo contract. +- Final verification record with command outputs and manual UI sign-off notes. + +DEPENDS_ON: Steps `1.3`, `2.4`, `3.1`, `3.2`, `3.3`, `3.4` +COORDINATES_WITH: none +Parallelizable with: none + +TDD: tests to write first: +- N/A (docs step), but verify against generated OpenAPI. + +TDD: tests to run for completion: +- `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test` +- `cd client && npm run lint` +- Manual owner sign-off checklist for AC2 + +Acceptance criteria mapped: +- AC2, AC7, AC10 + +Completion signal: +- Automated checks pass, docs/skills match implementation, and owner confirms graph UX behavior. + +## 7. Test Command Matrix (Execution Order) +1. `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test -- task-graph-scoping.test.mjs` +2. `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test -- image-scoping.test.mjs` +3. `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test -- project-id-routes-regression.test.mjs` +4. `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test -- openapi.test.mjs` +5. `cd api && set -a && source .env && set +a && NODE_ENV=test npm run test` +6. `cd client && npm run lint` + +## 8. Risks and Mitigations +- Risk: Existing tests do not currently cover image routes. + - Mitigation: Add dedicated `image-scoping.test.mjs` before finalizing route changes. +- Risk: No token-to-project mapping exists today, while SPEC asks for cross-project denial. + - Mitigation: Enforce project ownership via route path/resource relationship and assert `403` for cross-project resource access attempts. +- Risk: Schema changes can drift from OpenAPI docs. + - Mitigation: add explicit OpenAPI path-add/path-remove assertions and keep schema-first edits in `api/src/schemas/images.js`. + +## 9. Approval Checklist +- [ ] Owner accepts the AC traceability matrix and phase structure. +- [ ] Owner accepts assumption that cross-project checks are route/resource-ownership based (not membership system redesign). +- [ ] Owner approves proceeding with this CODEX plan as implementation source. diff --git a/api/__tests__/helpers/testDatabase.js b/api/__tests__/helpers/testDatabase.js index db8327b3..a7615128 100644 --- a/api/__tests__/helpers/testDatabase.js +++ b/api/__tests__/helpers/testDatabase.js @@ -1,5 +1,5 @@ import { db } from '../../lib/db/index.js'; -import { USERS, PROJECTS, DELIVERABLES, TASKS, TAGS, TASK_TAGS, TASK_RELATIONS } from '../../lib/db/schema.js'; +import { USERS, PROJECTS, DELIVERABLES, TASKS, TAGS, TASK_TAGS, TASK_RELATIONS, IMAGE_METADATA, IMAGE_DATA } from '../../lib/db/schema.js'; import { eq, and, sql } from 'drizzle-orm'; /** @@ -58,7 +58,10 @@ export async function validateTestEnvironment() { */ export async function clearTaskData() { await validateTestEnvironment(); - + + // Explicitly clear image tables so image route tests remain isolated. + await db.delete(IMAGE_DATA); + await db.delete(IMAGE_METADATA); await db.delete(TASK_RELATIONS); await db.delete(TASK_TAGS); await db.delete(TASKS); diff --git a/api/__tests__/helpers/testServerWithSwagger.js b/api/__tests__/helpers/testServerWithSwagger.js index d410a51e..944d2fda 100644 --- a/api/__tests__/helpers/testServerWithSwagger.js +++ b/api/__tests__/helpers/testServerWithSwagger.js @@ -24,7 +24,14 @@ export async function createTestServerWithSwagger() { openapi: '3.1.0', info: { title: 'Zazz Board API', - description: 'Test', + description: `Kanban-style orchestration API for coordinating AI agents and humans. + +**Common operations (agent quick reference)**: +- Create deliverable +- Create task +- Update deliverable +- Change deliverable status +- Change task status`, version: '1.0.0' }, servers: [{ url: BASE_URL, description: 'Local' }], @@ -42,5 +49,7 @@ export async function createTestServerWithSwagger() { await tokenService.initialize(); + await app.ready(); + return app; } diff --git a/api/__tests__/routes/image-scoping.test.mjs b/api/__tests__/routes/image-scoping.test.mjs new file mode 100644 index 00000000..34754bc1 --- /dev/null +++ b/api/__tests__/routes/image-scoping.test.mjs @@ -0,0 +1,204 @@ +import * as pactum from 'pactum'; +import { db } from '../../lib/db/index.js'; +import { IMAGE_METADATA } from '../../lib/db/schema.js'; +import { clearTaskData, createTestDeliverable, createTestTask, resetProjectDefaults } from '../helpers/testDatabase.js'; + +const { spec } = pactum; +const VALID_TOKEN = '550e8400-e29b-41d4-a716-446655440000'; + +const SAMPLE_IMAGE = { + originalName: 'tiny.png', + contentType: 'image/png', + fileSize: 68, + base64Data: 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/w8AAgMBgU6m4QAAAABJRU5ErkJggg==' +}; + +async function uploadTaskImage(projectCode, deliverableId, taskId) { + const response = await spec() + .post(`/projects/${projectCode}/deliverables/${deliverableId}/tasks/${taskId}/images/upload`) + .withHeaders('TB_TOKEN', VALID_TOKEN) + .withJson({ images: [SAMPLE_IMAGE] }) + .expectStatus(201) + .returns('res.body'); + + return response.images[0]; +} + +async function uploadDeliverableImage(projectCode, deliverableId) { + const response = await spec() + .post(`/projects/${projectCode}/deliverables/${deliverableId}/images/upload`) + .withHeaders('TB_TOKEN', VALID_TOKEN) + .withJson({ images: [SAMPLE_IMAGE] }) + .expectStatus(201) + .returns('res.body'); + + return response.images[0]; +} + +describe('Project-scoped image routes', () => { + beforeEach(async () => { + await clearTaskData(); + await resetProjectDefaults(); + }); + + it('should support task image upload/list/get/delete in same project scope', async () => { + const deliverable = await createTestDeliverable(1, { name: 'Task image scope' }); + const task = await createTestTask(1, { deliverableId: deliverable.id, title: 'Task image task' }); + + const uploaded = await uploadTaskImage('ZAZZ', deliverable.id, task.id); + expect(uploaded.taskId).toBe(task.id); + expect(uploaded.deliverableId).toBeNull(); + + const images = await spec() + .get(`/projects/ZAZZ/deliverables/${deliverable.id}/tasks/${task.id}/images`) + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(200) + .returns('res.body'); + + expect(images.length).toBe(1); + expect(images[0].id).toBe(uploaded.id); + + await spec() + .get(`/projects/ZAZZ/images/${uploaded.id}/metadata`) + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(200) + .expectJsonLike({ id: uploaded.id, taskId: task.id }); + + await spec() + .get(`/projects/ZAZZ/images/${uploaded.id}`) + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(200) + .expectHeaderContains('content-type', 'image/png'); + + await spec() + .delete(`/projects/ZAZZ/deliverables/${deliverable.id}/tasks/${task.id}/images/${uploaded.id}`) + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(200); + }); + + it('should support deliverable image upload/list/get/delete in same project scope', async () => { + const deliverable = await createTestDeliverable(1, { name: 'Deliverable image scope' }); + const uploaded = await uploadDeliverableImage('ZAZZ', deliverable.id); + expect(uploaded.taskId).toBeNull(); + expect(uploaded.deliverableId).toBe(deliverable.id); + + const images = await spec() + .get(`/projects/ZAZZ/deliverables/${deliverable.id}/images`) + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(200) + .returns('res.body'); + + expect(images.length).toBe(1); + expect(images[0].id).toBe(uploaded.id); + + await spec() + .get(`/projects/ZAZZ/images/${uploaded.id}/metadata`) + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(200) + .expectJsonLike({ id: uploaded.id, deliverableId: deliverable.id }); + + await spec() + .delete(`/projects/ZAZZ/deliverables/${deliverable.id}/images/${uploaded.id}`) + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(200); + }); + + it('should return 403 for cross-project image fetch by id', async () => { + const deliverable = await createTestDeliverable(2, { name: 'Other project deliverable' }); + const task = await createTestTask(2, { deliverableId: deliverable.id, title: 'Other project task' }); + const uploaded = await uploadTaskImage('MOBDEV', deliverable.id, task.id); + + await spec() + .get(`/projects/ZAZZ/images/${uploaded.id}`) + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(403); + + await spec() + .get(`/projects/ZAZZ/images/${uploaded.id}/metadata`) + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(403); + }); + + it('should return 403 for cross-project image mutation routes', async () => { + const deliverable = await createTestDeliverable(2, { name: 'Cross project mutate' }); + const task = await createTestTask(2, { deliverableId: deliverable.id, title: 'Cross project mutation task' }); + const taskImage = await uploadTaskImage('MOBDEV', deliverable.id, task.id); + const deliverableImage = await uploadDeliverableImage('MOBDEV', deliverable.id); + + await spec() + .delete(`/projects/ZAZZ/deliverables/${deliverable.id}/tasks/${task.id}/images/${taskImage.id}`) + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(403); + + await spec() + .delete(`/projects/ZAZZ/deliverables/${deliverable.id}/images/${deliverableImage.id}`) + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(403); + }); + + it('should require authentication for scoped image routes', async () => { + const deliverable = await createTestDeliverable(1, { name: 'Auth image deliverable' }); + const task = await createTestTask(1, { deliverableId: deliverable.id, title: 'Auth image task' }); + + await spec() + .get(`/projects/ZAZZ/deliverables/${deliverable.id}/tasks/${task.id}/images`) + .expectStatus(401); + }); + + it('should return 404 for removed legacy image routes', async () => { + await spec() + .get('/tasks/1/images') + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(404); + + await spec() + .post('/tasks/1/images/upload') + .withHeaders('TB_TOKEN', VALID_TOKEN) + .withJson({ images: [SAMPLE_IMAGE] }) + .expectStatus(404); + + await spec() + .delete('/tasks/1/images/1') + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(404); + + await spec() + .get('/images/1') + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(404); + + await spec() + .get('/images/1/metadata') + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(404); + }); + + it('should enforce single-owner DB constraint in IMAGE_METADATA', async () => { + const deliverable = await createTestDeliverable(1, { name: 'Constraint deliverable' }); + const task = await createTestTask(1, { deliverableId: deliverable.id, title: 'Constraint task' }); + + await expect( + db.insert(IMAGE_METADATA).values({ + task_id: task.id, + deliverable_id: deliverable.id, + original_name: 'bad-both.png', + content_type: 'image/png', + file_size: 1, + url: '/projects/ZAZZ/images/999', + storage_type: 'local' + }) + ).rejects.toThrow(); + + await expect( + db.insert(IMAGE_METADATA).values({ + task_id: null, + deliverable_id: null, + original_name: 'bad-neither.png', + content_type: 'image/png', + file_size: 1, + url: '/projects/ZAZZ/images/998', + storage_type: 'local' + }) + ).rejects.toThrow(); + }); +}); diff --git a/api/__tests__/routes/openapi.test.mjs b/api/__tests__/routes/openapi.test.mjs index ddc58de4..e2a81dba 100644 --- a/api/__tests__/routes/openapi.test.mjs +++ b/api/__tests__/routes/openapi.test.mjs @@ -7,7 +7,7 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import OpenAPISchemaValidatorPkg from 'openapi-schema-validator'; import { createTestServerWithSwagger } from '../helpers/testServerWithSwagger.js'; -const OpenAPISchemaValidator = OpenAPISchemaValidatorPkg.default; +const OpenAPISchemaValidator = OpenAPISchemaValidatorPkg.default || OpenAPISchemaValidatorPkg; let app; @@ -97,6 +97,14 @@ describe('OpenAPI / Swagger documentation', () => { '/projects/{code}/deliverables/{delivId}/tasks', '/projects/{code}/deliverables/{delivId}/tasks/{taskId}', '/projects/{code}/deliverables/{delivId}/graph', + '/projects/{code}/deliverables/{delivId}/tasks/{taskId}/images', + '/projects/{code}/deliverables/{delivId}/tasks/{taskId}/images/upload', + '/projects/{code}/deliverables/{delivId}/tasks/{taskId}/images/{imageId}', + '/projects/{code}/deliverables/{delivId}/images', + '/projects/{code}/deliverables/{delivId}/images/upload', + '/projects/{code}/deliverables/{delivId}/images/{imageId}', + '/projects/{code}/images/{id}', + '/projects/{code}/images/{id}/metadata', '/projects/{code}/tasks/{taskId}/relations', '/projects/{code}/tasks/{taskId}/readiness', '/health' @@ -121,4 +129,15 @@ describe('OpenAPI / Swagger documentation', () => { expect(desc).toContain('Change deliverable status'); expect(desc).toContain('Change task status'); }); + + it('should not document removed project-wide graph and legacy image routes', async () => { + const spec = await app.swagger(); + + expect(spec.paths['/projects/{code}/graph']).toBeUndefined(); + expect(spec.paths['/tasks/{taskId}/images']).toBeUndefined(); + expect(spec.paths['/tasks/{taskId}/images/upload']).toBeUndefined(); + expect(spec.paths['/tasks/{taskId}/images/{imageId}']).toBeUndefined(); + expect(spec.paths['/images/{id}']).toBeUndefined(); + expect(spec.paths['/images/{id}/metadata']).toBeUndefined(); + }); }); diff --git a/api/__tests__/routes/project-id-routes-regression.test.mjs b/api/__tests__/routes/project-id-routes-regression.test.mjs new file mode 100644 index 00000000..6aee0808 --- /dev/null +++ b/api/__tests__/routes/project-id-routes-regression.test.mjs @@ -0,0 +1,61 @@ +import * as pactum from 'pactum'; +import { clearTaskData, createTestDeliverable, createTestTask, resetProjectDefaults } from '../helpers/testDatabase.js'; + +const { spec } = pactum; +const VALID_TOKEN = '550e8400-e29b-41d4-a716-446655440000'; + +describe('Project-id route regressions', () => { + beforeEach(async () => { + await clearTaskData(); + await resetProjectDefaults(); + }); + + it('should keep GET /projects/:id functional', async () => { + const project = await spec() + .get('/projects/1') + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(200) + .returns('res.body'); + + expect(project.id).toBe(1); + expect(project.code).toBe('ZAZZ'); + }); + + it('should keep GET /projects/:id/tasks functional', async () => { + const deliverable = await createTestDeliverable(1, { name: 'Project task list regression' }); + await createTestTask(1, { + deliverableId: deliverable.id, + title: 'Project id route task', + status: 'READY' + }); + + const tasks = await spec() + .get('/projects/1/tasks') + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(200) + .returns('res.body'); + + expect(Array.isArray(tasks)).toBe(true); + expect(tasks.length).toBe(1); + expect(tasks[0].projectId).toBe(1); + }); + + it('should keep GET /projects/:id/kanban/tasks/column/:status functional', async () => { + const deliverable = await createTestDeliverable(1, { name: 'Column position regression' }); + const task = await createTestTask(1, { + deliverableId: deliverable.id, + title: 'Column task', + status: 'READY', + position: 30 + }); + + const columnTasks = await spec() + .get('/projects/1/kanban/tasks/column/READY') + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(200) + .returns('res.body'); + + expect(Array.isArray(columnTasks)).toBe(true); + expect(columnTasks.some((row) => row.id === task.id)).toBe(true); + }); +}); diff --git a/api/__tests__/routes/task-graph-scoping.test.mjs b/api/__tests__/routes/task-graph-scoping.test.mjs new file mode 100644 index 00000000..854a9f60 --- /dev/null +++ b/api/__tests__/routes/task-graph-scoping.test.mjs @@ -0,0 +1,56 @@ +import * as pactum from 'pactum'; +import { clearTaskData, createTestDeliverable, createTestTask, resetProjectDefaults } from '../helpers/testDatabase.js'; + +const { spec } = pactum; +const VALID_TOKEN = '550e8400-e29b-41d4-a716-446655440000'; + +describe('Task graph scoping', () => { + beforeEach(async () => { + await clearTaskData(); + await resetProjectDefaults(); + }); + + it('should return 404 for removed project-wide graph endpoint', async () => { + await createTestDeliverable(1, { name: 'Graph test deliverable' }); + + await spec() + .get('/projects/ZAZZ/graph') + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(404); + }); + + it('should return deliverable-scoped graph for a valid project and deliverable', async () => { + const deliverable = await createTestDeliverable(1, { name: 'Scoped graph deliverable' }); + await createTestTask(1, { title: 'Task A', deliverableId: deliverable.id, phase: 1, phaseTaskId: '1.1' }); + await createTestTask(1, { title: 'Task B', deliverableId: deliverable.id, phase: 1, phaseTaskId: '1.2' }); + + const graph = await spec() + .get(`/projects/ZAZZ/deliverables/${deliverable.id}/graph`) + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(200) + .returns('res.body'); + + expect(graph.deliverableId).toBe(deliverable.id); + expect(graph.projectCode).toBe('ZAZZ'); + expect(Array.isArray(graph.tasks)).toBe(true); + expect(Array.isArray(graph.relations)).toBe(true); + expect(graph.tasks).toHaveLength(2); + }); + + it('should return 404 when deliverable is not in the project path', async () => { + const otherProjectDeliverable = await createTestDeliverable(2, { name: 'Other project deliverable' }); + + await spec() + .get(`/projects/ZAZZ/deliverables/${otherProjectDeliverable.id}/graph`) + .withHeaders('TB_TOKEN', VALID_TOKEN) + .expectStatus(404); + }); + + it('should require authentication for deliverable graph endpoint', async () => { + const deliverable = await createTestDeliverable(1, { name: 'Auth check deliverable' }); + + await spec() + .get(`/projects/ZAZZ/deliverables/${deliverable.id}/graph`) + .expectStatus(401); + }); +}); diff --git a/api/lib/db/schema.js b/api/lib/db/schema.js index b3ce6008..4fd3c09c 100644 --- a/api/lib/db/schema.js +++ b/api/lib/db/schema.js @@ -1,4 +1,4 @@ -import { pgTable, serial, varchar, text, timestamp, integer, boolean, jsonb, primaryKey, index, unique } from 'drizzle-orm/pg-core'; +import { pgTable, serial, varchar, text, timestamp, integer, boolean, jsonb, primaryKey, index, unique, check } from 'drizzle-orm/pg-core'; import { pgEnum } from 'drizzle-orm/pg-core'; import { sql } from 'drizzle-orm'; import { relations } from 'drizzle-orm'; @@ -182,14 +182,20 @@ export const TASK_RELATIONS = pgTable('TASK_RELATIONS', { // Image metadata table export const IMAGE_METADATA = pgTable('IMAGE_METADATA', { id: serial('id').primaryKey(), - task_id: integer('task_id').notNull().references(() => TASKS.id, { onDelete: 'cascade' }), + task_id: integer('task_id').references(() => TASKS.id, { onDelete: 'cascade' }), + deliverable_id: integer('deliverable_id').references(() => DELIVERABLES.id, { onDelete: 'cascade' }), original_name: varchar('original_name', { length: 255 }).notNull(), content_type: varchar('content_type', { length: 100 }).notNull(), file_size: integer('file_size').notNull(), url: varchar('url', { length: 500 }).notNull(), // Local DB URL or S3 URL storage_type: varchar('storage_type', { length: 20 }).notNull().default('local'), // 'local' or 's3' created_at: timestamp('created_at', { withTimezone: true }).defaultNow().notNull(), -}); +}, (table) => [ + check( + 'image_metadata_single_owner_chk', + sql`((${table.task_id} IS NOT NULL AND ${table.deliverable_id} IS NULL) OR (${table.task_id} IS NULL AND ${table.deliverable_id} IS NOT NULL))` + ), +]); // Image binary data table (for local storage) export const IMAGE_DATA = pgTable('IMAGE_DATA', { @@ -225,6 +231,7 @@ export const deliverablesRelations = relations(DELIVERABLES, ({ one, many }) => references: [USERS.id], }), tasks: many(TASKS), + images: many(IMAGE_METADATA), })); export const tasksRelations = relations(TASKS, ({ one, many }) => ({ @@ -275,6 +282,10 @@ export const imageMetadataRelations = relations(IMAGE_METADATA, ({ one }) => ({ fields: [IMAGE_METADATA.task_id], references: [TASKS.id], }), + deliverable: one(DELIVERABLES, { + fields: [IMAGE_METADATA.deliverable_id], + references: [DELIVERABLES.id], + }), imageData: one(IMAGE_DATA, { fields: [IMAGE_METADATA.id], references: [IMAGE_DATA.id], diff --git a/api/src/routes/images.js b/api/src/routes/images.js index baf1101c..84d2b620 100644 --- a/api/src/routes/images.js +++ b/api/src/routes/images.js @@ -1,166 +1,344 @@ import { authMiddleware } from '../middleware/authMiddleware.js'; import { imageSchemas } from '../schemas/validation.js'; +function parseId(value) { + return parseInt(value, 10); +} + export default async function imageRoutes(fastify, options) { const { dbService } = options; // Add authentication middleware to all image routes fastify.addHook('preHandler', authMiddleware); - // GET /tasks/:taskId/images - Get all images for a task - fastify.get('/tasks/:taskId/images', { schema: imageSchemas.getTaskImages }, async (request, reply) => { - try { - const { taskId } = request.params; - request.log.info(`Fetching images for task ${taskId}`); - - const images = await dbService.getTaskImages(parseInt(taskId)); - request.log.info(`Found ${images.length} images for task ${taskId}`); - - reply.send(images); - } catch (error) { - request.log.error(error, 'Failed to fetch task images'); - reply.code(500).send({ error: 'Failed to fetch task images' }); - } - }); - - // POST /tasks/:taskId/images/upload - Upload images to a task - fastify.post('/tasks/:taskId/images/upload', { schema: imageSchemas.uploadTaskImages }, async (request, reply) => { - try { - const { taskId } = request.params; - const { images } = request.body; - - request.log.info(`Uploading ${images?.length || 0} images to task ${taskId}`); - - if (!images || !Array.isArray(images) || images.length === 0) { - return reply.code(400).send({ error: 'No images provided' }); + async function resolveScopedTask(request, reply) { + const { code, delivId, taskId } = request.params; + const deliverableId = parseId(delivId); + const taskIdNum = parseId(taskId); + + const project = await dbService.getProjectByCode(code); + if (!project) { + reply.code(404).send({ error: 'Project not found' }); + return null; + } + + const deliverable = await dbService.getDeliverableById(deliverableId); + if (!deliverable) { + reply.code(404).send({ error: 'Deliverable not found' }); + return null; + } + if (deliverable.projectId !== project.id) { + reply.code(403).send({ error: 'Deliverable does not belong to this project' }); + return null; + } + + const task = await dbService.getTaskById(taskIdNum); + if (!task) { + reply.code(404).send({ error: 'Task not found' }); + return null; + } + if (task.projectId !== project.id || task.deliverableId !== deliverableId) { + reply.code(403).send({ error: 'Task does not belong to this project/deliverable scope' }); + return null; + } + + return { project, deliverable, task, deliverableId, taskIdNum }; + } + + async function resolveScopedDeliverable(request, reply) { + const { code, delivId } = request.params; + const deliverableId = parseId(delivId); + + const project = await dbService.getProjectByCode(code); + if (!project) { + reply.code(404).send({ error: 'Project not found' }); + return null; + } + + const deliverable = await dbService.getDeliverableById(deliverableId); + if (!deliverable) { + reply.code(404).send({ error: 'Deliverable not found' }); + return null; + } + if (deliverable.projectId !== project.id) { + reply.code(403).send({ error: 'Deliverable does not belong to this project' }); + return null; + } + + return { project, deliverable, deliverableId }; + } + + async function resolveImageOwnerProjectId(metadata) { + if (metadata.taskId) { + const task = await dbService.getTaskById(metadata.taskId); + return task?.projectId ?? null; + } + if (metadata.deliverableId) { + const deliverable = await dbService.getDeliverableById(metadata.deliverableId); + return deliverable?.projectId ?? null; + } + return null; + } + + // GET /projects/:code/deliverables/:delivId/tasks/:taskId/images + fastify.get( + '/projects/:code/deliverables/:delivId/tasks/:taskId/images', + { schema: imageSchemas.getScopedTaskImages }, + async (request, reply) => { + try { + const scoped = await resolveScopedTask(request, reply); + if (!scoped) return; + + const images = await dbService.getTaskImages(scoped.taskIdNum); + reply.send(images); + } catch (error) { + request.log.error(error, 'Failed to fetch scoped task images'); + reply.code(500).send({ error: 'Failed to fetch task images' }); } - - const uploadedImages = []; - - for (const imageData of images) { - const { originalName, contentType, fileSize, base64Data } = imageData; - - // Validate required fields - if (!originalName || !contentType || !fileSize || !base64Data) { - request.log.warn('Skipping invalid image data', { originalName, contentType }); - continue; - } - - // Validate content type - if (!contentType.startsWith('image/')) { - request.log.warn('Skipping non-image file', { originalName, contentType }); - continue; - } - - const storedImage = await dbService.storeTaskImage(parseInt(taskId), { - originalName, - contentType, - fileSize, - base64Data + } + ); + + // POST /projects/:code/deliverables/:delivId/tasks/:taskId/images/upload + fastify.post( + '/projects/:code/deliverables/:delivId/tasks/:taskId/images/upload', + { schema: imageSchemas.uploadScopedTaskImages }, + async (request, reply) => { + try { + const scoped = await resolveScopedTask(request, reply); + if (!scoped) return; + + const { images } = request.body; + if (!images || !Array.isArray(images) || images.length === 0) { + return reply.code(400).send({ error: 'No images provided' }); + } + + const uploadedImages = []; + const imageUrlBase = `/projects/${encodeURIComponent(scoped.project.code)}/images`; + + for (const imageData of images) { + const { originalName, contentType, fileSize, base64Data } = imageData; + if (!originalName || !contentType || !fileSize || !base64Data) continue; + if (!contentType.startsWith('image/')) continue; + + const storedImage = await dbService.storeTaskImage( + scoped.taskIdNum, + { originalName, contentType, fileSize, base64Data }, + imageUrlBase + ); + uploadedImages.push(storedImage); + } + + reply.code(201).send({ + success: true, + images: uploadedImages, + count: uploadedImages.length }); - - uploadedImages.push(storedImage); - } - - request.log.info(`Successfully uploaded ${uploadedImages.length} images`); - - reply.code(201).send({ - success: true, - images: uploadedImages, - count: uploadedImages.length - }); - } catch (error) { - request.log.error(error, 'Failed to upload images'); - reply.code(500).send({ error: 'Failed to upload images' }); - } - }); - - // GET /images/:id - Serve individual image - fastify.get('/images/:id', { schema: imageSchemas.getImageById }, async (request, reply) => { - try { - const { id } = request.params; - const imageId = parseInt(id); - - if (isNaN(imageId)) { - return reply.code(400).send({ error: 'Invalid image ID' }); + } catch (error) { + request.log.error(error, 'Failed to upload scoped task images'); + reply.code(500).send({ error: 'Failed to upload task images' }); } - - const imageWithData = await dbService.getImageWithData(imageId); - - if (!imageWithData || !imageWithData.data) { - return reply.code(404).send({ error: 'Image not found' }); + } + ); + + // DELETE /projects/:code/deliverables/:delivId/tasks/:taskId/images/:imageId + fastify.delete( + '/projects/:code/deliverables/:delivId/tasks/:taskId/images/:imageId', + { schema: imageSchemas.deleteScopedTaskImage }, + async (request, reply) => { + try { + const scoped = await resolveScopedTask(request, reply); + if (!scoped) return; + + const imageIdNum = parseId(request.params.imageId); + const imageMetadata = await dbService.getImageMetadata(imageIdNum); + if (!imageMetadata) { + return reply.code(404).send({ error: 'Image not found' }); + } + if (imageMetadata.taskId !== scoped.taskIdNum) { + return reply.code(403).send({ error: 'Image does not belong to the specified task scope' }); + } + + const deletedImage = await dbService.deleteImage(imageIdNum); + if (!deletedImage) { + return reply.code(404).send({ error: 'Image not found' }); + } + + reply.send({ message: 'Image deleted successfully', image: deletedImage }); + } catch (error) { + request.log.error(error, 'Failed to delete scoped task image'); + reply.code(500).send({ error: 'Failed to delete task image' }); } - - // Convert base64 to binary - const binaryData = Buffer.from(imageWithData.data, 'base64'); - - // Set proper headers - reply - .header('Content-Type', imageWithData.contentType) - .header('Content-Length', binaryData.length) - .header('Cache-Control', 'public, max-age=31536000') // Cache for 1 year - .send(binaryData); - - } catch (error) { - request.log.error(error, 'Failed to serve image'); - reply.code(500).send({ error: 'Failed to serve image' }); - } - }); - - // GET /images/:id/metadata - Get image metadata only - fastify.get('/images/:id/metadata', { schema: imageSchemas.getImageMetadata }, async (request, reply) => { - try { - const { id } = request.params; - const imageId = parseInt(id); - - if (isNaN(imageId)) { - return reply.code(400).send({ error: 'Invalid image ID' }); + } + ); + + // GET /projects/:code/deliverables/:delivId/images + fastify.get( + '/projects/:code/deliverables/:delivId/images', + { schema: imageSchemas.getScopedDeliverableImages }, + async (request, reply) => { + try { + const scoped = await resolveScopedDeliverable(request, reply); + if (!scoped) return; + + const images = await dbService.getDeliverableImages(scoped.deliverableId); + reply.send(images); + } catch (error) { + request.log.error(error, 'Failed to fetch scoped deliverable images'); + reply.code(500).send({ error: 'Failed to fetch deliverable images' }); } - - const metadata = await dbService.getImageMetadata(imageId); - - if (!metadata) { - return reply.code(404).send({ error: 'Image not found' }); + } + ); + + // POST /projects/:code/deliverables/:delivId/images/upload + fastify.post( + '/projects/:code/deliverables/:delivId/images/upload', + { schema: imageSchemas.uploadScopedDeliverableImages }, + async (request, reply) => { + try { + const scoped = await resolveScopedDeliverable(request, reply); + if (!scoped) return; + + const { images } = request.body; + if (!images || !Array.isArray(images) || images.length === 0) { + return reply.code(400).send({ error: 'No images provided' }); + } + + const uploadedImages = []; + const imageUrlBase = `/projects/${encodeURIComponent(scoped.project.code)}/images`; + + for (const imageData of images) { + const { originalName, contentType, fileSize, base64Data } = imageData; + if (!originalName || !contentType || !fileSize || !base64Data) continue; + if (!contentType.startsWith('image/')) continue; + + const storedImage = await dbService.storeDeliverableImage( + scoped.deliverableId, + { originalName, contentType, fileSize, base64Data }, + imageUrlBase + ); + uploadedImages.push(storedImage); + } + + reply.code(201).send({ + success: true, + images: uploadedImages, + count: uploadedImages.length + }); + } catch (error) { + request.log.error(error, 'Failed to upload scoped deliverable images'); + reply.code(500).send({ error: 'Failed to upload deliverable images' }); } - - reply.send(metadata); - } catch (error) { - request.log.error(error, 'Failed to fetch image metadata'); - reply.code(500).send({ error: 'Failed to fetch image metadata' }); - } - }); - - // DELETE /tasks/:taskId/images/:imageId - Delete specific image from task (with security validation) - fastify.delete('/tasks/:taskId/images/:imageId', { schema: imageSchemas.deleteTaskImage }, async (request, reply) => { - try { - const { taskId, imageId } = request.params; - const taskIdNum = parseInt(taskId); - const imageIdNum = parseInt(imageId); - - if (isNaN(taskIdNum) || isNaN(imageIdNum)) { - return reply.code(400).send({ error: 'Invalid task ID or image ID' }); + } + ); + + // DELETE /projects/:code/deliverables/:delivId/images/:imageId + fastify.delete( + '/projects/:code/deliverables/:delivId/images/:imageId', + { schema: imageSchemas.deleteScopedDeliverableImage }, + async (request, reply) => { + try { + const scoped = await resolveScopedDeliverable(request, reply); + if (!scoped) return; + + const imageIdNum = parseId(request.params.imageId); + const imageMetadata = await dbService.getImageMetadata(imageIdNum); + if (!imageMetadata) { + return reply.code(404).send({ error: 'Image not found' }); + } + if (imageMetadata.deliverableId !== scoped.deliverableId) { + return reply.code(403).send({ error: 'Image does not belong to the specified deliverable scope' }); + } + + const deletedImage = await dbService.deleteImage(imageIdNum); + if (!deletedImage) { + return reply.code(404).send({ error: 'Image not found' }); + } + + reply.send({ message: 'Image deleted successfully', image: deletedImage }); + } catch (error) { + request.log.error(error, 'Failed to delete scoped deliverable image'); + reply.code(500).send({ error: 'Failed to delete deliverable image' }); } - - request.log.info(`Deleting image ${imageIdNum} from task ${taskIdNum}`); - - // First verify the image belongs to the specified task - const imageMetadata = await dbService.getImageMetadata(imageIdNum); - - if (!imageMetadata) { - return reply.code(404).send({ error: 'Image not found' }); + } + ); + + // GET /projects/:code/images/:id + fastify.get( + '/projects/:code/images/:id', + { schema: imageSchemas.getScopedImageById }, + async (request, reply) => { + try { + const { code, id } = request.params; + const imageId = parseId(id); + + const project = await dbService.getProjectByCode(code); + if (!project) { + return reply.code(404).send({ error: 'Project not found' }); + } + + const metadata = await dbService.getImageMetadata(imageId); + if (!metadata) { + return reply.code(404).send({ error: 'Image not found' }); + } + + const ownerProjectId = await resolveImageOwnerProjectId(metadata); + if (!ownerProjectId) { + return reply.code(404).send({ error: 'Image owner not found' }); + } + if (ownerProjectId !== project.id) { + return reply.code(403).send({ error: 'Image does not belong to this project' }); + } + + const imageWithData = await dbService.getImageWithData(imageId); + if (!imageWithData || !imageWithData.data) { + return reply.code(404).send({ error: 'Image not found' }); + } + + const binaryData = Buffer.from(imageWithData.data, 'base64'); + reply + .header('Content-Type', imageWithData.contentType) + .header('Content-Length', binaryData.length) + .header('Cache-Control', 'public, max-age=31536000') + .send(binaryData); + } catch (error) { + request.log.error(error, 'Failed to serve scoped image'); + reply.code(500).send({ error: 'Failed to serve image' }); } - - if (imageMetadata.taskId !== taskIdNum) { - return reply.code(403).send({ error: 'Image does not belong to the specified task' }); + } + ); + + // GET /projects/:code/images/:id/metadata + fastify.get( + '/projects/:code/images/:id/metadata', + { schema: imageSchemas.getScopedImageMetadata }, + async (request, reply) => { + try { + const { code, id } = request.params; + const imageId = parseId(id); + + const project = await dbService.getProjectByCode(code); + if (!project) { + return reply.code(404).send({ error: 'Project not found' }); + } + + const metadata = await dbService.getImageMetadata(imageId); + if (!metadata) { + return reply.code(404).send({ error: 'Image not found' }); + } + + const ownerProjectId = await resolveImageOwnerProjectId(metadata); + if (!ownerProjectId) { + return reply.code(404).send({ error: 'Image owner not found' }); + } + if (ownerProjectId !== project.id) { + return reply.code(403).send({ error: 'Image does not belong to this project' }); + } + + reply.send(metadata); + } catch (error) { + request.log.error(error, 'Failed to fetch scoped image metadata'); + reply.code(500).send({ error: 'Failed to fetch image metadata' }); } - - // Now delete the image - const deletedImage = await dbService.deleteImage(imageIdNum); - - reply.send({ message: 'Image deleted successfully', image: deletedImage }); - } catch (error) { - request.log.error(error, 'Failed to delete image'); - reply.code(500).send({ error: 'Failed to delete image' }); - } - }); + } + ); } diff --git a/api/src/routes/index.js b/api/src/routes/index.js index f319a4e9..91b5fd41 100644 --- a/api/src/routes/index.js +++ b/api/src/routes/index.js @@ -34,7 +34,7 @@ export default async function routes(fastify, options) { reply.send({ message: 'Zazz Board API', version: '1.0.0', - endpoints: ['/health', '/users', '/projects', '/deliverables', '/tasks', '/tags', '/images', '/translations', '/status-definitions', '/coordination-types'] + endpoints: ['/health', '/users', '/projects', '/deliverables', '/tasks', '/tags', '/projects/:code/images/:id', '/translations', '/status-definitions', '/coordination-types'] }); }); diff --git a/api/src/routes/statusDefinitions.js b/api/src/routes/statusDefinitions.js index 4f77455a..0c7990b6 100644 --- a/api/src/routes/statusDefinitions.js +++ b/api/src/routes/statusDefinitions.js @@ -19,7 +19,7 @@ export default async function statusDefinitionsRoutes(fastify, options) { type: 'object', properties: { code: { type: 'string' }, - description: { type: ['string', 'null'] }, + description: { type: 'string', nullable: true }, createdAt: { type: 'string' }, updatedAt: { type: 'string' } } diff --git a/api/src/routes/taskGraph.js b/api/src/routes/taskGraph.js index 48acf91c..97762946 100644 --- a/api/src/routes/taskGraph.js +++ b/api/src/routes/taskGraph.js @@ -7,32 +7,6 @@ export default async function taskGraphRoutes(fastify, options) { // Add authentication middleware to all task graph routes fastify.addHook('preHandler', authMiddleware); - // GET /projects/:code/graph - Get full task graph for a project - fastify.get('/projects/:code/graph', { - schema: taskGraphSchemas.getProjectGraph - }, async (request, reply) => { - try { - const { code } = request.params; - - const project = await dbService.getProjectByCode(code); - if (!project) { - return reply.code(404).send({ error: 'Project not found' }); - } - - const graph = await dbService.getProjectTaskGraph(project.id); - reply.send({ - projectId: project.id, - projectCode: project.code, - taskGraphLayoutDirection: project.taskGraphLayoutDirection, - completionCriteriaStatus: project.completionCriteriaStatus, - ...graph - }); - } catch (error) { - request.log.error(error, 'Failed to fetch project task graph'); - reply.code(500).send({ error: 'Failed to fetch project task graph' }); - } - }); - // GET /projects/:code/tasks/:taskId/relations - Get all relations for a task fastify.get('/projects/:code/tasks/:taskId/relations', { schema: taskGraphSchemas.getTaskRelations diff --git a/api/src/schemas/images.js b/api/src/schemas/images.js index f7afe725..68a7d6e5 100644 --- a/api/src/schemas/images.js +++ b/api/src/schemas/images.js @@ -1,126 +1,190 @@ /** - * Image route schemas. + * Project-scoped image route schemas. */ +const scopedProjectDeliverableParams = { + type: 'object', + required: ['code', 'delivId'], + properties: { + code: { type: 'string', pattern: '^[A-Z0-9]+$', description: 'Project code (e.g. ZAZZ).' }, + delivId: { type: 'string', pattern: '^\\d+$', description: 'Numeric deliverable id.' } + } +}; + +const scopedProjectDeliverableTaskParams = { + type: 'object', + required: ['code', 'delivId', 'taskId'], + properties: { + code: { type: 'string', pattern: '^[A-Z0-9]+$', description: 'Project code (e.g. ZAZZ).' }, + delivId: { type: 'string', pattern: '^\\d+$', description: 'Numeric deliverable id.' }, + taskId: { type: 'string', pattern: '^\\d+$', description: 'Numeric task id.' } + } +}; + +const scopedImageIdParams = { + type: 'object', + required: ['code', 'id'], + properties: { + code: { type: 'string', pattern: '^[A-Z0-9]+$', description: 'Project code (e.g. ZAZZ).' }, + id: { type: 'string', pattern: '^\\d+$', description: 'Numeric image id.' } + } +}; + +const imageUploadBody = { + type: 'object', + required: ['images'], + properties: { + images: { + type: 'array', + items: { + type: 'object', + required: ['originalName', 'contentType', 'fileSize', 'base64Data'], + properties: { + originalName: { type: 'string' }, + contentType: { type: 'string', pattern: '^image/' }, + fileSize: { type: 'integer', minimum: 1 }, + base64Data: { type: 'string' } + }, + additionalProperties: false + } + } + }, + additionalProperties: false +}; + +const imageMetadataSchema = { + type: 'object', + properties: { + id: { type: 'number' }, + taskId: { type: 'number', nullable: true }, + deliverableId: { type: 'number', nullable: true }, + originalName: { type: 'string' }, + contentType: { type: 'string' }, + fileSize: { type: 'number' }, + url: { type: 'string' }, + storageType: { type: 'string' }, + createdAt: { type: 'string', format: 'date-time' } + } +}; + +const uploadResponseSchema = { + type: 'object', + properties: { + success: { type: 'boolean' }, + images: { type: 'array', items: imageMetadataSchema }, + count: { type: 'number' } + } +}; + export const imageSchemas = { - getTaskImages: { + getScopedTaskImages: { tags: ['images'], - summary: 'List task images', - description: 'Returns metadata (id, originalName, contentType, fileSize) for images attached to a task. Use GET /images/:id for binary.', - params: { - type: 'object', - required: ['taskId'], - properties: { taskId: { type: 'string', pattern: '^\\d+$', description: 'Numeric task id.' } } - }, + summary: 'List task images (project scoped)', + description: 'Returns image metadata for a task scoped to project + deliverable + task.', + params: scopedProjectDeliverableTaskParams, response: { 200: { - description: 'List of image metadata', + description: 'Task image metadata list', type: 'array', - items: { - type: 'object', - properties: { - id: { type: 'number' }, - taskId: { type: 'number' }, - originalName: { type: 'string' }, - contentType: { type: 'string' }, - fileSize: { type: 'number' } - } - } - } + items: imageMetadataSchema + }, + 401: { description: 'Unauthorized' }, + 403: { description: 'Task/deliverable scope mismatch' }, + 404: { description: 'Project, deliverable, or task not found' } } }, - uploadTaskImages: { + uploadScopedTaskImages: { tags: ['images'], - summary: 'Upload task images', - description: 'Upload one or more images as base64. Body: { images: [{ originalName, contentType, fileSize, base64Data }] }. contentType must be image/*.', + summary: 'Upload task images (project scoped)', + description: 'Uploads one or more task-owned images scoped by project + deliverable + task.', + params: scopedProjectDeliverableTaskParams, + body: imageUploadBody, + response: { + 201: { + description: 'Task images uploaded', + ...uploadResponseSchema + }, + 401: { description: 'Unauthorized' }, + 403: { description: 'Task/deliverable scope mismatch' }, + 404: { description: 'Project, deliverable, or task not found' } + } + }, + + deleteScopedTaskImage: { + tags: ['images'], + summary: 'Delete task image (project scoped)', + description: 'Deletes a task-owned image scoped by project + deliverable + task.', params: { type: 'object', - required: ['taskId'], - properties: { taskId: { type: 'string', pattern: '^\\d+$', description: 'Numeric task id.' } } - }, - body: { - type: 'object', - required: ['images'], + required: ['code', 'delivId', 'taskId', 'imageId'], properties: { - images: { - type: 'array', - items: { - type: 'object', - required: ['originalName', 'contentType', 'fileSize', 'base64Data'], - properties: { - originalName: { type: 'string' }, - contentType: { type: 'string', pattern: '^image/' }, - fileSize: { type: 'integer', minimum: 1 }, - base64Data: { type: 'string' } - } - } - } + code: { type: 'string', pattern: '^[A-Z0-9]+$' }, + delivId: { type: 'string', pattern: '^\\d+$' }, + taskId: { type: 'string', pattern: '^\\d+$' }, + imageId: { type: 'string', pattern: '^\\d+$' } } }, response: { - 201: { - description: 'Images uploaded', + 200: { + description: 'Image deleted', type: 'object', properties: { - success: { type: 'boolean' }, - images: { type: 'array', items: { type: 'object' } }, - count: { type: 'number' } + message: { type: 'string' }, + image: { type: 'object' } } - } + }, + 401: { description: 'Unauthorized' }, + 403: { description: 'Image/task scope mismatch' }, + 404: { description: 'Project, deliverable, task, or image not found' } } }, - getImageById: { + getScopedDeliverableImages: { tags: ['images'], - summary: 'Get image binary', - description: 'Returns image binary. Use id from GET /tasks/:taskId/images. Content-Type indicates format.', - params: { - type: 'object', - required: ['id'], - properties: { id: { type: 'string', pattern: '^\\d+$', description: 'Numeric image id.' } } - }, + summary: 'List deliverable images (project scoped)', + description: 'Returns image metadata for images attached directly to a deliverable.', + params: scopedProjectDeliverableParams, response: { - 200: { description: 'Image binary' }, - 404: { description: 'Image not found' } + 200: { + description: 'Deliverable image metadata list', + type: 'array', + items: imageMetadataSchema + }, + 401: { description: 'Unauthorized' }, + 403: { description: 'Deliverable does not belong to project' }, + 404: { description: 'Project or deliverable not found' } } }, - getImageMetadata: { + uploadScopedDeliverableImages: { tags: ['images'], - summary: 'Get image metadata', - description: 'Returns image metadata (id, taskId, originalName, contentType, fileSize) without binary. Use when you need metadata only.', - params: { - type: 'object', - required: ['id'], - properties: { id: { type: 'string', pattern: '^\\d+$', description: 'Numeric image id.' } } - }, + summary: 'Upload deliverable images (project scoped)', + description: 'Uploads one or more deliverable-owned images scoped by project + deliverable.', + params: scopedProjectDeliverableParams, + body: imageUploadBody, response: { - 200: { - description: 'Image metadata', - type: 'object', - properties: { - id: { type: 'number' }, - taskId: { type: 'number' }, - originalName: { type: 'string' }, - contentType: { type: 'string' }, - fileSize: { type: 'number' } - } + 201: { + description: 'Deliverable images uploaded', + ...uploadResponseSchema }, - 404: { description: 'Image not found' } + 401: { description: 'Unauthorized' }, + 403: { description: 'Deliverable does not belong to project' }, + 404: { description: 'Project or deliverable not found' } } }, - deleteTaskImage: { + deleteScopedDeliverableImage: { tags: ['images'], - summary: 'Delete task image', - description: 'Deletes an image. Verifies image belongs to the specified task. Returns 403 if image belongs to different task.', + summary: 'Delete deliverable image (project scoped)', + description: 'Deletes a deliverable-owned image scoped by project + deliverable.', params: { type: 'object', - required: ['taskId', 'imageId'], + required: ['code', 'delivId', 'imageId'], properties: { - taskId: { type: 'string', pattern: '^\\d+$', description: 'Numeric task id.' }, - imageId: { type: 'string', pattern: '^\\d+$', description: 'Numeric image id.' } + code: { type: 'string', pattern: '^[A-Z0-9]+$' }, + delivId: { type: 'string', pattern: '^\\d+$' }, + imageId: { type: 'string', pattern: '^\\d+$' } } }, response: { @@ -132,8 +196,38 @@ export const imageSchemas = { image: { type: 'object' } } }, - 403: { description: 'Image does not belong to the specified task' }, - 404: { description: 'Image not found' } + 401: { description: 'Unauthorized' }, + 403: { description: 'Image/deliverable scope mismatch' }, + 404: { description: 'Project, deliverable, or image not found' } + } + }, + + getScopedImageById: { + tags: ['images'], + summary: 'Get image binary (project scoped)', + description: 'Returns image binary for an image that belongs to the specified project.', + params: scopedImageIdParams, + response: { + 200: { description: 'Image binary' }, + 401: { description: 'Unauthorized' }, + 403: { description: 'Image belongs to a different project' }, + 404: { description: 'Project or image not found' } + } + }, + + getScopedImageMetadata: { + tags: ['images'], + summary: 'Get image metadata (project scoped)', + description: 'Returns image metadata for an image that belongs to the specified project.', + params: scopedImageIdParams, + response: { + 200: { + description: 'Image metadata', + ...imageMetadataSchema + }, + 401: { description: 'Unauthorized' }, + 403: { description: 'Image belongs to a different project' }, + 404: { description: 'Project or image not found' } } } }; diff --git a/api/src/schemas/taskGraph.js b/api/src/schemas/taskGraph.js index 422eff2b..b640fc27 100644 --- a/api/src/schemas/taskGraph.js +++ b/api/src/schemas/taskGraph.js @@ -2,8 +2,6 @@ * Task graph and relation route schemas. */ -import { codeParam } from './common.js'; - const graphTaskItem = { type: 'object', properties: { @@ -34,26 +32,6 @@ const graphRelationItem = { }; export const taskGraphSchemas = { - getProjectGraph: { - tags: ['task-graph'], - summary: 'Get full task graph for a project', - description: 'Returns all tasks and intra-project relations. Polled every 3 s by the UI for live updates.', - params: codeParam, - response: { - 200: { - type: 'object', - properties: { - projectId: { type: 'integer' }, - projectCode: { type: 'string' }, - taskGraphLayoutDirection: { type: 'string', enum: ['LR', 'TB'] }, - completionCriteriaStatus: { type: 'string' }, - tasks: { type: 'array', items: graphTaskItem }, - relations: { type: 'array', items: graphRelationItem } - } - } - } - }, - getTaskRelations: { tags: ['task-graph'], summary: 'Get task relations', diff --git a/api/src/server.js b/api/src/server.js index 0b239154..b71c68b4 100644 --- a/api/src/server.js +++ b/api/src/server.js @@ -64,7 +64,7 @@ const start = async () => { { name: 'tags', description: 'Tags' }, { name: 'translations', description: 'i18n' }, { name: 'status-definitions', description: 'Status lookup' }, - { name: 'images', description: 'Task images' } + { name: 'images', description: 'Project-scoped task and deliverable images' } ], components: { securitySchemes: { @@ -117,4 +117,3 @@ const start = async () => { }; start(); - diff --git a/api/src/services/databaseService.js b/api/src/services/databaseService.js index 494be7d3..a26e3449 100644 --- a/api/src/services/databaseService.js +++ b/api/src/services/databaseService.js @@ -1248,6 +1248,8 @@ class DatabaseService { async getTaskImages(taskId) { const images = await db.select({ id: IMAGE_METADATA.id, + taskId: IMAGE_METADATA.task_id, + deliverableId: IMAGE_METADATA.deliverable_id, originalName: IMAGE_METADATA.original_name, contentType: IMAGE_METADATA.content_type, fileSize: IMAGE_METADATA.file_size, @@ -1263,24 +1265,48 @@ class DatabaseService { } /** - * Store image with metadata and binary data + * Get all images attached directly to a deliverable + */ + async getDeliverableImages(deliverableId) { + const images = await db.select({ + id: IMAGE_METADATA.id, + taskId: IMAGE_METADATA.task_id, + deliverableId: IMAGE_METADATA.deliverable_id, + originalName: IMAGE_METADATA.original_name, + contentType: IMAGE_METADATA.content_type, + fileSize: IMAGE_METADATA.file_size, + url: IMAGE_METADATA.url, + storageType: IMAGE_METADATA.storage_type, + createdAt: IMAGE_METADATA.created_at + }) + .from(IMAGE_METADATA) + .where(eq(IMAGE_METADATA.deliverable_id, deliverableId)) + .orderBy(asc(IMAGE_METADATA.created_at)); + + return images; + } + + /** + * Store task-owned image with metadata and binary data */ - async storeTaskImage(taskId, imageData) { + async storeTaskImage(taskId, imageData, imageUrlBase = '/images') { // Insert image metadata const [metadata] = await db.insert(IMAGE_METADATA) .values({ task_id: taskId, + deliverable_id: null, original_name: imageData.originalName, content_type: imageData.contentType, file_size: imageData.fileSize, - url: `/images/0`, // Temporary, will update with actual ID + url: `${imageUrlBase}/0`, // Temporary, will update with actual ID storage_type: 'local' }) .returning(); - + // Update URL with actual image ID + const finalUrl = `${imageUrlBase}/${metadata.id}`; await db.update(IMAGE_METADATA) - .set({ url: `/images/${metadata.id}` }) + .set({ url: finalUrl }) .where(eq(IMAGE_METADATA.id, metadata.id)); // Insert binary data @@ -1293,10 +1319,53 @@ class DatabaseService { return { id: metadata.id, + taskId: metadata.task_id, + deliverableId: metadata.deliverable_id, + originalName: metadata.original_name, + contentType: metadata.content_type, + fileSize: metadata.file_size, + url: finalUrl, + storageType: metadata.storage_type, + createdAt: metadata.created_at + }; + } + + /** + * Store deliverable-owned image with metadata and binary data + */ + async storeDeliverableImage(deliverableId, imageData, imageUrlBase = '/images') { + const [metadata] = await db.insert(IMAGE_METADATA) + .values({ + task_id: null, + deliverable_id: deliverableId, + original_name: imageData.originalName, + content_type: imageData.contentType, + file_size: imageData.fileSize, + url: `${imageUrlBase}/0`, + storage_type: 'local' + }) + .returning(); + + const finalUrl = `${imageUrlBase}/${metadata.id}`; + await db.update(IMAGE_METADATA) + .set({ url: finalUrl }) + .where(eq(IMAGE_METADATA.id, metadata.id)); + + await db.insert(IMAGE_DATA) + .values({ + id: metadata.id, + data: imageData.base64Data, + thumbnail_data: null + }); + + return { + id: metadata.id, + taskId: metadata.task_id, + deliverableId: metadata.deliverable_id, originalName: metadata.original_name, contentType: metadata.content_type, fileSize: metadata.file_size, - url: `/images/${metadata.id}`, + url: finalUrl, storageType: metadata.storage_type, createdAt: metadata.created_at }; @@ -1309,9 +1378,13 @@ class DatabaseService { const [result] = await db .select({ id: IMAGE_METADATA.id, + taskId: IMAGE_METADATA.task_id, + deliverableId: IMAGE_METADATA.deliverable_id, originalName: IMAGE_METADATA.original_name, contentType: IMAGE_METADATA.content_type, fileSize: IMAGE_METADATA.file_size, + url: IMAGE_METADATA.url, + storageType: IMAGE_METADATA.storage_type, data: IMAGE_DATA.data, thumbnailData: IMAGE_DATA.thumbnail_data }) @@ -1330,6 +1403,7 @@ class DatabaseService { const [image] = await db.select({ id: IMAGE_METADATA.id, taskId: IMAGE_METADATA.task_id, + deliverableId: IMAGE_METADATA.deliverable_id, originalName: IMAGE_METADATA.original_name, contentType: IMAGE_METADATA.content_type, fileSize: IMAGE_METADATA.file_size, @@ -1359,6 +1433,8 @@ class DatabaseService { return deletedImage ? { id: deletedImage.id, + taskId: deletedImage.task_id, + deliverableId: deletedImage.deliverable_id, originalName: deletedImage.original_name, contentType: deletedImage.content_type } : null; diff --git a/client/src/App.jsx b/client/src/App.jsx index d12e7816..80cb886f 100644 --- a/client/src/App.jsx +++ b/client/src/App.jsx @@ -222,6 +222,11 @@ function AppContent() { console.log('=================='); }, []); + // Reset graph deliverable scope when switching projects. + useEffect(() => { + setSelectedDeliverableId(null); + }, [selectedProject?.id]); + const toggleTheme = () => { const newTheme = colorScheme === 'dark' ? 'light' : 'dark'; setColorScheme(newTheme); @@ -576,11 +581,8 @@ function AppContent() { {isProjectPage && selectedProject && ( isTaskGraphPage ? (