From 78ab5a4325f12bf38dcf4c4c00ea2c19268ef4ad Mon Sep 17 00:00:00 2001 From: Sami Rusani Date: Sat, 14 Mar 2026 09:53:13 +0100 Subject: [PATCH] Sprint 5C: task artifact records and registration --- .ai/active/SPRINT_PACKET.md | 177 ++++++----- ARCHITECTURE.md | 39 ++- BUILD_REPORT.md | 128 ++++++-- REVIEW_REPORT.md | 47 +-- .../versions/20260313_0023_task_artifacts.py | 87 ++++++ apps/api/src/alicebot_api/artifacts.py | 173 ++++++++++ apps/api/src/alicebot_api/contracts.py | 42 +++ apps/api/src/alicebot_api/main.py | 82 +++++ apps/api/src/alicebot_api/store.py | 145 +++++++++ tests/integration/test_migrations.py | 10 + tests/integration/test_task_artifacts_api.py | 293 +++++++++++++++++ .../unit/test_20260313_0023_task_artifacts.py | 46 +++ tests/unit/test_artifacts.py | 295 ++++++++++++++++++ tests/unit/test_artifacts_main.py | 154 +++++++++ tests/unit/test_main.py | 3 + tests/unit/test_task_artifact_store.py | 228 ++++++++++++++ 16 files changed, 1812 insertions(+), 137 deletions(-) create mode 100644 apps/api/alembic/versions/20260313_0023_task_artifacts.py create mode 100644 apps/api/src/alicebot_api/artifacts.py create mode 100644 tests/integration/test_task_artifacts_api.py create mode 100644 tests/unit/test_20260313_0023_task_artifacts.py create mode 100644 tests/unit/test_artifacts.py create mode 100644 tests/unit/test_artifacts_main.py create mode 100644 tests/unit/test_task_artifact_store.py diff --git a/.ai/active/SPRINT_PACKET.md b/.ai/active/SPRINT_PACKET.md index 5f56f34..9c1979c 100644 --- a/.ai/active/SPRINT_PACKET.md +++ b/.ai/active/SPRINT_PACKET.md @@ -2,115 +2,124 @@ ## Sprint Title -Sprint 5B: Project Truth Compaction 01 +Sprint 5C: Task Artifact Records and Registration ## Sprint Type -refactor +feature ## Sprint Reason -The live project-truth files are now materially stale and redundant relative to the accepted repo state through Sprint 5A. `ROADMAP.md` and `.ai/handoff/CURRENT_STATE.md` still describe the project as pre-Milestone-5 and pre-step-linked approval/execution/workspace work, which will degrade planning and review quality if not compacted now. +Milestone 5 should continue on top of the shipped task-workspace boundary. Before document ingestion or connectors can safely rely on workspaces, the repo needs explicit, reviewable artifact records tied to those workspaces instead of ad hoc filesystem assumptions. ## Sprint Intent -Compact and synchronize the live project-truth files so Control Tower, builders, and reviewers operate from a smaller, current, non-redundant source-of-truth set without changing product scope or runtime behavior. +Add durable task-artifact records plus a narrow local artifact registration path on top of existing `task_workspaces`, so later document ingestion and retrieval can consume explicit artifact metadata instead of raw workspace scanning. ## Git Instructions -- Branch Name: `codex/refactor-project-truth-compaction-01` +- Branch Name: `codex/sprint-5c-task-artifacts` - Base Branch: `main` - PR Strategy: one sprint branch, one PR, no stacked PRs unless Control Tower explicitly opens a follow-up sprint - Merge Policy: squash merge only after reviewer `PASS` and explicit Control Tower merge approval -## Why This Sprint Matters +## Why This Sprint -- `ROADMAP.md` and `.ai/handoff/CURRENT_STATE.md` are behind the accepted repo state. -- `ARCHITECTURE.md` now reflects Sprint 5A, so the other live truth artifacts need to catch up and shed stale milestone text. -- A narrow compaction sprint is safer than letting outdated truth leak into future planning or review work. -- This restores clean project truth without changing product scope. +- Sprint 5A shipped deterministic rooted task-workspace provisioning. +- Sprint 5B cleaned and synchronized live project truth, so Milestone 5 planning can proceed from current facts. +- The roadmap says artifact and workspace boundaries should be explicit before document-heavy work lands. +- The narrowest next step is artifact records and registration only, not ingestion, chunking, connectors, or UI. ## In Scope -- compact and synchronize `.ai/handoff/CURRENT_STATE.md` -- compact and synchronize `ROADMAP.md` -- prune `RULES.md` only if it contains stale or duplicate guidance after truth sync -- slim `README.md` only if it duplicates active planning truth instead of onboarding -- archive stale planning/history docs into `docs/archive/` when they are no longer appropriate as live context -- update internal references so canonical files point to the right archive locations -- update `ARCHITECTURE.md` only if a stale duplicate or outdated boundary statement remains after the Sprint 5A truth sync +- Add schema and migration support for: + - `task_artifacts` +- Define typed contracts for: + - artifact registration requests + - artifact create responses + - artifact list responses + - artifact detail responses +- Implement a narrow artifact seam that: + - registers one local file path under an existing visible task workspace + - persists one user-scoped artifact record linked to that workspace and task + - validates that the artifact path stays rooted under the workspace local path + - stores explicit artifact metadata such as relative path, media type hint if supplied, and status fields needed for later ingestion + - exposes deterministic list and detail reads +- Implement the minimal API or service paths needed for: + - registering one artifact for a task workspace + - listing artifacts + - reading one artifact by id +- Add unit and integration tests for: + - artifact registration + - rooted path validation against the workspace boundary + - duplicate registration behavior for the same workspace-relative path + - per-user isolation + - stable response shape ## Out of Scope -- new product features -- source code changes unrelated to doc-link or archive-link integrity -- UI improvements -- backend refactors -- new architecture decisions unless a current truth file is factually inaccurate -- changing roadmap priorities beyond removing stale historical clutter -- artifact, document, connector, or runner implementation work +- No document ingestion. +- No chunking, embeddings, or document retrieval. +- No background scanning of workspace directories. +- No Gmail or Calendar connector scope. +- No runner-style orchestration. +- No new proxy handlers or broader side-effect expansion. +- No UI work. -## Files / Modules In Scope +## Required Deliverables -- `.ai/handoff/CURRENT_STATE.md` -- `ROADMAP.md` -- `RULES.md` only if needed for stale/duplicate guidance cleanup -- `README.md` only if needed for onboarding/truth separation -- `docs/archive/**` -- `ARCHITECTURE.md` only if duplicate or stale sections must be cleaned after truth sync - -## Constraints - -- do not delete information unless it is safely archived -- preserve historical traceability -- do not change product scope -- do not change runtime behavior -- keep canonical files concise, current, and durable -- prefer archive over deletion -- use `PRODUCT_BRIEF.md`, `ARCHITECTURE.md`, accepted build/review reports, and the implemented repo state as the truth basis - -## Relevant Rules - -- active sprint packet is the top scope boundary for implementation work -- never represent planned architecture as implemented behavior -- roadmap should be future-facing once historical milestone state has been distilled elsewhere -- rules should contain only durable reusable guidance -- when live context becomes noisy, reduce and archive instead of letting stale state accumulate - -## Design Source of Truth - -- `DESIGN_SYSTEM.md` if it is later introduced -- otherwise N/A for this sprint - -## Architecture Source of Truth - -- `ARCHITECTURE.md` +- Migration for `task_artifacts`. +- Stable artifact register/list/detail contracts. +- Minimal deterministic artifact-registration and persistence path over existing task workspaces. +- Unit and integration coverage for rooted-path safety, duplicate handling, ordering, and isolation. +- Updated `BUILD_REPORT.md` with exact verification results and explicit deferred scope. ## Acceptance Criteria -- `.ai/handoff/CURRENT_STATE.md` is concise, current, and non-redundant through Sprint 5A. -- `ROADMAP.md` no longer presents stale pre-Sprint-5 state and is future-facing from the current repo position. -- `RULES.md` contains only durable rules and no stale scope-era leftovers. -- Any stale planning/history material moved out of live context is archived under `docs/archive/`. -- All archive links and references resolve correctly. -- No product behavior, scope, or runtime code was changed. -- Control Tower can plan from a smaller, cleaner active context set after this sprint. - -## Required Tests - -- manual review of canonical files for duplication, staleness, and truth alignment -- link/path sanity check for moved archive files -- confirm no runtime or schema behavior changed -- run docs/path validation only if any existing tooling references moved files - -## Docs To Update - -- `.ai/handoff/CURRENT_STATE.md` -- `ROADMAP.md` -- `RULES.md` if needed -- `README.md` if needed -- `ARCHITECTURE.md` only if stale duplication remains - -## Definition of Done - -The live project-truth files are smaller, cleaner, and aligned to the accepted repo state through Sprint 5A; stale planning/history material is preserved in archive; and the next sprint can be planned from a trustworthy active context set. +- A client can register one artifact under an existing visible task workspace. +- Every artifact record stores a workspace-relative path and remains rooted under the persisted workspace local path. +- Duplicate registration behavior for the same workspace-relative path is deterministic and documented. +- Artifact list and detail reads are deterministic and user-scoped. +- `./.venv/bin/python -m pytest tests/unit` passes. +- `./.venv/bin/python -m pytest tests/integration` passes. +- No document ingestion, connector, runner, handler-expansion, or broader side-effect scope enters the sprint. + +## Implementation Constraints + +- Keep the artifact seam narrow and boring. +- Reuse the existing `task_workspaces` boundary; do not invent a parallel storage contract. +- Prefer explicit artifact registration over implicit directory scanning in this sprint. +- Keep rooted-path validation deterministic and local-filesystem-only. +- Do not parse, chunk, embed, or retrieve artifact contents in the same sprint. + +## Suggested Work Breakdown + +1. Add `task_artifacts` schema and migration. +2. Define artifact register/list/detail contracts. +3. Implement deterministic rooted artifact-path validation against the persisted workspace path. +4. Implement artifact registration, list, and detail behavior. +5. Add unit and integration tests. +6. Update `BUILD_REPORT.md` with executed verification. + +## Build Report Requirements + +`BUILD_REPORT.md` must include: +- the exact artifact schema and contract changes introduced +- the artifact-path rooting and duplicate-handling rule used +- exact commands run +- unit and integration test results +- one example artifact registration response +- one example artifact detail response +- what remains intentionally deferred to later milestones + +## Review Focus + +`REVIEW_REPORT.md` should verify: +- the sprint stayed limited to task artifact records and registration +- artifact paths are deterministic, rooted safely under existing workspaces, and user-scoped +- duplicate handling, ordering, and isolation are test-backed +- no hidden document ingestion, connector, runner, UI, handler-expansion, or broader side-effect scope entered the sprint + +## Exit Condition + +This sprint is complete when the repo can persist deterministic user-scoped task-artifact records rooted under existing task workspaces, expose stable artifact reads, and verify the full path with Postgres-backed tests, while still deferring document ingestion, retrieval, and connector work. diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 9cefafb..f097b55 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -2,16 +2,16 @@ ## Current Implemented Slice -AliceBot now implements the accepted repo slice through Sprint 5A. The shipped backend includes: +AliceBot now implements the accepted repo slice through Sprint 5C. The shipped backend includes: - foundation continuity storage over `users`, `threads`, `sessions`, and append-only `events` - deterministic tracing and context compilation over durable continuity, memory, entity, and entity-edge records - governed memory admission, explicit-preference extraction, memory review labels, review queue reads, evaluation summary reads, explicit embedding config and memory-embedding storage, direct semantic retrieval, and deterministic hybrid compile-path memory merge - deterministic prompt assembly and one no-tools response path that persists assistant replies as immutable continuity events - user-scoped consents, policies, policy evaluation, tool registry, allowlist evaluation, tool routing, approval request persistence, approval resolution, approved-only proxy execution through the in-process `proxy.echo` handler, durable execution review, and execution-budget lifecycle plus enforcement -- durable `tasks`, `task_steps`, and `task_workspaces`, deterministic task-step sequencing, explicit task-step transitions, explicit manual continuation with lineage through `parent_step_id`, `source_approval_id`, and `source_execution_id`, explicit `tool_executions.task_step_id` linkage for execution synchronization, and deterministic rooted local task-workspace provisioning +- durable `tasks`, `task_steps`, `task_workspaces`, and `task_artifacts`, deterministic task-step sequencing, explicit task-step transitions, explicit manual continuation with lineage through `parent_step_id`, `source_approval_id`, and `source_execution_id`, explicit `tool_executions.task_step_id` linkage for execution synchronization, deterministic rooted local task-workspace provisioning, and explicit rooted local artifact registration plus deterministic artifact reads -The current multi-step boundary is narrow and explicit. Manual continuation is implemented and review-passed. Approval resolution and proxy execution now both use explicit task-step linkage rather than first-step inference. Task workspaces are now implemented only as deterministic rooted local boundaries. Broader runner-style orchestration, automatic multi-step progression, artifact indexing, document ingestion, connectors, and new side-effect surfaces are still planned later and must not be described as live behavior. +The current multi-step boundary is narrow and explicit. Manual continuation is implemented and review-passed. Approval resolution and proxy execution now both use explicit task-step linkage rather than first-step inference. Task workspaces are now implemented only as deterministic rooted local boundaries, and task artifacts are now implemented only as explicit rooted local-file registrations under those workspaces. Broader runner-style orchestration, automatic multi-step progression, artifact indexing, document ingestion, connectors, and new side-effect surfaces are still planned later and must not be described as live behavior. ## Implemented Now @@ -24,7 +24,7 @@ The current multi-step boundary is narrow and explicit. Manual continuation is i - memory and retrieval: `POST /v0/memories/admit`, `POST /v0/memories/extract-explicit-preferences`, `GET /v0/memories`, `GET /v0/memories/review-queue`, `GET /v0/memories/evaluation-summary`, `POST /v0/memories/semantic-retrieval`, `GET /v0/memories/{memory_id}`, `GET /v0/memories/{memory_id}/revisions`, `POST /v0/memories/{memory_id}/labels`, `GET /v0/memories/{memory_id}/labels` - embeddings and graph seams: `POST /v0/embedding-configs`, `GET /v0/embedding-configs`, `POST /v0/memory-embeddings`, `GET /v0/memories/{memory_id}/embeddings`, `GET /v0/memory-embeddings/{memory_embedding_id}`, `POST /v0/entities`, `GET /v0/entities`, `GET /v0/entities/{entity_id}`, `POST /v0/entity-edges`, `GET /v0/entities/{entity_id}/edges` - governance: `POST /v0/consents`, `GET /v0/consents`, `POST /v0/policies`, `GET /v0/policies`, `GET /v0/policies/{policy_id}`, `POST /v0/policies/evaluate`, `POST /v0/tools`, `GET /v0/tools`, `GET /v0/tools/{tool_id}`, `POST /v0/tools/allowlist/evaluate`, `POST /v0/tools/route`, `POST /v0/approvals/requests`, `GET /v0/approvals`, `GET /v0/approvals/{approval_id}`, `POST /v0/approvals/{approval_id}/approve`, `POST /v0/approvals/{approval_id}/reject`, `POST /v0/approvals/{approval_id}/execute` - - task and execution review: `GET /v0/tasks`, `GET /v0/tasks/{task_id}`, `POST /v0/tasks/{task_id}/workspace`, `GET /v0/task-workspaces`, `GET /v0/task-workspaces/{task_workspace_id}`, `GET /v0/tasks/{task_id}/steps`, `GET /v0/task-steps/{task_step_id}`, `POST /v0/tasks/{task_id}/steps`, `POST /v0/task-steps/{task_step_id}/transition`, `POST /v0/execution-budgets`, `GET /v0/execution-budgets`, `GET /v0/execution-budgets/{execution_budget_id}`, `POST /v0/execution-budgets/{execution_budget_id}/deactivate`, `POST /v0/execution-budgets/{execution_budget_id}/supersede`, `GET /v0/tool-executions`, `GET /v0/tool-executions/{execution_id}` + - task and execution review: `GET /v0/tasks`, `GET /v0/tasks/{task_id}`, `POST /v0/tasks/{task_id}/workspace`, `GET /v0/task-workspaces`, `GET /v0/task-workspaces/{task_workspace_id}`, `POST /v0/task-workspaces/{task_workspace_id}/artifacts`, `GET /v0/task-artifacts`, `GET /v0/task-artifacts/{task_artifact_id}`, `GET /v0/tasks/{task_id}/steps`, `GET /v0/task-steps/{task_step_id}`, `POST /v0/tasks/{task_id}/steps`, `POST /v0/task-steps/{task_step_id}/transition`, `POST /v0/execution-budgets`, `GET /v0/execution-budgets`, `GET /v0/execution-budgets/{execution_budget_id}`, `POST /v0/execution-budgets/{execution_budget_id}/deactivate`, `POST /v0/execution-budgets/{execution_budget_id}/supersede`, `GET /v0/tool-executions`, `GET /v0/tool-executions/{execution_id}` - `apps/web` and `workers` remain starter shells only. ### Data Foundation @@ -37,7 +37,7 @@ The current multi-step boundary is narrow and explicit. Manual continuation is i - memory and retrieval tables: `memories`, `memory_revisions`, `memory_review_labels`, `embedding_configs`, `memory_embeddings` - graph tables: `entities`, `entity_edges` - governance tables: `consents`, `policies`, `tools`, `approvals`, `tool_executions`, `execution_budgets` - - task lifecycle tables: `tasks`, `task_steps`, `task_workspaces` + - task lifecycle tables: `tasks`, `task_steps`, `task_workspaces`, `task_artifacts` - `events`, `trace_events`, and `memory_revisions` are append-only by application contract and database enforcement. - `memory_review_labels` are append-only by database enforcement. - `tasks` are explicit user-scoped lifecycle records keyed to one thread and one tool, with durable request/tool snapshots, status in `pending_approval | approved | executed | denied | blocked`, and latest approval/execution pointers for the current narrow lifecycle seam. @@ -49,17 +49,18 @@ The current multi-step boundary is narrow and explicit. Manual continuation is i - Lineage fields are guarded by composite user-scoped foreign keys and a self-reference check so a step cannot cite itself as its parent. - `tool_executions` now persist an explicit `task_step_id` linked by a composite foreign key to `task_steps(id, user_id)`. - `task_workspaces` persist one active workspace record per visible task and user, store a deterministic `local_path`, and enforce that active uniqueness through a partial unique index on `(user_id, task_id)`. +- `task_artifacts` persist explicit user-scoped artifact rows linked to both `tasks` and `task_workspaces`, store `status = registered`, `ingestion_status = pending`, store only a workspace-relative `relative_path` plus optional `media_type_hint`, and enforce deterministic duplicate rejection through a unique index on `(user_id, task_workspace_id, relative_path)`. - `execution_budgets` enforce at most one active budget per `(user_id, tool_key, domain_hint)` selector scope through a partial unique index. - Per-request user context is set in the database through `app.current_user_id()`. - `TASK_WORKSPACE_ROOT` defines the only allowed base directory for workspace provisioning, and the live path rule is `resolved_root / user_id / task_id`. ### Repo Boundaries In This Slice -- `apps/api`: implemented API, store, contracts, service logic, and migrations for continuity, tracing, memory, embeddings, entities, policies, tools, approvals, proxy execution, execution budgets, tasks, task steps, and task workspaces. +- `apps/api`: implemented API, store, contracts, service logic, and migrations for continuity, tracing, memory, embeddings, entities, policies, tools, approvals, proxy execution, execution budgets, tasks, task steps, task workspaces, and task artifacts. - `apps/web`: minimal shell only; no shipped workflow UI. - `workers`: scaffold only; no background jobs or runner logic are implemented. - `infra`: local development bootstrap assets only. -- `tests`: unit and Postgres-backed integration coverage for the shipped seams above, including Sprint 4O task-step lineage/manual continuation, Sprint 4S step-linked execution synchronization, and Sprint 5A task-workspace provisioning. +- `tests`: unit and Postgres-backed integration coverage for the shipped seams above, including Sprint 4O task-step lineage/manual continuation, Sprint 4S step-linked execution synchronization, Sprint 5A task-workspace provisioning, and Sprint 5C task-artifact registration. ## Core Flows Implemented Now @@ -162,12 +163,24 @@ The current multi-step boundary is narrow and explicit. Manual continuation is i 8. `GET /v0/task-workspaces` lists visible workspaces in deterministic `created_at ASC, id ASC` order. 9. `GET /v0/task-workspaces/{task_workspace_id}` returns one user-visible workspace detail record. +### Task Artifact Registration And Reads + +1. Accept a user-scoped `POST /v0/task-workspaces/{task_workspace_id}/artifacts` request for one visible workspace. +2. Resolve the provided local file path and require it to exist as a regular file. +3. Resolve the persisted workspace `local_path` and reject registration if the file path escapes that rooted workspace boundary. +4. Persist only the workspace-relative POSIX path plus optional `media_type_hint`; no absolute artifact path is stored. +5. Lock registration for the target workspace before checking for an existing artifact with the same relative path. +6. Reject duplicate registration for the same visible `(task_workspace_id, relative_path)` deterministically. +7. Persist one `task_artifacts` row with `status = registered` and `ingestion_status = pending`. +8. `GET /v0/task-artifacts` lists visible artifact rows in deterministic `created_at ASC, id ASC` order. +9. `GET /v0/task-artifacts/{task_artifact_id}` returns one user-visible artifact detail record. + ## Security Model Implemented Now -- User-owned continuity, trace, memory, embedding, entity, governance, task, task-step, and task-workspace tables enforce row-level security. +- User-owned continuity, trace, memory, embedding, entity, governance, task, task-step, task-workspace, and task-artifact tables enforce row-level security. - The runtime role is limited to the narrow `SELECT` / `INSERT` / `UPDATE` permissions required by the shipped seams; there is no broad DDL or unrestricted table access at runtime. - Cross-user references are constrained through composite foreign keys on `(id, user_id)` where the schema needs ownership-linked joins. -- Approval, execution, memory, entity, task/task-step, and task-workspace reads all operate only inside the current user scope. +- Approval, execution, memory, entity, task/task-step, task-workspace, and task-artifact reads all operate only inside the current user scope. - Task-step manual continuation adds both schema-level and service-level lineage protection: - schema-level: user-scoped foreign keys and parent-not-self check - service-level: same-task, latest-step, visible-approval, visible-execution, and parent-outcome-match validation @@ -176,7 +189,7 @@ The current multi-step boundary is narrow and explicit. Manual continuation is i ## Testing Coverage Implemented Now - Unit and integration tests cover continuity, compiler, response generation, memory admission, review labels, review queue, embeddings, semantic retrieval, entities, policies, tools, approvals, proxy execution, execution budgets, and execution review. -- Sprint 4O, Sprint 4S, and Sprint 5A added explicit task lifecycle coverage: +- Sprint 4O, Sprint 4S, Sprint 5A, and Sprint 5C added explicit task lifecycle coverage: - migrations for `tasks`, `task_steps`, and task-step lineage - staged/backfilled migration coverage for `tool_executions.task_step_id` - task and task-step store contracts @@ -189,6 +202,10 @@ The current multi-step boundary is narrow and explicit. Manual continuation is i - workspace create/list/detail response shape - duplicate active workspace rejection - task-workspace per-user isolation + - artifact register/list/detail response shape + - rooted artifact-path enforcement beneath the persisted workspace path + - duplicate artifact registration rejection for the same workspace-relative path + - task-artifact per-user isolation - trace visibility for continuation and transition events - user isolation for task and task-step reads and mutations - adversarial lineage validation for cross-task, cross-user, and parent-step mismatch cases @@ -198,7 +215,7 @@ The current multi-step boundary is narrow and explicit. Manual continuation is i The following areas remain planned later and must not be described as implemented: - runner-style orchestration and automatic multi-step progression beyond the current explicit manual continuation seam -- artifact storage, artifact indexing, and document ingestion beyond the current rooted local workspace boundary +- artifact indexing, artifact content processing, and document ingestion beyond the current explicit rooted local registration boundary - read-only Gmail and Calendar connectors - broader tool proxying and real-world side effects beyond the current no-I/O `proxy.echo` handler - model-driven extraction, reranking, and broader memory review automation diff --git a/BUILD_REPORT.md b/BUILD_REPORT.md index 1589baf..a8fde01 100644 --- a/BUILD_REPORT.md +++ b/BUILD_REPORT.md @@ -2,47 +2,127 @@ ## sprint objective -Compact and synchronize the live project-truth files so the active docs reflect the accepted repo state through Sprint 5A, reduce redundancy, and move stale sprint-history material out of the live context set. +Implement Sprint 5C: Task Artifact Records and Registration by adding durable user-scoped `task_artifacts` records on top of existing `task_workspaces`, plus a narrow rooted local-file registration path and deterministic artifact reads. ## completed work -- Rewrote `.ai/handoff/CURRENT_STATE.md` into a compact current-state snapshot with canonical truth pointers, implemented boundaries, non-implemented boundaries, active risks, and accepted Sprint 5A verification. -- Rewrote `ROADMAP.md` so it is future-facing from the current repo position instead of repeating milestone-history detail. -- Pruned `RULES.md` down to durable reusable scope, safety, architecture, data, and testing rules. -- Slimmed `README.md` to onboarding and current-slice orientation, removing stale sprint-by-sprint implementation narration. -- Archived the prior Sprint 5A build and review reports under `docs/archive/sprints/`. -- Removed `REVIEW_REPORT.md` from the repo root and updated live references to point at the archive location. -- Left `ARCHITECTURE.md` unchanged because it was already aligned to the accepted Sprint 5A state. +- Added `task_artifacts` schema via `apps/api/alembic/versions/20260313_0023_task_artifacts.py`. +- Added artifact contracts in `apps/api/src/alicebot_api/contracts.py`: + - `TaskArtifactRegisterInput` + - `TaskArtifactRecord` + - `TaskArtifactCreateResponse` + - `TaskArtifactListResponse` + - `TaskArtifactDetailResponse` + - `TaskArtifactStatus = "registered"` + - `TaskArtifactIngestionStatus = "pending"` + - `TASK_ARTIFACT_LIST_ORDER = ["created_at_asc", "id_asc"]` +- Added artifact persistence and reads in `apps/api/src/alicebot_api/store.py`: + - `TaskArtifactRow` + - create/get/list/duplicate-lookup methods + - advisory lock for per-workspace artifact registration +- Added narrow artifact registration service in `apps/api/src/alicebot_api/artifacts.py`. +- Added API routes in `apps/api/src/alicebot_api/main.py`: + - `POST /v0/task-workspaces/{task_workspace_id}/artifacts` + - `GET /v0/task-artifacts` + - `GET /v0/task-artifacts/{task_artifact_id}` +- Added unit and integration coverage for rooted path validation, duplicate behavior, deterministic ordering, response shape, migration coverage, and per-user isolation. + +Artifact schema introduced: + +- `id uuid PRIMARY KEY` +- `user_id uuid NOT NULL` +- `task_id uuid NOT NULL` +- `task_workspace_id uuid NOT NULL` +- `status text NOT NULL CHECK (status IN ('registered'))` +- `ingestion_status text NOT NULL CHECK (ingestion_status IN ('pending'))` +- `relative_path text NOT NULL` +- `media_type_hint text NULL` +- `created_at timestamptz NOT NULL` +- `updated_at timestamptz NOT NULL` +- foreign keys to `(tasks.id, user_id)` and `(task_workspaces.id, user_id)` +- unique index on `(user_id, task_workspace_id, relative_path)` +- user-owned RLS policy with runtime grants limited to `SELECT, INSERT` + +Artifact-path rooting and duplicate-handling rule: + +- Registration accepts one existing regular file path. +- The file path is resolved locally and must stay rooted under the persisted workspace `local_path`. +- The persisted record stores only the workspace-relative POSIX path, not an absolute artifact path. +- Duplicate registration for the same `(user_id, task_workspace_id, relative_path)` is rejected with HTTP `409`. + +Example artifact registration response: + +```json +{ + "artifact": { + "id": "11111111-1111-1111-1111-111111111111", + "task_id": "22222222-2222-2222-2222-222222222222", + "task_workspace_id": "33333333-3333-3333-3333-333333333333", + "status": "registered", + "ingestion_status": "pending", + "relative_path": "docs/spec.txt", + "media_type_hint": "text/plain", + "created_at": "2026-03-13T10:00:00+00:00", + "updated_at": "2026-03-13T10:00:00+00:00" + } +} +``` + +Example artifact detail response: + +```json +{ + "artifact": { + "id": "11111111-1111-1111-1111-111111111111", + "task_id": "22222222-2222-2222-2222-222222222222", + "task_workspace_id": "33333333-3333-3333-3333-333333333333", + "status": "registered", + "ingestion_status": "pending", + "relative_path": "docs/spec.txt", + "media_type_hint": "text/plain", + "created_at": "2026-03-13T10:00:00+00:00", + "updated_at": "2026-03-13T10:00:00+00:00" + } +} +``` ## incomplete work -- None within Sprint 5B scope. +- None within Sprint 5C scope. ## files changed -- `.ai/handoff/CURRENT_STATE.md` -- `ROADMAP.md` -- `RULES.md` -- `README.md` -- `docs/archive/sprints/2026-03-13-sprint-5a-build-report.md` -- `docs/archive/sprints/2026-03-13-sprint-5a-review-report.md` -- `REVIEW_REPORT.md` (removed from live root) +- `apps/api/alembic/versions/20260313_0023_task_artifacts.py` +- `apps/api/src/alicebot_api/artifacts.py` +- `apps/api/src/alicebot_api/contracts.py` +- `apps/api/src/alicebot_api/main.py` +- `apps/api/src/alicebot_api/store.py` +- `tests/integration/test_migrations.py` +- `tests/integration/test_task_artifacts_api.py` +- `tests/unit/test_20260313_0023_task_artifacts.py` +- `tests/unit/test_artifacts.py` +- `tests/unit/test_artifacts_main.py` +- `tests/unit/test_main.py` +- `tests/unit/test_task_artifact_store.py` - `BUILD_REPORT.md` ## tests run -- Manual review of `ARCHITECTURE.md`, `PRODUCT_BRIEF.md`, the live truth files, and the archived Sprint 5A reports for truth alignment and duplication. -- `rg -n "REVIEW_REPORT|docs/archive/sprints|CURRENT_STATE|ROADMAP|RULES" .` -- `find docs/archive -maxdepth 3 -type f | sort` +- `./.venv/bin/python -m pytest tests/unit/test_artifacts.py tests/unit/test_artifacts_main.py tests/unit/test_task_artifact_store.py tests/unit/test_20260313_0023_task_artifacts.py tests/unit/test_main.py tests/integration/test_task_artifacts_api.py tests/integration/test_migrations.py` + - result: `54 passed, 3 errors` + - note: the 3 errors were sandboxed local-Postgres connection failures before rerunning with elevated access +- `./.venv/bin/python -m pytest tests/unit` + - result: `332 passed` +- `./.venv/bin/python -m pytest tests/integration` + - result: `100 passed in 29.55s` - `git diff --check` -- `git diff --stat -- .ai/handoff/CURRENT_STATE.md ROADMAP.md RULES.md README.md docs/archive/sprints BUILD_REPORT.md REVIEW_REPORT.md` -- `test -f docs/archive/sprints/2026-03-13-sprint-5a-build-report.md && test -f docs/archive/sprints/2026-03-13-sprint-5a-review-report.md && test ! -f REVIEW_REPORT.md && echo ok` + - result: passed ## blockers/issues -- No implementation blockers. -- No runtime or schema tests were run because this sprint was intentionally docs-only and the sprint packet required manual truth review plus path/link sanity checks, not behavior changes. +- No remaining implementation blockers. +- Document ingestion, chunking, embeddings over artifacts, retrieval, connectors, scanning, UI work, and broader side effects remain intentionally deferred. ## recommended next step -Plan the next sprint from the compact live truth set now in `PRODUCT_BRIEF.md`, `ARCHITECTURE.md`, `ROADMAP.md`, `RULES.md`, and `.ai/handoff/CURRENT_STATE.md`; pull historical detail from `docs/archive/sprints/` only when needed. +Build the next milestone on top of these explicit artifact records by adding a separate ingestion workflow that consumes `task_artifacts` metadata without weakening the task-workspace boundary or reintroducing implicit directory scanning. diff --git a/REVIEW_REPORT.md b/REVIEW_REPORT.md index ce0e041..1863373 100644 --- a/REVIEW_REPORT.md +++ b/REVIEW_REPORT.md @@ -6,42 +6,53 @@ PASS ## criteria met -- `.ai/handoff/CURRENT_STATE.md` is materially smaller, current through Sprint 5A, and now points readers to the canonical live truth files instead of repeating stale milestone-state detail. -- `ROADMAP.md` is future-facing from the shipped Sprint 5A position and no longer presents the repo as pre-Milestone-5. -- `RULES.md` was pruned to durable reusable guidance only; stale sprint-era scope narration was removed. -- `README.md` is now focused on onboarding, current slice orientation, and canonical file pointers instead of duplicating active planning truth. -- Stale sprint-history material was archived under `docs/archive/sprints/`, and the archived Sprint 5A build/review reports were preserved intact. -- Archive references resolve correctly in the live docs, and the expected archive files exist on disk. -- No product behavior, schema, or runtime code changed in the tracked diff; the modified tracked files are docs only. -- The live truth set is smaller and cleaner, and it now aligns with the implemented Sprint 5A state already described in `ARCHITECTURE.md`. +- Added the required `task_artifacts` migration and kept the schema narrow: user-scoped rows linked to both `tasks` and `task_workspaces`, explicit status fields, a unique `(user_id, task_workspace_id, relative_path)` constraint, and RLS-enabled runtime access limited to `SELECT, INSERT`. +- Added stable typed contracts for artifact registration, create, list, and detail responses in `apps/api/src/alicebot_api/contracts.py`. +- Implemented the required narrow API seam: + - `POST /v0/task-workspaces/{task_workspace_id}/artifacts` + - `GET /v0/task-artifacts` + - `GET /v0/task-artifacts/{task_artifact_id}` +- Registration is rooted safely under the persisted workspace path: the local file path is resolved, required to exist as a regular file, checked against the resolved workspace root, and persisted only as a workspace-relative POSIX path. +- Duplicate registration behavior is deterministic and documented in code/tests: the same workspace-relative path returns HTTP `409`. +- Artifact reads are deterministic and user-scoped: list order is `created_at ASC, id ASC`, detail/list reads run inside the current user DB scope, and isolation is covered by integration tests. +- The sprint stayed within scope. No ingestion, chunking, retrieval, connector, runner, UI, or broader side-effect work entered the diff. +- `BUILD_REPORT.md` includes the schema summary, contract changes, duplicate/rooting rules, exact commands, example responses, test results, and deferred scope. +- Verification passed: + - `./.venv/bin/python -m pytest tests/unit` -> `332 passed` + - `./.venv/bin/python -m pytest tests/integration` -> `100 passed in 27.20s` + - `git diff --check` -> passed ## criteria missed -- None. +- None on the sprint packet’s functional acceptance criteria. ## quality issues -- None blocking. -- Process note: the prior root `REVIEW_REPORT.md` was correctly archived and removed from the live context set; this file is the new current review artifact for Sprint 5B. +- None blocking in the implementation reviewed. ## regression risks -- Low risk. The tracked diff is docs-only, and `git diff --name-only -- '*.py' '*.ts' '*.tsx' '*.js' '*.jsx' '*.sql' '*.yaml' '*.yml' '*.toml' '*.json' '*.sh' 'Dockerfile*'` returned no runtime-file changes. -- The main residual risk is future doc drift if later sprints re-expand live truth files instead of continuing to archive stale history. +- Low. The change is narrowly scoped to artifact registration/read behavior on top of the existing workspace seam. +- The follow-up change is documentation-only and aligns `ARCHITECTURE.md` with the already reviewed implementation. ## docs issues -- None. The live docs are internally consistent with the accepted Sprint 5A architecture and with the archived sprint history. +- None blocking. The previous architecture drift is now fixed. +- `RULES.md` does not appear to need changes for this sprint. ## should anything be added to RULES.md? -- No. The revised rules already capture the durable guidance this sprint was meant to preserve: truth accuracy, archive-over-delete, scope control, and testing expectations. +- No. The existing rules already cover the durable guidance this sprint exercised: narrow scope, typed contracts, migration-backed schemas, RLS on user-owned tables, and test-backed delivery. ## should anything update ARCHITECTURE.md? -- No. `ARCHITECTURE.md` already matches the accepted Sprint 5A implemented boundary, and this sprint appropriately treated it as the architecture source of truth. +- No additional update is required for this sprint. The follow-up doc change now reflects the shipped Sprint 5C boundary: + - `task_artifacts` is listed in the implemented data model + - the artifact register/list/detail endpoints are listed in the live API surface + - the rooted registration and duplicate-rejection rules are documented + - deferred scope is narrowed to indexing/content processing/ingestion beyond the current registration seam ## recommended next action -- Accept Sprint 5B and plan the next sprint from `PRODUCT_BRIEF.md`, `ARCHITECTURE.md`, `ROADMAP.md`, `RULES.md`, and `.ai/handoff/CURRENT_STATE.md`. -- Keep future sprint build/review artifacts under `docs/archive/sprints/` so the live context set stays compact. +- Accept Sprint 5C. +- Keep later milestone work focused on artifact indexing and ingestion as a separate seam on top of these explicit artifact records. diff --git a/apps/api/alembic/versions/20260313_0023_task_artifacts.py b/apps/api/alembic/versions/20260313_0023_task_artifacts.py new file mode 100644 index 0000000..a3d32c1 --- /dev/null +++ b/apps/api/alembic/versions/20260313_0023_task_artifacts.py @@ -0,0 +1,87 @@ +"""Add user-scoped task artifact records.""" + +from __future__ import annotations + +from alembic import op + + +revision = "20260313_0023" +down_revision = "20260313_0022" +branch_labels = None +depends_on = None + +_RLS_TABLES = ("task_artifacts",) + +_UPGRADE_SCHEMA_STATEMENT = """ + CREATE TABLE task_artifacts ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + user_id uuid NOT NULL REFERENCES users(id) ON DELETE CASCADE, + task_id uuid NOT NULL, + task_workspace_id uuid NOT NULL, + status text NOT NULL, + ingestion_status text NOT NULL, + relative_path text NOT NULL, + media_type_hint text, + created_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now(), + UNIQUE (id, user_id), + CONSTRAINT task_artifacts_task_user_fk + FOREIGN KEY (task_id, user_id) + REFERENCES tasks(id, user_id) + ON DELETE CASCADE, + CONSTRAINT task_artifacts_workspace_user_fk + FOREIGN KEY (task_workspace_id, user_id) + REFERENCES task_workspaces(id, user_id) + ON DELETE CASCADE, + CONSTRAINT task_artifacts_status_check + CHECK (status IN ('registered')), + CONSTRAINT task_artifacts_ingestion_status_check + CHECK (ingestion_status IN ('pending')), + CONSTRAINT task_artifacts_relative_path_nonempty_check + CHECK (length(relative_path) > 0), + CONSTRAINT task_artifacts_media_type_hint_nonempty_check + CHECK (media_type_hint IS NULL OR length(media_type_hint) > 0) + ); + + CREATE INDEX task_artifacts_user_created_idx + ON task_artifacts (user_id, created_at, id); + + CREATE UNIQUE INDEX task_artifacts_workspace_relative_path_idx + ON task_artifacts (user_id, task_workspace_id, relative_path); + """ + +_UPGRADE_GRANT_STATEMENTS = ( + "GRANT SELECT, INSERT ON task_artifacts TO alicebot_app", +) + +_UPGRADE_POLICY_STATEMENT = """ + CREATE POLICY task_artifacts_is_owner ON task_artifacts + USING (user_id = app.current_user_id()) + WITH CHECK (user_id = app.current_user_id()); + """ + +_DOWNGRADE_STATEMENTS = ( + "DROP TABLE IF EXISTS task_artifacts", +) + + +def _execute_statements(statements: tuple[str, ...]) -> None: + for statement in statements: + op.execute(statement) + + +def _enable_row_level_security() -> None: + for table_name in _RLS_TABLES: + op.execute(f"ALTER TABLE {table_name} ENABLE ROW LEVEL SECURITY") + op.execute(f"ALTER TABLE {table_name} FORCE ROW LEVEL SECURITY") + + +def upgrade() -> None: + op.execute(_UPGRADE_SCHEMA_STATEMENT) + _execute_statements(_UPGRADE_GRANT_STATEMENTS) + _enable_row_level_security() + op.execute(_UPGRADE_POLICY_STATEMENT) + + +def downgrade() -> None: + _execute_statements(_DOWNGRADE_STATEMENTS) diff --git a/apps/api/src/alicebot_api/artifacts.py b/apps/api/src/alicebot_api/artifacts.py new file mode 100644 index 0000000..c91f77c --- /dev/null +++ b/apps/api/src/alicebot_api/artifacts.py @@ -0,0 +1,173 @@ +from __future__ import annotations + +from pathlib import Path +from typing import cast +from uuid import UUID + +import psycopg + +from alicebot_api.contracts import ( + TASK_ARTIFACT_LIST_ORDER, + TaskArtifactCreateResponse, + TaskArtifactDetailResponse, + TaskArtifactListResponse, + TaskArtifactRecord, + TaskArtifactRegisterInput, + TaskArtifactStatus, + TaskArtifactIngestionStatus, +) +from alicebot_api.store import ContinuityStore, TaskArtifactRow +from alicebot_api.workspaces import TaskWorkspaceNotFoundError + + +class TaskArtifactNotFoundError(LookupError): + """Raised when a task artifact is not visible inside the current user scope.""" + + +class TaskArtifactAlreadyExistsError(RuntimeError): + """Raised when the same workspace-relative artifact path is registered twice.""" + + +class TaskArtifactValidationError(ValueError): + """Raised when a local artifact path cannot satisfy registration constraints.""" + + +def resolve_artifact_path(local_path: str) -> Path: + return Path(local_path).expanduser().resolve() + + +def ensure_artifact_path_is_rooted(*, workspace_path: Path, artifact_path: Path) -> None: + resolved_workspace_path = workspace_path.resolve() + resolved_artifact_path = artifact_path.resolve() + try: + resolved_artifact_path.relative_to(resolved_workspace_path) + except ValueError as exc: + raise TaskArtifactValidationError( + f"artifact path {resolved_artifact_path} escapes workspace root {resolved_workspace_path}" + ) from exc + + +def build_workspace_relative_artifact_path(*, workspace_path: Path, artifact_path: Path) -> str: + relative_path = artifact_path.relative_to(workspace_path).as_posix() + if relative_path in ("", "."): + raise TaskArtifactValidationError( + f"artifact path {artifact_path} must point to a file beneath workspace root {workspace_path}" + ) + return relative_path + + +def _require_existing_file(artifact_path: Path) -> None: + if not artifact_path.exists(): + raise TaskArtifactValidationError(f"artifact path {artifact_path} was not found") + if not artifact_path.is_file(): + raise TaskArtifactValidationError(f"artifact path {artifact_path} is not a regular file") + + +def _duplicate_registration_message(*, task_workspace_id: UUID, relative_path: str) -> str: + return ( + f"artifact {relative_path} is already registered for task workspace {task_workspace_id}" + ) + + +def serialize_task_artifact_row(row: TaskArtifactRow) -> TaskArtifactRecord: + return { + "id": str(row["id"]), + "task_id": str(row["task_id"]), + "task_workspace_id": str(row["task_workspace_id"]), + "status": cast(TaskArtifactStatus, row["status"]), + "ingestion_status": cast(TaskArtifactIngestionStatus, row["ingestion_status"]), + "relative_path": row["relative_path"], + "media_type_hint": row["media_type_hint"], + "created_at": row["created_at"].isoformat(), + "updated_at": row["updated_at"].isoformat(), + } + + +def register_task_artifact_record( + store: ContinuityStore, + *, + user_id: UUID, + request: TaskArtifactRegisterInput, +) -> TaskArtifactCreateResponse: + del user_id + + workspace = store.get_task_workspace_optional(request.task_workspace_id) + if workspace is None: + raise TaskWorkspaceNotFoundError( + f"task workspace {request.task_workspace_id} was not found" + ) + + workspace_path = Path(workspace["local_path"]).expanduser().resolve() + artifact_path = resolve_artifact_path(request.local_path) + _require_existing_file(artifact_path) + ensure_artifact_path_is_rooted( + workspace_path=workspace_path, + artifact_path=artifact_path, + ) + relative_path = build_workspace_relative_artifact_path( + workspace_path=workspace_path, + artifact_path=artifact_path, + ) + + store.lock_task_artifacts(workspace["id"]) + existing = store.get_task_artifact_by_workspace_relative_path_optional( + task_workspace_id=workspace["id"], + relative_path=relative_path, + ) + if existing is not None: + raise TaskArtifactAlreadyExistsError( + _duplicate_registration_message( + task_workspace_id=workspace["id"], + relative_path=relative_path, + ) + ) + + try: + row = store.create_task_artifact( + task_id=workspace["task_id"], + task_workspace_id=workspace["id"], + status="registered", + ingestion_status="pending", + relative_path=relative_path, + media_type_hint=request.media_type_hint, + ) + except psycopg.errors.UniqueViolation as exc: + raise TaskArtifactAlreadyExistsError( + _duplicate_registration_message( + task_workspace_id=workspace["id"], + relative_path=relative_path, + ) + ) from exc + + return {"artifact": serialize_task_artifact_row(row)} + + +def list_task_artifact_records( + store: ContinuityStore, + *, + user_id: UUID, +) -> TaskArtifactListResponse: + del user_id + + items = [serialize_task_artifact_row(row) for row in store.list_task_artifacts()] + return { + "items": items, + "summary": { + "total_count": len(items), + "order": list(TASK_ARTIFACT_LIST_ORDER), + }, + } + + +def get_task_artifact_record( + store: ContinuityStore, + *, + user_id: UUID, + task_artifact_id: UUID, +) -> TaskArtifactDetailResponse: + del user_id + + row = store.get_task_artifact_optional(task_artifact_id) + if row is None: + raise TaskArtifactNotFoundError(f"task artifact {task_artifact_id} was not found") + return {"artifact": serialize_task_artifact_row(row)} diff --git a/apps/api/src/alicebot_api/contracts.py b/apps/api/src/alicebot_api/contracts.py index fc794c2..06113c4 100644 --- a/apps/api/src/alicebot_api/contracts.py +++ b/apps/api/src/alicebot_api/contracts.py @@ -20,6 +20,8 @@ ApprovalResolutionOutcome = Literal["resolved", "duplicate_rejected", "conflict_rejected"] TaskStatus = Literal["pending_approval", "approved", "executed", "denied", "blocked"] TaskWorkspaceStatus = Literal["active"] +TaskArtifactStatus = Literal["registered"] +TaskArtifactIngestionStatus = Literal["pending"] TaskLifecycleSource = Literal[ "approval_request", "approval_resolution", @@ -129,6 +131,7 @@ APPROVAL_LIST_ORDER = ["created_at_asc", "id_asc"] TASK_LIST_ORDER = ["created_at_asc", "id_asc"] TASK_WORKSPACE_LIST_ORDER = ["created_at_asc", "id_asc"] +TASK_ARTIFACT_LIST_ORDER = ["created_at_asc", "id_asc"] TASK_STEP_LIST_ORDER = ["sequence_no_asc", "created_at_asc", "id_asc"] TOOL_EXECUTION_LIST_ORDER = ["executed_at_asc", "id_asc"] EXECUTION_BUDGET_LIST_ORDER = ["created_at_asc", "id_asc"] @@ -136,6 +139,8 @@ EXECUTION_BUDGET_STATUSES = ["active", "inactive", "superseded"] TASK_STATUSES = ["pending_approval", "approved", "executed", "denied", "blocked"] TASK_WORKSPACE_STATUSES = ["active"] +TASK_ARTIFACT_STATUSES = ["registered"] +TASK_ARTIFACT_INGESTION_STATUSES = ["pending"] TASK_STEP_KINDS = ["governed_request"] TASK_STEP_STATUSES = ["created", "approved", "executed", "blocked", "denied"] APPROVAL_REQUEST_VERSION_V0 = "approval_request_v0" @@ -1594,6 +1599,43 @@ class TaskWorkspaceDetailResponse(TypedDict): workspace: TaskWorkspaceRecord +@dataclass(frozen=True, slots=True) +class TaskArtifactRegisterInput: + task_workspace_id: UUID + local_path: str + media_type_hint: str | None = None + + +class TaskArtifactRecord(TypedDict): + id: str + task_id: str + task_workspace_id: str + status: TaskArtifactStatus + ingestion_status: TaskArtifactIngestionStatus + relative_path: str + media_type_hint: str | None + created_at: str + updated_at: str + + +class TaskArtifactCreateResponse(TypedDict): + artifact: TaskArtifactRecord + + +class TaskArtifactListSummary(TypedDict): + total_count: int + order: list[str] + + +class TaskArtifactListResponse(TypedDict): + items: list[TaskArtifactRecord] + summary: TaskArtifactListSummary + + +class TaskArtifactDetailResponse(TypedDict): + artifact: TaskArtifactRecord + + class TaskStepTraceLink(TypedDict): trace_id: str trace_kind: str diff --git a/apps/api/src/alicebot_api/main.py b/apps/api/src/alicebot_api/main.py index 764812d..5bce9bf 100644 --- a/apps/api/src/alicebot_api/main.py +++ b/apps/api/src/alicebot_api/main.py @@ -50,6 +50,7 @@ ProxyExecutionStatus, ToolAllowlistEvaluationRequestInput, ProxyExecutionRequestInput, + TaskArtifactRegisterInput, TaskStepKind, TaskStepLineageInput, TaskStepNextCreateInput, @@ -60,6 +61,14 @@ ToolRoutingRequestInput, ToolCreateInput, ) +from alicebot_api.artifacts import ( + TaskArtifactAlreadyExistsError, + TaskArtifactNotFoundError, + TaskArtifactValidationError, + get_task_artifact_record, + list_task_artifact_records, + register_task_artifact_record, +) from alicebot_api.approvals import ( ApprovalNotFoundError, ApprovalResolutionConflictError, @@ -398,6 +407,12 @@ class CreateTaskWorkspaceRequest(BaseModel): user_id: UUID +class RegisterTaskArtifactRequest(BaseModel): + user_id: UUID + local_path: str = Field(min_length=1, max_length=4000) + media_type_hint: str | None = Field(default=None, min_length=1, max_length=200) + + class TaskStepRequestSnapshot(BaseModel): thread_id: UUID tool_id: UUID @@ -1172,6 +1187,73 @@ def get_task_step(task_step_id: UUID, user_id: UUID) -> JSONResponse: ) +@app.post("/v0/task-workspaces/{task_workspace_id}/artifacts") +def register_task_artifact( + task_workspace_id: UUID, + request: RegisterTaskArtifactRequest, +) -> JSONResponse: + settings = get_settings() + + try: + with user_connection(settings.database_url, request.user_id) as conn: + payload = register_task_artifact_record( + ContinuityStore(conn), + user_id=request.user_id, + request=TaskArtifactRegisterInput( + task_workspace_id=task_workspace_id, + local_path=request.local_path, + media_type_hint=request.media_type_hint, + ), + ) + except TaskWorkspaceNotFoundError as exc: + return JSONResponse(status_code=404, content={"detail": str(exc)}) + except TaskArtifactValidationError as exc: + return JSONResponse(status_code=400, content={"detail": str(exc)}) + except TaskArtifactAlreadyExistsError as exc: + return JSONResponse(status_code=409, content={"detail": str(exc)}) + + return JSONResponse( + status_code=201, + content=jsonable_encoder(payload), + ) + + +@app.get("/v0/task-artifacts") +def list_task_artifacts(user_id: UUID) -> JSONResponse: + settings = get_settings() + + with user_connection(settings.database_url, user_id) as conn: + payload = list_task_artifact_records( + ContinuityStore(conn), + user_id=user_id, + ) + + return JSONResponse( + status_code=200, + content=jsonable_encoder(payload), + ) + + +@app.get("/v0/task-artifacts/{task_artifact_id}") +def get_task_artifact(task_artifact_id: UUID, user_id: UUID) -> JSONResponse: + settings = get_settings() + + try: + with user_connection(settings.database_url, user_id) as conn: + payload = get_task_artifact_record( + ContinuityStore(conn), + user_id=user_id, + task_artifact_id=task_artifact_id, + ) + except TaskArtifactNotFoundError as exc: + return JSONResponse(status_code=404, content={"detail": str(exc)}) + + return JSONResponse( + status_code=200, + content=jsonable_encoder(payload), + ) + + @app.post("/v0/tasks/{task_id}/steps") def create_next_task_step(task_id: UUID, request: CreateNextTaskStepRequest) -> JSONResponse: settings = get_settings() diff --git a/apps/api/src/alicebot_api/store.py b/apps/api/src/alicebot_api/store.py index 8c3b551..c849a35 100644 --- a/apps/api/src/alicebot_api/store.py +++ b/apps/api/src/alicebot_api/store.py @@ -245,6 +245,19 @@ class TaskWorkspaceRow(TypedDict): updated_at: datetime +class TaskArtifactRow(TypedDict): + id: UUID + user_id: UUID + task_id: UUID + task_workspace_id: UUID + status: str + ingestion_status: str + relative_path: str + media_type_hint: str | None + created_at: datetime + updated_at: datetime + + class TaskStepRow(TypedDict): id: UUID user_id: UUID @@ -344,6 +357,7 @@ class LabelCountRow(TypedDict): LOCK_THREAD_EVENTS_SQL = "SELECT pg_advisory_xact_lock(hashtextextended(%s::text, 0))" LOCK_TASK_STEPS_SQL = "SELECT pg_advisory_xact_lock(hashtextextended(%s::text, 2))" LOCK_TASK_WORKSPACES_SQL = "SELECT pg_advisory_xact_lock(hashtextextended(%s::text, 3))" +LOCK_TASK_ARTIFACTS_SQL = "SELECT pg_advisory_xact_lock(hashtextextended(%s::text, 4))" INSERT_EVENT_SQL = """ WITH next_sequence AS ( @@ -1416,6 +1430,93 @@ class LabelCountRow(TypedDict): ORDER BY created_at ASC, id ASC """ +INSERT_TASK_ARTIFACT_SQL = """ + INSERT INTO task_artifacts ( + user_id, + task_id, + task_workspace_id, + status, + ingestion_status, + relative_path, + media_type_hint, + created_at, + updated_at + ) + VALUES ( + app.current_user_id(), + %s, + %s, + %s, + %s, + %s, + %s, + clock_timestamp(), + clock_timestamp() + ) + RETURNING + id, + user_id, + task_id, + task_workspace_id, + status, + ingestion_status, + relative_path, + media_type_hint, + created_at, + updated_at + """ + +GET_TASK_ARTIFACT_SQL = """ + SELECT + id, + user_id, + task_id, + task_workspace_id, + status, + ingestion_status, + relative_path, + media_type_hint, + created_at, + updated_at + FROM task_artifacts + WHERE id = %s + """ + +GET_TASK_ARTIFACT_BY_WORKSPACE_RELATIVE_PATH_SQL = """ + SELECT + id, + user_id, + task_id, + task_workspace_id, + status, + ingestion_status, + relative_path, + media_type_hint, + created_at, + updated_at + FROM task_artifacts + WHERE task_workspace_id = %s + AND relative_path = %s + ORDER BY created_at ASC, id ASC + LIMIT 1 + """ + +LIST_TASK_ARTIFACTS_SQL = """ + SELECT + id, + user_id, + task_id, + task_workspace_id, + status, + ingestion_status, + relative_path, + media_type_hint, + created_at, + updated_at + FROM task_artifacts + ORDER BY created_at ASC, id ASC + """ + INSERT_TASK_STEP_SQL = """ INSERT INTO task_steps ( user_id, @@ -2505,6 +2606,50 @@ def get_active_task_workspace_for_task_optional(self, task_id: UUID) -> TaskWork def list_task_workspaces(self) -> list[TaskWorkspaceRow]: return self._fetch_all(LIST_TASK_WORKSPACES_SQL) + def lock_task_artifacts(self, task_workspace_id: UUID) -> None: + with self.conn.cursor() as cur: + cur.execute(LOCK_TASK_ARTIFACTS_SQL, (str(task_workspace_id),)) + + def create_task_artifact( + self, + *, + task_id: UUID, + task_workspace_id: UUID, + status: str, + ingestion_status: str, + relative_path: str, + media_type_hint: str | None, + ) -> TaskArtifactRow: + return self._fetch_one( + "create_task_artifact", + INSERT_TASK_ARTIFACT_SQL, + ( + task_id, + task_workspace_id, + status, + ingestion_status, + relative_path, + media_type_hint, + ), + ) + + def get_task_artifact_optional(self, task_artifact_id: UUID) -> TaskArtifactRow | None: + return self._fetch_optional_one(GET_TASK_ARTIFACT_SQL, (task_artifact_id,)) + + def get_task_artifact_by_workspace_relative_path_optional( + self, + *, + task_workspace_id: UUID, + relative_path: str, + ) -> TaskArtifactRow | None: + return self._fetch_optional_one( + GET_TASK_ARTIFACT_BY_WORKSPACE_RELATIVE_PATH_SQL, + (task_workspace_id, relative_path), + ) + + def list_task_artifacts(self) -> list[TaskArtifactRow]: + return self._fetch_all(LIST_TASK_ARTIFACTS_SQL) + def lock_task_steps(self, task_id: UUID) -> None: with self.conn.cursor() as cur: cur.execute(LOCK_TASK_STEPS_SQL, (str(task_id),)) diff --git a/tests/integration/test_migrations.py b/tests/integration/test_migrations.py index 434645e..250c270 100644 --- a/tests/integration/test_migrations.py +++ b/tests/integration/test_migrations.py @@ -299,6 +299,8 @@ def test_migrations_upgrade_and_downgrade(database_urls): assert cur.fetchone()[0] == "tasks" cur.execute("SELECT to_regclass('public.task_workspaces')") assert cur.fetchone()[0] == "task_workspaces" + cur.execute("SELECT to_regclass('public.task_artifacts')") + assert cur.fetchone()[0] == "task_artifacts" cur.execute("SELECT to_regclass('public.task_steps')") assert cur.fetchone()[0] == "task_steps" cur.execute( @@ -380,6 +382,7 @@ def test_migrations_upgrade_and_downgrade(database_urls): 'approvals', 'tasks', 'task_workspaces', + 'task_artifacts', 'task_steps', 'execution_budgets', 'tool_executions' @@ -401,6 +404,7 @@ def test_migrations_upgrade_and_downgrade(database_urls): ("memory_revisions", True, True), ("policies", True, True), ("sessions", True, True), + ("task_artifacts", True, True), ("task_steps", True, True), ("task_workspaces", True, True), ("tasks", True, True), @@ -467,6 +471,8 @@ def test_migrations_upgrade_and_downgrade(database_urls): has_table_privilege('alicebot_app', 'tasks', 'DELETE'), has_table_privilege('alicebot_app', 'task_workspaces', 'UPDATE'), has_table_privilege('alicebot_app', 'task_workspaces', 'DELETE'), + has_table_privilege('alicebot_app', 'task_artifacts', 'UPDATE'), + has_table_privilege('alicebot_app', 'task_artifacts', 'DELETE'), has_table_privilege('alicebot_app', 'task_steps', 'UPDATE'), has_table_privilege('alicebot_app', 'task_steps', 'DELETE'), has_table_privilege('alicebot_app', 'execution_budgets', 'UPDATE'), @@ -504,6 +510,8 @@ def test_migrations_upgrade_and_downgrade(database_urls): False, False, False, + False, + False, True, False, True, @@ -516,6 +524,8 @@ def test_migrations_upgrade_and_downgrade(database_urls): with psycopg.connect(database_urls["admin"]) as conn: with conn.cursor() as cur: + cur.execute("SELECT to_regclass('public.task_artifacts')") + assert cur.fetchone()[0] is None cur.execute("SELECT to_regclass('public.task_workspaces')") assert cur.fetchone()[0] is None cur.execute( diff --git a/tests/integration/test_task_artifacts_api.py b/tests/integration/test_task_artifacts_api.py new file mode 100644 index 0000000..ff78f47 --- /dev/null +++ b/tests/integration/test_task_artifacts_api.py @@ -0,0 +1,293 @@ +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any +from urllib.parse import urlencode +from uuid import UUID, uuid4 + +import anyio + +import apps.api.src.alicebot_api.main as main_module +from apps.api.src.alicebot_api.config import Settings +from alicebot_api.db import user_connection +from alicebot_api.store import ContinuityStore + + +def invoke_request( + method: str, + path: str, + *, + query_params: dict[str, str] | None = None, + payload: dict[str, Any] | None = None, +) -> tuple[int, dict[str, Any]]: + messages: list[dict[str, object]] = [] + encoded_body = b"" if payload is None else json.dumps(payload).encode() + request_received = False + + async def receive() -> dict[str, object]: + nonlocal request_received + if request_received: + return {"type": "http.disconnect"} + + request_received = True + return {"type": "http.request", "body": encoded_body, "more_body": False} + + async def send(message: dict[str, object]) -> None: + messages.append(message) + + query_string = urlencode(query_params or {}).encode() + scope = { + "type": "http", + "asgi": {"version": "3.0"}, + "http_version": "1.1", + "method": method, + "scheme": "http", + "path": path, + "raw_path": path.encode(), + "query_string": query_string, + "headers": [(b"content-type", b"application/json")], + "client": ("testclient", 50000), + "server": ("testserver", 80), + "root_path": "", + } + + anyio.run(main_module.app, scope, receive, send) + + start_message = next(message for message in messages if message["type"] == "http.response.start") + body = b"".join( + message.get("body", b"") + for message in messages + if message["type"] == "http.response.body" + ) + return start_message["status"], json.loads(body) + + +def seed_task(database_url: str, *, email: str) -> dict[str, UUID]: + user_id = uuid4() + + with user_connection(database_url, user_id) as conn: + store = ContinuityStore(conn) + store.create_user(user_id, email, email.split("@", 1)[0].title()) + thread = store.create_thread("Artifact thread") + tool = store.create_tool( + tool_key="proxy.echo", + name="Proxy Echo", + description="Deterministic proxy handler.", + version="1.0.0", + metadata_version="tool_metadata_v0", + active=True, + tags=["proxy"], + action_hints=["tool.run"], + scope_hints=["workspace"], + domain_hints=[], + risk_hints=[], + metadata={"transport": "proxy"}, + ) + task = store.create_task( + thread_id=thread["id"], + tool_id=tool["id"], + status="approved", + request={ + "thread_id": str(thread["id"]), + "tool_id": str(tool["id"]), + "action": "tool.run", + "scope": "workspace", + "domain_hint": None, + "risk_hint": None, + "attributes": {}, + }, + tool={ + "id": str(tool["id"]), + "tool_key": "proxy.echo", + "name": "Proxy Echo", + "description": "Deterministic proxy handler.", + "version": "1.0.0", + "metadata_version": "tool_metadata_v0", + "active": True, + "tags": ["proxy"], + "action_hints": ["tool.run"], + "scope_hints": ["workspace"], + "domain_hints": [], + "risk_hints": [], + "metadata": {"transport": "proxy"}, + "created_at": tool["created_at"].isoformat(), + }, + latest_approval_id=None, + latest_execution_id=None, + ) + + return { + "user_id": user_id, + "task_id": task["id"], + } + + +def test_task_artifact_endpoints_register_list_detail_isolate_and_reject_duplicates( + migrated_database_urls, + monkeypatch, + tmp_path, +) -> None: + owner = seed_task(migrated_database_urls["app"], email="owner@example.com") + intruder = seed_task(migrated_database_urls["app"], email="intruder@example.com") + workspace_root = tmp_path / "task-workspaces" + monkeypatch.setattr( + main_module, + "get_settings", + lambda: Settings( + database_url=migrated_database_urls["app"], + task_workspace_root=str(workspace_root), + ), + ) + + workspace_status, workspace_payload = invoke_request( + "POST", + f"/v0/tasks/{owner['task_id']}/workspace", + payload={"user_id": str(owner["user_id"])}, + ) + assert workspace_status == 201 + + workspace_path = Path(workspace_payload["workspace"]["local_path"]) + first_file = workspace_path / "docs" / "spec.txt" + first_file.parent.mkdir(parents=True) + first_file.write_text("spec") + second_file = workspace_path / "notes" / "plan.md" + second_file.parent.mkdir(parents=True) + second_file.write_text("plan") + outside_file = tmp_path / "escape.txt" + outside_file.write_text("escape") + + first_create_status, first_create_payload = invoke_request( + "POST", + f"/v0/task-workspaces/{workspace_payload['workspace']['id']}/artifacts", + payload={ + "user_id": str(owner["user_id"]), + "local_path": str(first_file), + "media_type_hint": "text/plain", + }, + ) + second_create_status, second_create_payload = invoke_request( + "POST", + f"/v0/task-workspaces/{workspace_payload['workspace']['id']}/artifacts", + payload={ + "user_id": str(owner["user_id"]), + "local_path": str(second_file), + "media_type_hint": "text/markdown", + }, + ) + list_status, list_payload = invoke_request( + "GET", + "/v0/task-artifacts", + query_params={"user_id": str(owner["user_id"])}, + ) + detail_status, detail_payload = invoke_request( + "GET", + f"/v0/task-artifacts/{first_create_payload['artifact']['id']}", + query_params={"user_id": str(owner["user_id"])}, + ) + duplicate_status, duplicate_payload = invoke_request( + "POST", + f"/v0/task-workspaces/{workspace_payload['workspace']['id']}/artifacts", + payload={ + "user_id": str(owner["user_id"]), + "local_path": str(first_file), + "media_type_hint": "text/plain", + }, + ) + escaped_status, escaped_payload = invoke_request( + "POST", + f"/v0/task-workspaces/{workspace_payload['workspace']['id']}/artifacts", + payload={ + "user_id": str(owner["user_id"]), + "local_path": str(outside_file), + }, + ) + isolated_list_status, isolated_list_payload = invoke_request( + "GET", + "/v0/task-artifacts", + query_params={"user_id": str(intruder["user_id"])}, + ) + isolated_detail_status, isolated_detail_payload = invoke_request( + "GET", + f"/v0/task-artifacts/{first_create_payload['artifact']['id']}", + query_params={"user_id": str(intruder["user_id"])}, + ) + isolated_create_status, isolated_create_payload = invoke_request( + "POST", + f"/v0/task-workspaces/{workspace_payload['workspace']['id']}/artifacts", + payload={ + "user_id": str(intruder["user_id"]), + "local_path": str(first_file), + }, + ) + + assert first_create_status == 201 + assert first_create_payload == { + "artifact": { + "id": first_create_payload["artifact"]["id"], + "task_id": str(owner["task_id"]), + "task_workspace_id": workspace_payload["workspace"]["id"], + "status": "registered", + "ingestion_status": "pending", + "relative_path": "docs/spec.txt", + "media_type_hint": "text/plain", + "created_at": first_create_payload["artifact"]["created_at"], + "updated_at": first_create_payload["artifact"]["updated_at"], + } + } + + assert second_create_status == 201 + assert second_create_payload == { + "artifact": { + "id": second_create_payload["artifact"]["id"], + "task_id": str(owner["task_id"]), + "task_workspace_id": workspace_payload["workspace"]["id"], + "status": "registered", + "ingestion_status": "pending", + "relative_path": "notes/plan.md", + "media_type_hint": "text/markdown", + "created_at": second_create_payload["artifact"]["created_at"], + "updated_at": second_create_payload["artifact"]["updated_at"], + } + } + + assert list_status == 200 + assert list_payload == { + "items": [ + first_create_payload["artifact"], + second_create_payload["artifact"], + ], + "summary": {"total_count": 2, "order": ["created_at_asc", "id_asc"]}, + } + + assert detail_status == 200 + assert detail_payload == {"artifact": first_create_payload["artifact"]} + + assert duplicate_status == 409 + assert duplicate_payload == { + "detail": ( + "artifact docs/spec.txt is already registered for task workspace " + f"{workspace_payload['workspace']['id']}" + ) + } + + assert escaped_status == 400 + assert escaped_payload == { + "detail": f"artifact path {outside_file.resolve()} escapes workspace root {workspace_path.resolve()}" + } + + assert isolated_list_status == 200 + assert isolated_list_payload == { + "items": [], + "summary": {"total_count": 0, "order": ["created_at_asc", "id_asc"]}, + } + + assert isolated_detail_status == 404 + assert isolated_detail_payload == { + "detail": f"task artifact {first_create_payload['artifact']['id']} was not found" + } + + assert isolated_create_status == 404 + assert isolated_create_payload == { + "detail": f"task workspace {workspace_payload['workspace']['id']} was not found" + } diff --git a/tests/unit/test_20260313_0023_task_artifacts.py b/tests/unit/test_20260313_0023_task_artifacts.py new file mode 100644 index 0000000..f6fd17d --- /dev/null +++ b/tests/unit/test_20260313_0023_task_artifacts.py @@ -0,0 +1,46 @@ +from __future__ import annotations + +import importlib + + +MODULE_NAME = "apps.api.alembic.versions.20260313_0023_task_artifacts" + + +def load_migration_module(): + return importlib.import_module(MODULE_NAME) + + +def test_upgrade_executes_expected_statements_in_order(monkeypatch) -> None: + module = load_migration_module() + executed: list[str] = [] + + monkeypatch.setattr(module.op, "execute", executed.append) + + module.upgrade() + + assert executed == [ + module._UPGRADE_SCHEMA_STATEMENT, + *module._UPGRADE_GRANT_STATEMENTS, + "ALTER TABLE task_artifacts ENABLE ROW LEVEL SECURITY", + "ALTER TABLE task_artifacts FORCE ROW LEVEL SECURITY", + module._UPGRADE_POLICY_STATEMENT, + ] + + +def test_downgrade_executes_expected_statements_in_order(monkeypatch) -> None: + module = load_migration_module() + executed: list[str] = [] + + monkeypatch.setattr(module.op, "execute", executed.append) + + module.downgrade() + + assert executed == list(module._DOWNGRADE_STATEMENTS) + + +def test_task_artifact_privileges_allow_only_expected_runtime_writes() -> None: + module = load_migration_module() + + assert module._UPGRADE_GRANT_STATEMENTS == ( + "GRANT SELECT, INSERT ON task_artifacts TO alicebot_app", + ) diff --git a/tests/unit/test_artifacts.py b/tests/unit/test_artifacts.py new file mode 100644 index 0000000..33d6fb7 --- /dev/null +++ b/tests/unit/test_artifacts.py @@ -0,0 +1,295 @@ +from __future__ import annotations + +from datetime import UTC, datetime, timedelta +from pathlib import Path +from uuid import UUID, uuid4 + +import pytest + +from alicebot_api.artifacts import ( + TaskArtifactAlreadyExistsError, + TaskArtifactNotFoundError, + TaskArtifactValidationError, + build_workspace_relative_artifact_path, + ensure_artifact_path_is_rooted, + get_task_artifact_record, + list_task_artifact_records, + register_task_artifact_record, + serialize_task_artifact_row, +) +from alicebot_api.contracts import TaskArtifactRegisterInput +from alicebot_api.workspaces import TaskWorkspaceNotFoundError + + +class ArtifactStoreStub: + def __init__(self) -> None: + self.base_time = datetime(2026, 3, 13, 10, 0, tzinfo=UTC) + self.workspaces: list[dict[str, object]] = [] + self.artifacts: list[dict[str, object]] = [] + self.locked_workspace_ids: list[UUID] = [] + + def create_task_workspace(self, *, task_workspace_id: UUID, task_id: UUID, user_id: UUID, local_path: str) -> dict[str, object]: + workspace = { + "id": task_workspace_id, + "user_id": user_id, + "task_id": task_id, + "status": "active", + "local_path": local_path, + "created_at": self.base_time, + "updated_at": self.base_time, + } + self.workspaces.append(workspace) + return workspace + + def get_task_workspace_optional(self, task_workspace_id: UUID) -> dict[str, object] | None: + return next((workspace for workspace in self.workspaces if workspace["id"] == task_workspace_id), None) + + def lock_task_artifacts(self, task_workspace_id: UUID) -> None: + self.locked_workspace_ids.append(task_workspace_id) + + def get_task_artifact_by_workspace_relative_path_optional( + self, + *, + task_workspace_id: UUID, + relative_path: str, + ) -> dict[str, object] | None: + return next( + ( + artifact + for artifact in self.artifacts + if artifact["task_workspace_id"] == task_workspace_id + and artifact["relative_path"] == relative_path + ), + None, + ) + + def create_task_artifact( + self, + *, + task_id: UUID, + task_workspace_id: UUID, + status: str, + ingestion_status: str, + relative_path: str, + media_type_hint: str | None, + ) -> dict[str, object]: + artifact = { + "id": uuid4(), + "user_id": self.workspaces[0]["user_id"], + "task_id": task_id, + "task_workspace_id": task_workspace_id, + "status": status, + "ingestion_status": ingestion_status, + "relative_path": relative_path, + "media_type_hint": media_type_hint, + "created_at": self.base_time + timedelta(minutes=len(self.artifacts)), + "updated_at": self.base_time + timedelta(minutes=len(self.artifacts)), + } + self.artifacts.append(artifact) + return artifact + + def list_task_artifacts(self) -> list[dict[str, object]]: + return sorted(self.artifacts, key=lambda artifact: (artifact["created_at"], artifact["id"])) + + def get_task_artifact_optional(self, task_artifact_id: UUID) -> dict[str, object] | None: + return next((artifact for artifact in self.artifacts if artifact["id"] == task_artifact_id), None) + + +def test_ensure_artifact_path_is_rooted_rejects_escape() -> None: + with pytest.raises(TaskArtifactValidationError, match="escapes workspace root"): + ensure_artifact_path_is_rooted( + workspace_path=Path("/tmp/alicebot/task-workspaces/user/task"), + artifact_path=Path("/tmp/alicebot/task-workspaces/user/task/../escape.txt"), + ) + + +def test_build_workspace_relative_artifact_path_returns_posix_path() -> None: + relative_path = build_workspace_relative_artifact_path( + workspace_path=Path("/tmp/alicebot/task-workspaces/user/task"), + artifact_path=Path("/tmp/alicebot/task-workspaces/user/task/docs/spec.txt"), + ) + + assert relative_path == "docs/spec.txt" + + +def test_register_task_artifact_record_persists_relative_path_and_returns_record(tmp_path) -> None: + store = ArtifactStoreStub() + user_id = uuid4() + task_id = uuid4() + task_workspace_id = uuid4() + workspace_path = tmp_path / "workspaces" / str(user_id) / str(task_id) + workspace_path.mkdir(parents=True) + artifact_path = workspace_path / "docs" / "spec.txt" + artifact_path.parent.mkdir(parents=True) + artifact_path.write_text("spec") + store.create_task_workspace( + task_workspace_id=task_workspace_id, + task_id=task_id, + user_id=user_id, + local_path=str(workspace_path), + ) + + response = register_task_artifact_record( + store, + user_id=user_id, + request=TaskArtifactRegisterInput( + task_workspace_id=task_workspace_id, + local_path=str(artifact_path), + media_type_hint="text/plain", + ), + ) + + assert response == { + "artifact": { + "id": response["artifact"]["id"], + "task_id": str(task_id), + "task_workspace_id": str(task_workspace_id), + "status": "registered", + "ingestion_status": "pending", + "relative_path": "docs/spec.txt", + "media_type_hint": "text/plain", + "created_at": "2026-03-13T10:00:00+00:00", + "updated_at": "2026-03-13T10:00:00+00:00", + } + } + assert store.locked_workspace_ids == [task_workspace_id] + + +def test_register_task_artifact_record_rejects_duplicate_relative_path(tmp_path) -> None: + store = ArtifactStoreStub() + user_id = uuid4() + task_id = uuid4() + task_workspace_id = uuid4() + workspace_path = tmp_path / "workspaces" / str(user_id) / str(task_id) + workspace_path.mkdir(parents=True) + artifact_path = workspace_path / "docs" / "spec.txt" + artifact_path.parent.mkdir(parents=True) + artifact_path.write_text("spec") + store.create_task_workspace( + task_workspace_id=task_workspace_id, + task_id=task_id, + user_id=user_id, + local_path=str(workspace_path), + ) + + register_task_artifact_record( + store, + user_id=user_id, + request=TaskArtifactRegisterInput( + task_workspace_id=task_workspace_id, + local_path=str(artifact_path), + media_type_hint="text/plain", + ), + ) + + with pytest.raises( + TaskArtifactAlreadyExistsError, + match=f"artifact docs/spec.txt is already registered for task workspace {task_workspace_id}", + ): + register_task_artifact_record( + store, + user_id=user_id, + request=TaskArtifactRegisterInput( + task_workspace_id=task_workspace_id, + local_path=str(artifact_path), + media_type_hint="text/plain", + ), + ) + + +def test_register_task_artifact_record_requires_visible_workspace(tmp_path) -> None: + artifact_path = tmp_path / "spec.txt" + artifact_path.write_text("spec") + + with pytest.raises(TaskWorkspaceNotFoundError, match="was not found"): + register_task_artifact_record( + ArtifactStoreStub(), + user_id=uuid4(), + request=TaskArtifactRegisterInput( + task_workspace_id=uuid4(), + local_path=str(artifact_path), + media_type_hint=None, + ), + ) + + +def test_register_task_artifact_record_rejects_paths_outside_workspace(tmp_path) -> None: + store = ArtifactStoreStub() + user_id = uuid4() + task_id = uuid4() + task_workspace_id = uuid4() + workspace_path = tmp_path / "workspaces" / str(user_id) / str(task_id) + workspace_path.mkdir(parents=True) + outside_path = tmp_path / "escape.txt" + outside_path.write_text("escape") + store.create_task_workspace( + task_workspace_id=task_workspace_id, + task_id=task_id, + user_id=user_id, + local_path=str(workspace_path), + ) + + with pytest.raises(TaskArtifactValidationError, match="escapes workspace root"): + register_task_artifact_record( + store, + user_id=user_id, + request=TaskArtifactRegisterInput( + task_workspace_id=task_workspace_id, + local_path=str(outside_path), + media_type_hint=None, + ), + ) + + +def test_list_and_get_task_artifact_records_are_deterministic() -> None: + store = ArtifactStoreStub() + user_id = uuid4() + task_id = uuid4() + task_workspace_id = uuid4() + store.create_task_workspace( + task_workspace_id=task_workspace_id, + task_id=task_id, + user_id=user_id, + local_path="/tmp/alicebot/task-workspaces/user/task", + ) + first = store.create_task_artifact( + task_id=task_id, + task_workspace_id=task_workspace_id, + status="registered", + ingestion_status="pending", + relative_path="docs/a.txt", + media_type_hint="text/plain", + ) + second = store.create_task_artifact( + task_id=task_id, + task_workspace_id=task_workspace_id, + status="registered", + ingestion_status="pending", + relative_path="docs/b.txt", + media_type_hint=None, + ) + + assert list_task_artifact_records(store, user_id=user_id) == { + "items": [ + serialize_task_artifact_row(first), + serialize_task_artifact_row(second), + ], + "summary": { + "total_count": 2, + "order": ["created_at_asc", "id_asc"], + }, + } + assert get_task_artifact_record( + store, + user_id=user_id, + task_artifact_id=first["id"], + ) == {"artifact": serialize_task_artifact_row(first)} + + +def test_get_task_artifact_record_raises_when_artifact_is_missing() -> None: + with pytest.raises(TaskArtifactNotFoundError, match="was not found"): + get_task_artifact_record( + ArtifactStoreStub(), + user_id=uuid4(), + task_artifact_id=uuid4(), + ) diff --git a/tests/unit/test_artifacts_main.py b/tests/unit/test_artifacts_main.py new file mode 100644 index 0000000..a791655 --- /dev/null +++ b/tests/unit/test_artifacts_main.py @@ -0,0 +1,154 @@ +from __future__ import annotations + +import json +from contextlib import contextmanager +from uuid import uuid4 + +import apps.api.src.alicebot_api.main as main_module +from apps.api.src.alicebot_api.config import Settings +from alicebot_api.artifacts import ( + TaskArtifactAlreadyExistsError, + TaskArtifactNotFoundError, + TaskArtifactValidationError, +) +from alicebot_api.workspaces import TaskWorkspaceNotFoundError + + +def test_list_task_artifacts_endpoint_returns_payload(monkeypatch) -> None: + user_id = uuid4() + settings = Settings(database_url="postgresql://app") + + @contextmanager + def fake_user_connection(*_args, **_kwargs): + yield object() + + monkeypatch.setattr(main_module, "get_settings", lambda: settings) + monkeypatch.setattr(main_module, "user_connection", fake_user_connection) + monkeypatch.setattr( + main_module, + "list_task_artifact_records", + lambda *_args, **_kwargs: { + "items": [], + "summary": {"total_count": 0, "order": ["created_at_asc", "id_asc"]}, + }, + ) + + response = main_module.list_task_artifacts(user_id) + + assert response.status_code == 200 + assert json.loads(response.body) == { + "items": [], + "summary": {"total_count": 0, "order": ["created_at_asc", "id_asc"]}, + } + + +def test_get_task_artifact_endpoint_maps_not_found_to_404(monkeypatch) -> None: + user_id = uuid4() + task_artifact_id = uuid4() + settings = Settings(database_url="postgresql://app") + + @contextmanager + def fake_user_connection(*_args, **_kwargs): + yield object() + + def fake_get_task_artifact_record(*_args, **_kwargs): + raise TaskArtifactNotFoundError(f"task artifact {task_artifact_id} was not found") + + monkeypatch.setattr(main_module, "get_settings", lambda: settings) + monkeypatch.setattr(main_module, "user_connection", fake_user_connection) + monkeypatch.setattr(main_module, "get_task_artifact_record", fake_get_task_artifact_record) + + response = main_module.get_task_artifact(task_artifact_id, user_id) + + assert response.status_code == 404 + assert json.loads(response.body) == {"detail": f"task artifact {task_artifact_id} was not found"} + + +def test_register_task_artifact_endpoint_maps_workspace_not_found_to_404(monkeypatch) -> None: + user_id = uuid4() + task_workspace_id = uuid4() + settings = Settings(database_url="postgresql://app") + + @contextmanager + def fake_user_connection(*_args, **_kwargs): + yield object() + + def fake_register_task_artifact_record(*_args, **_kwargs): + raise TaskWorkspaceNotFoundError(f"task workspace {task_workspace_id} was not found") + + monkeypatch.setattr(main_module, "get_settings", lambda: settings) + monkeypatch.setattr(main_module, "user_connection", fake_user_connection) + monkeypatch.setattr(main_module, "register_task_artifact_record", fake_register_task_artifact_record) + + response = main_module.register_task_artifact( + task_workspace_id, + main_module.RegisterTaskArtifactRequest( + user_id=user_id, + local_path="/tmp/example.txt", + ), + ) + + assert response.status_code == 404 + assert json.loads(response.body) == {"detail": f"task workspace {task_workspace_id} was not found"} + + +def test_register_task_artifact_endpoint_maps_validation_to_400(monkeypatch) -> None: + user_id = uuid4() + task_workspace_id = uuid4() + settings = Settings(database_url="postgresql://app") + + @contextmanager + def fake_user_connection(*_args, **_kwargs): + yield object() + + def fake_register_task_artifact_record(*_args, **_kwargs): + raise TaskArtifactValidationError("artifact path /tmp/escape.txt escapes workspace root /tmp/workspace") + + monkeypatch.setattr(main_module, "get_settings", lambda: settings) + monkeypatch.setattr(main_module, "user_connection", fake_user_connection) + monkeypatch.setattr(main_module, "register_task_artifact_record", fake_register_task_artifact_record) + + response = main_module.register_task_artifact( + task_workspace_id, + main_module.RegisterTaskArtifactRequest( + user_id=user_id, + local_path="/tmp/escape.txt", + ), + ) + + assert response.status_code == 400 + assert json.loads(response.body) == { + "detail": "artifact path /tmp/escape.txt escapes workspace root /tmp/workspace" + } + + +def test_register_task_artifact_endpoint_maps_duplicate_to_409(monkeypatch) -> None: + user_id = uuid4() + task_workspace_id = uuid4() + settings = Settings(database_url="postgresql://app") + + @contextmanager + def fake_user_connection(*_args, **_kwargs): + yield object() + + def fake_register_task_artifact_record(*_args, **_kwargs): + raise TaskArtifactAlreadyExistsError( + f"artifact docs/spec.txt is already registered for task workspace {task_workspace_id}" + ) + + monkeypatch.setattr(main_module, "get_settings", lambda: settings) + monkeypatch.setattr(main_module, "user_connection", fake_user_connection) + monkeypatch.setattr(main_module, "register_task_artifact_record", fake_register_task_artifact_record) + + response = main_module.register_task_artifact( + task_workspace_id, + main_module.RegisterTaskArtifactRequest( + user_id=user_id, + local_path="/tmp/docs/spec.txt", + ), + ) + + assert response.status_code == 409 + assert json.loads(response.body) == { + "detail": f"artifact docs/spec.txt is already registered for task workspace {task_workspace_id}" + } diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index dc1e5ca..20b7a00 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -126,6 +126,9 @@ def test_healthcheck_route_is_registered() -> None: assert "/v0/tasks/{task_id}/steps" in route_paths assert "/v0/task-workspaces" in route_paths assert "/v0/task-workspaces/{task_workspace_id}" in route_paths + assert "/v0/task-workspaces/{task_workspace_id}/artifacts" in route_paths + assert "/v0/task-artifacts" in route_paths + assert "/v0/task-artifacts/{task_artifact_id}" in route_paths assert "/v0/task-steps/{task_step_id}" in route_paths assert "/v0/task-steps/{task_step_id}/transition" in route_paths assert "/v0/entities/{entity_id}" in route_paths diff --git a/tests/unit/test_task_artifact_store.py b/tests/unit/test_task_artifact_store.py new file mode 100644 index 0000000..c6f6277 --- /dev/null +++ b/tests/unit/test_task_artifact_store.py @@ -0,0 +1,228 @@ +from __future__ import annotations + +from typing import Any +from uuid import uuid4 + +from alicebot_api.store import ContinuityStore + + +class RecordingCursor: + def __init__(self, fetchone_results: list[dict[str, Any]], fetchall_result: list[dict[str, Any]] | None = None) -> None: + self.executed: list[tuple[str, tuple[object, ...] | None]] = [] + self.fetchone_results = list(fetchone_results) + self.fetchall_result = fetchall_result or [] + + def __enter__(self) -> "RecordingCursor": + return self + + def __exit__(self, exc_type, exc, tb) -> None: + return None + + def execute(self, query: str, params: tuple[object, ...] | None = None) -> None: + self.executed.append((query, params)) + + def fetchone(self) -> dict[str, Any] | None: + if not self.fetchone_results: + return None + return self.fetchone_results.pop(0) + + def fetchall(self) -> list[dict[str, Any]]: + return self.fetchall_result + + +class RecordingConnection: + def __init__(self, cursor: RecordingCursor) -> None: + self.cursor_instance = cursor + + def cursor(self) -> RecordingCursor: + return self.cursor_instance + + +def test_task_artifact_store_methods_use_expected_queries() -> None: + task_artifact_id = uuid4() + task_id = uuid4() + task_workspace_id = uuid4() + cursor = RecordingCursor( + fetchone_results=[ + { + "id": task_artifact_id, + "user_id": uuid4(), + "task_id": task_id, + "task_workspace_id": task_workspace_id, + "status": "registered", + "ingestion_status": "pending", + "relative_path": "docs/spec.txt", + "media_type_hint": "text/plain", + "created_at": "2026-03-13T10:00:00+00:00", + "updated_at": "2026-03-13T10:00:00+00:00", + }, + { + "id": task_artifact_id, + "user_id": uuid4(), + "task_id": task_id, + "task_workspace_id": task_workspace_id, + "status": "registered", + "ingestion_status": "pending", + "relative_path": "docs/spec.txt", + "media_type_hint": "text/plain", + "created_at": "2026-03-13T10:00:00+00:00", + "updated_at": "2026-03-13T10:00:00+00:00", + }, + { + "id": task_artifact_id, + "user_id": uuid4(), + "task_id": task_id, + "task_workspace_id": task_workspace_id, + "status": "registered", + "ingestion_status": "pending", + "relative_path": "docs/spec.txt", + "media_type_hint": "text/plain", + "created_at": "2026-03-13T10:00:00+00:00", + "updated_at": "2026-03-13T10:00:00+00:00", + }, + ], + fetchall_result=[ + { + "id": task_artifact_id, + "user_id": uuid4(), + "task_id": task_id, + "task_workspace_id": task_workspace_id, + "status": "registered", + "ingestion_status": "pending", + "relative_path": "docs/spec.txt", + "media_type_hint": "text/plain", + "created_at": "2026-03-13T10:00:00+00:00", + "updated_at": "2026-03-13T10:00:00+00:00", + } + ], + ) + store = ContinuityStore(RecordingConnection(cursor)) + + created = store.create_task_artifact( + task_id=task_id, + task_workspace_id=task_workspace_id, + status="registered", + ingestion_status="pending", + relative_path="docs/spec.txt", + media_type_hint="text/plain", + ) + fetched = store.get_task_artifact_optional(task_artifact_id) + duplicate = store.get_task_artifact_by_workspace_relative_path_optional( + task_workspace_id=task_workspace_id, + relative_path="docs/spec.txt", + ) + listed = store.list_task_artifacts() + store.lock_task_artifacts(task_workspace_id) + + assert created["id"] == task_artifact_id + assert fetched is not None + assert duplicate is not None + assert listed[0]["id"] == task_artifact_id + assert cursor.executed == [ + ( + """ + INSERT INTO task_artifacts ( + user_id, + task_id, + task_workspace_id, + status, + ingestion_status, + relative_path, + media_type_hint, + created_at, + updated_at + ) + VALUES ( + app.current_user_id(), + %s, + %s, + %s, + %s, + %s, + %s, + clock_timestamp(), + clock_timestamp() + ) + RETURNING + id, + user_id, + task_id, + task_workspace_id, + status, + ingestion_status, + relative_path, + media_type_hint, + created_at, + updated_at + """, + ( + task_id, + task_workspace_id, + "registered", + "pending", + "docs/spec.txt", + "text/plain", + ), + ), + ( + """ + SELECT + id, + user_id, + task_id, + task_workspace_id, + status, + ingestion_status, + relative_path, + media_type_hint, + created_at, + updated_at + FROM task_artifacts + WHERE id = %s + """, + (task_artifact_id,), + ), + ( + """ + SELECT + id, + user_id, + task_id, + task_workspace_id, + status, + ingestion_status, + relative_path, + media_type_hint, + created_at, + updated_at + FROM task_artifacts + WHERE task_workspace_id = %s + AND relative_path = %s + ORDER BY created_at ASC, id ASC + LIMIT 1 + """, + (task_workspace_id, "docs/spec.txt"), + ), + ( + """ + SELECT + id, + user_id, + task_id, + task_workspace_id, + status, + ingestion_status, + relative_path, + media_type_hint, + created_at, + updated_at + FROM task_artifacts + ORDER BY created_at ASC, id ASC + """, + None, + ), + ( + "SELECT pg_advisory_xact_lock(hashtextextended(%s::text, 4))", + (str(task_workspace_id),), + ), + ]