From f2b736b08e012ffac50b4f7ac734dc96b6c1e523 Mon Sep 17 00:00:00 2001 From: JC Date: Wed, 29 Jul 2026 19:02:23 +0000 Subject: [PATCH] feat: add return to service workflow --- .../versions/0012_return_to_service.py | 253 ++++ backend/app/identity.py | 11 + backend/app/repairs/__init__.py | 14 + backend/app/repairs/exceptions.py | 43 + backend/app/repairs/models.py | 21 +- backend/app/repairs/repository.py | 120 +- backend/app/repairs/router.py | 44 + backend/app/repairs/service.py | 139 ++- backend/tests/test_repair_timeline.py | 5 + backend/tests/test_repairs_router.py | 6 +- backend/tests/test_return_to_service.py | 1070 +++++++++++++++++ docs/ROADMAP.md | 10 +- docs/releases/v0.2-repair-workflow.md | 7 +- docs/tasks/003-return-to-service.md | 814 +++++++++++++ scripts/test-begin-repair-migration.sh | 8 +- scripts/test-dev-baseline.sh | 2 +- scripts/test-repair-timeline-migration.sh | 8 +- scripts/test-return-to-service-migration.sh | 295 +++++ scripts/test-return-to-service.sh | 59 + 19 files changed, 2883 insertions(+), 46 deletions(-) create mode 100644 backend/alembic/versions/0012_return_to_service.py create mode 100644 backend/tests/test_return_to_service.py create mode 100644 docs/tasks/003-return-to-service.md create mode 100755 scripts/test-return-to-service-migration.sh create mode 100755 scripts/test-return-to-service.sh diff --git a/backend/alembic/versions/0012_return_to_service.py b/backend/alembic/versions/0012_return_to_service.py new file mode 100644 index 0000000..46e1359 --- /dev/null +++ b/backend/alembic/versions/0012_return_to_service.py @@ -0,0 +1,253 @@ +"""add return to service workflow + +Revision ID: 0012_return_to_service +Revises: 0011_repair_timeline +Create Date: 2026-07-29 +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + + +revision: str = "0012_return_to_service" +down_revision: Union[str, None] = "0011_repair_timeline" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + +REPAIR_REASONS = ( + "HARDWARE_FAILURE", + "LOW_HASHRATE", + "CONTROL_BOARD", + "HASHBOARD", + "PSU", + "FAN", + "OVERHEATING", + "NETWORKING", + "PREVENTIVE_MAINTENANCE", + "INSPECTION", + "OTHER", +) + + +def _quoted(values: Sequence[str]) -> str: + return ", ".join(f"'{value}'" for value in values) + + +def _create_event_constraints() -> None: + op.create_check_constraint( + "ck_repair_events_type", + "repair_events", + "event_type IN ('REPAIR_STARTED', 'INITIAL_NOTE', " + "'REPAIR_COMPLETED')", + ) + op.create_check_constraint( + "ck_repair_events_payload_shape", + "repair_events", + "(" + "event_type = 'REPAIR_STARTED' " + "AND payload ?& ARRAY['reason', 'from_lifecycle', 'to_lifecycle'] " + f"AND payload ->> 'reason' IN ({_quoted(REPAIR_REASONS)}) " + "AND payload ->> 'from_lifecycle' IN ('active', 'spare') " + "AND payload ->> 'to_lifecycle' = 'in_repair'" + ") OR (" + "event_type = 'INITIAL_NOTE' " + "AND jsonb_typeof(payload -> 'note') = 'string' " + "AND trim(payload ->> 'note') <> ''" + ") OR (" + "event_type = 'REPAIR_COMPLETED' " + "AND payload ?& ARRAY[" + "'previous_miner_lifecycle', 'resulting_miner_lifecycle'" + "] " + "AND payload ->> 'previous_miner_lifecycle' = 'in_repair' " + "AND payload ->> 'resulting_miner_lifecycle' " + "IN ('active', 'spare') " + "AND (NOT payload ? 'pre_repair_lifecycle' " + "OR payload ->> 'pre_repair_lifecycle' IN ('active', 'spare')) " + "AND (NOT payload ? 'completion_summary' OR (" + "jsonb_typeof(payload -> 'completion_summary') = 'string' " + "AND trim(payload ->> 'completion_summary') <> '' " + "AND char_length(payload ->> 'completion_summary') <= 4000" + "))" + ")", + ) + + +def _create_previous_event_constraints() -> None: + op.create_check_constraint( + "ck_repair_events_type", + "repair_events", + "event_type IN ('REPAIR_STARTED', 'INITIAL_NOTE')", + ) + op.create_check_constraint( + "ck_repair_events_payload_shape", + "repair_events", + "(" + "event_type = 'REPAIR_STARTED' " + "AND payload ?& ARRAY['reason', 'from_lifecycle', 'to_lifecycle'] " + f"AND payload ->> 'reason' IN ({_quoted(REPAIR_REASONS)}) " + "AND payload ->> 'from_lifecycle' IN ('active', 'spare') " + "AND payload ->> 'to_lifecycle' = 'in_repair'" + ") OR (" + "event_type = 'INITIAL_NOTE' " + "AND jsonb_typeof(payload -> 'note') = 'string' " + "AND trim(payload ->> 'note') <> ''" + ")", + ) + + +def upgrade() -> None: + connection = op.get_bind() + + connection.exec_driver_sql( + """ + INSERT INTO permissions (key, description) + VALUES ('repairs.complete', 'Return repaired miners to service') + ON CONFLICT (key) DO UPDATE + SET description = EXCLUDED.description + """ + ) + connection.exec_driver_sql( + """ + INSERT INTO role_permissions (role_id, permission_id) + SELECT roles.id, permissions.id + FROM roles + JOIN permissions ON permissions.key = 'repairs.complete' + WHERE roles.name IN ('Admin', 'Manager', 'Technician') + ON CONFLICT DO NOTHING + """ + ) + + op.add_column( + "repairs", + sa.Column("completion_summary", sa.Text(), nullable=True), + ) + op.add_column( + "repairs", + sa.Column("resulting_lifecycle", sa.Text(), nullable=True), + ) + op.create_check_constraint( + "ck_repairs_resulting_lifecycle", + "repairs", + "resulting_lifecycle IS NULL " + "OR resulting_lifecycle IN ('active', 'spare')", + ) + op.create_check_constraint( + "ck_repairs_completion_summary", + "repairs", + "completion_summary IS NULL OR (" + "trim(completion_summary) <> '' " + "AND char_length(completion_summary) <= 4000" + ")", + ) + # NOT VALID preserves truthful legacy COMPLETED rows whose destination was + # never recorded, while PostgreSQL still enforces the check for every new + # or updated row after this migration. + connection.exec_driver_sql( + """ + ALTER TABLE repairs + ADD CONSTRAINT ck_repairs_completion_metadata_state + CHECK ( + ( + status = 'COMPLETED' + AND resulting_lifecycle IS NOT NULL + ) + OR ( + status <> 'COMPLETED' + AND completion_summary IS NULL + AND resulting_lifecycle IS NULL + ) + ) + NOT VALID + """ + ) + + op.drop_constraint( + "ck_repair_events_payload_shape", + "repair_events", + type_="check", + ) + op.drop_constraint( + "ck_repair_events_type", + "repair_events", + type_="check", + ) + _create_event_constraints() + + +def downgrade() -> None: + connection = op.get_bind() + + connection.exec_driver_sql( + """ + DO $$ + DECLARE + completion_count BIGINT; + BEGIN + SELECT count(*) + INTO completion_count + FROM repairs + WHERE resulting_lifecycle IS NOT NULL + OR completion_summary IS NOT NULL; + + completion_count := completion_count + ( + SELECT count(*) + FROM repair_events + WHERE event_type = 'REPAIR_COMPLETED' + ); + + IF completion_count > 0 THEN + RAISE EXCEPTION USING + MESSAGE = format( + 'Cannot downgrade Return to Service: %s completion records or events would lose authoritative history', + completion_count + ), + HINT = 'Preserve or explicitly migrate Return to Service history before downgrading; MinerOps will not invent or discard lifecycle, actor, summary, or event values.'; + END IF; + END + $$; + """ + ) + + op.drop_constraint( + "ck_repair_events_payload_shape", + "repair_events", + type_="check", + ) + op.drop_constraint( + "ck_repair_events_type", + "repair_events", + type_="check", + ) + _create_previous_event_constraints() + + op.drop_constraint( + "ck_repairs_completion_metadata_state", + "repairs", + type_="check", + ) + op.drop_constraint( + "ck_repairs_completion_summary", + "repairs", + type_="check", + ) + op.drop_constraint( + "ck_repairs_resulting_lifecycle", + "repairs", + type_="check", + ) + op.drop_column("repairs", "resulting_lifecycle") + op.drop_column("repairs", "completion_summary") + + connection.exec_driver_sql( + """ + DELETE FROM role_permissions + WHERE permission_id = ( + SELECT id FROM permissions WHERE key = 'repairs.complete' + ) + """ + ) + connection.exec_driver_sql( + "DELETE FROM permissions WHERE key = 'repairs.complete'" + ) diff --git a/backend/app/identity.py b/backend/app/identity.py index 5330a6e..eabf514 100644 --- a/backend/app/identity.py +++ b/backend/app/identity.py @@ -682,11 +682,22 @@ def repair_timeline_request(path: str) -> bool: ) +def repair_completion_request(path: str) -> bool: + parts = path.split("/") + return ( + len(parts) == 5 + and parts[:3] == ["", "api", "repairs"] + and parts[4] == "complete" + ) + + def required_api_permission(method: str, path: str) -> str | None: if path.startswith("/api/admin/"): return "users.manage" if method == "POST" and path == "/api/repairs": return "repairs.create" + if method == "POST" and repair_completion_request(path): + return "repairs.complete" if method == "GET" and repair_timeline_request(path): return "repairs.read" if method == "GET": diff --git a/backend/app/repairs/__init__.py b/backend/app/repairs/__init__.py index 365f364..40c5fd5 100644 --- a/backend/app/repairs/__init__.py +++ b/backend/app/repairs/__init__.py @@ -4,10 +4,14 @@ from app.repairs.exceptions import ( ActiveRepairConflict, IncompatibleMinerLifecycle, InconsistentRepairState, + RepairActiveMismatch, RepairActorRequired, + RepairCompletionActorRequired, RepairConflict, RepairError, RepairMinerNotFound, + RepairMinerNotInRepair, + RepairNotActive, RepairNotFound, RepairPersistenceError, ) @@ -15,7 +19,10 @@ from app.repairs.models import ( BeginRepairRequest, BeginRepairResult, RepairActor, + RepairCompleteRequest, + RepairCompletedPayload, RepairDetail, + RepairPostLifecycle, RepairReason, RepairStatus, RepairTimeline, @@ -30,14 +37,21 @@ __all__ = [ "BeginRepairResult", "IncompatibleMinerLifecycle", "InconsistentRepairState", + "RepairActiveMismatch", "RepairActor", "RepairActorRequired", + "RepairCompleteRequest", + "RepairCompletedPayload", + "RepairCompletionActorRequired", "RepairConflict", "RepairDetail", "RepairError", "RepairMinerNotFound", + "RepairMinerNotInRepair", + "RepairNotActive", "RepairNotFound", "RepairPersistenceError", + "RepairPostLifecycle", "RepairReason", "RepairRepository", "RepairService", diff --git a/backend/app/repairs/exceptions.py b/backend/app/repairs/exceptions.py index 9b7938e..65ac36c 100644 --- a/backend/app/repairs/exceptions.py +++ b/backend/app/repairs/exceptions.py @@ -62,5 +62,48 @@ class RepairActorRequired(RepairError): super().__init__("An authenticated actor is required to begin repair") +class RepairCompletionActorRequired(RepairError): + """Raised when Return to Service lacks an authenticated actor.""" + + def __init__(self): + super().__init__( + "An authenticated actor is required to return a miner to service" + ) + + +class RepairNotActive(RepairConflict): + """Raised when a repair is no longer active.""" + + def __init__(self, repair_id: object, repair_status: object): + self.repair_id = repair_id + self.repair_status = repair_status + super().__init__( + f"Repair is not active: {repair_id} ({repair_status})" + ) + + +class RepairMinerNotInRepair(RepairConflict): + """Raised when a repair's miner has already left repair lifecycle.""" + + def __init__(self, miner_id: object, lifecycle_status: object): + self.miner_id = miner_id + self.lifecycle_status = lifecycle_status + super().__init__( + "Miner is not in repair lifecycle: " + f"{miner_id} ({lifecycle_status})" + ) + + +class RepairActiveMismatch(RepairConflict): + """Raised when the requested repair is not the miner's active repair.""" + + def __init__(self, repair_id: object, miner_id: object): + self.repair_id = repair_id + self.miner_id = miner_id + super().__init__( + f"Repair is not the active repair for miner: {repair_id}" + ) + + class RepairPersistenceError(RepairError): """Raised when a required repair persistence operation fails.""" diff --git a/backend/app/repairs/models.py b/backend/app/repairs/models.py index 5129618..5ae1cb4 100644 --- a/backend/app/repairs/models.py +++ b/backend/app/repairs/models.py @@ -28,8 +28,13 @@ RepairStatus = Literal[ "COMPLETED", "CANCELLED", ] -RepairEventType = Literal["REPAIR_STARTED", "INITIAL_NOTE"] +RepairEventType = Literal[ + "REPAIR_STARTED", + "INITIAL_NOTE", + "REPAIR_COMPLETED", +] RepairEligibleLifecycle = Literal["active", "spare"] +RepairPostLifecycle = Literal["active", "spare"] RepairMinerLifecycle = Literal["active", "spare", "retired", "in_repair"] @@ -41,6 +46,13 @@ class BeginRepairRequest(BaseModel): notes: str | None = Field(default=None, max_length=4000) +class RepairCompleteRequest(BaseModel): + model_config = ConfigDict(extra="forbid") + + target_lifecycle: RepairPostLifecycle + completion_summary: str | None = Field(default=None, max_length=4000) + + class RepairStartedPayload(BaseModel): reason: RepairReason from_lifecycle: RepairEligibleLifecycle @@ -51,6 +63,13 @@ class InitialRepairNotePayload(BaseModel): note: str = Field(min_length=1, max_length=4000) +class RepairCompletedPayload(BaseModel): + previous_miner_lifecycle: Literal["in_repair"] = "in_repair" + resulting_miner_lifecycle: RepairPostLifecycle + pre_repair_lifecycle: RepairEligibleLifecycle | None = None + completion_summary: str | None = Field(default=None, max_length=4000) + + class BeginRepairResult(BaseModel): repair_id: UUID miner_id: UUID diff --git a/backend/app/repairs/repository.py b/backend/app/repairs/repository.py index 4013a47..dd20ba1 100644 --- a/backend/app/repairs/repository.py +++ b/backend/app/repairs/repository.py @@ -39,7 +39,7 @@ def default_connection_factory() -> psycopg.Connection: class RepairRepository: - """Persistence boundary for the atomic Begin Repair workflow.""" + """Persistence boundary for explicit atomic repair workflows.""" def __init__( self, @@ -224,6 +224,124 @@ class RepairRepository: "Miner lifecycle transition did not update one record" ) + def get_repair_identity(self, repair_id: UUID) -> dict | None: + with self._connection_scope() as connection: + with connection.cursor(row_factory=dict_row) as cur: + cur.execute( + """ + SELECT id, miner_id + FROM repairs + WHERE id = %s + """, + (repair_id,), + ) + row = cur.fetchone() + return dict(row) if row else None + + def lock_repair_for_update(self, repair_id: UUID) -> dict | None: + with self._connection_scope() as connection: + with connection.cursor(row_factory=dict_row) as cur: + cur.execute( + """ + SELECT + id, + miner_id, + status, + reason, + opened_at, + opened_by, + completed_at, + completed_by, + completion_summary, + resulting_lifecycle + FROM repairs + WHERE id = %s + FOR UPDATE + """, + (repair_id,), + ) + row = cur.fetchone() + return dict(row) if row else None + + def get_next_event_sequence(self, repair_id: UUID) -> int: + with self._connection_scope() as connection: + with connection.cursor() as cur: + cur.execute( + """ + SELECT COALESCE(max(sequence), 0) + 1 + FROM repair_events + WHERE repair_id = %s + """, + (repair_id,), + ) + return int(cur.fetchone()[0]) + + def get_pre_repair_lifecycle(self, repair_id: UUID) -> str | None: + with self._connection_scope() as connection: + with connection.cursor() as cur: + cur.execute( + """ + SELECT event.payload ->> 'from_lifecycle' + FROM repair_events AS event + WHERE event.repair_id = %s + AND event.event_type = 'REPAIR_STARTED' + ORDER BY event.sequence + LIMIT 2 + """, + (repair_id,), + ) + rows = cur.fetchall() + + if len(rows) != 1: + return None + lifecycle_status = rows[0][0] + if lifecycle_status not in {"active", "spare"}: + return None + return str(lifecycle_status) + + def complete_repair( + self, + repair_id: UUID, + *, + completed_by: UUID, + completion_summary: str | None, + resulting_lifecycle: str, + ) -> dict | None: + with self._connection_scope(write=True) as connection: + with connection.cursor(row_factory=dict_row) as cur: + cur.execute( + """ + UPDATE repairs + SET status = 'COMPLETED', + completed_at = now(), + completed_by = %s, + completion_summary = %s, + resulting_lifecycle = %s + WHERE id = %s + AND status = ANY(%s) + RETURNING + id, + miner_id, + status, + reason, + opened_at, + opened_by, + completed_at, + completed_by, + completion_summary, + resulting_lifecycle + """, + ( + completed_by, + completion_summary, + resulting_lifecycle, + repair_id, + list(ACTIVE_REPAIR_STATUSES), + ), + ) + row = cur.fetchone() + return dict(row) if row else None + def get_repair_detail(self, repair_id: UUID) -> dict | None: with self._connection_scope() as connection: with connection.cursor(row_factory=dict_row) as cur: diff --git a/backend/app/repairs/router.py b/backend/app/repairs/router.py index 5ae9ec7..7f010ca 100644 --- a/backend/app/repairs/router.py +++ b/backend/app/repairs/router.py @@ -9,6 +9,7 @@ from app.identity import require_permission from app.repairs.dependencies import get_repair_service from app.repairs.exceptions import ( RepairActorRequired, + RepairCompletionActorRequired, RepairConflict, RepairMinerNotFound, RepairNotFound, @@ -16,6 +17,7 @@ from app.repairs.exceptions import ( from app.repairs.models import ( BeginRepairRequest, BeginRepairResponse, + RepairCompleteRequest, RepairDetailResponse, RepairTimelineResponse, ) @@ -25,6 +27,7 @@ from app.repairs.service import RepairService router = APIRouter(prefix="/api/repairs", tags=["repairs"]) require_repair_create = require_permission("repairs.create") require_repair_read = require_permission("repairs.read") +require_repair_complete = require_permission("repairs.complete") RepairServiceDependency = Annotated[ RepairService, @@ -32,6 +35,7 @@ RepairServiceDependency = Annotated[ ] RepairCreateUser = Annotated[dict, Depends(require_repair_create)] RepairReadUser = Annotated[dict, Depends(require_repair_read)] +RepairCompleteUser = Annotated[dict, Depends(require_repair_complete)] @router.post( @@ -68,6 +72,46 @@ def begin_repair( return BeginRepairResponse(data=repair) +@router.post( + "/{repair_id}/complete", + response_model=RepairDetailResponse, +) +def complete_repair( + repair_id: UUID, + request: RepairCompleteRequest, + service: RepairServiceDependency, + current_user: RepairCompleteUser, +) -> RepairDetailResponse: + try: + repair = service.complete_repair( + repair_id, + request, + actor_user_id=UUID(str(current_user["id"])), + ) + except RepairNotFound as exc: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail="Repair not found", + ) from exc + except RepairMinerNotFound as exc: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="Repair miner no longer exists", + ) from exc + except RepairConflict as exc: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=str(exc), + ) from exc + except RepairCompletionActorRequired as exc: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=str(exc), + ) from exc + + return RepairDetailResponse(data=repair) + + @router.get( "/{repair_id}", response_model=RepairDetailResponse, diff --git a/backend/app/repairs/service.py b/backend/app/repairs/service.py index e841377..648036c 100644 --- a/backend/app/repairs/service.py +++ b/backend/app/repairs/service.py @@ -6,15 +6,22 @@ from app.repairs.exceptions import ( ActiveRepairConflict, IncompatibleMinerLifecycle, InconsistentRepairState, + RepairActiveMismatch, RepairActorRequired, + RepairCompletionActorRequired, RepairMinerNotFound, + RepairMinerNotInRepair, + RepairNotActive, RepairNotFound, + RepairPersistenceError, ) from app.repairs.models import ( BeginRepairRequest, BeginRepairResult, InitialRepairNotePayload, RepairActor, + RepairCompleteRequest, + RepairCompletedPayload, RepairDetail, RepairTimeline, RepairTimelineEvent, @@ -24,6 +31,9 @@ from app.repairs.repository import RepairRepository ELIGIBLE_LIFECYCLE_STATUSES = frozenset({"active", "spare"}) +ACTIVE_REPAIR_STATUSES = frozenset( + {"OPEN", "IN_PROGRESS", "WAITING_FOR_PARTS", "READY_FOR_TEST"} +) REPAIR_LIFECYCLE_STATUS = "in_repair" @@ -40,6 +50,32 @@ class RepairService: normalized = str(note).strip() return normalized or None + @staticmethod + def _map_repair_detail(repair: dict) -> RepairDetail: + closed_by = None + if repair["closed_by_user_id"] is not None: + closed_by = RepairActor( + user_id=repair["closed_by_user_id"], + display_name=repair["closed_by_display_name"], + ) + + return RepairDetail( + repair_id=repair["repair_id"], + miner_id=repair["miner_id"], + miner_asset_id=repair["miner_asset_id"], + status=repair["status"], + reason=repair["reason"], + opened_at=repair["opened_at"], + opened_by=RepairActor( + user_id=repair["opened_by_user_id"], + display_name=repair["opened_by_display_name"], + ), + closed_at=repair["closed_at"], + closed_by=closed_by, + miner_lifecycle=repair["miner_lifecycle"], + latest_event_sequence=repair["latest_event_sequence"], + ) + def begin_repair( self, request: BeginRepairRequest, @@ -107,34 +143,89 @@ class RepairService: opened_by=repair["opened_by"], ) + def complete_repair( + self, + repair_id: UUID, + request: RepairCompleteRequest, + *, + actor_user_id: UUID | None, + ) -> RepairDetail: + if actor_user_id is None: + raise RepairCompletionActorRequired() + + completion_summary = self.normalize_note(request.completion_summary) + + with self.repository.transaction() as repository: + repair_identity = repository.get_repair_identity(repair_id) + if repair_identity is None: + raise RepairNotFound(repair_id) + + miner_id = repair_identity["miner_id"] + # Begin Repair also locks the miner first. Keeping this order makes + # competing lifecycle workflows serialize on the durable asset. + miner = repository.lock_miner_for_update(miner_id) + if miner is None: + raise RepairMinerNotFound(miner_id) + + repair = repository.lock_repair_for_update(repair_id) + if repair is None: + raise RepairNotFound(repair_id) + if repair["miner_id"] != miner_id: + raise RepairActiveMismatch(repair_id, miner_id) + if repair["status"] not in ACTIVE_REPAIR_STATUSES: + raise RepairNotActive(repair_id, repair["status"]) + + active_repair = repository.get_active_repair(miner_id) + if active_repair is None or active_repair["id"] != repair_id: + raise RepairActiveMismatch(repair_id, miner_id) + + lifecycle_status = str(miner["lifecycle_status"]) + if lifecycle_status != REPAIR_LIFECYCLE_STATUS: + raise RepairMinerNotInRepair(miner_id, lifecycle_status) + + next_sequence = repository.get_next_event_sequence(repair_id) + pre_repair_lifecycle = repository.get_pre_repair_lifecycle( + repair_id + ) + completed = repository.complete_repair( + repair_id, + completed_by=actor_user_id, + completion_summary=completion_summary, + resulting_lifecycle=request.target_lifecycle, + ) + if completed is None: + raise RepairNotActive(repair_id, repair["status"]) + + repository.set_miner_lifecycle( + miner_id, + request.target_lifecycle, + ) + payload = RepairCompletedPayload( + resulting_miner_lifecycle=request.target_lifecycle, + pre_repair_lifecycle=pre_repair_lifecycle, + completion_summary=completion_summary, + ) + repository.create_event( + repair_id, + sequence=next_sequence, + event_type="REPAIR_COMPLETED", + created_by=actor_user_id, + payload=payload.model_dump(mode="json", exclude_none=True), + ) + + detail = repository.get_repair_detail(repair_id) + if detail is None: + raise RepairPersistenceError( + "Completed repair could not be read in its transaction" + ) + + return self._map_repair_detail(detail) + def get_repair(self, repair_id: UUID) -> RepairDetail: repair = self.repository.get_repair_detail(repair_id) if repair is None: raise RepairNotFound(repair_id) - - closed_by = None - if repair["closed_by_user_id"] is not None: - closed_by = RepairActor( - user_id=repair["closed_by_user_id"], - display_name=repair["closed_by_display_name"], - ) - - return RepairDetail( - repair_id=repair["repair_id"], - miner_id=repair["miner_id"], - miner_asset_id=repair["miner_asset_id"], - status=repair["status"], - reason=repair["reason"], - opened_at=repair["opened_at"], - opened_by=RepairActor( - user_id=repair["opened_by_user_id"], - display_name=repair["opened_by_display_name"], - ), - closed_at=repair["closed_at"], - closed_by=closed_by, - miner_lifecycle=repair["miner_lifecycle"], - latest_event_sequence=repair["latest_event_sequence"], - ) + return self._map_repair_detail(repair) def get_repair_timeline(self, repair_id: UUID) -> RepairTimeline: events = self.repository.get_repair_events(repair_id) diff --git a/backend/tests/test_repair_timeline.py b/backend/tests/test_repair_timeline.py index 7a6a605..e0ad5c9 100644 --- a/backend/tests/test_repair_timeline.py +++ b/backend/tests/test_repair_timeline.py @@ -715,6 +715,11 @@ class RepairTimelineArchitectureTests(unittest.TestCase): "minerops_repair_parts_summary_api", ), ("POST", "/api/repairs", "begin_repair"), + ( + "POST", + "/api/repairs/{repair_id}/complete", + "complete_repair", + ), ("GET", "/api/repairs/{repair_id}", "get_repair"), ( "GET", diff --git a/backend/tests/test_repairs_router.py b/backend/tests/test_repairs_router.py index cc0f111..0ce9ef1 100644 --- a/backend/tests/test_repairs_router.py +++ b/backend/tests/test_repairs_router.py @@ -232,11 +232,13 @@ class RepairRouterTests(unittest.TestCase): self.assertEqual(response.status_code, 422) self.assertIsNone(self.service.request) - def test_router_registers_exactly_one_post_route(self): + def test_router_registers_exact_begin_repair_route(self): routes = [ (route.path, route.methods) for route in router.routes - if hasattr(route, "methods") and "POST" in route.methods + if hasattr(route, "methods") + and "POST" in route.methods + and route.path == "/api/repairs" ] self.assertEqual(routes, [("/api/repairs", {"POST"})]) diff --git a/backend/tests/test_return_to_service.py b/backend/tests/test_return_to_service.py new file mode 100644 index 0000000..ed78dcb --- /dev/null +++ b/backend/tests/test_return_to_service.py @@ -0,0 +1,1070 @@ +import ast +import unittest +from concurrent.futures import ThreadPoolExecutor +from contextlib import contextmanager +from copy import deepcopy +from datetime import datetime, timezone +from pathlib import Path +from threading import Barrier +from uuid import UUID, uuid4 + +from fastapi import FastAPI +from fastapi.testclient import TestClient +from psycopg.errors import CheckViolation, RaiseException +from psycopg.rows import dict_row +from starlette.routing import Match + +from app.identity import get_current_user, required_api_permission +from app.repairs.dependencies import get_repair_service +from app.repairs.exceptions import ( + RepairActiveMismatch, + RepairCompletionActorRequired, + RepairMinerNotInRepair, + RepairNotActive, + RepairNotFound, +) +from app.repairs.models import ( + BeginRepairRequest, + RepairActor, + RepairCompleteRequest, + RepairDetail, +) +from app.repairs.repository import RepairRepository +from app.repairs.router import router +from app.repairs.service import RepairService + + +NOW = datetime(2026, 7, 29, 16, 0, tzinfo=timezone.utc) +ACTIVE_STATUSES = { + "OPEN", + "IN_PROGRESS", + "WAITING_FOR_PARTS", + "READY_FOR_TEST", +} + + +def completed_detail(repair_id=None, miner_id=None, actor_id=None): + return RepairDetail( + repair_id=repair_id or uuid4(), + miner_id=miner_id or uuid4(), + miner_asset_id="RETURN-TO-SERVICE-001", + status="COMPLETED", + reason="FAN", + opened_at=NOW, + opened_by=RepairActor( + user_id=uuid4(), + display_name="Opening Technician", + ), + closed_at=NOW, + closed_by=RepairActor( + user_id=actor_id or uuid4(), + display_name="Completing Technician", + ), + miner_lifecycle="active", + latest_event_sequence=3, + ) + + +class FakeCompletionRepository: + def __init__(self, *, status="OPEN", lifecycle="in_repair"): + self.repair_id = uuid4() + self.miner_id = uuid4() + self.opened_by = uuid4() + self.closed_by = uuid4() + self.miner = { + "id": self.miner_id, + "lifecycle_status": lifecycle, + } + self.repair = { + "id": self.repair_id, + "miner_id": self.miner_id, + "status": status, + "reason": "FAN", + "opened_at": NOW, + "opened_by": self.opened_by, + "completed_at": NOW if status == "COMPLETED" else None, + "completed_by": self.closed_by if status == "COMPLETED" else None, + "completion_summary": None, + "resulting_lifecycle": None, + } + self.events = [ + { + "id": uuid4(), + "repair_id": self.repair_id, + "sequence": 1, + "event_type": "REPAIR_STARTED", + "created_at": NOW, + "created_by": self.opened_by, + "payload": { + "reason": "FAN", + "from_lifecycle": "spare", + "to_lifecycle": "in_repair", + }, + }, + { + "id": uuid4(), + "repair_id": self.repair_id, + "sequence": 2, + "event_type": "INITIAL_NOTE", + "created_at": NOW, + "created_by": self.opened_by, + "payload": {"note": "Replace fan."}, + }, + ] + self.calls = [] + self.completion_error = None + self.lifecycle_error = None + self.event_error = None + self.identity_missing = False + self.active_mismatch = False + + @contextmanager + def transaction(self): + self.calls.append("transaction") + snapshot = deepcopy((self.miner, self.repair, self.events)) + try: + yield self + except Exception: + self.miner, self.repair, self.events = snapshot + raise + + def get_repair_identity(self, repair_id): + self.calls.append("get_repair_identity") + if self.identity_missing or repair_id != self.repair_id: + return None + return {"id": self.repair_id, "miner_id": self.miner_id} + + def lock_miner_for_update(self, miner_id): + self.calls.append("lock_miner") + return self.miner if miner_id == self.miner_id else None + + def lock_repair_for_update(self, repair_id): + self.calls.append("lock_repair") + return self.repair if repair_id == self.repair_id else None + + def get_active_repair(self, miner_id): + self.calls.append("get_active_repair") + if self.active_mismatch or self.repair["status"] not in ACTIVE_STATUSES: + return None + return self.repair + + def get_next_event_sequence(self, repair_id): + self.calls.append("get_next_event_sequence") + return max(event["sequence"] for event in self.events) + 1 + + def get_pre_repair_lifecycle(self, repair_id): + self.calls.append("get_pre_repair_lifecycle") + return "spare" + + def complete_repair( + self, + repair_id, + *, + completed_by, + completion_summary, + resulting_lifecycle, + ): + self.calls.append("complete_repair") + if self.completion_error is not None: + raise self.completion_error + self.repair.update( + { + "status": "COMPLETED", + "completed_at": NOW, + "completed_by": completed_by, + "completion_summary": completion_summary, + "resulting_lifecycle": resulting_lifecycle, + } + ) + return self.repair + + def set_miner_lifecycle(self, miner_id, lifecycle_status): + self.calls.append("set_miner_lifecycle") + if self.lifecycle_error is not None: + raise self.lifecycle_error + self.miner["lifecycle_status"] = lifecycle_status + + def create_event( + self, + repair_id, + *, + sequence, + event_type, + created_by, + payload, + ): + self.calls.append("create_event:REPAIR_COMPLETED") + if self.event_error is not None: + raise self.event_error + event = { + "id": uuid4(), + "repair_id": repair_id, + "sequence": sequence, + "event_type": event_type, + "created_at": NOW, + "created_by": created_by, + "payload": dict(payload), + } + self.events.append(event) + return event + + def get_repair_detail(self, repair_id): + self.calls.append("get_repair_detail") + if repair_id != self.repair_id: + return None + return { + "repair_id": self.repair_id, + "miner_id": self.miner_id, + "miner_asset_id": "RETURN-TO-SERVICE-001", + "status": self.repair["status"], + "reason": self.repair["reason"], + "opened_at": self.repair["opened_at"], + "opened_by_user_id": self.opened_by, + "opened_by_display_name": "Opening Technician", + "closed_at": self.repair["completed_at"], + "closed_by_user_id": self.repair["completed_by"], + "closed_by_display_name": "Completing Technician", + "miner_lifecycle": self.miner["lifecycle_status"], + "latest_event_sequence": max( + event["sequence"] for event in self.events + ), + } + + +class ReturnToServiceServiceTests(unittest.TestCase): + def setUp(self): + self.repository = FakeCompletionRepository() + self.service = RepairService(self.repository) + self.actor_id = uuid4() + + def complete(self, target="active", summary=" Load test passed. "): + return self.service.complete_repair( + self.repository.repair_id, + RepairCompleteRequest( + target_lifecycle=target, + completion_summary=summary, + ), + actor_user_id=self.actor_id, + ) + + def test_complete_repair_is_atomic_and_returns_detail(self): + result = self.complete() + + self.assertEqual(result.status, "COMPLETED") + self.assertEqual(result.miner_lifecycle, "active") + self.assertEqual(result.closed_by.user_id, self.actor_id) + self.assertEqual(result.latest_event_sequence, 3) + self.assertEqual(self.repository.repair["completion_summary"], "Load test passed.") + self.assertEqual(self.repository.repair["resulting_lifecycle"], "active") + event = self.repository.events[-1] + self.assertEqual(event["event_type"], "REPAIR_COMPLETED") + self.assertEqual(event["sequence"], 3) + self.assertEqual(event["created_by"], self.actor_id) + self.assertEqual( + event["payload"], + { + "previous_miner_lifecycle": "in_repair", + "resulting_miner_lifecycle": "active", + "pre_repair_lifecycle": "spare", + "completion_summary": "Load test passed.", + }, + ) + self.assertEqual( + self.repository.calls, + [ + "transaction", + "get_repair_identity", + "lock_miner", + "lock_repair", + "get_active_repair", + "get_next_event_sequence", + "get_pre_repair_lifecycle", + "complete_repair", + "set_miner_lifecycle", + "create_event:REPAIR_COMPLETED", + "get_repair_detail", + ], + ) + + def test_spare_target_and_blank_summary_are_preserved_truthfully(self): + result = self.complete(target="spare", summary=" \t ") + + self.assertEqual(result.miner_lifecycle, "spare") + self.assertIsNone(self.repository.repair["completion_summary"]) + self.assertEqual( + self.repository.events[-1]["payload"], + { + "previous_miner_lifecycle": "in_repair", + "resulting_miner_lifecycle": "spare", + "pre_repair_lifecycle": "spare", + }, + ) + + def test_maximum_length_summary_is_preserved(self): + summary = "x" * 4000 + + self.complete(summary=summary) + + self.assertEqual( + self.repository.repair["completion_summary"], + summary, + ) + self.assertEqual( + self.repository.events[-1]["payload"]["completion_summary"], + summary, + ) + + def test_completion_actor_is_required_before_transaction(self): + with self.assertRaises(RepairCompletionActorRequired): + self.service.complete_repair( + self.repository.repair_id, + RepairCompleteRequest(target_lifecycle="active"), + actor_user_id=None, + ) + self.assertEqual(self.repository.calls, []) + + def test_unknown_repair_is_not_found_without_writes(self): + with self.assertRaises(RepairNotFound): + self.service.complete_repair( + uuid4(), + RepairCompleteRequest(target_lifecycle="active"), + actor_user_id=self.actor_id, + ) + self.assertEqual(self.repository.repair["status"], "OPEN") + + def test_completed_repair_is_conflict(self): + repository = FakeCompletionRepository(status="COMPLETED", lifecycle="active") + with self.assertRaises(RepairNotActive): + RepairService(repository).complete_repair( + repository.repair_id, + RepairCompleteRequest(target_lifecycle="active"), + actor_user_id=self.actor_id, + ) + + def test_every_non_active_repair_status_is_a_conflict(self): + for status in ("COMPLETED", "CANCELLED"): + with self.subTest(status=status): + repository = FakeCompletionRepository( + status=status, + lifecycle="in_repair", + ) + with self.assertRaises(RepairNotActive): + RepairService(repository).complete_repair( + repository.repair_id, + RepairCompleteRequest(target_lifecycle="active"), + actor_user_id=self.actor_id, + ) + + def test_miner_outside_repair_is_conflict(self): + repository = FakeCompletionRepository(lifecycle="active") + with self.assertRaises(RepairMinerNotInRepair): + RepairService(repository).complete_repair( + repository.repair_id, + RepairCompleteRequest(target_lifecycle="spare"), + actor_user_id=self.actor_id, + ) + + def test_active_repair_mismatch_is_conflict(self): + self.repository.active_mismatch = True + with self.assertRaises(RepairActiveMismatch): + self.complete() + + def test_completion_failure_rolls_back_every_change(self): + self.repository.completion_error = RuntimeError("completion failed") + with self.assertRaisesRegex(RuntimeError, "completion failed"): + self.complete() + self.assertEqual(self.repository.repair["status"], "OPEN") + self.assertEqual(self.repository.miner["lifecycle_status"], "in_repair") + self.assertEqual(len(self.repository.events), 2) + + def test_lifecycle_failure_rolls_back_every_change(self): + self.repository.lifecycle_error = RuntimeError("lifecycle failed") + with self.assertRaisesRegex(RuntimeError, "lifecycle failed"): + self.complete() + self.assertEqual(self.repository.repair["status"], "OPEN") + self.assertEqual(self.repository.miner["lifecycle_status"], "in_repair") + self.assertEqual(len(self.repository.events), 2) + + def test_event_failure_rolls_back_every_change(self): + self.repository.event_error = RuntimeError("event failed") + with self.assertRaisesRegex(RuntimeError, "event failed"): + self.complete() + self.assertEqual(self.repository.repair["status"], "OPEN") + self.assertEqual(self.repository.miner["lifecycle_status"], "in_repair") + self.assertEqual(len(self.repository.events), 2) + + +class ReturnToServiceRepositoryTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.repository = RepairRepository() + cls.actor_id = UUID("00000000-0000-0000-0000-000000001203") + + def setUp(self): + with self.repository._connection_scope(write=True) as connection: + with connection.cursor() as cur: + cur.execute( + "DROP TRIGGER IF EXISTS test_completion_event_failure " + "ON repair_events" + ) + cur.execute( + "DROP TRIGGER IF EXISTS test_completion_lifecycle_failure " + "ON miners" + ) + cur.execute( + "DROP FUNCTION IF EXISTS test_fail_completion_event()" + ) + cur.execute( + "DROP FUNCTION IF EXISTS test_fail_completion_lifecycle()" + ) + cur.execute("DELETE FROM repair_events") + cur.execute("DELETE FROM repairs") + cur.execute("DELETE FROM miner_tags") + cur.execute("DELETE FROM miner_asset_id_history") + cur.execute("DELETE FROM miners") + cur.execute( + "DELETE FROM users WHERE username = 'return-service-test'" + ) + cur.execute( + """ + INSERT INTO users ( + id, username, display_name, password_hash, role_id + ) + SELECT %s, %s, %s, %s, id + FROM roles + WHERE name = 'Admin' + """, + ( + self.actor_id, + "return-service-test", + "Return Service Test", + "not-used", + ), + ) + + def create_miner(self, lifecycle="active"): + with self.repository._connection_scope(write=True) as connection: + with connection.cursor(row_factory=dict_row) as cur: + cur.execute( + """ + INSERT INTO miners ( + asset_tag, status, lifecycle_status, + first_seen_at, last_seen_at + ) + VALUES (%s, 'unknown', %s, NULL, NULL) + RETURNING id + """, + (f"RETURN-{uuid4()}", lifecycle), + ) + return cur.fetchone()["id"] + + def begin(self, *, lifecycle="active", notes=None): + miner_id = self.create_miner(lifecycle) + result = RepairService(self.repository).begin_repair( + BeginRepairRequest( + miner_id=miner_id, + reason="FAN", + notes=notes, + ), + actor_user_id=self.actor_id, + ) + return miner_id, result.repair_id + + def fetch_repair(self, repair_id): + with self.repository._connection_scope() as connection: + with connection.cursor(row_factory=dict_row) as cur: + cur.execute("SELECT * FROM repairs WHERE id = %s", (repair_id,)) + return dict(cur.fetchone()) + + def fetch_events(self, repair_id): + with self.repository._connection_scope() as connection: + with connection.cursor(row_factory=dict_row) as cur: + cur.execute( + """ + SELECT * FROM repair_events + WHERE repair_id = %s + ORDER BY sequence + """, + (repair_id,), + ) + return [dict(row) for row in cur.fetchall()] + + def lifecycle(self, miner_id): + with self.repository._connection_scope() as connection: + with connection.cursor() as cur: + cur.execute( + "SELECT lifecycle_status FROM miners WHERE id = %s", + (miner_id,), + ) + return cur.fetchone()[0] + + def test_completion_persists_actor_time_summary_lifecycle_and_event(self): + miner_id, repair_id = self.begin(notes="Replace failed fan.") + before = datetime.now(timezone.utc) + + result = RepairService(self.repository).complete_repair( + repair_id, + RepairCompleteRequest( + target_lifecycle="active", + completion_summary=" Load test passed. ", + ), + actor_user_id=self.actor_id, + ) + + repair = self.fetch_repair(repair_id) + events = self.fetch_events(repair_id) + self.assertEqual(repair["status"], "COMPLETED") + self.assertGreaterEqual(repair["completed_at"], before) + self.assertEqual(repair["completed_by"], self.actor_id) + self.assertEqual(repair["completion_summary"], "Load test passed.") + self.assertEqual(repair["resulting_lifecycle"], "active") + self.assertEqual(self.lifecycle(miner_id), "active") + self.assertEqual(result.status, "COMPLETED") + self.assertEqual(result.closed_by.user_id, self.actor_id) + self.assertEqual(result.latest_event_sequence, 3) + self.assertEqual( + [(event["sequence"], event["event_type"]) for event in events], + [ + (1, "REPAIR_STARTED"), + (2, "INITIAL_NOTE"), + (3, "REPAIR_COMPLETED"), + ], + ) + self.assertEqual(events[-1]["created_by"], self.actor_id) + self.assertEqual( + events[-1]["payload"], + { + "previous_miner_lifecycle": "in_repair", + "resulting_miner_lifecycle": "active", + "pre_repair_lifecycle": "active", + "completion_summary": "Load test passed.", + }, + ) + + def test_spare_return_and_blank_summary(self): + miner_id, repair_id = self.begin(lifecycle="spare") + RepairService(self.repository).complete_repair( + repair_id, + RepairCompleteRequest( + target_lifecycle="spare", + completion_summary=" ", + ), + actor_user_id=self.actor_id, + ) + repair = self.fetch_repair(repair_id) + self.assertIsNone(repair["completion_summary"]) + self.assertEqual(repair["resulting_lifecycle"], "spare") + self.assertEqual(self.lifecycle(miner_id), "spare") + self.assertNotIn( + "completion_summary", + self.fetch_events(repair_id)[-1]["payload"], + ) + + def test_pre_repair_lifecycle_requires_exactly_one_start_event(self): + changes = ( + ("delete", "DELETE FROM repair_events WHERE repair_id = %s"), + ( + "duplicate", + """ + INSERT INTO repair_events ( + repair_id, sequence, event_type, created_by, payload + ) + SELECT + repair_id, + 2, + event_type, + created_by, + payload + FROM repair_events + WHERE repair_id = %s + AND event_type = 'REPAIR_STARTED' + """, + ), + ) + for name, change_sql in changes: + with self.subTest(change=name): + _, repair_id = self.begin() + with self.repository._connection_scope(write=True) as connection: + with connection.cursor() as cur: + cur.execute(change_sql, (repair_id,)) + + RepairService(self.repository).complete_repair( + repair_id, + RepairCompleteRequest(target_lifecycle="active"), + actor_user_id=self.actor_id, + ) + + payload = self.fetch_events(repair_id)[-1]["payload"] + self.assertNotIn("pre_repair_lifecycle", payload) + + def test_completion_releases_active_repair_constraint(self): + miner_id, repair_id = self.begin() + service = RepairService(self.repository) + service.complete_repair( + repair_id, + RepairCompleteRequest(target_lifecycle="active"), + actor_user_id=self.actor_id, + ) + later = service.begin_repair( + BeginRepairRequest(miner_id=miner_id, reason="INSPECTION"), + actor_user_id=self.actor_id, + ) + self.assertNotEqual(later.repair_id, repair_id) + + def test_event_failure_rolls_back_completion_and_lifecycle(self): + miner_id, repair_id = self.begin() + with self.repository._connection_scope(write=True) as connection: + with connection.cursor() as cur: + cur.execute( + """ + CREATE FUNCTION test_fail_completion_event() + RETURNS trigger AS $$ + BEGIN + IF NEW.event_type = 'REPAIR_COMPLETED' THEN + RAISE EXCEPTION 'forced completion event failure'; + END IF; + RETURN NEW; + END; + $$ LANGUAGE plpgsql + """ + ) + cur.execute( + """ + CREATE TRIGGER test_completion_event_failure + BEFORE INSERT ON repair_events + FOR EACH ROW + EXECUTE FUNCTION test_fail_completion_event() + """ + ) + + with self.assertRaises(RaiseException): + RepairService(self.repository).complete_repair( + repair_id, + RepairCompleteRequest(target_lifecycle="active"), + actor_user_id=self.actor_id, + ) + self.assertEqual(self.fetch_repair(repair_id)["status"], "OPEN") + self.assertEqual(self.lifecycle(miner_id), "in_repair") + self.assertEqual(len(self.fetch_events(repair_id)), 1) + + def test_lifecycle_failure_rolls_back_completion_and_event(self): + miner_id, repair_id = self.begin() + with self.repository._connection_scope(write=True) as connection: + with connection.cursor() as cur: + cur.execute( + """ + CREATE FUNCTION test_fail_completion_lifecycle() + RETURNS trigger AS $$ + BEGIN + IF OLD.lifecycle_status = 'in_repair' + AND NEW.lifecycle_status <> 'in_repair' THEN + RAISE EXCEPTION 'forced completion lifecycle failure'; + END IF; + RETURN NEW; + END; + $$ LANGUAGE plpgsql + """ + ) + cur.execute( + """ + CREATE TRIGGER test_completion_lifecycle_failure + BEFORE UPDATE ON miners + FOR EACH ROW + EXECUTE FUNCTION test_fail_completion_lifecycle() + """ + ) + + with self.assertRaises(RaiseException): + RepairService(self.repository).complete_repair( + repair_id, + RepairCompleteRequest(target_lifecycle="active"), + actor_user_id=self.actor_id, + ) + self.assertEqual(self.fetch_repair(repair_id)["status"], "OPEN") + self.assertEqual(self.lifecycle(miner_id), "in_repair") + self.assertEqual(len(self.fetch_events(repair_id)), 1) + + def test_concurrent_completion_succeeds_once(self): + miner_id, repair_id = self.begin() + barrier = Barrier(2) + + def complete(): + barrier.wait() + return RepairService(RepairRepository()).complete_repair( + repair_id, + RepairCompleteRequest(target_lifecycle="active"), + actor_user_id=self.actor_id, + ) + + outcomes = [] + with ThreadPoolExecutor(max_workers=2) as executor: + futures = [executor.submit(complete), executor.submit(complete)] + for future in futures: + try: + outcomes.append(future.result(timeout=10)) + except RepairNotActive as exc: + outcomes.append(exc) + + self.assertEqual( + sum(not isinstance(item, Exception) for item in outcomes), + 1, + ) + self.assertEqual( + sum(isinstance(item, RepairNotActive) for item in outcomes), + 1, + ) + completed_events = [ + event + for event in self.fetch_events(repair_id) + if event["event_type"] == "REPAIR_COMPLETED" + ] + self.assertEqual(len(completed_events), 1) + self.assertEqual(self.lifecycle(miner_id), "active") + + def test_database_constraints_reject_invalid_completion_metadata(self): + miner_id, repair_id = self.begin() + with self.assertRaises(CheckViolation): + with self.repository._connection_scope(write=True) as connection: + with connection.cursor() as cur: + cur.execute( + """ + UPDATE repairs + SET resulting_lifecycle = 'active' + WHERE id = %s + """, + (repair_id,), + ) + with self.assertRaises(CheckViolation): + with self.repository._connection_scope(write=True) as connection: + with connection.cursor() as cur: + cur.execute( + """ + UPDATE repairs + SET status = 'COMPLETED', + completed_at = now(), + completed_by = %s, + resulting_lifecycle = 'retired' + WHERE id = %s + """, + (self.actor_id, repair_id), + ) + with self.assertRaises(CheckViolation): + with self.repository._connection_scope(write=True) as connection: + with connection.cursor() as cur: + cur.execute( + """ + UPDATE repairs + SET status = 'COMPLETED', + completed_at = now(), + completed_by = %s + WHERE id = %s + """, + (self.actor_id, repair_id), + ) + self.assertEqual(self.fetch_repair(repair_id)["status"], "OPEN") + self.assertEqual(self.lifecycle(miner_id), "in_repair") + + def test_database_rejects_invalid_completion_event_payload(self): + _, repair_id = self.begin() + with self.assertRaises(CheckViolation): + with self.repository._connection_scope(write=True) as connection: + with connection.cursor() as cur: + cur.execute( + """ + INSERT INTO repair_events ( + repair_id, sequence, event_type, + created_by, payload + ) VALUES ( + %s, 2, 'REPAIR_COMPLETED', %s, + '{"resulting_miner_lifecycle":"active"}'::jsonb + ) + """, + (repair_id, self.actor_id), + ) + + +class StubCompletionService: + def __init__(self): + self.result = None + self.error = None + self.repair_id = None + self.request = None + self.actor_id = None + + def complete_repair(self, repair_id, request, *, actor_user_id): + self.repair_id = repair_id + self.request = request + self.actor_id = actor_user_id + if self.error is not None: + raise self.error + return self.result + + +class ReturnToServiceRouterTests(unittest.TestCase): + def setUp(self): + self.service = StubCompletionService() + self.app = FastAPI() + self.app.include_router(router) + self.app.dependency_overrides[get_repair_service] = lambda: self.service + self.client = TestClient(self.app) + + def tearDown(self): + self.client.close() + + def authenticate(self, *, role="Technician", permissions=("repairs.complete",)): + actor_id = uuid4() + self.app.dependency_overrides[get_current_user] = lambda: { + "id": str(actor_id), + "username": role.lower().replace(" ", "-"), + "role": role, + "permissions": list(permissions), + } + return actor_id + + def test_authentication_and_permission_are_required(self): + repair_id = uuid4() + self.assertEqual( + self.client.post( + f"/api/repairs/{repair_id}/complete", + json={"target_lifecycle": "active"}, + ).status_code, + 401, + ) + self.authenticate(permissions=("repairs.read",)) + self.assertEqual( + self.client.post( + f"/api/repairs/{repair_id}/complete", + json={"target_lifecycle": "active"}, + ).status_code, + 403, + ) + + def test_admin_manager_and_technician_can_complete(self): + repair_id = uuid4() + for role in ("Admin", "Manager", "Technician"): + with self.subTest(role=role): + actor_id = self.authenticate(role=role) + self.service.result = completed_detail( + repair_id=repair_id, + actor_id=actor_id, + ) + response = self.client.post( + f"/api/repairs/{repair_id}/complete", + json={"target_lifecycle": "active"}, + ) + self.assertEqual(response.status_code, 200) + + def test_read_only_cannot_complete(self): + self.authenticate(role="Read Only", permissions=("repairs.read",)) + response = self.client.post( + f"/api/repairs/{uuid4()}/complete", + json={"target_lifecycle": "active"}, + ) + self.assertEqual(response.status_code, 403) + + def test_success_returns_detail_and_authenticated_actor(self): + repair_id = uuid4() + miner_id = uuid4() + actor_id = self.authenticate() + self.service.result = completed_detail( + repair_id=repair_id, + miner_id=miner_id, + actor_id=actor_id, + ) + + response = self.client.post( + f"/api/repairs/{repair_id}/complete", + json={ + "target_lifecycle": "spare", + "completion_summary": "Ready for shelf.", + }, + ) + + self.assertEqual(response.status_code, 200) + body = response.json() + self.assertEqual(body["data"]["repair_id"], str(repair_id)) + self.assertEqual(body["data"]["status"], "COMPLETED") + self.assertEqual(body["data"]["latest_event_sequence"], 3) + self.assertEqual(self.service.actor_id, actor_id) + self.assertEqual(self.service.request.target_lifecycle, "spare") + self.assertEqual(body["meta"], {}) + self.assertIsNone(body["error"]) + + def test_unknown_repair_and_conflicts_map_to_http(self): + actor_id = self.authenticate() + repair_id = uuid4() + self.service.error = RepairNotFound(repair_id) + self.assertEqual( + self.client.post( + f"/api/repairs/{repair_id}/complete", + json={"target_lifecycle": "active"}, + ).status_code, + 404, + ) + self.service.error = RepairNotActive(repair_id, "COMPLETED") + self.assertEqual( + self.client.post( + f"/api/repairs/{repair_id}/complete", + json={"target_lifecycle": "active"}, + ).status_code, + 409, + ) + self.assertIsNotNone(actor_id) + + def test_request_validation(self): + self.authenticate() + repair_id = uuid4() + invalid_bodies = ( + {}, + {"target_lifecycle": "inactive"}, + {"target_lifecycle": "in_repair"}, + {"target_lifecycle": "retired"}, + {"target_lifecycle": "unknown"}, + { + "target_lifecycle": "active", + "completion_summary": "x" * 4001, + }, + {"target_lifecycle": "active", "completed_by": str(uuid4())}, + {"target_lifecycle": "active", "completed_at": NOW.isoformat()}, + {"target_lifecycle": "active", "payload": {}}, + ) + for body in invalid_bodies: + with self.subTest(body=list(body)): + response = self.client.post( + f"/api/repairs/{repair_id}/complete", + json=body, + ) + self.assertEqual(response.status_code, 422) + + def test_maximum_length_summary_is_accepted(self): + repair_id = uuid4() + actor_id = self.authenticate() + self.service.result = completed_detail( + repair_id=repair_id, + actor_id=actor_id, + ) + + response = self.client.post( + f"/api/repairs/{repair_id}/complete", + json={ + "target_lifecycle": "active", + "completion_summary": "x" * 4000, + }, + ) + + self.assertEqual(response.status_code, 200) + self.assertEqual( + self.service.request.completion_summary, + "x" * 4000, + ) + + def test_malformed_uuid_returns_validation_error(self): + self.authenticate() + response = self.client.post( + "/api/repairs/not-a-uuid/complete", + json={"target_lifecycle": "active"}, + ) + self.assertEqual(response.status_code, 422) + + +class ReturnToServiceArchitectureTests(unittest.TestCase): + @staticmethod + def full_matches(app, method, path): + scope = { + "type": "http", + "method": method, + "path": path, + "root_path": "", + } + return [ + route + for route in app.routes + if route.matches(scope)[0] is Match.FULL + ] + + def test_exactly_one_completion_route_and_permission_mapping(self): + complete_routes = [ + route + for route in router.routes + if route.path == "/api/repairs/{repair_id}/complete" + and "POST" in route.methods + ] + self.assertEqual(len(complete_routes), 1) + repair_id = uuid4() + self.assertEqual( + required_api_permission( + "POST", f"/api/repairs/{repair_id}/complete" + ), + "repairs.complete", + ) + self.assertEqual( + required_api_permission("POST", "/api/repairs"), + "repairs.create", + ) + self.assertEqual( + required_api_permission( + "GET", f"/api/repairs/{repair_id}" + ), + "repairs.read", + ) + + def test_static_and_uuid_routes_keep_their_handlers(self): + from app.main import app + + cases = ( + ("GET", "/api/repairs/queue", "minerops_repair_queue_api"), + ( + "GET", + "/api/repairs/parts-summary", + "minerops_repair_parts_summary_api", + ), + ( + "POST", + f"/api/repairs/{uuid4()}/complete", + "complete_repair", + ), + ) + for method, path, endpoint in cases: + with self.subTest(path=path): + matches = self.full_matches(app, method, path) + self.assertGreaterEqual(len(matches), 1) + self.assertEqual(matches[0].endpoint.__name__, endpoint) + + def test_layering_and_immutable_event_surface(self): + repairs_path = Path(__file__).parents[1] / "app" / "repairs" + router_source = (repairs_path / "router.py").read_text( + encoding="utf-8" + ) + for keyword in ( + "SELECT ", + "INSERT ", + "UPDATE ", + "DELETE ", + ".commit(", + ".rollback(", + ".transaction(", + ): + self.assertNotIn(keyword.upper(), router_source.upper()) + for filename in ("service.py", "repository.py"): + tree = ast.parse( + (repairs_path / filename).read_text(encoding="utf-8") + ) + modules = [] + for node in ast.walk(tree): + if isinstance(node, ast.Import): + modules.extend(alias.name for alias in node.names) + elif isinstance(node, ast.ImportFrom) and node.module: + modules.append(node.module) + self.assertFalse( + any( + module == "fastapi" or module.startswith("fastapi.") + for module in modules + ) + ) + self.assertFalse(hasattr(RepairRepository, "update_repair")) + self.assertFalse(hasattr(RepairRepository, "update_repair_event")) + self.assertFalse(hasattr(RepairRepository, "delete_repair_event")) + + +if __name__ == "__main__": + unittest.main() diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index deb62ca..c681da1 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -99,11 +99,11 @@ durable asset record. Repairs, maintenance, work orders, parts usage, equipment movement, testing, and return-to-service history become coherent operational records rather than disconnected actions. -The Begin Repair and read-only Repair Timeline workflows are complete. An -authorized actor can open a repair against a durable miner UUID, record -sequenced starting events, and move the miner into repair atomically. Authorized -readers can inspect the repair and its ordered immutable events. Later repair -lifecycle workflows remain planned. +The Begin Repair, read-only Repair Timeline, and Return to Service workflows are +complete. An authorized actor can open a repair against a durable miner UUID, +record sequenced events, and move the miner into or out of repair atomically. +Authorized readers can inspect the repair and its ordered immutable events. +Later repair lifecycle workflows remain planned. Technician experience is a priority. Responsive and QR-assisted workflows may reduce lookup and data-entry effort where they reinforce the same asset record. diff --git a/docs/releases/v0.2-repair-workflow.md b/docs/releases/v0.2-repair-workflow.md index 915d758..d185fce 100644 --- a/docs/releases/v0.2-repair-workflow.md +++ b/docs/releases/v0.2-repair-workflow.md @@ -15,10 +15,10 @@ and immutable history. - [x] Begin Repair (`POST /api/repairs`) - [x] Repair detail and timeline read APIs +- [x] Return to Service (`POST /api/repairs/{repair_id}/complete`) ## Planned -- [ ] Return to Service - [ ] Control Board Replacement - [ ] Repair Parts Tracking @@ -41,9 +41,8 @@ not make these workflows complete. ## Candidate Task Order -1. `003-return-to-service.md` -2. `004-control-board-replacement.md` -3. `005-repair-parts-tracking.md` +1. `004-control-board-replacement.md` +2. `005-repair-parts-tracking.md` These are proposed future files under `docs/tasks/`. They do not exist yet and must not be treated as approved specifications until they are created and diff --git a/docs/tasks/003-return-to-service.md b/docs/tasks/003-return-to-service.md new file mode 100644 index 0000000..445cb3a --- /dev/null +++ b/docs/tasks/003-return-to-service.md @@ -0,0 +1,814 @@ +# Task 003: Return to Service + +## Status + +Ready for implementation. + +## Objective + +Complete the MinerOps repair lifecycle by adding an explicit **Return to Service** operation that closes an active repair, records an immutable completion event, and transitions the miner from `in_repair` to an eligible post-repair lifecycle state. + +The operation must be atomic, permission-controlled, auditable, safe under concurrent requests, and compatible with the existing Begin Repair and Repair Timeline implementations. + +## Product Intent + +A repair should not remain open indefinitely after work is complete. + +An authorized operator must be able to: + +1. identify the active repair, +2. confirm the miner is ready to leave repair, +3. select or confirm the resulting lifecycle state, +4. optionally record a concise completion summary, +5. close the repair, +6. transition the miner out of `in_repair`, +7. preserve immutable operational history. + +This is an operational command, not generic repair editing. + +## Background + +Task 001 introduced the Begin Repair workflow. + +Task 002 introduced repair detail and immutable repair timeline read APIs. + +The current repair model supports: + +- creating an active repair, +- moving a miner into `in_repair`, +- preserving the miner's pre-repair lifecycle in the repair-start event payload, +- enforcing one active repair per miner, +- reading repair details, +- reading ordered repair events. + +Task 003 completes that lifecycle. + +The implementation must inspect the repository and reuse current names, models, exception conventions, migration patterns, event sequencing, and transaction infrastructure rather than inventing parallel abstractions. + +## Scope + +This task includes: + +- a new `repairs.complete` permission, +- a new explicit Return to Service API, +- request and response models, +- service-layer completion workflow, +- focused repository methods, +- repair completion persistence, +- miner lifecycle transition, +- immutable completion event, +- concurrency protection, +- migration and permission assignment, +- full authorization, domain, migration, routing, and regression tests, +- release-document update after implementation passes. + +## Explicitly Out of Scope + +Do not implement: + +- generic repair update or patch endpoints, +- reopening a completed repair, +- cancelling a repair, +- abandoning a repair, +- repair diagnosis fields, +- repair photos, +- repair parts consumption, +- technician assignment, +- work orders, +- control board replacement, +- MAC transfer, +- automatic monitoring confirmation, +- automated burn-in or test workflows, +- bulk Return to Service, +- notifications, +- UI changes unless the repository already requires generated API client updates, +- changes to unrelated legacy repair routes. + +Those belong in later tasks. + +## User and Operator Workflow + +A typical flow is: + +1. A technician opens the active repair. +2. The technician completes physical work and any required validation. +3. The technician invokes Return to Service. +4. The technician chooses the miner's destination lifecycle: + - the original pre-repair lifecycle, when eligible, + - or another explicitly allowed lifecycle. +5. The technician may enter a completion summary. +6. MinerOps atomically: + - verifies the repair is active, + - verifies the miner is still in repair, + - validates the destination lifecycle, + - closes the repair, + - transitions the miner, + - appends a completion event. +7. The repair becomes read-only historical data. + +## API + +Add one explicit action route. + +```http +POST /api/repairs/{repair_id}/complete +``` + +Use `complete` unless existing route naming conventions make `return-to-service` materially more consistent. Do not add both aliases. + +### Authentication + +Required. + +### Permission + +```text +repairs.complete +``` + +### Path Parameter + +- `repair_id`: UUID + +### Request Body + +The request should contain: + +```json +{ + "target_lifecycle": "active", + "completion_summary": "Replaced failed fan and completed load test." +} +``` + +Use repository-established lifecycle values and naming. + +#### `target_lifecycle` + +Required unless the service can unambiguously restore the original lifecycle under existing product rules. + +Preferred design: require an explicit target lifecycle in the request so the operator confirms the resulting operational state. + +Allowed target values must be restricted to valid non-repair lifecycle states. + +At minimum: + +- `active` +- `inactive` + +Also allow other existing non-terminal lifecycle states only if they are already legitimate post-repair states in the current domain model. + +Never allow: + +- `in_repair` +- unknown values +- deprecated values +- terminal or destructive states that are not valid Return to Service destinations + +Do not invent new lifecycle states. + +#### `completion_summary` + +Optional. + +Requirements: + +- trim leading and trailing whitespace, +- empty after trimming becomes `null`, +- enforce a sensible maximum length using current project conventions, +- preserve text exactly after normalization, +- do not interpret it as structured diagnosis or parts data. + +### Successful Response + +Return the completed repair using the existing repair-detail response model where practical. + +The response must allow the client to determine: + +- repair UUID, +- miner UUID, +- completed status, +- completion timestamp, +- completing actor where exposed by current conventions, +- resulting miner lifecycle, +- timeline or event count only if already part of the established response. + +Do not expand the response with unrelated miner details. + +### Status Codes + +#### `200 OK` + +Preferred when returning the completed repair representation. + +Use `201` only if existing operational-command conventions clearly require it. Do not use `204` if the existing repair response can be returned cleanly. + +#### `401 Unauthorized` + +Unauthenticated request. + +#### `403 Forbidden` + +Authenticated actor lacks `repairs.complete`. + +#### `404 Not Found` + +The repair UUID does not exist. + +Do not reveal authorization-sensitive information beyond existing project behavior. + +#### `409 Conflict` + +Use for valid requests that conflict with current domain state, including: + +- repair is already completed, +- repair is otherwise not active, +- miner is no longer in `in_repair`, +- repair does not match the miner's active repair, +- target lifecycle is known but not eligible for Return to Service, +- a concurrent request completed the repair first. + +#### `422 Unprocessable Entity` + +Use for malformed UUIDs, invalid JSON, missing required fields, unknown lifecycle values, excessive summary length, or other request-validation errors. + +## Permissions + +Add: + +```text +repairs.complete +``` + +### Intended Role Assignment + +Follow current role conventions and migrations. + +Expected behavior: + +- Administrator: allowed +- Technician: allowed +- Read Only / Viewer: denied +- Auditor: denied unless current policy explicitly grants operational actions +- Unauthenticated: denied + +Do not grant the permission to every role by default. + +Do not overload `repairs.create`. + +## Domain Rules + +### Active Repair Requirement + +Only an active repair can be completed. + +A completed repair cannot be completed again. + +### Miner Lifecycle Requirement + +The associated miner must currently be in `in_repair`. + +If it is not, return a domain conflict and do not modify anything. + +### Destination Lifecycle + +The destination must: + +- exist in the current lifecycle domain, +- not equal `in_repair`, +- be explicitly allowed as a post-repair state, +- be applied only after all validation succeeds. + +### Original Lifecycle + +The Begin Repair workflow preserves the previous lifecycle in the `REPAIR_STARTED` event payload. + +The service may use this value for: + +- validation, +- response context, +- defaulting only if the API contract and existing model conventions support it safely. + +The implementation must not assume every miner should return to `active`. + +If restoration of the previous lifecycle is supported, preserve exact historical meaning. Do not infer or fabricate it when missing. + +### Completion Is Final + +Once completed: + +- the repair is historical, +- the completion event is immutable, +- the active-repair constraint is released, +- a later repair must be a new repair record. + +### No Silent Repair Mutation + +Do not edit prior events. + +Do not replace the start event. + +Do not overwrite historical lifecycle values. + +## Architecture + +Preserve: + +```text +Router -> Service -> Repository -> PostgreSQL +``` + +### Router Responsibilities + +The router should: + +- define the action route, +- validate the UUID and request model, +- enforce authentication and `repairs.complete`, +- call the service, +- map established domain exceptions to HTTP responses, +- return the established response model. + +The router must not: + +- contain SQL, +- manage commits or rollbacks, +- choose lifecycle transitions, +- construct event sequence values, +- read actor UUIDs from the request body. + +### Service Responsibilities + +The service should: + +- load and lock the repair, +- verify it exists and is active, +- load and lock the associated miner when required, +- verify the miner is in repair, +- validate target lifecycle, +- normalize summary, +- close the repair, +- update miner lifecycle, +- append the completion event, +- return the completed domain result, +- ensure the operation is atomic. + +### Repository Responsibilities + +Repositories should provide focused methods for: + +- retrieving an active repair for update, +- retrieving the associated miner for update, +- marking a repair completed, +- updating lifecycle, +- obtaining or allocating the next repair event sequence, +- appending a repair event, +- reading the completed repair representation efficiently. + +Do not introduce a broad generic repair-update method. + +## Database and Migration + +A migration is required if the current schema does not already support all required completion data and permission assignment. + +Inspect the existing repair table before adding columns. + +### Preferred Repair Completion Fields + +If not already present, add only the fields needed to represent completion authoritatively: + +- `completed_at`: timezone-aware timestamp, nullable while active +- `completed_by_user_id`: UUID foreign key, nullable while active +- `completion_summary`: nullable text +- `resulting_lifecycle`: constrained lifecycle value, nullable while active + +Use existing naming conventions if equivalent columns already exist. + +### Status Representation + +Do not add redundant mutable status data if active versus completed can be derived safely from established fields. + +If the current repair schema already uses a status column, reuse and constrain it rather than introducing a second status mechanism. + +### Constraints + +Enforce critical invariants in PostgreSQL where practical. + +Examples: + +- active repair has no completion timestamp, +- completed repair has a completion timestamp, +- completed repair has a completing actor, +- resulting lifecycle is null while active and present when completed, +- resulting lifecycle cannot equal `in_repair`, +- completion summary length is constrained if the project uses database length checks, +- one active repair per miner remains enforced. + +Adapt the exact checks to the existing schema. + +### Foreign Keys + +`completed_by_user_id` should reference the established user table. + +Use the project's existing historical-record deletion policy. Operational history should not silently disappear. + +### Indexes + +Add only indexes justified by actual read patterns. + +Likely useful: + +- existing active-repair partial uniqueness, +- lookup by miner, +- lookup by completion timestamp only if current repair queue/history queries need it. + +Avoid speculative indexes. + +### Migration Data Handling + +Existing active repairs must remain active. + +Existing completed repairs, if legacy data exists, must be handled without inventing actor IDs, summaries, or lifecycle states. + +If new non-null completion invariants cannot be truthfully applied to legacy completed rows: + +- use nullable historical fields with conditional checks, +- preserve legacy data, +- document the limitation, +- do not fabricate values. + +### Downgrade + +The downgrade must restore the previous schema safely. + +Before dropping completion columns or constraints, restore miner lifecycle only if the downgrade actually reverses newly created completion data and there is a truthful historical source. + +Do not map every historical completed repair to `active`. + +Do not invent original lifecycle values. + +If a fully safe downgrade is impossible once real completion records exist, use the project's established explicit-refusal pattern rather than destructive guessing. + +## Events and History + +Append one immutable completion event. + +Use the existing repair event naming conventions. Preferred semantic name: + +```text +REPAIR_COMPLETED +``` + +Reuse an existing equivalent event type if already defined. + +### Event Fields + +The event must include: + +- event UUID, +- repair UUID, +- next explicit sequence number, +- authenticated actor UUID, +- server/database timestamp, +- structured payload. + +### Payload + +Include only durable, meaningful values. + +Recommended payload: + +```json +{ + "previous_miner_lifecycle": "in_repair", + "resulting_miner_lifecycle": "active", + "pre_repair_lifecycle": "inactive", + "completion_summary": "Replaced failed fan and completed load test." +} +``` + +`pre_repair_lifecycle` may be omitted if the existing event model or service cannot retrieve it reliably without ambiguity. + +Do not duplicate arbitrary repair fields in the payload. + +Do not store display names as authoritative actor identity. + +### Event Ordering + +Use the established explicit sequence mechanism. + +Do not calculate order from timestamps alone. + +Concurrent event insertion must not produce duplicate sequence values. + +## Models and Validation + +Create or extend focused models. + +Suggested names, adapted to current conventions: + +- `RepairCompleteRequest` +- `RepairDetailResponse` or existing equivalent +- internal completion result model if needed + +### Validation Requirements + +- UUID path validation, +- finite lifecycle validation, +- summary trimming, +- empty summary normalization, +- maximum summary length, +- no client-supplied actor, +- no client-supplied completion timestamp, +- no arbitrary event payload supplied by the client. + +## Concurrency and Atomicity + +The entire operation must use one transaction. + +The following must succeed or fail together: + +1. repair validation, +2. miner validation, +3. repair completion update, +4. miner lifecycle transition, +5. completion event insertion. + +### Concurrent Completion + +Two simultaneous completion requests must not both succeed. + +Expected result: + +- one request succeeds, +- the other receives `409 Conflict`, +- one completion event exists, +- one final lifecycle transition exists. + +Use row locking, compare-and-update semantics, or another database-backed mechanism consistent with the repository. + +### Concurrent Miner Changes + +A simultaneous lifecycle-changing operation must not leave: + +- a completed repair with miner still in `in_repair`, +- an active repair with miner already returned, +- conflicting lifecycle history. + +Lock ordering must be consistent with Begin Repair and other miner workflows to reduce deadlock risk. + +Document the chosen lock order in code comments only where it is not obvious. + +## Error and Failure Behaviour + +On any failure after the transaction starts: + +- no partial completion fields persist, +- miner lifecycle remains unchanged, +- no completion event persists, +- repair remains active. + +Unexpected database integrity failures should be translated using established service/repository exception conventions. + +Do not catch broad exceptions merely to convert them into `409`. + +## Tests + +Add focused tests following current test organization. + +### Authorization Tests + +Verify: + +- unauthenticated request is denied, +- user without `repairs.complete` is denied, +- Read Only / Viewer is denied, +- Technician is allowed, +- Administrator is allowed, +- existing `repairs.read` behavior is unchanged, +- existing `repairs.create` behavior is unchanged. + +### Happy Path Tests + +Verify: + +- active repair completes successfully, +- miner transitions from `in_repair` to requested eligible lifecycle, +- repair is marked completed, +- completion timestamp is server-controlled, +- completing actor is authenticated user, +- optional summary is persisted, +- blank summary normalizes to null, +- completion event is appended, +- event sequence follows existing events, +- response represents the completed repair. + +Test at least: + +- return to `active`, +- return to `inactive`, +- restoration to a valid original lifecycle when relevant. + +### Domain Conflict Tests + +Verify `409` for: + +- already completed repair, +- inactive/non-active repair, +- miner not in `in_repair`, +- mismatched active repair state, +- disallowed destination lifecycle, +- duplicate concurrent completion. + +### Validation Tests + +Verify `422` for: + +- malformed repair UUID, +- missing required lifecycle, +- unknown lifecycle, +- `in_repair` as destination, +- summary over maximum length, +- invalid request shape. + +If lifecycle defaulting is intentionally supported, replace the missing-field expectation with tests for documented default behavior. + +### Missing Resource Test + +Verify `404` for unknown repair UUID. + +### Transaction Rollback Tests + +Force failures at meaningful steps, such as: + +- miner update failure, +- repair completion update failure, +- event insertion failure. + +Verify no partial state persists. + +### Event Tests + +Verify: + +- exactly one completion event, +- immutable event behavior remains, +- actor is correct, +- payload is structured, +- ordering is explicit, +- prior events remain unchanged. + +### Database Constraint Tests + +Verify applicable constraints directly, including: + +- completed record requirements, +- active/completed consistency, +- resulting lifecycle restriction, +- one-active-repair rule remains valid. + +### Migration Tests + +Verify: + +- previous head upgrades, +- new permission exists, +- intended roles receive the permission, +- unintended roles do not, +- schema constraints exist, +- downgrade works safely or refuses explicitly according to project convention, +- re-upgrade works, +- empty database upgrades through head. + +### Routing Tests + +Verify: + +- static repair routes such as `/api/repairs/queue` and `/api/repairs/parts-summary` still resolve, +- `/api/repairs/{repair_id}/complete` resolves correctly, +- UUID detail and event routes remain unchanged, +- malformed static-looking paths are not accidentally consumed by UUID routes, +- expected route count increases only by the intended amount. + +### Regression Tests + +Run at minimum: + +- Begin Repair tests, +- Repair Timeline tests, +- inventory lifecycle tests, +- permission tests, +- migration lifecycle suite, +- relevant legacy repair tests, +- complete project suite if practical. + +## Documentation Updates + +After implementation and validation succeed: + +- mark Return to Service complete in `docs/releases/v0.2-repair-workflow.md`, +- update `docs/ROADMAP.md` only if the repository uses it as current implementation status, +- update API documentation if maintained separately. + +Do not mark Control Board Replacement or Repair Parts Tracking complete. + +Do not perform broad stale-document cleanup in this task unless a directly changed statement would become materially false. + +## Acceptance Criteria + +The task is complete when all of the following are true: + +1. `POST /api/repairs/{repair_id}/complete` exists. +2. Authentication and `repairs.complete` are enforced. +3. Only an active repair can be completed. +4. The associated miner must be in `in_repair`. +5. The target lifecycle is explicit and validated. +6. Repair completion and miner transition are atomic. +7. Exactly one immutable completion event is appended. +8. Concurrent completion cannot succeed twice. +9. Existing repair static and UUID routes still resolve. +10. Existing Begin Repair and Repair Timeline behavior remains unchanged. +11. Migration lifecycle validation passes. +12. Permission assignments are correct. +13. Focused and regression tests pass. +14. `git diff --check` passes. +15. No unrelated files are changed. +16. No commit or push is performed. + +## Validation Commands + +Use repository-native commands where available. + +At minimum run: + +```bash +python -m compileall . +``` + +Run shell syntax checks for changed shell files, if any. + +Run: + +- focused Return to Service tests, +- Begin Repair tests, +- Repair Timeline tests, +- inventory lifecycle tests, +- permission tests, +- migration tests, +- route audits, +- architecture audits, +- full suite when practical. + +Validate migrations through: + +- previous head to new head, +- downgrade, +- re-upgrade, +- empty database to head, +- disposable development baseline. + +Run: + +```bash +git diff --check +git status --short +``` + +## Architecture Audit + +Confirm: + +- no SQL in routers, +- no database driver imports in routers, +- no transaction management in routers, +- no FastAPI imports in services or repositories, +- no generic repair update repository introduced, +- actor identity comes from authentication, +- timestamps are server-controlled, +- events are append-only, +- route ordering is safe. + +## Completion Report + +Return: + +- branch and starting commit, +- files created, +- files modified, +- migration revision and parent, +- route added, +- permission added and role assignments, +- domain behavior, +- concurrency strategy, +- event type and payload, +- tests added, +- focused test results, +- regression results, +- migration validation results, +- architecture audit results, +- `git diff --check`, +- known limitations, +- final `git status --short`. + +Do not commit. +Do not push. diff --git a/scripts/test-begin-repair-migration.sh b/scripts/test-begin-repair-migration.sh index a43dc3e..c6f4886 100755 --- a/scripts/test-begin-repair-migration.sh +++ b/scripts/test-begin-repair-migration.sh @@ -57,7 +57,7 @@ echo "Upgrading the previous head to Begin Repair..." run_alembic upgrade 0009_inventory_asset_id_history previous_revision="$(run_alembic current 2>/dev/null)" [[ "${previous_revision}" == *"0009_inventory_asset_id_history"* ]] -run_alembic upgrade head +run_alembic upgrade 0010_begin_repair current_revision="$(run_alembic current 2>/dev/null)" [[ "${current_revision}" == *"0010_begin_repair"* ]] @@ -534,17 +534,17 @@ SELECT SQL )" [[ "${downgrade_state}" == "t" ]] -run_alembic upgrade head +run_alembic upgrade 0010_begin_repair reupgraded_revision="$(run_alembic current 2>/dev/null)" [[ "${reupgraded_revision}" == *"0010_begin_repair"* ]] -echo "Migrating an empty database from base through head..." +echo "Migrating an empty database from base through Begin Repair..." "${compose[@]}" exec -T postgres sh -ec \ 'psql -v ON_ERROR_STOP=1 -U "$POSTGRES_USER" -d postgres \ -c "CREATE DATABASE minerops_repair_empty"' >/dev/null "${compose[@]}" run --rm --no-deps \ -e "POSTGRES_DB=${empty_database}" \ - backend alembic upgrade head + backend alembic upgrade 0010_begin_repair empty_revision="$( "${compose[@]}" run --rm --no-deps \ diff --git a/scripts/test-dev-baseline.sh b/scripts/test-dev-baseline.sh index f9ccfb7..379b887 100755 --- a/scripts/test-dev-baseline.sh +++ b/scripts/test-dev-baseline.sh @@ -88,7 +88,7 @@ health_json="$(cat "${health_file}")" echo "Checking migrations and required schema..." current_revision="$("${compose[@]}" exec -T backend alembic current 2>/dev/null)" -[[ "${current_revision}" == *"0011_repair_timeline"* ]] +[[ "${current_revision}" == *"0012_return_to_service"* ]] missing_tables="$( "${compose[@]}" exec -T postgres sh -c \ diff --git a/scripts/test-repair-timeline-migration.sh b/scripts/test-repair-timeline-migration.sh index 08dd87c..78c47a9 100755 --- a/scripts/test-repair-timeline-migration.sh +++ b/scripts/test-repair-timeline-migration.sh @@ -57,7 +57,7 @@ echo "Upgrading the previous head to Repair Timeline..." run_alembic upgrade 0010_begin_repair previous_revision="$(run_alembic current 2>/dev/null)" [[ "${previous_revision}" == *"0010_begin_repair"* ]] -run_alembic upgrade head +run_alembic upgrade 0011_repair_timeline current_revision="$(run_alembic current 2>/dev/null)" [[ "${current_revision}" == *"0011_repair_timeline"* ]] @@ -149,17 +149,17 @@ SQL downgraded_revision="$(run_alembic current 2>/dev/null)" [[ "${downgraded_revision}" == *"0010_begin_repair"* ]] -run_alembic upgrade head +run_alembic upgrade 0011_repair_timeline reupgraded_revision="$(run_alembic current 2>/dev/null)" [[ "${reupgraded_revision}" == *"0011_repair_timeline"* ]] -echo "Migrating an empty database from base through head..." +echo "Migrating an empty database from base through Repair Timeline..." "${compose[@]}" exec -T postgres sh -ec \ 'psql -v ON_ERROR_STOP=1 -U "$POSTGRES_USER" -d postgres \ -c "CREATE DATABASE minerops_timeline_empty"' >/dev/null "${compose[@]}" run --rm --no-deps \ -e "POSTGRES_DB=${empty_database}" \ - backend alembic upgrade head + backend alembic upgrade 0011_repair_timeline empty_revision="$( "${compose[@]}" run --rm --no-deps \ diff --git a/scripts/test-return-to-service-migration.sh b/scripts/test-return-to-service-migration.sh new file mode 100755 index 0000000..964cf70 --- /dev/null +++ b/scripts/test-return-to-service-migration.sh @@ -0,0 +1,295 @@ +#!/usr/bin/env bash +set -euo pipefail + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +cd "${repo_root}" + +project_name="${MINEROPS_RETURN_MIGRATION_PROJECT:-minerops-return-migration-$$}" +empty_database="minerops_return_empty" +compose=( + docker compose + --project-name "${project_name}" + --env-file .env.dev + -f compose.dev.yaml +) + +cleanup() { + status=$? + if (( status != 0 )); then + echo + echo "Return to Service migration test failed. Recent PostgreSQL logs:" + "${compose[@]}" logs --no-color --tail 160 postgres || true + fi + "${compose[@]}" down --volumes --remove-orphans >/dev/null 2>&1 || true + exit "${status}" +} + +trap cleanup EXIT + +if [[ ! -f .env.dev ]]; then + echo "Missing .env.dev. Run ./scripts/dev-env-init.sh first." >&2 + exit 1 +fi + +run_alembic() { + "${compose[@]}" run --rm --no-deps backend alembic "$@" +} + +echo "Starting disposable PostgreSQL..." +"${compose[@]}" up --detach postgres +for attempt in $(seq 1 60); do + if "${compose[@]}" exec -T postgres sh -ec \ + 'pg_isready -U "$POSTGRES_USER" -d "$POSTGRES_DB"' \ + >/dev/null 2>&1; then + break + fi + if (( attempt == 60 )); then + echo "PostgreSQL did not become ready." >&2 + exit 1 + fi + sleep 1 +done + +echo "Building migration image..." +"${compose[@]}" build backend + +echo "Preparing previous head and truthful legacy completion data..." +run_alembic upgrade 0011_repair_timeline +previous_revision="$(run_alembic current 2>/dev/null)" +[[ "${previous_revision}" == *"0011_repair_timeline"* ]] +"${compose[@]}" exec -T postgres sh -ec \ + 'psql -v ON_ERROR_STOP=1 -U "$POSTGRES_USER" -d "$POSTGRES_DB"' <<'SQL' +INSERT INTO users ( + id, username, display_name, password_hash, role_id +) +SELECT + '00000000-0000-0000-0000-000000001301', + 'return-migration-test', + 'Return Migration Test', + 'not-used', + id +FROM roles +WHERE name = 'Admin'; + +INSERT INTO miners ( + id, asset_tag, status, lifecycle_status, first_seen_at, last_seen_at +) VALUES ( + '00000000-0000-0000-0000-000000001302', + 'RETURN-MIGRATION-LEGACY', + 'unknown', + 'active', + NULL, + NULL +); + +INSERT INTO repairs ( + id, miner_id, status, reason, opened_by, completed_at, completed_by +) VALUES ( + '00000000-0000-0000-0000-000000001303', + '00000000-0000-0000-0000-000000001302', + 'COMPLETED', + 'OTHER', + '00000000-0000-0000-0000-000000001301', + now(), + '00000000-0000-0000-0000-000000001301' +); +SQL + +echo "Upgrading previous head to Return to Service..." +run_alembic upgrade head +current_revision="$(run_alembic current 2>/dev/null)" +[[ "${current_revision}" == *"0012_return_to_service"* ]] + +echo "Checking schema, permission matrix, constraints, and legacy preservation..." +"${compose[@]}" exec -T postgres sh -ec \ + 'psql -v ON_ERROR_STOP=1 -U "$POSTGRES_USER" -d "$POSTGRES_DB"' <<'SQL' +DO $$ +DECLARE + v_roles TEXT[]; + v_constraint_count INTEGER; +BEGIN + IF ( + SELECT count(*) + FROM information_schema.columns + WHERE table_schema = 'public' + AND table_name = 'repairs' + AND column_name IN ('completion_summary', 'resulting_lifecycle') + ) <> 2 THEN + RAISE EXCEPTION 'Return to Service columns are missing'; + END IF; + + IF EXISTS ( + SELECT 1 + FROM repairs + WHERE id = '00000000-0000-0000-0000-000000001303' + AND ( + completion_summary IS NOT NULL + OR resulting_lifecycle IS NOT NULL + ) + ) THEN + RAISE EXCEPTION 'Legacy completion metadata was fabricated'; + END IF; + + IF (SELECT count(*) FROM permissions WHERE key = 'repairs.complete') <> 1 THEN + RAISE EXCEPTION 'repairs.complete permission was not registered once'; + END IF; + + SELECT array_agg(roles.name ORDER BY roles.name) + INTO v_roles + FROM role_permissions + JOIN roles ON roles.id = role_permissions.role_id + JOIN permissions ON permissions.id = role_permissions.permission_id + WHERE permissions.key = 'repairs.complete'; + IF v_roles <> ARRAY['Admin', 'Manager', 'Technician'] THEN + RAISE EXCEPTION 'Unexpected repairs.complete role matrix: %', v_roles; + END IF; + + SELECT count(*) + INTO v_constraint_count + FROM pg_constraint + WHERE conrelid IN ('repairs'::regclass, 'repair_events'::regclass) + AND conname IN ( + 'ck_repairs_resulting_lifecycle', + 'ck_repairs_completion_summary', + 'ck_repairs_completion_metadata_state', + 'ck_repair_events_type', + 'ck_repair_events_payload_shape' + ); + IF v_constraint_count <> 5 THEN + RAISE EXCEPTION 'Expected Return to Service constraints are missing'; + END IF; + + IF ( + SELECT convalidated + FROM pg_constraint + WHERE conrelid = 'repairs'::regclass + AND conname = 'ck_repairs_completion_metadata_state' + ) THEN + RAISE EXCEPTION 'Legacy completion compatibility constraint was unexpectedly validated'; + END IF; + + BEGIN + INSERT INTO repairs ( + miner_id, status, reason, opened_by, completed_at, completed_by + ) VALUES ( + '00000000-0000-0000-0000-000000001302', + 'COMPLETED', + 'OTHER', + '00000000-0000-0000-0000-000000001301', + now(), + '00000000-0000-0000-0000-000000001301' + ); + RAISE EXCEPTION 'New completed repair without resulting lifecycle was accepted'; + EXCEPTION + WHEN check_violation THEN NULL; + END; +END +$$; + +INSERT INTO permissions (key, description) +VALUES ('repairs.complete', 'Return repaired miners to service') +ON CONFLICT (key) DO UPDATE +SET description = EXCLUDED.description; +INSERT INTO role_permissions (role_id, permission_id) +SELECT roles.id, permissions.id +FROM roles +JOIN permissions ON permissions.key = 'repairs.complete' +WHERE roles.name IN ('Admin', 'Manager', 'Technician') +ON CONFLICT DO NOTHING; + +DO $$ +BEGIN + IF (SELECT count(*) FROM permissions WHERE key = 'repairs.complete') <> 1 + OR ( + SELECT count(*) + FROM role_permissions + JOIN permissions ON permissions.id = role_permissions.permission_id + WHERE permissions.key = 'repairs.complete' + ) <> 3 THEN + RAISE EXCEPTION 'repairs.complete seeding is not idempotent'; + END IF; +END +$$; +SQL + +echo "Downgrading and re-upgrading without new completion history..." +run_alembic downgrade 0011_repair_timeline +downgrade_state="$( + "${compose[@]}" exec -T postgres sh -ec \ + 'psql -v ON_ERROR_STOP=1 -U "$POSTGRES_USER" -d "$POSTGRES_DB" -At' <<'SQL' +SELECT + NOT EXISTS ( + SELECT 1 FROM permissions WHERE key = 'repairs.complete' + ) + AND EXISTS ( + SELECT 1 FROM permissions WHERE key = 'repairs.read' + ) + AND NOT EXISTS ( + SELECT 1 + FROM information_schema.columns + WHERE table_schema = 'public' + AND table_name = 'repairs' + AND column_name IN ('completion_summary', 'resulting_lifecycle') + ) + AND EXISTS ( + SELECT 1 + FROM repairs + WHERE id = '00000000-0000-0000-0000-000000001303' + AND status = 'COMPLETED' + AND completed_at IS NOT NULL + AND completed_by IS NOT NULL + ); +SQL +)" +[[ "${downgrade_state}" == "t" ]] +downgraded_revision="$(run_alembic current 2>/dev/null)" +[[ "${downgraded_revision}" == *"0011_repair_timeline"* ]] +run_alembic upgrade head +reupgraded_revision="$(run_alembic current 2>/dev/null)" +[[ "${reupgraded_revision}" == *"0012_return_to_service"* ]] + +echo "Checking downgrade refusal after authoritative completion history..." +"${compose[@]}" exec -T postgres sh -ec \ + 'psql -v ON_ERROR_STOP=1 -U "$POSTGRES_USER" -d "$POSTGRES_DB"' <<'SQL' +UPDATE repairs +SET completion_summary = 'Migration completion', + resulting_lifecycle = 'active' +WHERE id = '00000000-0000-0000-0000-000000001303'; + +INSERT INTO repair_events ( + repair_id, sequence, event_type, created_by, payload +) VALUES ( + '00000000-0000-0000-0000-000000001303', + 1, + 'REPAIR_COMPLETED', + '00000000-0000-0000-0000-000000001301', + '{ + "previous_miner_lifecycle":"in_repair", + "resulting_miner_lifecycle":"active", + "completion_summary":"Migration completion" + }'::jsonb +); +SQL +if downgrade_error="$(run_alembic downgrade 0011_repair_timeline 2>&1)"; then + echo "Downgrade discarded Return to Service history." >&2 + exit 1 +fi +[[ "${downgrade_error}" == *"would lose authoritative history"* ]] +failed_revision="$(run_alembic current 2>/dev/null)" +[[ "${failed_revision}" == *"0012_return_to_service"* ]] + +echo "Migrating an empty database from base through head..." +"${compose[@]}" exec -T postgres sh -ec \ + 'psql -v ON_ERROR_STOP=1 -U "$POSTGRES_USER" -d postgres \ + -c "CREATE DATABASE minerops_return_empty"' >/dev/null +"${compose[@]}" run --rm --no-deps \ + -e "POSTGRES_DB=${empty_database}" \ + backend alembic upgrade head +empty_revision="$( + "${compose[@]}" run --rm --no-deps \ + -e "POSTGRES_DB=${empty_database}" \ + backend alembic current 2>/dev/null +)" +[[ "${empty_revision}" == *"0012_return_to_service"* ]] + +echo "Return to Service migration test passed." diff --git a/scripts/test-return-to-service.sh b/scripts/test-return-to-service.sh new file mode 100755 index 0000000..9a8ce91 --- /dev/null +++ b/scripts/test-return-to-service.sh @@ -0,0 +1,59 @@ +#!/usr/bin/env bash +set -euo pipefail + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +cd "${repo_root}" + +project_name="${MINEROPS_RETURN_SERVICE_TEST_PROJECT:-minerops-return-service-$$}" +compose=( + docker compose + --project-name "${project_name}" + --env-file .env.dev + -f compose.dev.yaml +) + +cleanup() { + status=$? + if (( status != 0 )); then + echo + echo "Return to Service tests failed. Recent PostgreSQL logs:" + "${compose[@]}" logs --no-color --tail 120 postgres || true + fi + "${compose[@]}" down --volumes --remove-orphans >/dev/null 2>&1 || true + exit "${status}" +} + +trap cleanup EXIT + +if [[ ! -f .env.dev ]]; then + echo "Missing .env.dev. Run ./scripts/dev-env-init.sh first." >&2 + exit 1 +fi + +echo "Starting disposable Return to Service database..." +"${compose[@]}" up --detach postgres +for attempt in $(seq 1 60); do + if "${compose[@]}" exec -T postgres sh -ec \ + 'pg_isready -U "$POSTGRES_USER" -d "$POSTGRES_DB"' \ + >/dev/null 2>&1; then + break + fi + if (( attempt == 60 )); then + echo "PostgreSQL did not become ready." >&2 + exit 1 + fi + sleep 1 +done + +echo "Building Return to Service test image..." +"${compose[@]}" build backend + +echo "Migrating disposable database..." +"${compose[@]}" run --rm --no-deps backend alembic upgrade head + +echo "Running Return to Service repository, service, and router tests..." +"${compose[@]}" run --rm --no-deps \ + -v "${repo_root}/backend/tests:/app/tests:ro" \ + backend python -m unittest -v tests.test_return_to_service + +echo "Return to Service tests passed."