From 52bd0c38b2caab70080fcf277ad47d2d4b185239 Mon Sep 17 00:00:00 2001 From: Jonah Date: Wed, 11 Feb 2026 13:56:37 -0500 Subject: [PATCH 1/9] feat(OMN-2143): Checkpoint effect + validate compute nodes for pipeline resilience MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add two ONEX nodes for pipeline checkpoint persistence and validation: - node_checkpoint_effect (EFFECT): Write/read/list checkpoint YAML files with append-only semantics (per-attempt files, never overwrite) - node_checkpoint_validate_compute (COMPUTE): Pure structural validation of checkpoint data (path normalization, SHA format, phase consistency) - Shared models: ModelCheckpoint with discriminated union of 5 per-phase payloads - EnumCheckpointPhase: IMPLEMENT → LOCAL_REVIEW → CREATE_PR → PR_RELEASE_READY → READY_FOR_MERGE - 22 unit tests covering write/read/list round-trips and validation logic --- src/omnibase_infra/enums/__init__.py | 2 + .../enums/enum_checkpoint_phase.py | 59 +++++ .../models/checkpoint/__init__.py | 41 ++++ .../models/checkpoint/model_checkpoint.py | 123 ++++++++++ .../model_phase_payload_create_pr.py | 44 ++++ .../model_phase_payload_implement.py | 44 ++++ .../model_phase_payload_local_review.py | 43 ++++ .../model_phase_payload_pr_release_ready.py | 38 +++ .../model_phase_payload_ready_for_merge.py | 33 +++ .../nodes/node_checkpoint_effect/__init__.py | 10 + .../node_checkpoint_effect/contract.yaml | 53 +++++ .../handlers/__init__.py | 22 ++ .../handlers/handler_checkpoint_list.py | 157 +++++++++++++ .../handlers/handler_checkpoint_read.py | 186 +++++++++++++++ .../handlers/handler_checkpoint_write.py | 187 +++++++++++++++ .../node_checkpoint_effect/models/__init__.py | 18 ++ .../models/model_checkpoint_effect_input.py | 65 ++++++ .../models/model_checkpoint_effect_output.py | 70 ++++++ .../nodes/node_checkpoint_effect/node.py | 32 +++ .../registry/__init__.py | 12 + .../registry_infra_checkpoint_effect.py | 102 ++++++++ .../__init__.py | 12 + .../contract.yaml | 30 +++ .../handlers/__init__.py | 12 + .../handlers/handler_checkpoint_validate.py | 197 ++++++++++++++++ .../models/__init__.py | 18 ++ .../models/model_checkpoint_validate_input.py | 32 +++ .../model_checkpoint_validate_output.py | 52 +++++ .../node_checkpoint_validate_compute/node.py | 31 +++ .../registry/__init__.py | 12 + ...istry_infra_checkpoint_validate_compute.py | 90 +++++++ .../validation/infra_validators.py | 5 +- .../validation/validation_exemptions.yaml | 24 ++ .../test_node_checkpoint_effect/__init__.py | 3 + .../test_handler_checkpoint_list.py | 214 +++++++++++++++++ .../test_handler_checkpoint_read.py | 203 ++++++++++++++++ .../test_handler_checkpoint_write.py | 186 +++++++++++++++ .../__init__.py | 3 + .../test_handler_checkpoint_validate.py | 221 ++++++++++++++++++ 39 files changed, 2685 insertions(+), 1 deletion(-) create mode 100644 src/omnibase_infra/enums/enum_checkpoint_phase.py create mode 100644 src/omnibase_infra/models/checkpoint/__init__.py create mode 100644 src/omnibase_infra/models/checkpoint/model_checkpoint.py create mode 100644 src/omnibase_infra/models/checkpoint/model_phase_payload_create_pr.py create mode 100644 src/omnibase_infra/models/checkpoint/model_phase_payload_implement.py create mode 100644 src/omnibase_infra/models/checkpoint/model_phase_payload_local_review.py create mode 100644 src/omnibase_infra/models/checkpoint/model_phase_payload_pr_release_ready.py create mode 100644 src/omnibase_infra/models/checkpoint/model_phase_payload_ready_for_merge.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_effect/__init__.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_effect/contract.yaml create mode 100644 src/omnibase_infra/nodes/node_checkpoint_effect/handlers/__init__.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_effect/models/__init__.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_effect/models/model_checkpoint_effect_input.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_effect/models/model_checkpoint_effect_output.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_effect/node.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_effect/registry/__init__.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_effect/registry/registry_infra_checkpoint_effect.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_validate_compute/__init__.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_validate_compute/contract.yaml create mode 100644 src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/__init__.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/handler_checkpoint_validate.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_validate_compute/models/__init__.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_validate_compute/models/model_checkpoint_validate_input.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_validate_compute/models/model_checkpoint_validate_output.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_validate_compute/node.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_validate_compute/registry/__init__.py create mode 100644 src/omnibase_infra/nodes/node_checkpoint_validate_compute/registry/registry_infra_checkpoint_validate_compute.py create mode 100644 tests/unit/nodes/test_node_checkpoint_effect/__init__.py create mode 100644 tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_list.py create mode 100644 tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_read.py create mode 100644 tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_write.py create mode 100644 tests/unit/nodes/test_node_checkpoint_validate_compute/__init__.py create mode 100644 tests/unit/nodes/test_node_checkpoint_validate_compute/test_handler_checkpoint_validate.py diff --git a/src/omnibase_infra/enums/__init__.py b/src/omnibase_infra/enums/__init__.py index 6adae9c140..83a3c15a5f 100644 --- a/src/omnibase_infra/enums/__init__.py +++ b/src/omnibase_infra/enums/__init__.py @@ -60,6 +60,7 @@ from omnibase_infra.enums.enum_capture_outcome import EnumCaptureOutcome from omnibase_infra.enums.enum_capture_state import EnumCaptureState from omnibase_infra.enums.enum_chain_violation_type import EnumChainViolationType +from omnibase_infra.enums.enum_checkpoint_phase import EnumCheckpointPhase from omnibase_infra.enums.enum_circuit_state import EnumCircuitState from omnibase_infra.enums.enum_confirmation_event_type import EnumConfirmationEventType from omnibase_infra.enums.enum_consumer_group_purpose import EnumConsumerGroupPurpose @@ -113,6 +114,7 @@ "EnumBackendType", "EnumCaptureOutcome", "EnumCaptureState", + "EnumCheckpointPhase", "EnumChainViolationType", "EnumCircuitState", "EnumConfirmationEventType", diff --git a/src/omnibase_infra/enums/enum_checkpoint_phase.py b/src/omnibase_infra/enums/enum_checkpoint_phase.py new file mode 100644 index 0000000000..be2a4880c4 --- /dev/null +++ b/src/omnibase_infra/enums/enum_checkpoint_phase.py @@ -0,0 +1,59 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Pipeline checkpoint phases for resume and replay. + +Each phase represents a completed step in the ticket pipeline workflow. +Checkpoints are written after each phase completes (never during). + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from enum import Enum, unique + + +@unique +class EnumCheckpointPhase(str, Enum): + """Pipeline phase that a checkpoint records. + + Attributes: + IMPLEMENT: Code implementation completed. + LOCAL_REVIEW: Local review iterations completed. + CREATE_PR: Pull request created on remote. + PR_RELEASE_READY: PR passed release-ready review. + READY_FOR_MERGE: PR is ready for merge. + """ + + IMPLEMENT = "implement" + """Code implementation phase completed.""" + + LOCAL_REVIEW = "local_review" + """Local review iterations completed.""" + + CREATE_PR = "create_pr" + """Pull request created on remote.""" + + PR_RELEASE_READY = "pr_release_ready" + """PR passed release-ready review.""" + + READY_FOR_MERGE = "ready_for_merge" + """PR is ready for merge.""" + + def __str__(self) -> str: + """Return the string value for serialization.""" + return self.value + + @property + def phase_number(self) -> int: + """Return the 1-based ordinal position of this phase.""" + ordered = list(EnumCheckpointPhase) + return ordered.index(self) + 1 + + @property + def filename(self) -> str: + """Return the canonical checkpoint filename for this phase.""" + return f"phase_{self.phase_number}_{self.value}.yaml" + + +__all__: list[str] = ["EnumCheckpointPhase"] diff --git a/src/omnibase_infra/models/checkpoint/__init__.py b/src/omnibase_infra/models/checkpoint/__init__.py new file mode 100644 index 0000000000..95fc18cf3e --- /dev/null +++ b/src/omnibase_infra/models/checkpoint/__init__.py @@ -0,0 +1,41 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Checkpoint models for pipeline resilience and resume. + +Provides the core checkpoint data model with per-phase payloads for the +five-phase ticket pipeline workflow. + +Ticket: OMN-2143 +""" + +from omnibase_infra.models.checkpoint.model_checkpoint import ( + CHECKPOINT_SCHEMA_VERSION, + ModelCheckpoint, + PhasePayload, +) +from omnibase_infra.models.checkpoint.model_phase_payload_create_pr import ( + ModelPhasePayloadCreatePr, +) +from omnibase_infra.models.checkpoint.model_phase_payload_implement import ( + ModelPhasePayloadImplement, +) +from omnibase_infra.models.checkpoint.model_phase_payload_local_review import ( + ModelPhasePayloadLocalReview, +) +from omnibase_infra.models.checkpoint.model_phase_payload_pr_release_ready import ( + ModelPhasePayloadPrReleaseReady, +) +from omnibase_infra.models.checkpoint.model_phase_payload_ready_for_merge import ( + ModelPhasePayloadReadyForMerge, +) + +__all__: list[str] = [ + "CHECKPOINT_SCHEMA_VERSION", + "ModelCheckpoint", + "ModelPhasePayloadCreatePr", + "ModelPhasePayloadImplement", + "ModelPhasePayloadLocalReview", + "ModelPhasePayloadPrReleaseReady", + "ModelPhasePayloadReadyForMerge", + "PhasePayload", +] diff --git a/src/omnibase_infra/models/checkpoint/model_checkpoint.py b/src/omnibase_infra/models/checkpoint/model_checkpoint.py new file mode 100644 index 0000000000..e42b2c03b8 --- /dev/null +++ b/src/omnibase_infra/models/checkpoint/model_checkpoint.py @@ -0,0 +1,123 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Core checkpoint model with common header and discriminated phase payload. + +A checkpoint records the successful completion of a pipeline phase. Checkpoints +are append-only: re-runs produce a new checkpoint with an incremented +``attempt_number`` rather than overwriting the previous one. + +Invariants: + - Written *after* each phase completes, never during. + - ``artifact_paths`` must contain only relative paths (no absolute machine paths). + - ``repo_commit_map`` keys are relative to ``~/.claude/`` or repo root. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from datetime import datetime +from typing import Annotated, Union +from uuid import UUID + +from pydantic import BaseModel, ConfigDict, Discriminator, Field, Tag, model_validator + +from omnibase_infra.enums.enum_checkpoint_phase import EnumCheckpointPhase +from omnibase_infra.models.checkpoint.model_phase_payload_create_pr import ( + ModelPhasePayloadCreatePr, +) +from omnibase_infra.models.checkpoint.model_phase_payload_implement import ( + ModelPhasePayloadImplement, +) +from omnibase_infra.models.checkpoint.model_phase_payload_local_review import ( + ModelPhasePayloadLocalReview, +) +from omnibase_infra.models.checkpoint.model_phase_payload_pr_release_ready import ( + ModelPhasePayloadPrReleaseReady, +) +from omnibase_infra.models.checkpoint.model_phase_payload_ready_for_merge import ( + ModelPhasePayloadReadyForMerge, +) + +PhasePayload = Annotated[ + Union[ + Annotated[ModelPhasePayloadImplement, Tag("implement")], + Annotated[ModelPhasePayloadLocalReview, Tag("local_review")], + Annotated[ModelPhasePayloadCreatePr, Tag("create_pr")], + Annotated[ModelPhasePayloadPrReleaseReady, Tag("pr_release_ready")], + Annotated[ModelPhasePayloadReadyForMerge, Tag("ready_for_merge")], + ], + Discriminator("phase"), +] +"""Discriminated union of per-phase payloads, keyed on the ``phase`` literal.""" + +CHECKPOINT_SCHEMA_VERSION = "1.0.0" +"""Current schema version for checkpoint files.""" + + +class ModelCheckpoint(BaseModel): + """Immutable record of a completed pipeline phase. + + The common header fields (``schema_version`` through ``attempt_number``) + are present on every checkpoint. The ``phase_payload`` carries + phase-specific data needed to resume the pipeline from this point. + """ + + model_config = ConfigDict(frozen=True, extra="forbid", from_attributes=True) + + # ── Common header ──────────────────────────────────────────────── + schema_version: str = Field( + default=CHECKPOINT_SCHEMA_VERSION, + description="Forward-compatibility version string.", + ) + run_id: UUID = Field( + ..., + description="Correlation ID for the entire pipeline run.", + ) + ticket_id: str = Field( + ..., + min_length=1, + max_length=64, + description="Linear ticket identifier (e.g. OMN-2143).", + ) + phase: EnumCheckpointPhase = Field( + ..., + description="Which pipeline phase completed.", + ) + timestamp_utc: datetime = Field( + ..., + description="UTC timestamp when the phase completed.", + ) + repo_commit_map: dict[str, str] = Field( + default_factory=dict, + description="Mapping of relative repo path to commit SHA.", + ) + artifact_paths: tuple[str, ...] = Field( + default_factory=tuple, + description="Relative filesystem paths serving as resume evidence.", + ) + attempt_number: int = Field( + default=1, + ge=1, + description="Monotonically increasing attempt counter (append-only).", + ) + + # ── Phase-specific payload ─────────────────────────────────────── + phase_payload: PhasePayload = Field( + ..., + description="Per-phase data required to resume the pipeline.", + ) + + @model_validator(mode="after") + def _phase_matches_payload(self) -> ModelCheckpoint: + """Enforce that the header ``phase`` matches ``phase_payload.phase``.""" + if self.phase.value != self.phase_payload.phase: + msg = ( + f"Header phase '{self.phase.value}' does not match " + f"payload phase '{self.phase_payload.phase}'" + ) + raise ValueError(msg) + return self + + +__all__: list[str] = ["ModelCheckpoint", "PhasePayload", "CHECKPOINT_SCHEMA_VERSION"] diff --git a/src/omnibase_infra/models/checkpoint/model_phase_payload_create_pr.py b/src/omnibase_infra/models/checkpoint/model_phase_payload_create_pr.py new file mode 100644 index 0000000000..3001dcb551 --- /dev/null +++ b/src/omnibase_infra/models/checkpoint/model_phase_payload_create_pr.py @@ -0,0 +1,44 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Phase payload for the create_pr phase. + +Records PR URL, PR number, and head SHA after PR creation. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from typing import Literal + +from pydantic import BaseModel, ConfigDict, Field + + +class ModelPhasePayloadCreatePr(BaseModel): + """Payload captured after the create_pr phase completes.""" + + model_config = ConfigDict(frozen=True, extra="forbid", from_attributes=True) + + phase: Literal["create_pr"] = Field( + default="create_pr", + description="Discriminator field for phase payload union.", + ) + pr_url: str = Field( + ..., + min_length=1, + description="Full URL of the created pull request.", + ) + pr_number: int = Field( + ..., + ge=1, + description="Pull request number on the remote.", + ) + head_sha: str = Field( + ..., + min_length=7, + max_length=40, + description="HEAD SHA pushed to the remote branch.", + ) + + +__all__: list[str] = ["ModelPhasePayloadCreatePr"] diff --git a/src/omnibase_infra/models/checkpoint/model_phase_payload_implement.py b/src/omnibase_infra/models/checkpoint/model_phase_payload_implement.py new file mode 100644 index 0000000000..7ac57c0d10 --- /dev/null +++ b/src/omnibase_infra/models/checkpoint/model_phase_payload_implement.py @@ -0,0 +1,44 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Phase payload for the implement phase. + +Records branch name, commit SHA, and files changed during implementation. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from typing import Literal + +from pydantic import BaseModel, ConfigDict, Field + + +class ModelPhasePayloadImplement(BaseModel): + """Payload captured after the implement phase completes.""" + + model_config = ConfigDict(frozen=True, extra="forbid", from_attributes=True) + + phase: Literal["implement"] = Field( + default="implement", + description="Discriminator field for phase payload union.", + ) + branch_name: str = Field( + ..., + min_length=1, + max_length=512, + description="Git branch name created for this ticket.", + ) + commit_sha: str = Field( + ..., + min_length=7, + max_length=40, + description="HEAD commit SHA after implementation.", + ) + files_changed: tuple[str, ...] = Field( + default_factory=tuple, + description="Relative paths of files changed during implementation.", + ) + + +__all__: list[str] = ["ModelPhasePayloadImplement"] diff --git a/src/omnibase_infra/models/checkpoint/model_phase_payload_local_review.py b/src/omnibase_infra/models/checkpoint/model_phase_payload_local_review.py new file mode 100644 index 0000000000..200c0f6a9a --- /dev/null +++ b/src/omnibase_infra/models/checkpoint/model_phase_payload_local_review.py @@ -0,0 +1,43 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Phase payload for the local_review phase. + +Records iteration count, issue fingerprints, and the last clean commit SHA. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from typing import Literal + +from pydantic import BaseModel, ConfigDict, Field + + +class ModelPhasePayloadLocalReview(BaseModel): + """Payload captured after the local_review phase completes.""" + + model_config = ConfigDict(frozen=True, extra="forbid", from_attributes=True) + + phase: Literal["local_review"] = Field( + default="local_review", + description="Discriminator field for phase payload union.", + ) + iteration_count: int = Field( + ..., + ge=1, + description="Number of review-fix iterations performed.", + ) + issue_fingerprints: tuple[str, ...] = Field( + default_factory=tuple, + description="Fingerprints of issues found and resolved.", + ) + last_clean_sha: str = Field( + ..., + min_length=7, + max_length=40, + description="Commit SHA of the last clean (review-passing) state.", + ) + + +__all__: list[str] = ["ModelPhasePayloadLocalReview"] diff --git a/src/omnibase_infra/models/checkpoint/model_phase_payload_pr_release_ready.py b/src/omnibase_infra/models/checkpoint/model_phase_payload_pr_release_ready.py new file mode 100644 index 0000000000..9e1cd7200a --- /dev/null +++ b/src/omnibase_infra/models/checkpoint/model_phase_payload_pr_release_ready.py @@ -0,0 +1,38 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Phase payload for the pr_release_ready phase. + +Records last review SHA and issue fingerprints after PR release-ready review. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from typing import Literal + +from pydantic import BaseModel, ConfigDict, Field + + +class ModelPhasePayloadPrReleaseReady(BaseModel): + """Payload captured after the pr_release_ready phase completes.""" + + model_config = ConfigDict(frozen=True, extra="forbid", from_attributes=True) + + phase: Literal["pr_release_ready"] = Field( + default="pr_release_ready", + description="Discriminator field for phase payload union.", + ) + last_review_sha: str = Field( + ..., + min_length=7, + max_length=40, + description="Commit SHA of the last review pass.", + ) + issue_fingerprints: tuple[str, ...] = Field( + default_factory=tuple, + description="Fingerprints of issues found and resolved.", + ) + + +__all__: list[str] = ["ModelPhasePayloadPrReleaseReady"] diff --git a/src/omnibase_infra/models/checkpoint/model_phase_payload_ready_for_merge.py b/src/omnibase_infra/models/checkpoint/model_phase_payload_ready_for_merge.py new file mode 100644 index 0000000000..89bd95fec1 --- /dev/null +++ b/src/omnibase_infra/models/checkpoint/model_phase_payload_ready_for_merge.py @@ -0,0 +1,33 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Phase payload for the ready_for_merge phase. + +Records the timestamp when the merge-ready label was applied. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from datetime import datetime +from typing import Literal + +from pydantic import BaseModel, ConfigDict, Field + + +class ModelPhasePayloadReadyForMerge(BaseModel): + """Payload captured after the ready_for_merge phase completes.""" + + model_config = ConfigDict(frozen=True, extra="forbid", from_attributes=True) + + phase: Literal["ready_for_merge"] = Field( + default="ready_for_merge", + description="Discriminator field for phase payload union.", + ) + label_applied_at: datetime = Field( + ..., + description="UTC timestamp when the merge-ready label was applied.", + ) + + +__all__: list[str] = ["ModelPhasePayloadReadyForMerge"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/__init__.py b/src/omnibase_infra/nodes/node_checkpoint_effect/__init__.py new file mode 100644 index 0000000000..7cf05dd8dd --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/__init__.py @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Checkpoint effect node — filesystem I/O for pipeline checkpoints. + +Ticket: OMN-2143 +""" + +from omnibase_infra.nodes.node_checkpoint_effect.node import NodeCheckpointEffect + +__all__: list[str] = ["NodeCheckpointEffect"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/contract.yaml b/src/omnibase_infra/nodes/node_checkpoint_effect/contract.yaml new file mode 100644 index 0000000000..4b44749115 --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/contract.yaml @@ -0,0 +1,53 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +# Ticket: OMN-2143 +name: "node_checkpoint_effect" +node_type: "EFFECT_GENERIC" +contract_version: + major: 1 + minor: 0 + patch: 0 +node_version: "1.0.0" +description: > + EFFECT node that owns all filesystem I/O for pipeline checkpoint persistence. Supports write, read, and list operations on checkpoint YAML files stored under ~/.claude/checkpoints/{ticket_id}/{run_id}/. + +input_model: + name: "ModelCheckpointEffectInput" + module: "omnibase_infra.nodes.node_checkpoint_effect.models.model_checkpoint_effect_input" + description: "Input envelope for checkpoint effect operations." +output_model: + name: "ModelCheckpointEffectOutput" + module: "omnibase_infra.nodes.node_checkpoint_effect.models.model_checkpoint_effect_output" + description: "Result of checkpoint effect operations." +io_operations: + - name: "write_checkpoint" + description: "Write a phase checkpoint YAML to disk (append-only)." + input_fields: ["checkpoint"] + output_fields: ["success", "checkpoint_path"] + - name: "read_checkpoint" + description: "Read a specific checkpoint from disk." + input_fields: ["ticket_id", "run_id", "phase"] + output_fields: ["success", "checkpoint"] + - name: "list_checkpoints" + description: "List available checkpoints for a ticket/run." + input_fields: ["ticket_id", "run_id"] + output_fields: ["success", "checkpoints"] +handler_routing: + routing_strategy: "operation_match" + handlers: + - operation: "write_checkpoint" + handler: + name: "HandlerCheckpointWrite" + module: "omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_write" + - operation: "read_checkpoint" + handler: + name: "HandlerCheckpointRead" + module: "omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_read" + - operation: "list_checkpoints" + handler: + name: "HandlerCheckpointList" + module: "omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_list" +capabilities: + - "checkpoint.write" + - "checkpoint.read" + - "checkpoint.list" diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/__init__.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/__init__.py new file mode 100644 index 0000000000..8d59b44e17 --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/__init__.py @@ -0,0 +1,22 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Handlers for the checkpoint effect node. + +Ticket: OMN-2143 +""" + +from omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_list import ( + HandlerCheckpointList, +) +from omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_read import ( + HandlerCheckpointRead, +) +from omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_write import ( + HandlerCheckpointWrite, +) + +__all__: list[str] = [ + "HandlerCheckpointList", + "HandlerCheckpointRead", + "HandlerCheckpointWrite", +] diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py new file mode 100644 index 0000000000..a113a76234 --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py @@ -0,0 +1,157 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Handler that lists available checkpoints for a ticket/run. + +Returns all checkpoints found, across all phases and attempts. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +import logging +from pathlib import Path +from typing import TYPE_CHECKING +from uuid import UUID, uuid4 + +import yaml +from pydantic import ValidationError + +from omnibase_core.models.dispatch import ModelHandlerOutput +from omnibase_infra.enums import ( + EnumHandlerType, + EnumHandlerTypeCategory, + EnumInfraTransportType, +) +from omnibase_infra.errors import ModelInfraErrorContext, RuntimeHostError +from omnibase_infra.models.checkpoint.model_checkpoint import ModelCheckpoint +from omnibase_infra.nodes.node_checkpoint_effect.models.model_checkpoint_effect_output import ( + ModelCheckpointEffectOutput, +) + +if TYPE_CHECKING: + from omnibase_core.models.container.model_onex_container import ModelONEXContainer + +logger = logging.getLogger(__name__) + +_DEFAULT_BASE_DIR = Path.home() / ".claude" / "checkpoints" + + +class HandlerCheckpointList: + """Lists all checkpoint files for a ticket and optionally a specific run.""" + + def __init__(self, container: ModelONEXContainer) -> None: + self._container = container + self._initialized: bool = False + + @property + def handler_type(self) -> EnumHandlerType: + return EnumHandlerType.NODE_HANDLER + + @property + def handler_category(self) -> EnumHandlerTypeCategory: + return EnumHandlerTypeCategory.EFFECT + + async def initialize(self, config: dict[str, object]) -> None: + """Initialize the handler.""" + self._initialized = True + logger.info("HandlerCheckpointList initialized") + + async def shutdown(self) -> None: + """Shutdown the handler.""" + self._initialized = False + logger.info("HandlerCheckpointList shutdown") + + async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: + """List checkpoints for a ticket and optional run. + + Envelope keys: + ticket_id: str — which ticket. + run_id: UUID | None — optional run scope. + correlation_id: UUID for tracing. + base_dir: Optional override for the checkpoint root. + """ + correlation_id_raw = envelope.get("correlation_id") + context = ModelInfraErrorContext.with_correlation( + correlation_id=correlation_id_raw + if isinstance(correlation_id_raw, UUID) + else None, + transport_type=EnumInfraTransportType.FILESYSTEM, + operation="list_checkpoints", + target_name="checkpoint_yaml", + ) + # with_correlation() auto-generates a UUID when None is passed + corr_id = context.correlation_id or uuid4() + + ticket_id = envelope.get("ticket_id") + if not ticket_id: + raise RuntimeHostError( + "list_checkpoints requires ticket_id", + context=context, + ) + + # Resolve base directory + base_dir_raw = envelope.get("base_dir") + base_dir = Path(str(base_dir_raw)) if base_dir_raw else _DEFAULT_BASE_DIR + + ticket_dir = base_dir / str(ticket_id) + if not ticket_dir.is_dir(): + return ModelHandlerOutput.for_compute( + input_envelope_id=uuid4(), + correlation_id=corr_id, + handler_id="handler-checkpoint-list", + result=ModelCheckpointEffectOutput( + success=True, + correlation_id=corr_id, + checkpoints=(), + ), + ) + + # Optionally scope to a specific run + run_id_raw = envelope.get("run_id") + if run_id_raw is not None: + run_id = ( + UUID(str(run_id_raw)) + if not isinstance(run_id_raw, UUID) + else run_id_raw + ) + scan_dirs = [ticket_dir / str(run_id)] + else: + scan_dirs = [d for d in ticket_dir.iterdir() if d.is_dir()] + + checkpoints: list[ModelCheckpoint] = [] + for scan_dir in scan_dirs: + if not scan_dir.is_dir(): + continue + for yaml_file in sorted(scan_dir.glob("phase_*.yaml")): + try: + raw_data = yaml.safe_load(yaml_file.read_text(encoding="utf-8")) + checkpoint = ModelCheckpoint.model_validate(raw_data) + checkpoints.append(checkpoint) + except (ValidationError, yaml.YAMLError) as exc: + logger.warning( + "Skipping corrupt checkpoint %s: %s", + yaml_file.name, + str(exc)[:200], + ) + + logger.info( + "Listed %d checkpoints for ticket=%s", + len(checkpoints), + ticket_id, + ) + + result = ModelCheckpointEffectOutput( + success=True, + correlation_id=corr_id, + checkpoints=tuple(checkpoints), + ) + return ModelHandlerOutput.for_compute( + input_envelope_id=uuid4(), + correlation_id=corr_id, + handler_id="handler-checkpoint-list", + result=result, + ) + + +__all__: list[str] = ["HandlerCheckpointList"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py new file mode 100644 index 0000000000..27e35d6a2e --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py @@ -0,0 +1,186 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Handler that reads a specific checkpoint from disk. + +When multiple attempts exist for the same phase, returns the checkpoint with +the highest ``attempt_number`` (latest attempt). + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +import logging +from pathlib import Path +from typing import TYPE_CHECKING +from uuid import UUID, uuid4 + +import yaml +from pydantic import ValidationError + +from omnibase_core.models.dispatch import ModelHandlerOutput +from omnibase_infra.enums import ( + EnumHandlerType, + EnumHandlerTypeCategory, + EnumInfraTransportType, +) +from omnibase_infra.enums.enum_checkpoint_phase import EnumCheckpointPhase +from omnibase_infra.errors import ModelInfraErrorContext, RuntimeHostError +from omnibase_infra.models.checkpoint.model_checkpoint import ModelCheckpoint +from omnibase_infra.nodes.node_checkpoint_effect.models.model_checkpoint_effect_output import ( + ModelCheckpointEffectOutput, +) + +if TYPE_CHECKING: + from omnibase_core.models.container.model_onex_container import ModelONEXContainer + +logger = logging.getLogger(__name__) + +_DEFAULT_BASE_DIR = Path.home() / ".claude" / "checkpoints" + + +class HandlerCheckpointRead: + """Reads a checkpoint for a given ticket, run, and phase. + + If multiple attempts exist, the one with the highest ``attempt_number`` + is returned. + """ + + def __init__(self, container: ModelONEXContainer) -> None: + self._container = container + self._initialized: bool = False + + @property + def handler_type(self) -> EnumHandlerType: + return EnumHandlerType.NODE_HANDLER + + @property + def handler_category(self) -> EnumHandlerTypeCategory: + return EnumHandlerTypeCategory.EFFECT + + async def initialize(self, config: dict[str, object]) -> None: + """Initialize the handler.""" + self._initialized = True + logger.info("HandlerCheckpointRead initialized") + + async def shutdown(self) -> None: + """Shutdown the handler.""" + self._initialized = False + logger.info("HandlerCheckpointRead shutdown") + + async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: + """Read the latest checkpoint for the requested phase. + + Envelope keys: + ticket_id: str — which ticket. + run_id: UUID — which pipeline run. + phase: EnumCheckpointPhase — which phase to read. + correlation_id: UUID for tracing. + base_dir: Optional override for the checkpoint root. + """ + correlation_id_raw = envelope.get("correlation_id") + context = ModelInfraErrorContext.with_correlation( + correlation_id=correlation_id_raw + if isinstance(correlation_id_raw, UUID) + else None, + transport_type=EnumInfraTransportType.FILESYSTEM, + operation="read_checkpoint", + target_name="checkpoint_yaml", + ) + # with_correlation() auto-generates a UUID when None is passed + corr_id = context.correlation_id or uuid4() + + ticket_id = envelope.get("ticket_id") + run_id = envelope.get("run_id") + phase_raw = envelope.get("phase") + + if not ticket_id or not run_id or phase_raw is None: + raise RuntimeHostError( + "read_checkpoint requires ticket_id, run_id, and phase", + context=context, + ) + + # Coerce phase + if isinstance(phase_raw, EnumCheckpointPhase): + phase = phase_raw + elif isinstance(phase_raw, str): + phase = EnumCheckpointPhase(phase_raw) + else: + raise RuntimeHostError( + f"Unexpected phase type: {type(phase_raw).__name__}", + context=context, + ) + + run_id_val = UUID(str(run_id)) if not isinstance(run_id, UUID) else run_id + + # Resolve base directory + base_dir_raw = envelope.get("base_dir") + base_dir = Path(str(base_dir_raw)) if base_dir_raw else _DEFAULT_BASE_DIR + + # Scan for matching files + target_dir = base_dir / str(ticket_id) / str(run_id_val) + phase_prefix = f"phase_{phase.phase_number}_{phase.value}_a" + + if not target_dir.is_dir(): + return ModelHandlerOutput.for_compute( + input_envelope_id=uuid4(), + correlation_id=corr_id, + handler_id="handler-checkpoint-read", + result=ModelCheckpointEffectOutput( + success=False, + correlation_id=corr_id, + error=f"Checkpoint directory not found: {target_dir.name}", + ), + ) + + # Find all attempt files for this phase + matching_files = sorted( + target_dir.glob(f"{phase_prefix}*.yaml"), + key=lambda p: p.name, + ) + + if not matching_files: + return ModelHandlerOutput.for_compute( + input_envelope_id=uuid4(), + correlation_id=corr_id, + handler_id="handler-checkpoint-read", + result=ModelCheckpointEffectOutput( + success=False, + correlation_id=corr_id, + error=f"No checkpoint found for phase {phase.value}", + ), + ) + + # Read the latest attempt (last file alphabetically = highest attempt) + latest_file = matching_files[-1] + raw_data = yaml.safe_load(latest_file.read_text(encoding="utf-8")) + + try: + checkpoint = ModelCheckpoint.model_validate(raw_data) + except ValidationError as exc: + raise RuntimeHostError( + f"Corrupt checkpoint file {latest_file.name}: {exc.error_count()} errors", + context=context, + ) from exc + + logger.info( + "Checkpoint read: %s (phase=%s, attempt=%d)", + latest_file.name, + checkpoint.phase.value, + checkpoint.attempt_number, + ) + + result = ModelCheckpointEffectOutput( + success=True, + correlation_id=corr_id, + checkpoint=checkpoint, + ) + return ModelHandlerOutput.for_compute( + input_envelope_id=uuid4(), + correlation_id=corr_id, + handler_id="handler-checkpoint-read", + result=result, + ) + + +__all__: list[str] = ["HandlerCheckpointRead"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py new file mode 100644 index 0000000000..05c2495e48 --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py @@ -0,0 +1,187 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Handler that writes a pipeline checkpoint to disk. + +Writes are append-only: each attempt creates a new file with a monotonically +increasing ``attempt_number``. Existing checkpoint files are never modified. + +File layout:: + + {base_dir}/{ticket_id}/{run_id}/phase_{N}_{name}_a{attempt}.yaml + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +import logging +from pathlib import Path +from typing import TYPE_CHECKING +from uuid import UUID, uuid4 + +import yaml +from pydantic import ValidationError + +from omnibase_core.models.dispatch import ModelHandlerOutput +from omnibase_infra.enums import ( + EnumHandlerType, + EnumHandlerTypeCategory, + EnumInfraTransportType, +) +from omnibase_infra.errors import ModelInfraErrorContext, RuntimeHostError +from omnibase_infra.models.checkpoint.model_checkpoint import ModelCheckpoint +from omnibase_infra.nodes.node_checkpoint_effect.models.model_checkpoint_effect_output import ( + ModelCheckpointEffectOutput, +) + +if TYPE_CHECKING: + from omnibase_core.models.container.model_onex_container import ModelONEXContainer + +logger = logging.getLogger(__name__) + +_DEFAULT_BASE_DIR = Path.home() / ".claude" / "checkpoints" + + +def _checkpoint_dir(base_dir: Path, ticket_id: str, run_id: UUID) -> Path: + """Return the directory for a given ticket + run.""" + return base_dir / ticket_id / str(run_id) + + +def _checkpoint_filename(phase_number: int, phase_value: str, attempt: int) -> str: + """Return the canonical filename for a checkpoint attempt.""" + return f"phase_{phase_number}_{phase_value}_a{attempt}.yaml" + + +def _serialize_checkpoint(checkpoint: ModelCheckpoint) -> dict[str, object]: + """Serialize a checkpoint to a YAML-safe dict.""" + data: dict[str, object] = checkpoint.model_dump(mode="json") + return data + + +class HandlerCheckpointWrite: + """Writes a phase checkpoint YAML file to disk (append-only). + + Each call writes a new file; existing checkpoints are never overwritten. + """ + + def __init__(self, container: ModelONEXContainer) -> None: + self._container = container + self._initialized: bool = False + + @property + def handler_type(self) -> EnumHandlerType: + return EnumHandlerType.NODE_HANDLER + + @property + def handler_category(self) -> EnumHandlerTypeCategory: + return EnumHandlerTypeCategory.EFFECT + + async def initialize(self, config: dict[str, object]) -> None: + """Initialize the handler.""" + self._initialized = True + logger.info("HandlerCheckpointWrite initialized") + + async def shutdown(self) -> None: + """Shutdown the handler.""" + self._initialized = False + logger.info("HandlerCheckpointWrite shutdown") + + async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: + """Write a checkpoint to disk. + + Envelope keys: + checkpoint: ModelCheckpoint or dict to persist. + correlation_id: UUID for tracing. + base_dir: Optional override for the checkpoint root. + """ + correlation_id_raw = envelope.get("correlation_id") + context = ModelInfraErrorContext.with_correlation( + correlation_id=correlation_id_raw + if isinstance(correlation_id_raw, UUID) + else None, + transport_type=EnumInfraTransportType.FILESYSTEM, + operation="write_checkpoint", + target_name="checkpoint_yaml", + ) + # with_correlation() auto-generates a UUID when None is passed + corr_id = context.correlation_id or uuid4() + + raw_checkpoint = envelope.get("checkpoint") + if raw_checkpoint is None: + raise RuntimeHostError( + "write_checkpoint requires a 'checkpoint' in the envelope", + context=context, + ) + + # Coerce dict → ModelCheckpoint for validation + if isinstance(raw_checkpoint, dict): + try: + checkpoint = ModelCheckpoint.model_validate(raw_checkpoint) + except ValidationError as exc: + raise RuntimeHostError( + f"Invalid checkpoint data: {exc.error_count()} validation errors", + context=context, + ) from exc + elif isinstance(raw_checkpoint, ModelCheckpoint): + checkpoint = raw_checkpoint + else: + raise RuntimeHostError( + f"Unexpected checkpoint type: {type(raw_checkpoint).__name__}", + context=context, + ) + + # Resolve base directory + base_dir_raw = envelope.get("base_dir") + base_dir = Path(str(base_dir_raw)) if base_dir_raw else _DEFAULT_BASE_DIR + + # Build path + target_dir = _checkpoint_dir(base_dir, checkpoint.ticket_id, checkpoint.run_id) + filename = _checkpoint_filename( + checkpoint.phase.phase_number, + checkpoint.phase.value, + checkpoint.attempt_number, + ) + target_path = target_dir / filename + + # Validate path normalization: artifact_paths must be relative + for artifact in checkpoint.artifact_paths: + if Path(artifact).is_absolute(): + raise RuntimeHostError( + f"Absolute artifact path forbidden: {artifact}", + context=context, + ) + + # Write + target_dir.mkdir(parents=True, exist_ok=True) + yaml_content = yaml.dump( + _serialize_checkpoint(checkpoint), + default_flow_style=False, + sort_keys=False, + allow_unicode=True, + ) + target_path.write_text(yaml_content, encoding="utf-8") + + logger.info( + "Checkpoint written: %s (phase=%s, attempt=%d)", + target_path, + checkpoint.phase.value, + checkpoint.attempt_number, + ) + + # Return relative path from base_dir + relative_path = str(target_path.relative_to(base_dir)) + + result = ModelCheckpointEffectOutput( + success=True, + correlation_id=corr_id, + checkpoint_path=relative_path, + ) + return ModelHandlerOutput.for_compute( + input_envelope_id=uuid4(), + correlation_id=corr_id, + handler_id="handler-checkpoint-write", + result=result, + ) + + +__all__: list[str] = ["HandlerCheckpointWrite"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/models/__init__.py b/src/omnibase_infra/nodes/node_checkpoint_effect/models/__init__.py new file mode 100644 index 0000000000..ae1c7b3717 --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/models/__init__.py @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Models for the checkpoint effect node. + +Ticket: OMN-2143 +""" + +from omnibase_infra.nodes.node_checkpoint_effect.models.model_checkpoint_effect_input import ( + ModelCheckpointEffectInput, +) +from omnibase_infra.nodes.node_checkpoint_effect.models.model_checkpoint_effect_output import ( + ModelCheckpointEffectOutput, +) + +__all__: list[str] = [ + "ModelCheckpointEffectInput", + "ModelCheckpointEffectOutput", +] diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/models/model_checkpoint_effect_input.py b/src/omnibase_infra/nodes/node_checkpoint_effect/models/model_checkpoint_effect_input.py new file mode 100644 index 0000000000..679c53decf --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/models/model_checkpoint_effect_input.py @@ -0,0 +1,65 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Input model for checkpoint effect operations. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from uuid import UUID + +from pydantic import BaseModel, ConfigDict, Field + +from omnibase_infra.enums.enum_checkpoint_phase import EnumCheckpointPhase +from omnibase_infra.models.checkpoint.model_checkpoint import ModelCheckpoint + + +class ModelCheckpointEffectInput(BaseModel): + """Input envelope for checkpoint effect node operations. + + The ``operation`` field selects the handler (write / read / list). + Fields not relevant to the selected operation may be ``None``. + """ + + model_config = ConfigDict(frozen=True, extra="forbid", from_attributes=True) + + operation: str = Field( + ..., + min_length=1, + description="Operation to perform: write_checkpoint, read_checkpoint, list_checkpoints.", + ) + correlation_id: UUID = Field( + ..., + description="Correlation ID for tracing.", + ) + + # ── write_checkpoint fields ────────────────────────────────────── + checkpoint: ModelCheckpoint | None = Field( + default=None, + description="Full checkpoint to persist (write_checkpoint only).", + ) + + # ── read_checkpoint / list_checkpoints fields ──────────────────── + ticket_id: str | None = Field( + default=None, + max_length=64, + description="Ticket identifier to read/list checkpoints for.", + ) + run_id: UUID | None = Field( + default=None, + description="Pipeline run ID to scope the read/list.", + ) + phase: EnumCheckpointPhase | None = Field( + default=None, + description="Specific phase to read (read_checkpoint only).", + ) + + # ── optional base directory override (testing) ─────────────────── + base_dir: str | None = Field( + default=None, + description="Override base checkpoint directory (for testing). Relative paths preserved.", + ) + + +__all__: list[str] = ["ModelCheckpointEffectInput"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/models/model_checkpoint_effect_output.py b/src/omnibase_infra/nodes/node_checkpoint_effect/models/model_checkpoint_effect_output.py new file mode 100644 index 0000000000..18660f2aee --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/models/model_checkpoint_effect_output.py @@ -0,0 +1,70 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Output model for checkpoint effect operations. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from uuid import UUID + +from pydantic import BaseModel, ConfigDict, Field + +from omnibase_infra.models.checkpoint.model_checkpoint import ModelCheckpoint + + +class ModelCheckpointEffectOutput(BaseModel): + """Result of a checkpoint effect node operation. + + Warning: + **Non-standard __bool__ behavior**: Returns ``True`` only when + ``success`` is True. Differs from typical Pydantic behavior. + """ + + model_config = ConfigDict(frozen=True, extra="forbid", from_attributes=True) + + success: bool = Field( + ..., + description="Whether the operation completed successfully.", + ) + correlation_id: UUID = Field( + ..., + description="Correlation ID echoed from the input.", + ) + + # ── write_checkpoint result ────────────────────────────────────── + checkpoint_path: str | None = Field( + default=None, + description="Relative path of the written checkpoint file.", + ) + + # ── read_checkpoint result ─────────────────────────────────────── + checkpoint: ModelCheckpoint | None = Field( + default=None, + description="Deserialized checkpoint (read_checkpoint only).", + ) + + # ── list_checkpoints result ────────────────────────────────────── + checkpoints: tuple[ModelCheckpoint, ...] = Field( + default_factory=tuple, + description="All checkpoints found (list_checkpoints only).", + ) + + # ── error information ──────────────────────────────────────────── + error: str | None = Field( + default=None, + description="Error message if the operation failed.", + ) + + def __bool__(self) -> bool: + """Allow using result in boolean context. + + Warning: + **Non-standard __bool__ behavior**: Returns ``True`` only when + ``success`` is True. Differs from typical Pydantic behavior. + """ + return self.success + + +__all__: list[str] = ["ModelCheckpointEffectOutput"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/node.py b/src/omnibase_infra/nodes/node_checkpoint_effect/node.py new file mode 100644 index 0000000000..0d4bc5157a --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/node.py @@ -0,0 +1,32 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Declarative EFFECT node for checkpoint filesystem I/O. + +Owns all read/write/list operations on checkpoint YAML files. +All behavior is defined in contract.yaml and delegated to handlers. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from omnibase_core.nodes.node_effect import NodeEffect + +if TYPE_CHECKING: + from omnibase_core.models.container.model_onex_container import ModelONEXContainer + + +class NodeCheckpointEffect(NodeEffect): + """Declarative effect node for checkpoint persistence. + + Handlers: + - ``HandlerCheckpointWrite``: Writes a phase checkpoint to disk. + - ``HandlerCheckpointRead``: Reads a checkpoint for pipeline resume. + - ``HandlerCheckpointList``: Lists available checkpoints for a ticket/run. + """ + + def __init__(self, container: ModelONEXContainer) -> None: + """Initialize the checkpoint effect node.""" + super().__init__(container) diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/registry/__init__.py b/src/omnibase_infra/nodes/node_checkpoint_effect/registry/__init__.py new file mode 100644 index 0000000000..4894994b51 --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/registry/__init__.py @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Registry for checkpoint effect node. + +Ticket: OMN-2143 +""" + +from omnibase_infra.nodes.node_checkpoint_effect.registry.registry_infra_checkpoint_effect import ( + RegistryInfraCheckpointEffect, +) + +__all__: list[str] = ["RegistryInfraCheckpointEffect"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/registry/registry_infra_checkpoint_effect.py b/src/omnibase_infra/nodes/node_checkpoint_effect/registry/registry_infra_checkpoint_effect.py new file mode 100644 index 0000000000..25a6b83fe4 --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/registry/registry_infra_checkpoint_effect.py @@ -0,0 +1,102 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Dependency injection registry for the checkpoint effect node. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +import logging +from typing import TYPE_CHECKING + +from omnibase_infra.enums import EnumInfraTransportType +from omnibase_infra.errors import ModelInfraErrorContext, ProtocolConfigurationError + +if TYPE_CHECKING: + from omnibase_core.models.container.model_onex_container import ModelONEXContainer + from omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_list import ( + HandlerCheckpointList, + ) + from omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_read import ( + HandlerCheckpointRead, + ) + from omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_write import ( + HandlerCheckpointWrite, + ) + +logger = logging.getLogger(__name__) + +_HANDLER_STORAGE: dict[str, object] = {} +_PROTOCOL_METADATA: dict[str, dict[str, object]] = {} + + +class RegistryInfraCheckpointEffect: + """Registry for checkpoint effect node handlers.""" + + HANDLER_KEY = "handler_checkpoint" + + @staticmethod + def _is_registered(handler_key: str) -> bool: + return handler_key in _HANDLER_STORAGE + + @staticmethod + def register(_container: ModelONEXContainer) -> None: + """Register checkpoint effect metadata with the container.""" + _PROTOCOL_METADATA[RegistryInfraCheckpointEffect.HANDLER_KEY] = { + "handler": "HandlerCheckpointWrite", + "module": "omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_write", + "description": "Handler for checkpoint write operations", + "capabilities": [ + "checkpoint.write", + "checkpoint.read", + "checkpoint.list", + ], + } + + @staticmethod + def register_handler( + _container: ModelONEXContainer, + handler: HandlerCheckpointWrite | HandlerCheckpointRead | HandlerCheckpointList, + handler_type: str = "write", + ) -> None: + """Register a specific checkpoint handler.""" + required_methods = ["initialize", "shutdown", "execute"] + missing = [ + m for m in required_methods if not callable(getattr(handler, m, None)) + ] + if missing: + context = ModelInfraErrorContext.with_correlation( + transport_type=EnumInfraTransportType.FILESYSTEM, + operation="register_handler", + target_name="checkpoint_handler", + ) + raise ProtocolConfigurationError( + f"Handler missing required protocol methods: {missing}", + context=context, + ) + + handler_key = f"{RegistryInfraCheckpointEffect.HANDLER_KEY}.{handler_type}" + + if RegistryInfraCheckpointEffect._is_registered(handler_key): + logger.warning("Re-registering handler '%s'", handler_key) + + _HANDLER_STORAGE[handler_key] = handler + + @staticmethod + def get_handler( + _container: ModelONEXContainer, + handler_type: str = "write", + ) -> object | None: + """Retrieve a registered checkpoint handler.""" + handler_key = f"{RegistryInfraCheckpointEffect.HANDLER_KEY}.{handler_type}" + return _HANDLER_STORAGE.get(handler_key) + + @staticmethod + def clear() -> None: + """Clear all registered handlers. Call in test teardown.""" + _HANDLER_STORAGE.clear() + _PROTOCOL_METADATA.clear() + + +__all__: list[str] = ["RegistryInfraCheckpointEffect"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_validate_compute/__init__.py b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/__init__.py new file mode 100644 index 0000000000..3f40b7eb29 --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/__init__.py @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Checkpoint validate compute node — pure structural validation. + +Ticket: OMN-2143 +""" + +from omnibase_infra.nodes.node_checkpoint_validate_compute.node import ( + NodeCheckpointValidateCompute, +) + +__all__: list[str] = ["NodeCheckpointValidateCompute"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_validate_compute/contract.yaml b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/contract.yaml new file mode 100644 index 0000000000..9f8c0b26de --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/contract.yaml @@ -0,0 +1,30 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +# Ticket: OMN-2143 +name: "node_checkpoint_validate_compute" +node_type: "COMPUTE_GENERIC" +contract_version: + major: 1 + minor: 0 + patch: 0 +node_version: "1.0.0" +description: > + COMPUTE node that performs pure structural validation on checkpoint data. Verifies schema version, required fields, path normalization, phase-payload consistency, and commit SHA format. No filesystem I/O. + +input_model: + name: "ModelCheckpointValidateInput" + module: "omnibase_infra.nodes.node_checkpoint_validate_compute.models.model_checkpoint_validate_input" + description: "Checkpoint data to validate." +output_model: + name: "ModelCheckpointValidateOutput" + module: "omnibase_infra.nodes.node_checkpoint_validate_compute.models.model_checkpoint_validate_output" + description: "Validation result with errors and warnings." +handler_routing: + routing_strategy: "operation_match" + handlers: + - operation: "validate_checkpoint" + handler: + name: "HandlerCheckpointValidate" + module: "omnibase_infra.nodes.node_checkpoint_validate_compute.handlers.handler_checkpoint_validate" +capabilities: + - "checkpoint.validate" diff --git a/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/__init__.py b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/__init__.py new file mode 100644 index 0000000000..e74d056f2c --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/__init__.py @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Handlers for the checkpoint validate compute node. + +Ticket: OMN-2143 +""" + +from omnibase_infra.nodes.node_checkpoint_validate_compute.handlers.handler_checkpoint_validate import ( + HandlerCheckpointValidate, +) + +__all__: list[str] = ["HandlerCheckpointValidate"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/handler_checkpoint_validate.py b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/handler_checkpoint_validate.py new file mode 100644 index 0000000000..b808ad2be9 --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/handler_checkpoint_validate.py @@ -0,0 +1,197 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Pure COMPUTE handler for checkpoint structural validation. + +Validates data consistency, path normalization, commit SHA format, and +phase-payload agreement — with zero filesystem I/O. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +import logging +import re +from datetime import UTC, datetime, timedelta +from pathlib import PurePosixPath +from typing import TYPE_CHECKING +from uuid import UUID, uuid4 + +from pydantic import ValidationError + +from omnibase_core.models.dispatch import ModelHandlerOutput +from omnibase_infra.enums import EnumHandlerType, EnumHandlerTypeCategory +from omnibase_infra.models.checkpoint.model_checkpoint import ( + CHECKPOINT_SCHEMA_VERSION, + ModelCheckpoint, +) +from omnibase_infra.nodes.node_checkpoint_validate_compute.models.model_checkpoint_validate_output import ( + ModelCheckpointValidateOutput, +) + +if TYPE_CHECKING: + from omnibase_core.models.container.model_onex_container import ModelONEXContainer + +logger = logging.getLogger(__name__) + +_SHA_RE = re.compile(r"^[0-9a-f]{7,40}$") +"""Pattern matching a valid (possibly abbreviated) git commit SHA.""" + + +class HandlerCheckpointValidate: + """Pure validation of checkpoint data — no I/O, fully deterministic.""" + + def __init__(self, container: ModelONEXContainer) -> None: + self._container = container + self._initialized: bool = False + + @property + def handler_type(self) -> EnumHandlerType: + return EnumHandlerType.COMPUTE_HANDLER + + @property + def handler_category(self) -> EnumHandlerTypeCategory: + return EnumHandlerTypeCategory.COMPUTE + + async def initialize(self, config: dict[str, object]) -> None: + """Initialize the handler.""" + self._initialized = True + logger.info("HandlerCheckpointValidate initialized") + + async def shutdown(self) -> None: + """Shutdown the handler.""" + self._initialized = False + logger.info("HandlerCheckpointValidate shutdown") + + async def execute( + self, + envelope: dict[str, object], + ) -> ModelHandlerOutput[ModelCheckpointValidateOutput]: + """Validate checkpoint data structurally. + + Envelope keys: + checkpoint: ModelCheckpoint or dict to validate. + correlation_id: UUID for tracing. + """ + correlation_id_raw = envelope.get("correlation_id") + corr_id = ( + correlation_id_raw + if isinstance(correlation_id_raw, UUID) + else UUID("00000000-0000-0000-0000-000000000000") + ) + input_envelope_id = uuid4() + + raw_checkpoint = envelope.get("checkpoint") + + # ── Step 0: Deserialize if needed ──────────────────────────── + if isinstance(raw_checkpoint, dict): + try: + checkpoint = ModelCheckpoint.model_validate(raw_checkpoint) + except ValidationError as exc: + errors = tuple( + f"{'.'.join(str(loc) for loc in e['loc'])}: {e['msg']}" + for e in exc.errors() + ) + return ModelHandlerOutput.for_compute( + input_envelope_id=input_envelope_id, + correlation_id=corr_id, + handler_id="handler-checkpoint-validate", + result=ModelCheckpointValidateOutput( + is_valid=False, + correlation_id=corr_id, + errors=errors, + ), + ) + elif isinstance(raw_checkpoint, ModelCheckpoint): + checkpoint = raw_checkpoint + else: + return ModelHandlerOutput.for_compute( + input_envelope_id=input_envelope_id, + correlation_id=corr_id, + handler_id="handler-checkpoint-validate", + result=ModelCheckpointValidateOutput( + is_valid=False, + correlation_id=corr_id, + errors=("checkpoint field is missing or has unexpected type",), + ), + ) + + # ── Validate ───────────────────────────────────────────────── + result = self.validate(checkpoint, corr_id) + return ModelHandlerOutput.for_compute( + input_envelope_id=input_envelope_id, + correlation_id=corr_id, + handler_id="handler-checkpoint-validate", + result=result, + ) + + def validate( + self, + checkpoint: ModelCheckpoint, + correlation_id: UUID, + ) -> ModelCheckpointValidateOutput: + """Pure validation function — deterministic, no I/O. + + Checks: + 1. Schema version compatibility + 2. Attempt number is positive + 3. No absolute artifact paths + 4. Commit SHAs in repo_commit_map are valid hex + 5. Phase payload ``phase`` matches checkpoint ``phase`` + 6. Timestamp is not in the future + """ + errors: list[str] = [] + warnings: list[str] = [] + + # 1. Schema version + if checkpoint.schema_version != CHECKPOINT_SCHEMA_VERSION: + warnings.append( + f"Schema version mismatch: expected {CHECKPOINT_SCHEMA_VERSION}, " + f"got {checkpoint.schema_version}" + ) + + # 2. Attempt number + if checkpoint.attempt_number < 1: + errors.append( + f"attempt_number must be >= 1, got {checkpoint.attempt_number}" + ) + + # 3. Path normalization: no absolute paths in artifact_paths + for path_str in checkpoint.artifact_paths: + if PurePosixPath(path_str).is_absolute(): + errors.append(f"Absolute artifact path forbidden: {path_str}") + + # 4. Commit SHA format in repo_commit_map + for repo_path, sha in checkpoint.repo_commit_map.items(): + if not _SHA_RE.match(sha): + errors.append(f"Invalid commit SHA for repo '{repo_path}': '{sha}'") + + # 5. Phase payload consistency + payload_phase = checkpoint.phase_payload.phase + if payload_phase != checkpoint.phase.value: + errors.append( + f"Phase mismatch: header says '{checkpoint.phase.value}', " + f"payload says '{payload_phase}'" + ) + + # 6. Timestamp not in the future (with 60s tolerance) + now = datetime.now(UTC) + if checkpoint.timestamp_utc.tzinfo is not None: + ts = checkpoint.timestamp_utc + else: + ts = checkpoint.timestamp_utc.replace(tzinfo=UTC) + + if ts > now + timedelta(seconds=60): + warnings.append("timestamp_utc is in the future") + + is_valid = len(errors) == 0 + + return ModelCheckpointValidateOutput( + is_valid=is_valid, + correlation_id=correlation_id, + errors=tuple(errors), + warnings=tuple(warnings), + ) + + +__all__: list[str] = ["HandlerCheckpointValidate"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_validate_compute/models/__init__.py b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/models/__init__.py new file mode 100644 index 0000000000..5b108c8372 --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/models/__init__.py @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Models for the checkpoint validate compute node. + +Ticket: OMN-2143 +""" + +from omnibase_infra.nodes.node_checkpoint_validate_compute.models.model_checkpoint_validate_input import ( + ModelCheckpointValidateInput, +) +from omnibase_infra.nodes.node_checkpoint_validate_compute.models.model_checkpoint_validate_output import ( + ModelCheckpointValidateOutput, +) + +__all__: list[str] = [ + "ModelCheckpointValidateInput", + "ModelCheckpointValidateOutput", +] diff --git a/src/omnibase_infra/nodes/node_checkpoint_validate_compute/models/model_checkpoint_validate_input.py b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/models/model_checkpoint_validate_input.py new file mode 100644 index 0000000000..4259d68fda --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/models/model_checkpoint_validate_input.py @@ -0,0 +1,32 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Input model for checkpoint validation. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from uuid import UUID + +from pydantic import BaseModel, ConfigDict, Field + +from omnibase_infra.models.checkpoint.model_checkpoint import ModelCheckpoint + + +class ModelCheckpointValidateInput(BaseModel): + """Input for the checkpoint validate compute handler.""" + + model_config = ConfigDict(frozen=True, extra="forbid", from_attributes=True) + + checkpoint: ModelCheckpoint = Field( + ..., + description="The checkpoint to validate structurally.", + ) + correlation_id: UUID = Field( + ..., + description="Correlation ID for tracing.", + ) + + +__all__: list[str] = ["ModelCheckpointValidateInput"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_validate_compute/models/model_checkpoint_validate_output.py b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/models/model_checkpoint_validate_output.py new file mode 100644 index 0000000000..d29550ab6c --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/models/model_checkpoint_validate_output.py @@ -0,0 +1,52 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Output model for checkpoint validation. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from uuid import UUID + +from pydantic import BaseModel, ConfigDict, Field + + +class ModelCheckpointValidateOutput(BaseModel): + """Result of checkpoint structural validation. + + Warning: + **Non-standard __bool__ behavior**: Returns ``True`` only when + ``is_valid`` is True. Differs from typical Pydantic behavior. + """ + + model_config = ConfigDict(frozen=True, extra="forbid", from_attributes=True) + + is_valid: bool = Field( + ..., + description="Whether the checkpoint passed all validation checks.", + ) + correlation_id: UUID = Field( + ..., + description="Correlation ID echoed from the input.", + ) + errors: tuple[str, ...] = Field( + default_factory=tuple, + description="Validation errors that make the checkpoint unusable.", + ) + warnings: tuple[str, ...] = Field( + default_factory=tuple, + description="Non-fatal warnings about checkpoint quality.", + ) + + def __bool__(self) -> bool: + """Allow using result in boolean context. + + Warning: + **Non-standard __bool__ behavior**: Returns ``True`` only when + ``is_valid`` is True. Differs from typical Pydantic behavior. + """ + return self.is_valid + + +__all__: list[str] = ["ModelCheckpointValidateOutput"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_validate_compute/node.py b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/node.py new file mode 100644 index 0000000000..4c5860d721 --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/node.py @@ -0,0 +1,31 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Declarative COMPUTE node for checkpoint validation. + +Pure validation — verifies checkpoint data consistency without I/O. +All behavior is defined in contract.yaml and delegated to +HandlerCheckpointValidate. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from omnibase_core.nodes.node_compute import NodeCompute + +if TYPE_CHECKING: + from omnibase_core.models.container.model_onex_container import ModelONEXContainer + + +class NodeCheckpointValidateCompute(NodeCompute): + """Declarative compute node for checkpoint structural validation. + + Handler: + - ``HandlerCheckpointValidate``: Pure validation of checkpoint data. + """ + + def __init__(self, container: ModelONEXContainer) -> None: + """Initialize the checkpoint validate compute node.""" + super().__init__(container) diff --git a/src/omnibase_infra/nodes/node_checkpoint_validate_compute/registry/__init__.py b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/registry/__init__.py new file mode 100644 index 0000000000..327832e091 --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/registry/__init__.py @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Registry for checkpoint validate compute node. + +Ticket: OMN-2143 +""" + +from omnibase_infra.nodes.node_checkpoint_validate_compute.registry.registry_infra_checkpoint_validate_compute import ( + RegistryInfraCheckpointValidateCompute, +) + +__all__: list[str] = ["RegistryInfraCheckpointValidateCompute"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_validate_compute/registry/registry_infra_checkpoint_validate_compute.py b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/registry/registry_infra_checkpoint_validate_compute.py new file mode 100644 index 0000000000..7bb8ce8e5c --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/registry/registry_infra_checkpoint_validate_compute.py @@ -0,0 +1,90 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Dependency injection registry for the checkpoint validate compute node. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +import logging +from typing import TYPE_CHECKING + +from omnibase_infra.enums import EnumInfraTransportType +from omnibase_infra.errors import ModelInfraErrorContext, ProtocolConfigurationError + +if TYPE_CHECKING: + from omnibase_core.models.container.model_onex_container import ModelONEXContainer + from omnibase_infra.nodes.node_checkpoint_validate_compute.handlers.handler_checkpoint_validate import ( + HandlerCheckpointValidate, + ) + +logger = logging.getLogger(__name__) + +_HANDLER_STORAGE: dict[str, object] = {} +_PROTOCOL_METADATA: dict[str, dict[str, object]] = {} + + +class RegistryInfraCheckpointValidateCompute: + """Registry for checkpoint validate compute node handler.""" + + HANDLER_KEY = "handler_checkpoint_validate" + + @staticmethod + def _is_registered(handler_key: str) -> bool: + return handler_key in _HANDLER_STORAGE + + @staticmethod + def register(_container: ModelONEXContainer) -> None: + """Register checkpoint validate metadata.""" + _PROTOCOL_METADATA[RegistryInfraCheckpointValidateCompute.HANDLER_KEY] = { + "handler": "HandlerCheckpointValidate", + "module": "omnibase_infra.nodes.node_checkpoint_validate_compute.handlers.handler_checkpoint_validate", + "description": "Handler for checkpoint structural validation", + "capabilities": ["checkpoint.validate"], + } + + @staticmethod + def register_handler( + _container: ModelONEXContainer, + handler: HandlerCheckpointValidate, + ) -> None: + """Register a checkpoint validate handler.""" + required_methods = ["initialize", "shutdown", "execute"] + missing = [ + m for m in required_methods if not callable(getattr(handler, m, None)) + ] + if missing: + context = ModelInfraErrorContext.with_correlation( + transport_type=EnumInfraTransportType.RUNTIME, + operation="register_handler", + target_name="checkpoint_validate_handler", + ) + raise ProtocolConfigurationError( + f"Handler missing required protocol methods: {missing}", + context=context, + ) + + handler_key = f"{RegistryInfraCheckpointValidateCompute.HANDLER_KEY}.default" + + if RegistryInfraCheckpointValidateCompute._is_registered(handler_key): + logger.warning("Re-registering handler '%s'", handler_key) + + _HANDLER_STORAGE[handler_key] = handler + + @staticmethod + def get_handler( + _container: ModelONEXContainer, + ) -> object | None: + """Retrieve a registered checkpoint validate handler.""" + handler_key = f"{RegistryInfraCheckpointValidateCompute.HANDLER_KEY}.default" + return _HANDLER_STORAGE.get(handler_key) + + @staticmethod + def clear() -> None: + """Clear all registered handlers. Call in test teardown.""" + _HANDLER_STORAGE.clear() + _PROTOCOL_METADATA.clear() + + +__all__: list[str] = ["RegistryInfraCheckpointValidateCompute"] diff --git a/src/omnibase_infra/validation/infra_validators.py b/src/omnibase_infra/validation/infra_validators.py index 29a585c0b3..f9bcc5fc6f 100644 --- a/src/omnibase_infra/validation/infra_validators.py +++ b/src/omnibase_infra/validation/infra_validators.py @@ -452,7 +452,10 @@ def get_architecture_exemptions() -> list[ExemptionPattern]: # Note: OMN-1742 RequestResponseWiring uses UUID | None (optional, not counted) # Note: OMN-1869 ContractRegistration added IntentPayloadType union (118th) # Note: OMN-1990 ServiceTopicManager return type dict[str, list[str] | str] (119th) -INFRA_MAX_UNIONS = 119 +# - 121 (2026-02-11): OMN-2143 checkpoint nodes (+2 unions) +# PhasePayload: 5-type discriminated Union in ModelCheckpoint +# register_handler(): Write | Read | List handler type param +INFRA_MAX_UNIONS = 121 # Maximum allowed architecture violations in infrastructure code. # Set to 0 (strict enforcement) to ensure one-model-per-file principle is always followed. diff --git a/src/omnibase_infra/validation/validation_exemptions.yaml b/src/omnibase_infra/validation/validation_exemptions.yaml index 50d622f180..9b07487c51 100644 --- a/src/omnibase_infra/validation/validation_exemptions.yaml +++ b/src/omnibase_infra/validation/validation_exemptions.yaml @@ -2251,6 +2251,30 @@ pattern_exemptions: documentation: - CLAUDE.md (Node Architecture - COMPUTE nodes) ticket: OMN-2125 + # ========================================================================== + # ModelPhasePayloadImplement branch_name Exemption (OMN-2143) + # ========================================================================== + # branch_name is a git branch string, not an entity reference. + - file_pattern: 'model_phase_payload_implement\.py' + violation_pattern: "Field 'branch_name' might reference an entity" + reason: > + branch_name is a git branch string (e.g., "jonah/omn-2143-checkpoint-nodes") identifying the working branch for a pipeline phase. It is not a domain entity requiring ID + display_name. + + documentation: + - CLAUDE.md (Node Architecture - EFFECT nodes) + ticket: OMN-2143 + # ========================================================================== + # ModelCheckpoint ticket_id Exemption (OMN-2143) + # ========================================================================== + # ticket_id is a Linear ticket identifier string (e.g., "OMN-2143"), not a UUID. + - file_pattern: 'model_checkpoint\.py' + violation_pattern: "Field 'ticket_id' should use UUID type instead of str" + reason: > + ticket_id is a human-readable Linear ticket identifier (e.g., "OMN-2143") used for grouping checkpoint files by ticket. Linear ticket IDs are string-formatted identifiers, not UUIDs. + + documentation: + - CLAUDE.md (Node Architecture - EFFECT nodes) + ticket: OMN-2143 # Architecture validator exemptions # These handle one-model-per-file violations for domain-grouped protocols architecture_exemptions: diff --git a/tests/unit/nodes/test_node_checkpoint_effect/__init__.py b/tests/unit/nodes/test_node_checkpoint_effect/__init__.py new file mode 100644 index 0000000000..d90585ce93 --- /dev/null +++ b/tests/unit/nodes/test_node_checkpoint_effect/__init__.py @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Tests for checkpoint effect node. Ticket: OMN-2143.""" diff --git a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_list.py b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_list.py new file mode 100644 index 0000000000..ca219cfd6c --- /dev/null +++ b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_list.py @@ -0,0 +1,214 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Unit tests for HandlerCheckpointList. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from datetime import UTC, datetime +from pathlib import Path +from unittest.mock import MagicMock +from uuid import uuid4 + +import pytest + +from omnibase_infra.enums.enum_checkpoint_phase import EnumCheckpointPhase +from omnibase_infra.models.checkpoint.model_checkpoint import ModelCheckpoint +from omnibase_infra.models.checkpoint.model_phase_payload_create_pr import ( + ModelPhasePayloadCreatePr, +) +from omnibase_infra.models.checkpoint.model_phase_payload_implement import ( + ModelPhasePayloadImplement, +) +from omnibase_infra.models.checkpoint.model_phase_payload_local_review import ( + ModelPhasePayloadLocalReview, +) +from omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_list import ( + HandlerCheckpointList, +) +from omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_write import ( + HandlerCheckpointWrite, +) + + +@pytest.fixture +def mock_container() -> MagicMock: + return MagicMock() + + +@pytest.fixture +def writer(mock_container: MagicMock) -> HandlerCheckpointWrite: + return HandlerCheckpointWrite(mock_container) + + +@pytest.fixture +def lister(mock_container: MagicMock) -> HandlerCheckpointList: + return HandlerCheckpointList(mock_container) + + +class TestHandlerCheckpointList: + """Tests for HandlerCheckpointList.""" + + async def test_list_empty_returns_empty_tuple( + self, + lister: HandlerCheckpointList, + tmp_path: Path, + ) -> None: + """List returns empty tuple when no checkpoints exist.""" + await lister.initialize({}) + + env: dict[str, object] = { + "ticket_id": "OMN-NOEXIST", + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + result = await lister.execute(env) + assert result.result.success is True + assert result.result.checkpoints == () + + async def test_list_returns_all_phases( + self, + writer: HandlerCheckpointWrite, + lister: HandlerCheckpointList, + tmp_path: Path, + ) -> None: + """List returns checkpoints across all phases for a run.""" + await writer.initialize({}) + await lister.initialize({}) + + run_id = uuid4() + phases_and_payloads: list[tuple[EnumCheckpointPhase, object]] = [ + ( + EnumCheckpointPhase.IMPLEMENT, + ModelPhasePayloadImplement( + branch_name="branch", + commit_sha="abc1234", + ), + ), + ( + EnumCheckpointPhase.LOCAL_REVIEW, + ModelPhasePayloadLocalReview( + iteration_count=2, + last_clean_sha="def5678", + ), + ), + ( + EnumCheckpointPhase.CREATE_PR, + ModelPhasePayloadCreatePr( + pr_url="https://github.com/org/repo/pull/42", + pr_number=42, + head_sha="aabbccd", + ), + ), + ] + + for phase, payload in phases_and_payloads: + cp = ModelCheckpoint( + run_id=run_id, + ticket_id="OMN-2143", + phase=phase, + timestamp_utc=datetime.now(UTC), + attempt_number=1, + phase_payload=payload, + ) + await writer.execute( + { + "checkpoint": cp, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + ) + + # List all for this run + env: dict[str, object] = { + "ticket_id": "OMN-2143", + "run_id": run_id, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + result = await lister.execute(env) + assert result.result.success is True + assert len(result.result.checkpoints) == 3 + + async def test_list_includes_multiple_attempts( + self, + writer: HandlerCheckpointWrite, + lister: HandlerCheckpointList, + tmp_path: Path, + ) -> None: + """List returns all attempts for a given phase.""" + await writer.initialize({}) + await lister.initialize({}) + + run_id = uuid4() + + for attempt in (1, 2): + cp = ModelCheckpoint( + run_id=run_id, + ticket_id="OMN-2143", + phase=EnumCheckpointPhase.IMPLEMENT, + timestamp_utc=datetime.now(UTC), + attempt_number=attempt, + phase_payload=ModelPhasePayloadImplement( + branch_name="branch", + commit_sha="abc1234", + ), + ) + await writer.execute( + { + "checkpoint": cp, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + ) + + env: dict[str, object] = { + "ticket_id": "OMN-2143", + "run_id": run_id, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + result = await lister.execute(env) + assert result.result.success is True + assert len(result.result.checkpoints) == 2 + + async def test_list_without_run_id_scans_all_runs( + self, + writer: HandlerCheckpointWrite, + lister: HandlerCheckpointList, + tmp_path: Path, + ) -> None: + """List without run_id returns checkpoints across all runs.""" + await writer.initialize({}) + await lister.initialize({}) + + for _ in range(2): + cp = ModelCheckpoint( + run_id=uuid4(), + ticket_id="OMN-2143", + phase=EnumCheckpointPhase.IMPLEMENT, + timestamp_utc=datetime.now(UTC), + attempt_number=1, + phase_payload=ModelPhasePayloadImplement( + branch_name="branch", + commit_sha="abc1234", + ), + ) + await writer.execute( + { + "checkpoint": cp, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + ) + + env: dict[str, object] = { + "ticket_id": "OMN-2143", + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + result = await lister.execute(env) + assert result.result.success is True + assert len(result.result.checkpoints) == 2 diff --git a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_read.py b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_read.py new file mode 100644 index 0000000000..8c198fdc00 --- /dev/null +++ b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_read.py @@ -0,0 +1,203 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Unit tests for HandlerCheckpointRead. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from datetime import UTC, datetime +from pathlib import Path +from unittest.mock import MagicMock +from uuid import uuid4 + +import pytest +import yaml + +from omnibase_infra.enums.enum_checkpoint_phase import EnumCheckpointPhase +from omnibase_infra.models.checkpoint.model_checkpoint import ModelCheckpoint +from omnibase_infra.models.checkpoint.model_phase_payload_implement import ( + ModelPhasePayloadImplement, +) +from omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_read import ( + HandlerCheckpointRead, +) +from omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_write import ( + HandlerCheckpointWrite, +) + + +@pytest.fixture +def mock_container() -> MagicMock: + return MagicMock() + + +@pytest.fixture +def writer(mock_container: MagicMock) -> HandlerCheckpointWrite: + return HandlerCheckpointWrite(mock_container) + + +@pytest.fixture +def reader(mock_container: MagicMock) -> HandlerCheckpointRead: + return HandlerCheckpointRead(mock_container) + + +class TestHandlerCheckpointRead: + """Tests for HandlerCheckpointRead.""" + + async def test_read_returns_written_checkpoint( + self, + writer: HandlerCheckpointWrite, + reader: HandlerCheckpointRead, + tmp_path: Path, + ) -> None: + """Round-trip: write then read returns the same checkpoint.""" + await writer.initialize({}) + await reader.initialize({}) + + run_id = uuid4() + checkpoint = ModelCheckpoint( + run_id=run_id, + ticket_id="OMN-2143", + phase=EnumCheckpointPhase.IMPLEMENT, + timestamp_utc=datetime.now(UTC), + repo_commit_map={"infra": "deadbeef"}, + attempt_number=1, + phase_payload=ModelPhasePayloadImplement( + branch_name="feature-branch", + commit_sha="deadbeef", + files_changed=("src/foo.py",), + ), + ) + + # Write + write_env: dict[str, object] = { + "checkpoint": checkpoint, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + await writer.execute(write_env) + + # Read + read_env: dict[str, object] = { + "ticket_id": "OMN-2143", + "run_id": run_id, + "phase": EnumCheckpointPhase.IMPLEMENT, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + result = await reader.execute(read_env) + output = result.result + + assert output.success is True + assert output.checkpoint is not None + assert output.checkpoint.ticket_id == "OMN-2143" + assert output.checkpoint.repo_commit_map == {"infra": "deadbeef"} + + async def test_read_returns_latest_attempt( + self, + writer: HandlerCheckpointWrite, + reader: HandlerCheckpointRead, + tmp_path: Path, + ) -> None: + """When multiple attempts exist, read returns the highest attempt.""" + await writer.initialize({}) + await reader.initialize({}) + + run_id = uuid4() + + for attempt in (1, 2, 3): + cp = ModelCheckpoint( + run_id=run_id, + ticket_id="OMN-2143", + phase=EnumCheckpointPhase.LOCAL_REVIEW, + timestamp_utc=datetime.now(UTC), + attempt_number=attempt, + phase_payload={ + "phase": "local_review", + "iteration_count": attempt, + "issue_fingerprints": [], + "last_clean_sha": f"sha{attempt:04d}aa", + }, + ) + write_env: dict[str, object] = { + "checkpoint": cp, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + await writer.execute(write_env) + + # Read should return attempt 3 + read_env: dict[str, object] = { + "ticket_id": "OMN-2143", + "run_id": run_id, + "phase": EnumCheckpointPhase.LOCAL_REVIEW, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + result = await reader.execute(read_env) + assert result.result.success is True + assert result.result.checkpoint.attempt_number == 3 + + async def test_read_missing_directory_returns_error( + self, + reader: HandlerCheckpointRead, + tmp_path: Path, + ) -> None: + """Read returns failure when the checkpoint directory does not exist.""" + await reader.initialize({}) + + read_env: dict[str, object] = { + "ticket_id": "OMN-NOEXIST", + "run_id": uuid4(), + "phase": EnumCheckpointPhase.IMPLEMENT, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + result = await reader.execute(read_env) + assert result.result.success is False + assert "not found" in result.result.error + + async def test_read_missing_phase_returns_error( + self, + writer: HandlerCheckpointWrite, + reader: HandlerCheckpointRead, + tmp_path: Path, + ) -> None: + """Read returns failure when the requested phase has no checkpoints.""" + await writer.initialize({}) + await reader.initialize({}) + + run_id = uuid4() + # Write implement phase + cp = ModelCheckpoint( + run_id=run_id, + ticket_id="OMN-2143", + phase=EnumCheckpointPhase.IMPLEMENT, + timestamp_utc=datetime.now(UTC), + attempt_number=1, + phase_payload=ModelPhasePayloadImplement( + branch_name="branch", + commit_sha="abc1234", + ), + ) + await writer.execute( + { + "checkpoint": cp, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + ) + + # Try to read a different phase + read_env: dict[str, object] = { + "ticket_id": "OMN-2143", + "run_id": run_id, + "phase": EnumCheckpointPhase.CREATE_PR, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + result = await reader.execute(read_env) + assert result.result.success is False + assert "No checkpoint found" in result.result.error diff --git a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_write.py b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_write.py new file mode 100644 index 0000000000..b321c25dfe --- /dev/null +++ b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_write.py @@ -0,0 +1,186 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Unit tests for HandlerCheckpointWrite. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from datetime import UTC, datetime +from pathlib import Path +from unittest.mock import MagicMock +from uuid import uuid4 + +import pytest +import yaml + +from omnibase_infra.enums.enum_checkpoint_phase import EnumCheckpointPhase +from omnibase_infra.models.checkpoint.model_checkpoint import ModelCheckpoint +from omnibase_infra.models.checkpoint.model_phase_payload_implement import ( + ModelPhasePayloadImplement, +) +from omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_write import ( + HandlerCheckpointWrite, +) + + +@pytest.fixture +def mock_container() -> MagicMock: + return MagicMock() + + +@pytest.fixture +def handler(mock_container: MagicMock) -> HandlerCheckpointWrite: + return HandlerCheckpointWrite(mock_container) + + +@pytest.fixture +def sample_checkpoint() -> ModelCheckpoint: + return ModelCheckpoint( + run_id=uuid4(), + ticket_id="OMN-2143", + phase=EnumCheckpointPhase.IMPLEMENT, + timestamp_utc=datetime.now(UTC), + repo_commit_map={"omnibase_infra": "abc1234"}, + artifact_paths=("src/new_file.py",), + attempt_number=1, + phase_payload=ModelPhasePayloadImplement( + branch_name="jonah/omn-2143-checkpoint-nodes", + commit_sha="abc1234", + files_changed=("src/new_file.py",), + ), + ) + + +class TestHandlerCheckpointWrite: + """Tests for HandlerCheckpointWrite.""" + + async def test_write_creates_yaml_file( + self, + handler: HandlerCheckpointWrite, + sample_checkpoint: ModelCheckpoint, + tmp_path: Path, + ) -> None: + """Write creates a YAML file at the expected path.""" + await handler.initialize({}) + + envelope: dict[str, object] = { + "checkpoint": sample_checkpoint, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + + result = await handler.execute(envelope) + output = result.result + + assert output.success is True + assert output.checkpoint_path is not None + + # Verify file exists + full_path = tmp_path / output.checkpoint_path + assert full_path.exists() + + # Verify YAML content round-trips + raw = yaml.safe_load(full_path.read_text(encoding="utf-8")) + reloaded = ModelCheckpoint.model_validate(raw) + assert reloaded.ticket_id == "OMN-2143" + assert reloaded.phase == EnumCheckpointPhase.IMPLEMENT + assert reloaded.attempt_number == 1 + + async def test_write_preserves_attempt_number( + self, + handler: HandlerCheckpointWrite, + tmp_path: Path, + ) -> None: + """Re-runs create new files with incremented attempt_number.""" + await handler.initialize({}) + run_id = uuid4() + + for attempt in (1, 2, 3): + cp = ModelCheckpoint( + run_id=run_id, + ticket_id="OMN-2143", + phase=EnumCheckpointPhase.IMPLEMENT, + timestamp_utc=datetime.now(UTC), + attempt_number=attempt, + phase_payload=ModelPhasePayloadImplement( + branch_name="branch", + commit_sha="abc1234", + ), + ) + envelope: dict[str, object] = { + "checkpoint": cp, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + await handler.execute(envelope) + + # Verify 3 distinct files exist + run_dir = tmp_path / "OMN-2143" / str(run_id) + yaml_files = list(run_dir.glob("phase_1_implement_a*.yaml")) + assert len(yaml_files) == 3 + + async def test_write_rejects_absolute_artifact_paths( + self, + handler: HandlerCheckpointWrite, + tmp_path: Path, + ) -> None: + """Write rejects checkpoints with absolute artifact paths.""" + await handler.initialize({}) + + cp = ModelCheckpoint( + run_id=uuid4(), + ticket_id="OMN-2143", + phase=EnumCheckpointPhase.IMPLEMENT, + timestamp_utc=datetime.now(UTC), + artifact_paths=("/absolute/path/bad.py",), + attempt_number=1, + phase_payload=ModelPhasePayloadImplement( + branch_name="branch", + commit_sha="abc1234", + ), + ) + + envelope: dict[str, object] = { + "checkpoint": cp, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + + with pytest.raises(Exception, match="Absolute artifact path forbidden"): + await handler.execute(envelope) + + async def test_write_requires_checkpoint( + self, + handler: HandlerCheckpointWrite, + tmp_path: Path, + ) -> None: + """Write raises when checkpoint is missing from envelope.""" + await handler.initialize({}) + + envelope: dict[str, object] = { + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + + with pytest.raises(Exception, match=r"requires.*checkpoint"): + await handler.execute(envelope) + + async def test_write_accepts_dict_checkpoint( + self, + handler: HandlerCheckpointWrite, + sample_checkpoint: ModelCheckpoint, + tmp_path: Path, + ) -> None: + """Write accepts dict input and validates it via Pydantic.""" + await handler.initialize({}) + + envelope: dict[str, object] = { + "checkpoint": sample_checkpoint.model_dump(mode="json"), + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + + result = await handler.execute(envelope) + assert result.result.success is True diff --git a/tests/unit/nodes/test_node_checkpoint_validate_compute/__init__.py b/tests/unit/nodes/test_node_checkpoint_validate_compute/__init__.py new file mode 100644 index 0000000000..9c14fe65ec --- /dev/null +++ b/tests/unit/nodes/test_node_checkpoint_validate_compute/__init__.py @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Tests for checkpoint validate compute node. Ticket: OMN-2143.""" diff --git a/tests/unit/nodes/test_node_checkpoint_validate_compute/test_handler_checkpoint_validate.py b/tests/unit/nodes/test_node_checkpoint_validate_compute/test_handler_checkpoint_validate.py new file mode 100644 index 0000000000..ae28025f32 --- /dev/null +++ b/tests/unit/nodes/test_node_checkpoint_validate_compute/test_handler_checkpoint_validate.py @@ -0,0 +1,221 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Unit tests for HandlerCheckpointValidate. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from datetime import UTC, datetime +from unittest.mock import MagicMock +from uuid import uuid4 + +import pytest + +from omnibase_infra.enums.enum_checkpoint_phase import EnumCheckpointPhase +from omnibase_infra.models.checkpoint.model_checkpoint import ModelCheckpoint +from omnibase_infra.models.checkpoint.model_phase_payload_implement import ( + ModelPhasePayloadImplement, +) +from omnibase_infra.models.checkpoint.model_phase_payload_local_review import ( + ModelPhasePayloadLocalReview, +) +from omnibase_infra.nodes.node_checkpoint_validate_compute.handlers.handler_checkpoint_validate import ( + HandlerCheckpointValidate, +) + + +@pytest.fixture +def mock_container() -> MagicMock: + return MagicMock() + + +@pytest.fixture +def handler(mock_container: MagicMock) -> HandlerCheckpointValidate: + return HandlerCheckpointValidate(mock_container) + + +def _valid_checkpoint(**overrides: object) -> ModelCheckpoint: + """Create a valid checkpoint with optional overrides.""" + defaults: dict[str, object] = { + "run_id": uuid4(), + "ticket_id": "OMN-2143", + "phase": EnumCheckpointPhase.IMPLEMENT, + "timestamp_utc": datetime.now(UTC), + "repo_commit_map": {"omnibase_infra": "abc1234"}, + "artifact_paths": ("src/foo.py",), + "attempt_number": 1, + "phase_payload": ModelPhasePayloadImplement( + branch_name="feature-branch", + commit_sha="abc1234", + ), + } + defaults.update(overrides) + return ModelCheckpoint(**defaults) # type: ignore[arg-type] + + +class TestHandlerCheckpointValidate: + """Tests for HandlerCheckpointValidate.""" + + async def test_valid_checkpoint_passes( + self, + handler: HandlerCheckpointValidate, + ) -> None: + """A well-formed checkpoint passes validation.""" + await handler.initialize({}) + + env: dict[str, object] = { + "checkpoint": _valid_checkpoint(), + "correlation_id": uuid4(), + } + result = await handler.execute(env) + output = result.result + + assert output.is_valid is True + assert output.errors == () + + async def test_absolute_artifact_path_is_error( + self, + handler: HandlerCheckpointValidate, + ) -> None: + """Absolute paths in artifact_paths produce an error.""" + await handler.initialize({}) + + cp = _valid_checkpoint(artifact_paths=("/absolute/path.py",)) + + env: dict[str, object] = { + "checkpoint": cp, + "correlation_id": uuid4(), + } + result = await handler.execute(env) + output = result.result + + assert output.is_valid is False + assert any("Absolute artifact path" in e for e in output.errors) + + async def test_invalid_commit_sha_is_error( + self, + handler: HandlerCheckpointValidate, + ) -> None: + """Non-hex commit SHA produces an error.""" + await handler.initialize({}) + + cp = _valid_checkpoint(repo_commit_map={"repo": "not-a-sha!!"}) + + env: dict[str, object] = { + "checkpoint": cp, + "correlation_id": uuid4(), + } + result = await handler.execute(env) + output = result.result + + assert output.is_valid is False + assert any("Invalid commit SHA" in e for e in output.errors) + + async def test_phase_payload_mismatch_is_error( + self, + handler: HandlerCheckpointValidate, + ) -> None: + """Phase payload phase != header phase produces an error.""" + await handler.initialize({}) + + # Create a checkpoint where header says LOCAL_REVIEW but payload is IMPLEMENT + cp = _valid_checkpoint( + phase=EnumCheckpointPhase.LOCAL_REVIEW, + phase_payload=ModelPhasePayloadLocalReview( + iteration_count=1, + last_clean_sha="abc1234", + ), + ) + + # This should be valid — payload matches header + env: dict[str, object] = { + "checkpoint": cp, + "correlation_id": uuid4(), + } + result = await handler.execute(env) + assert result.result.is_valid is True + + async def test_schema_version_mismatch_is_warning( + self, + handler: HandlerCheckpointValidate, + ) -> None: + """Different schema version produces a warning, not error.""" + await handler.initialize({}) + + cp = _valid_checkpoint(schema_version="2.0.0") + + env: dict[str, object] = { + "checkpoint": cp, + "correlation_id": uuid4(), + } + result = await handler.execute(env) + output = result.result + + assert output.is_valid is True # Warning, not error + assert any("Schema version mismatch" in w for w in output.warnings) + + async def test_dict_input_validation( + self, + handler: HandlerCheckpointValidate, + ) -> None: + """Handler accepts dict input and validates via Pydantic.""" + await handler.initialize({}) + + cp = _valid_checkpoint() + env: dict[str, object] = { + "checkpoint": cp.model_dump(mode="json"), + "correlation_id": uuid4(), + } + result = await handler.execute(env) + assert result.result.is_valid is True + + async def test_invalid_dict_produces_errors( + self, + handler: HandlerCheckpointValidate, + ) -> None: + """Invalid dict input produces validation errors.""" + await handler.initialize({}) + + env: dict[str, object] = { + "checkpoint": {"not": "a checkpoint"}, + "correlation_id": uuid4(), + } + result = await handler.execute(env) + assert result.result.is_valid is False + assert len(result.result.errors) > 0 + + async def test_missing_checkpoint_produces_error( + self, + handler: HandlerCheckpointValidate, + ) -> None: + """Missing checkpoint field produces an error.""" + await handler.initialize({}) + + env: dict[str, object] = { + "correlation_id": uuid4(), + } + result = await handler.execute(env) + assert result.result.is_valid is False + + async def test_bool_reflects_validity( + self, + handler: HandlerCheckpointValidate, + ) -> None: + """Output __bool__ matches is_valid.""" + await handler.initialize({}) + + valid_env: dict[str, object] = { + "checkpoint": _valid_checkpoint(), + "correlation_id": uuid4(), + } + valid_result = await handler.execute(valid_env) + assert bool(valid_result.result) is True + + invalid_env: dict[str, object] = { + "checkpoint": _valid_checkpoint(artifact_paths=("/abs/path",)), + "correlation_id": uuid4(), + } + invalid_result = await handler.execute(invalid_env) + assert bool(invalid_result.result) is False From 4407daad9b1702bac51b2402004a09830b2fbb02 Mon Sep 17 00:00:00 2001 From: Jonah Date: Wed, 11 Feb 2026 14:08:19 -0500 Subject: [PATCH 2/9] =?UTF-8?q?fix(review):=20harden=20checkpoint=20handle?= =?UTF-8?q?rs=20=E2=80=94=20overwrite=20guard,=20YAML=20error=20handling,?= =?UTF-8?q?=20dead=20code?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Write handler now refuses to overwrite existing checkpoints (append-only guard) - Read handler catches yaml.YAMLError and rejects non-dict YAML content - List handler skips empty/non-mapping YAML files instead of crashing - Removed redundant `or uuid4()` fallback (with_correlation guarantees UUID) - Fixed __all__ alphabetical ordering in enums/__init__.py - Added handler classification docstring to HandlerCheckpointValidate - Added 4 new tests covering overwrite guard, invalid YAML, and empty files Review iteration: 1/10 --- src/omnibase_infra/enums/__init__.py | 2 +- .../handlers/handler_checkpoint_list.py | 8 ++- .../handlers/handler_checkpoint_read.py | 17 ++++-- .../handlers/handler_checkpoint_write.py | 11 ++-- .../handlers/handler_checkpoint_validate.py | 6 ++- .../test_handler_checkpoint_list.py | 48 +++++++++++++++++ .../test_handler_checkpoint_read.py | 52 +++++++++++++++++++ .../test_handler_checkpoint_write.py | 23 ++++++++ 8 files changed, 156 insertions(+), 11 deletions(-) diff --git a/src/omnibase_infra/enums/__init__.py b/src/omnibase_infra/enums/__init__.py index 83a3c15a5f..cead4860ea 100644 --- a/src/omnibase_infra/enums/__init__.py +++ b/src/omnibase_infra/enums/__init__.py @@ -114,8 +114,8 @@ "EnumBackendType", "EnumCaptureOutcome", "EnumCaptureState", - "EnumCheckpointPhase", "EnumChainViolationType", + "EnumCheckpointPhase", "EnumCircuitState", "EnumConfirmationEventType", "EnumConsumerGroupPurpose", diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py index a113a76234..c98295aa5c 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py @@ -80,8 +80,7 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: operation="list_checkpoints", target_name="checkpoint_yaml", ) - # with_correlation() auto-generates a UUID when None is passed - corr_id = context.correlation_id or uuid4() + corr_id = context.correlation_id # with_correlation() guarantees a UUID ticket_id = envelope.get("ticket_id") if not ticket_id: @@ -126,6 +125,11 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: for yaml_file in sorted(scan_dir.glob("phase_*.yaml")): try: raw_data = yaml.safe_load(yaml_file.read_text(encoding="utf-8")) + if not isinstance(raw_data, dict): + logger.warning( + "Skipping non-mapping checkpoint %s", yaml_file.name + ) + continue checkpoint = ModelCheckpoint.model_validate(raw_data) checkpoints.append(checkpoint) except (ValidationError, yaml.YAMLError) as exc: diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py index 27e35d6a2e..c58691169a 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py @@ -87,8 +87,7 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: operation="read_checkpoint", target_name="checkpoint_yaml", ) - # with_correlation() auto-generates a UUID when None is passed - corr_id = context.correlation_id or uuid4() + corr_id = context.correlation_id # with_correlation() guarantees a UUID ticket_id = envelope.get("ticket_id") run_id = envelope.get("run_id") @@ -153,10 +152,20 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: # Read the latest attempt (last file alphabetically = highest attempt) latest_file = matching_files[-1] - raw_data = yaml.safe_load(latest_file.read_text(encoding="utf-8")) - try: + raw_data = yaml.safe_load(latest_file.read_text(encoding="utf-8")) + if not isinstance(raw_data, dict): + raise RuntimeHostError( + f"Corrupt checkpoint file {latest_file.name}: expected mapping, " + f"got {type(raw_data).__name__}", + context=context, + ) checkpoint = ModelCheckpoint.model_validate(raw_data) + except yaml.YAMLError as exc: + raise RuntimeHostError( + f"Corrupt checkpoint file {latest_file.name}: invalid YAML", + context=context, + ) from exc except ValidationError as exc: raise RuntimeHostError( f"Corrupt checkpoint file {latest_file.name}: {exc.error_count()} errors", diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py index 05c2495e48..12d4557dbf 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py @@ -103,8 +103,7 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: operation="write_checkpoint", target_name="checkpoint_yaml", ) - # with_correlation() auto-generates a UUID when None is passed - corr_id = context.correlation_id or uuid4() + corr_id = context.correlation_id # with_correlation() guarantees a UUID raw_checkpoint = envelope.get("checkpoint") if raw_checkpoint is None: @@ -151,8 +150,14 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: context=context, ) - # Write + # Write (append-only: refuse to overwrite existing checkpoints) target_dir.mkdir(parents=True, exist_ok=True) + if target_path.exists(): + raise RuntimeHostError( + f"Checkpoint already exists: {filename} — " + f"increment attempt_number to create a new checkpoint", + context=context, + ) yaml_content = yaml.dump( _serialize_checkpoint(checkpoint), default_flow_style=False, diff --git a/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/handler_checkpoint_validate.py b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/handler_checkpoint_validate.py index b808ad2be9..d87699a5f7 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/handler_checkpoint_validate.py +++ b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/handler_checkpoint_validate.py @@ -39,7 +39,11 @@ class HandlerCheckpointValidate: - """Pure validation of checkpoint data — no I/O, fully deterministic.""" + """Pure validation of checkpoint data — no I/O, fully deterministic. + + Classification: ``COMPUTE_HANDLER`` + ``COMPUTE`` — this is a pure compute + handler. Effect handlers use ``NODE_HANDLER`` + ``EFFECT`` instead. + """ def __init__(self, container: ModelONEXContainer) -> None: self._container = container diff --git a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_list.py b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_list.py index ca219cfd6c..adc9c6f427 100644 --- a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_list.py +++ b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_list.py @@ -174,6 +174,54 @@ async def test_list_includes_multiple_attempts( assert result.result.success is True assert len(result.result.checkpoints) == 2 + async def test_list_skips_empty_yaml_files( + self, + writer: HandlerCheckpointWrite, + lister: HandlerCheckpointList, + tmp_path: Path, + ) -> None: + """List skips empty or non-dict YAML files without crashing.""" + await writer.initialize({}) + await lister.initialize({}) + + run_id = uuid4() + + # Write one valid checkpoint + cp = ModelCheckpoint( + run_id=run_id, + ticket_id="OMN-2143", + phase=EnumCheckpointPhase.IMPLEMENT, + timestamp_utc=datetime.now(UTC), + attempt_number=1, + phase_payload=ModelPhasePayloadImplement( + branch_name="branch", + commit_sha="abc1234", + ), + ) + await writer.execute( + { + "checkpoint": cp, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + ) + + # Place an empty YAML file alongside the valid one + run_dir = tmp_path / "OMN-2143" / str(run_id) + empty_file = run_dir / "phase_1_implement_a0.yaml" + empty_file.write_text("", encoding="utf-8") + + env: dict[str, object] = { + "ticket_id": "OMN-2143", + "run_id": run_id, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + result = await lister.execute(env) + assert result.result.success is True + # Only the valid checkpoint should be returned; the empty one is skipped + assert len(result.result.checkpoints) == 1 + async def test_list_without_run_id_scans_all_runs( self, writer: HandlerCheckpointWrite, diff --git a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_read.py b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_read.py index 8c198fdc00..355be2c59b 100644 --- a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_read.py +++ b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_read.py @@ -159,6 +159,58 @@ async def test_read_missing_directory_returns_error( assert result.result.success is False assert "not found" in result.result.error + async def test_read_handles_invalid_yaml( + self, + reader: HandlerCheckpointRead, + tmp_path: Path, + ) -> None: + """Read raises RuntimeHostError when checkpoint file contains invalid YAML.""" + await reader.initialize({}) + + run_id = uuid4() + run_dir = tmp_path / "OMN-2143" / str(run_id) + run_dir.mkdir(parents=True) + + # Write a corrupt YAML file + corrupt_file = run_dir / "phase_1_implement_a1.yaml" + corrupt_file.write_text(": : : [invalid yaml", encoding="utf-8") + + read_env: dict[str, object] = { + "ticket_id": "OMN-2143", + "run_id": run_id, + "phase": EnumCheckpointPhase.IMPLEMENT, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + with pytest.raises(Exception, match=r"Corrupt checkpoint file.*invalid YAML"): + await reader.execute(read_env) + + async def test_read_handles_empty_yaml( + self, + reader: HandlerCheckpointRead, + tmp_path: Path, + ) -> None: + """Read raises RuntimeHostError when checkpoint file is empty (None).""" + await reader.initialize({}) + + run_id = uuid4() + run_dir = tmp_path / "OMN-2143" / str(run_id) + run_dir.mkdir(parents=True) + + # Write an empty YAML file (safe_load returns None) + empty_file = run_dir / "phase_1_implement_a1.yaml" + empty_file.write_text("", encoding="utf-8") + + read_env: dict[str, object] = { + "ticket_id": "OMN-2143", + "run_id": run_id, + "phase": EnumCheckpointPhase.IMPLEMENT, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + with pytest.raises(Exception, match="expected mapping"): + await reader.execute(read_env) + async def test_read_missing_phase_returns_error( self, writer: HandlerCheckpointWrite, diff --git a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_write.py b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_write.py index b321c25dfe..b10f24e150 100644 --- a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_write.py +++ b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_write.py @@ -184,3 +184,26 @@ async def test_write_accepts_dict_checkpoint( result = await handler.execute(envelope) assert result.result.success is True + + async def test_write_rejects_duplicate_attempt( + self, + handler: HandlerCheckpointWrite, + sample_checkpoint: ModelCheckpoint, + tmp_path: Path, + ) -> None: + """Write refuses to overwrite an existing checkpoint (append-only).""" + await handler.initialize({}) + + envelope: dict[str, object] = { + "checkpoint": sample_checkpoint, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + + # First write succeeds + result = await handler.execute(envelope) + assert result.result.success is True + + # Second write with same checkpoint (same attempt_number) fails + with pytest.raises(Exception, match="Checkpoint already exists"): + await handler.execute(envelope) From 8360cc896abc21d6c0fb667dccdcdc9e34de2ba3 Mon Sep 17 00:00:00 2001 From: Jonah Date: Wed, 11 Feb 2026 14:15:35 -0500 Subject: [PATCH 3/9] fix(review): [critical] path traversal guard + [major] numeric attempt sort - All 3 effect handlers now validate resolved paths stay under base_dir via is_relative_to() to prevent ticket_id path traversal attacks - Read handler sorts checkpoint files by extracted attempt number instead of lexicographic filename order (fixes incorrect result for attempt >= 10) - Added 3 new tests: path traversal write/read, numeric sort verification Review iteration: 2/10 --- .../handlers/handler_checkpoint_list.py | 5 ++ .../handlers/handler_checkpoint_read.py | 19 +++++- .../handlers/handler_checkpoint_write.py | 7 ++- .../test_handler_checkpoint_read.py | 61 +++++++++++++++++++ .../test_handler_checkpoint_write.py | 29 +++++++++ 5 files changed, 117 insertions(+), 4 deletions(-) diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py index c98295aa5c..96ad488d73 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py @@ -94,6 +94,11 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: base_dir = Path(str(base_dir_raw)) if base_dir_raw else _DEFAULT_BASE_DIR ticket_dir = base_dir / str(ticket_id) + if not ticket_dir.resolve().is_relative_to(base_dir.resolve()): + raise RuntimeHostError( + "Path traversal detected: ticket_id escapes checkpoint root", + context=context, + ) if not ticket_dir.is_dir(): return ModelHandlerOutput.for_compute( input_envelope_id=uuid4(), diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py index c58691169a..a83ace2e8f 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py @@ -116,8 +116,13 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: base_dir_raw = envelope.get("base_dir") base_dir = Path(str(base_dir_raw)) if base_dir_raw else _DEFAULT_BASE_DIR - # Scan for matching files + # Scan for matching files (with path traversal guard) target_dir = base_dir / str(ticket_id) / str(run_id_val) + if not target_dir.resolve().is_relative_to(base_dir.resolve()): + raise RuntimeHostError( + "Path traversal detected: ticket_id escapes checkpoint root", + context=context, + ) phase_prefix = f"phase_{phase.phase_number}_{phase.value}_a" if not target_dir.is_dir(): @@ -132,10 +137,18 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: ), ) - # Find all attempt files for this phase + # Find all attempt files for this phase, sorted by attempt number + def _attempt_number(path: Path) -> int: + """Extract numeric attempt from filename like phase_1_implement_a3.yaml.""" + stem = path.stem # e.g. "phase_1_implement_a3" + after_a = stem.rsplit("_a", maxsplit=1) + if len(after_a) == 2 and after_a[1].isdigit(): + return int(after_a[1]) + return 0 + matching_files = sorted( target_dir.glob(f"{phase_prefix}*.yaml"), - key=lambda p: p.name, + key=_attempt_number, ) if not matching_files: diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py index 12d4557dbf..fca62ca4dc 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py @@ -133,8 +133,13 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: base_dir_raw = envelope.get("base_dir") base_dir = Path(str(base_dir_raw)) if base_dir_raw else _DEFAULT_BASE_DIR - # Build path + # Build path and guard against path traversal via ticket_id target_dir = _checkpoint_dir(base_dir, checkpoint.ticket_id, checkpoint.run_id) + if not target_dir.resolve().is_relative_to(base_dir.resolve()): + raise RuntimeHostError( + "Path traversal detected: ticket_id escapes checkpoint root", + context=context, + ) filename = _checkpoint_filename( checkpoint.phase.phase_number, checkpoint.phase.value, diff --git a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_read.py b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_read.py index 355be2c59b..b829667b97 100644 --- a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_read.py +++ b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_read.py @@ -253,3 +253,64 @@ async def test_read_missing_phase_returns_error( result = await reader.execute(read_env) assert result.result.success is False assert "No checkpoint found" in result.result.error + + async def test_read_rejects_path_traversal( + self, + reader: HandlerCheckpointRead, + tmp_path: Path, + ) -> None: + """Read rejects ticket_id containing path traversal sequences.""" + await reader.initialize({}) + + read_env: dict[str, object] = { + "ticket_id": "../../etc", + "run_id": uuid4(), + "phase": EnumCheckpointPhase.IMPLEMENT, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + with pytest.raises(Exception, match="Path traversal detected"): + await reader.execute(read_env) + + async def test_read_sorts_by_attempt_number_not_name( + self, + reader: HandlerCheckpointRead, + tmp_path: Path, + ) -> None: + """Read returns highest attempt even when attempt >= 10.""" + await reader.initialize({}) + + run_id = uuid4() + run_dir = tmp_path / "OMN-2143" / str(run_id) + run_dir.mkdir(parents=True) + + # Create files with attempts 1, 2, 10 (a10 sorts before a2 lexically) + for attempt in (1, 2, 10): + cp = ModelCheckpoint( + run_id=run_id, + ticket_id="OMN-2143", + phase=EnumCheckpointPhase.IMPLEMENT, + timestamp_utc=datetime.now(UTC), + attempt_number=attempt, + phase_payload=ModelPhasePayloadImplement( + branch_name="branch", + commit_sha="abc1234", + ), + ) + filename = f"phase_1_implement_a{attempt}.yaml" + data = cp.model_dump(mode="json") + (run_dir / filename).write_text( + yaml.dump(data, default_flow_style=False), encoding="utf-8" + ) + + read_env: dict[str, object] = { + "ticket_id": "OMN-2143", + "run_id": run_id, + "phase": EnumCheckpointPhase.IMPLEMENT, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + result = await reader.execute(read_env) + assert result.result.success is True + # Must return attempt 10, not attempt 2 (which would be the lexicographic last) + assert result.result.checkpoint.attempt_number == 10 diff --git a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_write.py b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_write.py index b10f24e150..eee1ec447b 100644 --- a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_write.py +++ b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_write.py @@ -207,3 +207,32 @@ async def test_write_rejects_duplicate_attempt( # Second write with same checkpoint (same attempt_number) fails with pytest.raises(Exception, match="Checkpoint already exists"): await handler.execute(envelope) + + async def test_write_rejects_path_traversal( + self, + handler: HandlerCheckpointWrite, + tmp_path: Path, + ) -> None: + """Write rejects ticket_id containing path traversal sequences.""" + await handler.initialize({}) + + cp = ModelCheckpoint( + run_id=uuid4(), + ticket_id="../../etc", + phase=EnumCheckpointPhase.IMPLEMENT, + timestamp_utc=datetime.now(UTC), + attempt_number=1, + phase_payload=ModelPhasePayloadImplement( + branch_name="branch", + commit_sha="abc1234", + ), + ) + + envelope: dict[str, object] = { + "checkpoint": cp, + "correlation_id": uuid4(), + "base_dir": str(tmp_path), + } + + with pytest.raises(Exception, match="Path traversal detected"): + await handler.execute(envelope) From 7dcf36e718f6275f446ce50599281e3e7e5784e1 Mon Sep 17 00:00:00 2001 From: Jonah Date: Wed, 11 Feb 2026 14:27:16 -0500 Subject: [PATCH 4/9] fix(review): [major] atomic writes + TOCTOU guard, [minor] 4 validation hardening fixes - Atomic checkpoint write via tempfile + os.link(O_CREAT-equivalent) prevents partial writes on crash and eliminates TOCTOU race on duplicate detection - timestamp_utc field_validator rejects naive datetimes, normalizes to UTC - List handler sorts by numeric attempt_number (consistent with read handler) - Operation field uses Literal type for compile-time validation - Removed dead naive-datetime branch in validate handler Review iteration: 1/10 --- .../models/checkpoint/model_checkpoint.py | 22 +++++++++- .../handlers/handler_checkpoint_list.py | 10 ++++- .../handlers/handler_checkpoint_write.py | 41 +++++++++++++++---- .../models/model_checkpoint_effect_input.py | 10 +++-- .../handlers/handler_checkpoint_validate.py | 8 +--- 5 files changed, 71 insertions(+), 20 deletions(-) diff --git a/src/omnibase_infra/models/checkpoint/model_checkpoint.py b/src/omnibase_infra/models/checkpoint/model_checkpoint.py index e42b2c03b8..e889cc577b 100644 --- a/src/omnibase_infra/models/checkpoint/model_checkpoint.py +++ b/src/omnibase_infra/models/checkpoint/model_checkpoint.py @@ -16,11 +16,19 @@ from __future__ import annotations -from datetime import datetime +from datetime import UTC, datetime from typing import Annotated, Union from uuid import UUID -from pydantic import BaseModel, ConfigDict, Discriminator, Field, Tag, model_validator +from pydantic import ( + BaseModel, + ConfigDict, + Discriminator, + Field, + Tag, + field_validator, + model_validator, +) from omnibase_infra.enums.enum_checkpoint_phase import EnumCheckpointPhase from omnibase_infra.models.checkpoint.model_phase_payload_create_pr import ( @@ -88,6 +96,16 @@ class ModelCheckpoint(BaseModel): ..., description="UTC timestamp when the phase completed.", ) + + @field_validator("timestamp_utc", mode="after") + @classmethod + def _ensure_utc(cls, v: datetime) -> datetime: + """Reject naive datetimes; normalize to UTC.""" + if v.tzinfo is None: + msg = "timestamp_utc must be timezone-aware (use datetime.now(UTC))" + raise ValueError(msg) + return v.astimezone(UTC) + repo_commit_map: dict[str, str] = Field( default_factory=dict, description="Mapping of relative repo path to commit SHA.", diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py index 96ad488d73..875d061f6a 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py @@ -123,11 +123,19 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: else: scan_dirs = [d for d in ticket_dir.iterdir() if d.is_dir()] + def _attempt_number(path: Path) -> int: + """Extract numeric attempt from filename like phase_1_implement_a3.yaml.""" + stem = path.stem + after_a = stem.rsplit("_a", maxsplit=1) + if len(after_a) == 2 and after_a[1].isdigit(): + return int(after_a[1]) + return 0 + checkpoints: list[ModelCheckpoint] = [] for scan_dir in scan_dirs: if not scan_dir.is_dir(): continue - for yaml_file in sorted(scan_dir.glob("phase_*.yaml")): + for yaml_file in sorted(scan_dir.glob("phase_*.yaml"), key=_attempt_number): try: raw_data = yaml.safe_load(yaml_file.read_text(encoding="utf-8")) if not isinstance(raw_data, dict): diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py index fca62ca4dc..535f2c552b 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py @@ -15,6 +15,8 @@ from __future__ import annotations import logging +import os +import tempfile from pathlib import Path from typing import TYPE_CHECKING from uuid import UUID, uuid4 @@ -156,20 +158,45 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: ) # Write (append-only: refuse to overwrite existing checkpoints) + # + # Atomic write: content goes to a temp file first, then os.link() + # atomically creates the target or raises FileExistsError (TOCTOU-safe). + # This ensures checkpoints are always complete — a crash mid-write + # leaves only the temp file, which is cleaned up on the next attempt. target_dir.mkdir(parents=True, exist_ok=True) - if target_path.exists(): - raise RuntimeHostError( - f"Checkpoint already exists: {filename} — " - f"increment attempt_number to create a new checkpoint", - context=context, - ) yaml_content = yaml.dump( _serialize_checkpoint(checkpoint), default_flow_style=False, sort_keys=False, allow_unicode=True, ) - target_path.write_text(yaml_content, encoding="utf-8") + + tmp_fd = -1 + tmp_path_str = "" + try: + tmp_fd, tmp_path_str = tempfile.mkstemp( + dir=str(target_dir), suffix=".tmp", prefix=".ckpt_" + ) + os.write(tmp_fd, yaml_content.encode("utf-8")) + os.fsync(tmp_fd) + os.close(tmp_fd) + tmp_fd = -1 + + # Atomic link: fails with FileExistsError if target already exists + os.link(tmp_path_str, str(target_path)) + Path(tmp_path_str).unlink() + tmp_path_str = "" + except FileExistsError: + raise RuntimeHostError( + f"Checkpoint already exists: {filename} — " + f"increment attempt_number to create a new checkpoint", + context=context, + ) + finally: + if tmp_fd >= 0: + os.close(tmp_fd) + if tmp_path_str and Path(tmp_path_str).exists(): + Path(tmp_path_str).unlink() logger.info( "Checkpoint written: %s (phase=%s, attempt=%d)", diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/models/model_checkpoint_effect_input.py b/src/omnibase_infra/nodes/node_checkpoint_effect/models/model_checkpoint_effect_input.py index 679c53decf..9200ce549d 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/models/model_checkpoint_effect_input.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/models/model_checkpoint_effect_input.py @@ -7,6 +7,7 @@ from __future__ import annotations +from typing import Literal from uuid import UUID from pydantic import BaseModel, ConfigDict, Field @@ -24,10 +25,11 @@ class ModelCheckpointEffectInput(BaseModel): model_config = ConfigDict(frozen=True, extra="forbid", from_attributes=True) - operation: str = Field( - ..., - min_length=1, - description="Operation to perform: write_checkpoint, read_checkpoint, list_checkpoints.", + operation: Literal["write_checkpoint", "read_checkpoint", "list_checkpoints"] = ( + Field( + ..., + description="Operation to perform: write_checkpoint, read_checkpoint, list_checkpoints.", + ) ) correlation_id: UUID = Field( ..., diff --git a/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/handler_checkpoint_validate.py b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/handler_checkpoint_validate.py index d87699a5f7..01689e44e5 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/handler_checkpoint_validate.py +++ b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/handler_checkpoint_validate.py @@ -179,13 +179,9 @@ def validate( ) # 6. Timestamp not in the future (with 60s tolerance) + # Note: ModelCheckpoint._ensure_utc guarantees tzinfo is never None. now = datetime.now(UTC) - if checkpoint.timestamp_utc.tzinfo is not None: - ts = checkpoint.timestamp_utc - else: - ts = checkpoint.timestamp_utc.replace(tzinfo=UTC) - - if ts > now + timedelta(seconds=60): + if checkpoint.timestamp_utc > now + timedelta(seconds=60): warnings.append("timestamp_utc is in the future") is_valid = len(errors) == 0 From 551f29d07d8bf34d5dcd09964e467a28e9bf0b1a Mon Sep 17 00:00:00 2001 From: Jonah Date: Wed, 11 Feb 2026 14:37:13 -0500 Subject: [PATCH 5/9] fix(review): [major] exception chaining + type narrowing, [minor] SHA hex pattern + UTC validator + dedup - Chain FileExistsError via `from exc` to preserve traceback - Assert corr_id is not None in all 3 effect handlers (mypy type narrowing) - Add hex pattern constraint (^[0-9a-f]+$) to SHA fields across all payload models - Add _ensure_utc validator to ModelPhasePayloadReadyForMerge.label_applied_at - Extract _attempt_number to module level in read/list handlers (eliminate per-call overhead) - Fix test fixture using non-hex SHA values Review iteration: 2/10 --- .../model_phase_payload_create_pr.py | 1 + .../model_phase_payload_implement.py | 1 + .../model_phase_payload_local_review.py | 1 + .../model_phase_payload_pr_release_ready.py | 1 + .../model_phase_payload_ready_for_merge.py | 13 ++++++++++-- .../handlers/handler_checkpoint_list.py | 20 ++++++++++--------- .../handlers/handler_checkpoint_read.py | 20 ++++++++++--------- .../handlers/handler_checkpoint_write.py | 7 ++++--- .../test_handler_checkpoint_read.py | 2 +- 9 files changed, 42 insertions(+), 24 deletions(-) diff --git a/src/omnibase_infra/models/checkpoint/model_phase_payload_create_pr.py b/src/omnibase_infra/models/checkpoint/model_phase_payload_create_pr.py index 3001dcb551..2e25471309 100644 --- a/src/omnibase_infra/models/checkpoint/model_phase_payload_create_pr.py +++ b/src/omnibase_infra/models/checkpoint/model_phase_payload_create_pr.py @@ -37,6 +37,7 @@ class ModelPhasePayloadCreatePr(BaseModel): ..., min_length=7, max_length=40, + pattern=r"^[0-9a-f]+$", description="HEAD SHA pushed to the remote branch.", ) diff --git a/src/omnibase_infra/models/checkpoint/model_phase_payload_implement.py b/src/omnibase_infra/models/checkpoint/model_phase_payload_implement.py index 7ac57c0d10..3f2e5919b4 100644 --- a/src/omnibase_infra/models/checkpoint/model_phase_payload_implement.py +++ b/src/omnibase_infra/models/checkpoint/model_phase_payload_implement.py @@ -33,6 +33,7 @@ class ModelPhasePayloadImplement(BaseModel): ..., min_length=7, max_length=40, + pattern=r"^[0-9a-f]+$", description="HEAD commit SHA after implementation.", ) files_changed: tuple[str, ...] = Field( diff --git a/src/omnibase_infra/models/checkpoint/model_phase_payload_local_review.py b/src/omnibase_infra/models/checkpoint/model_phase_payload_local_review.py index 200c0f6a9a..fb60352a89 100644 --- a/src/omnibase_infra/models/checkpoint/model_phase_payload_local_review.py +++ b/src/omnibase_infra/models/checkpoint/model_phase_payload_local_review.py @@ -36,6 +36,7 @@ class ModelPhasePayloadLocalReview(BaseModel): ..., min_length=7, max_length=40, + pattern=r"^[0-9a-f]+$", description="Commit SHA of the last clean (review-passing) state.", ) diff --git a/src/omnibase_infra/models/checkpoint/model_phase_payload_pr_release_ready.py b/src/omnibase_infra/models/checkpoint/model_phase_payload_pr_release_ready.py index 9e1cd7200a..c358b872c0 100644 --- a/src/omnibase_infra/models/checkpoint/model_phase_payload_pr_release_ready.py +++ b/src/omnibase_infra/models/checkpoint/model_phase_payload_pr_release_ready.py @@ -27,6 +27,7 @@ class ModelPhasePayloadPrReleaseReady(BaseModel): ..., min_length=7, max_length=40, + pattern=r"^[0-9a-f]+$", description="Commit SHA of the last review pass.", ) issue_fingerprints: tuple[str, ...] = Field( diff --git a/src/omnibase_infra/models/checkpoint/model_phase_payload_ready_for_merge.py b/src/omnibase_infra/models/checkpoint/model_phase_payload_ready_for_merge.py index 89bd95fec1..0534bd18be 100644 --- a/src/omnibase_infra/models/checkpoint/model_phase_payload_ready_for_merge.py +++ b/src/omnibase_infra/models/checkpoint/model_phase_payload_ready_for_merge.py @@ -9,10 +9,10 @@ from __future__ import annotations -from datetime import datetime +from datetime import UTC, datetime from typing import Literal -from pydantic import BaseModel, ConfigDict, Field +from pydantic import BaseModel, ConfigDict, Field, field_validator class ModelPhasePayloadReadyForMerge(BaseModel): @@ -29,5 +29,14 @@ class ModelPhasePayloadReadyForMerge(BaseModel): description="UTC timestamp when the merge-ready label was applied.", ) + @field_validator("label_applied_at", mode="after") + @classmethod + def _ensure_utc(cls, v: datetime) -> datetime: + """Reject naive datetimes; normalize to UTC.""" + if v.tzinfo is None: + msg = "label_applied_at must be timezone-aware (use datetime.now(UTC))" + raise ValueError(msg) + return v.astimezone(UTC) + __all__: list[str] = ["ModelPhasePayloadReadyForMerge"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py index 875d061f6a..8ce03a4899 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py @@ -37,6 +37,15 @@ _DEFAULT_BASE_DIR = Path.home() / ".claude" / "checkpoints" +def _attempt_number(path: Path) -> int: + """Extract numeric attempt from filename like ``phase_1_implement_a3.yaml``.""" + stem = path.stem + after_a = stem.rsplit("_a", maxsplit=1) + if len(after_a) == 2 and after_a[1].isdigit(): + return int(after_a[1]) + return 0 + + class HandlerCheckpointList: """Lists all checkpoint files for a ticket and optionally a specific run.""" @@ -80,7 +89,8 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: operation="list_checkpoints", target_name="checkpoint_yaml", ) - corr_id = context.correlation_id # with_correlation() guarantees a UUID + corr_id = context.correlation_id + assert corr_id is not None # with_correlation() guarantees a UUID ticket_id = envelope.get("ticket_id") if not ticket_id: @@ -123,14 +133,6 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: else: scan_dirs = [d for d in ticket_dir.iterdir() if d.is_dir()] - def _attempt_number(path: Path) -> int: - """Extract numeric attempt from filename like phase_1_implement_a3.yaml.""" - stem = path.stem - after_a = stem.rsplit("_a", maxsplit=1) - if len(after_a) == 2 and after_a[1].isdigit(): - return int(after_a[1]) - return 0 - checkpoints: list[ModelCheckpoint] = [] for scan_dir in scan_dirs: if not scan_dir.is_dir(): diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py index a83ace2e8f..194bb3c53d 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py @@ -39,6 +39,15 @@ _DEFAULT_BASE_DIR = Path.home() / ".claude" / "checkpoints" +def _attempt_number(path: Path) -> int: + """Extract numeric attempt from filename like ``phase_1_implement_a3.yaml``.""" + stem = path.stem + after_a = stem.rsplit("_a", maxsplit=1) + if len(after_a) == 2 and after_a[1].isdigit(): + return int(after_a[1]) + return 0 + + class HandlerCheckpointRead: """Reads a checkpoint for a given ticket, run, and phase. @@ -87,7 +96,8 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: operation="read_checkpoint", target_name="checkpoint_yaml", ) - corr_id = context.correlation_id # with_correlation() guarantees a UUID + corr_id = context.correlation_id + assert corr_id is not None # with_correlation() guarantees a UUID ticket_id = envelope.get("ticket_id") run_id = envelope.get("run_id") @@ -138,14 +148,6 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: ) # Find all attempt files for this phase, sorted by attempt number - def _attempt_number(path: Path) -> int: - """Extract numeric attempt from filename like phase_1_implement_a3.yaml.""" - stem = path.stem # e.g. "phase_1_implement_a3" - after_a = stem.rsplit("_a", maxsplit=1) - if len(after_a) == 2 and after_a[1].isdigit(): - return int(after_a[1]) - return 0 - matching_files = sorted( target_dir.glob(f"{phase_prefix}*.yaml"), key=_attempt_number, diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py index 535f2c552b..32df788f44 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py @@ -105,7 +105,8 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: operation="write_checkpoint", target_name="checkpoint_yaml", ) - corr_id = context.correlation_id # with_correlation() guarantees a UUID + corr_id = context.correlation_id + assert corr_id is not None # with_correlation() guarantees a UUID raw_checkpoint = envelope.get("checkpoint") if raw_checkpoint is None: @@ -186,12 +187,12 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: os.link(tmp_path_str, str(target_path)) Path(tmp_path_str).unlink() tmp_path_str = "" - except FileExistsError: + except FileExistsError as exc: raise RuntimeHostError( f"Checkpoint already exists: {filename} — " f"increment attempt_number to create a new checkpoint", context=context, - ) + ) from exc finally: if tmp_fd >= 0: os.close(tmp_fd) diff --git a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_read.py b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_read.py index b829667b97..7f1728f7dd 100644 --- a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_read.py +++ b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_read.py @@ -118,7 +118,7 @@ async def test_read_returns_latest_attempt( "phase": "local_review", "iteration_count": attempt, "issue_fingerprints": [], - "last_clean_sha": f"sha{attempt:04d}aa", + "last_clean_sha": f"aabbcc{attempt:04d}", }, ) write_env: dict[str, object] = { From 61ae68d745dd517b28e3086fcc9d2af8e73249fc Mon Sep 17 00:00:00 2001 From: Jonah Date: Wed, 11 Feb 2026 14:42:29 -0500 Subject: [PATCH 6/9] fix(review): [minor] stable phase ordinals, symlink guard, log sanitization, dedup helper - Replace fragile list().index() with explicit _PHASE_ORDINALS mapping for stable checkpoint filenames across enum reordering - Add symlink escape guard on scan_dirs in list handler - Log relative_path instead of full absolute path in write handler - Deduplicate _attempt_number: single definition in list handler, imported by read - Fix stale "alphabetically" comment in read handler Review iteration: 3/10 --- .../enums/enum_checkpoint_phase.py | 20 ++++++++++++++++--- .../handlers/handler_checkpoint_list.py | 4 ++++ .../handlers/handler_checkpoint_read.py | 17 ++++++---------- .../handlers/handler_checkpoint_write.py | 8 ++++---- 4 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/omnibase_infra/enums/enum_checkpoint_phase.py b/src/omnibase_infra/enums/enum_checkpoint_phase.py index be2a4880c4..42db80a6a0 100644 --- a/src/omnibase_infra/enums/enum_checkpoint_phase.py +++ b/src/omnibase_infra/enums/enum_checkpoint_phase.py @@ -46,9 +46,13 @@ def __str__(self) -> str: @property def phase_number(self) -> int: - """Return the 1-based ordinal position of this phase.""" - ordered = list(EnumCheckpointPhase) - return ordered.index(self) + 1 + """Return the stable 1-based ordinal for this phase. + + These ordinals are baked into checkpoint filenames on disk + (``phase_{N}_{value}_a{attempt}.yaml``). They MUST NOT change + even if enum members are reordered or new members are inserted. + """ + return _PHASE_ORDINALS[self] @property def filename(self) -> str: @@ -56,4 +60,14 @@ def filename(self) -> str: return f"phase_{self.phase_number}_{self.value}.yaml" +# Explicit mapping — add new phases at the end with the next available ordinal. +_PHASE_ORDINALS: dict[EnumCheckpointPhase, int] = { + EnumCheckpointPhase.IMPLEMENT: 1, + EnumCheckpointPhase.LOCAL_REVIEW: 2, + EnumCheckpointPhase.CREATE_PR: 3, + EnumCheckpointPhase.PR_RELEASE_READY: 4, + EnumCheckpointPhase.READY_FOR_MERGE: 5, +} + + __all__: list[str] = ["EnumCheckpointPhase"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py index 8ce03a4899..11bd0aec17 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py @@ -137,6 +137,10 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: for scan_dir in scan_dirs: if not scan_dir.is_dir(): continue + # Guard against symlinks escaping the checkpoint root + if not scan_dir.resolve().is_relative_to(base_dir.resolve()): + logger.warning("Skipping escaped path: %s", scan_dir.name) + continue for yaml_file in sorted(scan_dir.glob("phase_*.yaml"), key=_attempt_number): try: raw_data = yaml.safe_load(yaml_file.read_text(encoding="utf-8")) diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py index 194bb3c53d..22c0dce21c 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py @@ -27,6 +27,9 @@ from omnibase_infra.enums.enum_checkpoint_phase import EnumCheckpointPhase from omnibase_infra.errors import ModelInfraErrorContext, RuntimeHostError from omnibase_infra.models.checkpoint.model_checkpoint import ModelCheckpoint +from omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_list import ( + _attempt_number, +) from omnibase_infra.nodes.node_checkpoint_effect.models.model_checkpoint_effect_output import ( ModelCheckpointEffectOutput, ) @@ -39,15 +42,6 @@ _DEFAULT_BASE_DIR = Path.home() / ".claude" / "checkpoints" -def _attempt_number(path: Path) -> int: - """Extract numeric attempt from filename like ``phase_1_implement_a3.yaml``.""" - stem = path.stem - after_a = stem.rsplit("_a", maxsplit=1) - if len(after_a) == 2 and after_a[1].isdigit(): - return int(after_a[1]) - return 0 - - class HandlerCheckpointRead: """Reads a checkpoint for a given ticket, run, and phase. @@ -126,7 +120,8 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: base_dir_raw = envelope.get("base_dir") base_dir = Path(str(base_dir_raw)) if base_dir_raw else _DEFAULT_BASE_DIR - # Scan for matching files (with path traversal guard) + # Path traversal guard: covers ticket_id AND run_id components. + # run_id is additionally constrained by UUID() coercion above. target_dir = base_dir / str(ticket_id) / str(run_id_val) if not target_dir.resolve().is_relative_to(base_dir.resolve()): raise RuntimeHostError( @@ -165,7 +160,7 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: ), ) - # Read the latest attempt (last file alphabetically = highest attempt) + # Read the latest attempt (last by numeric attempt_number sort) latest_file = matching_files[-1] try: raw_data = yaml.safe_load(latest_file.read_text(encoding="utf-8")) diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py index 32df788f44..28c6ae5abb 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py @@ -199,16 +199,16 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: if tmp_path_str and Path(tmp_path_str).exists(): Path(tmp_path_str).unlink() + # Return relative path from base_dir + relative_path = str(target_path.relative_to(base_dir)) + logger.info( "Checkpoint written: %s (phase=%s, attempt=%d)", - target_path, + relative_path, checkpoint.phase.value, checkpoint.attempt_number, ) - # Return relative path from base_dir - relative_path = str(target_path.relative_to(base_dir)) - result = ModelCheckpointEffectOutput( success=True, correlation_id=corr_id, From e396966ec0055d444ebb1d035bd7f31bba65fa8f Mon Sep 17 00:00:00 2001 From: Jonah Date: Wed, 11 Feb 2026 15:14:23 -0500 Subject: [PATCH 7/9] =?UTF-8?q?fix(review):=20[major]=20extract=20shared?= =?UTF-8?q?=20util,=20model-level=20SHA=20+=20path=20validators,=20[minor]?= =?UTF-8?q?=20assert=E2=86=92raise,=20uuid4=20fallback,=20glob=20precision?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Major: - Extract _attempt_number to shared util_checkpoint_sorting.py (was cross-module private import) - Add field_validator on repo_commit_map to enforce hex SHA format at model level - Add field_validator on artifact_paths to reject absolute paths and traversal at model level Minor: - Replace bare assert with explicit RuntimeError raise in 3 effect handlers - Add os.link() same-filesystem constraint comment - Document attempt_number return-0 behavior for non-matching files - Use uuid4() fallback instead of zero-UUID in validate handler - Tighten glob pattern to [0-9]* for attempt-number precision - Update tests to use model_construct() for intentionally-invalid data --- .../models/checkpoint/model_checkpoint.py | 31 +++++++++++++ .../handlers/handler_checkpoint_list.py | 17 +++----- .../handlers/handler_checkpoint_read.py | 11 ++--- .../handlers/handler_checkpoint_write.py | 6 ++- .../handlers/util_checkpoint_sorting.py | 30 +++++++++++++ .../handlers/handler_checkpoint_validate.py | 4 +- .../test_handler_checkpoint_write.py | 4 +- .../test_handler_checkpoint_validate.py | 43 +++++++++++++++++-- 8 files changed, 120 insertions(+), 26 deletions(-) create mode 100644 src/omnibase_infra/nodes/node_checkpoint_effect/handlers/util_checkpoint_sorting.py diff --git a/src/omnibase_infra/models/checkpoint/model_checkpoint.py b/src/omnibase_infra/models/checkpoint/model_checkpoint.py index e889cc577b..916466a78e 100644 --- a/src/omnibase_infra/models/checkpoint/model_checkpoint.py +++ b/src/omnibase_infra/models/checkpoint/model_checkpoint.py @@ -16,6 +16,7 @@ from __future__ import annotations +import re from datetime import UTC, datetime from typing import Annotated, Union from uuid import UUID @@ -62,6 +63,8 @@ CHECKPOINT_SCHEMA_VERSION = "1.0.0" """Current schema version for checkpoint files.""" +_HEX_SHA_RE = re.compile(r"^[0-9a-f]+$") + class ModelCheckpoint(BaseModel): """Immutable record of a completed pipeline phase. @@ -110,10 +113,38 @@ def _ensure_utc(cls, v: datetime) -> datetime: default_factory=dict, description="Mapping of relative repo path to commit SHA.", ) + + @field_validator("repo_commit_map", mode="after") + @classmethod + def _validate_commit_shas(cls, v: dict[str, str]) -> dict[str, str]: + """Ensure every value in repo_commit_map is a valid lowercase hex SHA.""" + for repo_path, sha in v.items(): + if not _HEX_SHA_RE.match(sha): + msg = ( + f"repo_commit_map[{repo_path!r}] value {sha!r} is not a " + f"valid hex commit SHA (expected pattern ^[0-9a-f]+$)" + ) + raise ValueError(msg) + return v + artifact_paths: tuple[str, ...] = Field( default_factory=tuple, description="Relative filesystem paths serving as resume evidence.", ) + + @field_validator("artifact_paths", mode="after") + @classmethod + def _validate_artifact_paths(cls, v: tuple[str, ...]) -> tuple[str, ...]: + """Reject absolute paths and path-traversal segments in artifact_paths.""" + for p in v: + if p.startswith("/"): + msg = f"Absolute artifact path forbidden: {p!r}" + raise ValueError(msg) + if ".." in p.split("/"): + msg = f"Path traversal segment '..' forbidden in artifact path: {p!r}" + raise ValueError(msg) + return v + attempt_number: int = Field( default=1, ge=1, diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py index 11bd0aec17..179cd3a1e6 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_list.py @@ -25,6 +25,9 @@ ) from omnibase_infra.errors import ModelInfraErrorContext, RuntimeHostError from omnibase_infra.models.checkpoint.model_checkpoint import ModelCheckpoint +from omnibase_infra.nodes.node_checkpoint_effect.handlers.util_checkpoint_sorting import ( + attempt_number, +) from omnibase_infra.nodes.node_checkpoint_effect.models.model_checkpoint_effect_output import ( ModelCheckpointEffectOutput, ) @@ -37,15 +40,6 @@ _DEFAULT_BASE_DIR = Path.home() / ".claude" / "checkpoints" -def _attempt_number(path: Path) -> int: - """Extract numeric attempt from filename like ``phase_1_implement_a3.yaml``.""" - stem = path.stem - after_a = stem.rsplit("_a", maxsplit=1) - if len(after_a) == 2 and after_a[1].isdigit(): - return int(after_a[1]) - return 0 - - class HandlerCheckpointList: """Lists all checkpoint files for a ticket and optionally a specific run.""" @@ -90,7 +84,8 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: target_name="checkpoint_yaml", ) corr_id = context.correlation_id - assert corr_id is not None # with_correlation() guarantees a UUID + if corr_id is None: + raise RuntimeError("correlation_id must not be None") ticket_id = envelope.get("ticket_id") if not ticket_id: @@ -141,7 +136,7 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: if not scan_dir.resolve().is_relative_to(base_dir.resolve()): logger.warning("Skipping escaped path: %s", scan_dir.name) continue - for yaml_file in sorted(scan_dir.glob("phase_*.yaml"), key=_attempt_number): + for yaml_file in sorted(scan_dir.glob("phase_*.yaml"), key=attempt_number): try: raw_data = yaml.safe_load(yaml_file.read_text(encoding="utf-8")) if not isinstance(raw_data, dict): diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py index 22c0dce21c..ae3ea79414 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_read.py @@ -27,8 +27,8 @@ from omnibase_infra.enums.enum_checkpoint_phase import EnumCheckpointPhase from omnibase_infra.errors import ModelInfraErrorContext, RuntimeHostError from omnibase_infra.models.checkpoint.model_checkpoint import ModelCheckpoint -from omnibase_infra.nodes.node_checkpoint_effect.handlers.handler_checkpoint_list import ( - _attempt_number, +from omnibase_infra.nodes.node_checkpoint_effect.handlers.util_checkpoint_sorting import ( + attempt_number, ) from omnibase_infra.nodes.node_checkpoint_effect.models.model_checkpoint_effect_output import ( ModelCheckpointEffectOutput, @@ -91,7 +91,8 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: target_name="checkpoint_yaml", ) corr_id = context.correlation_id - assert corr_id is not None # with_correlation() guarantees a UUID + if corr_id is None: + raise RuntimeError("correlation_id must not be None") ticket_id = envelope.get("ticket_id") run_id = envelope.get("run_id") @@ -144,8 +145,8 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: # Find all attempt files for this phase, sorted by attempt number matching_files = sorted( - target_dir.glob(f"{phase_prefix}*.yaml"), - key=_attempt_number, + target_dir.glob(f"{phase_prefix}[0-9]*.yaml"), + key=attempt_number, ) if not matching_files: diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py index 28c6ae5abb..542fec4626 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/handler_checkpoint_write.py @@ -106,7 +106,8 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: target_name="checkpoint_yaml", ) corr_id = context.correlation_id - assert corr_id is not None # with_correlation() guarantees a UUID + if corr_id is None: + raise RuntimeError("correlation_id must not be None") raw_checkpoint = envelope.get("checkpoint") if raw_checkpoint is None: @@ -183,7 +184,8 @@ async def execute(self, envelope: dict[str, object]) -> ModelHandlerOutput: os.close(tmp_fd) tmp_fd = -1 - # Atomic link: fails with FileExistsError if target already exists + # Atomic link: fails with FileExistsError if target already exists. + # Hard link requires same filesystem; mkstemp(dir=target_dir) ensures this. os.link(tmp_path_str, str(target_path)) Path(tmp_path_str).unlink() tmp_path_str = "" diff --git a/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/util_checkpoint_sorting.py b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/util_checkpoint_sorting.py new file mode 100644 index 0000000000..67719a0311 --- /dev/null +++ b/src/omnibase_infra/nodes/node_checkpoint_effect/handlers/util_checkpoint_sorting.py @@ -0,0 +1,30 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 OmniNode Team +"""Shared sorting utilities for checkpoint file discovery. + +Extracted from handler_checkpoint_list to avoid cross-module imports of +private helpers. + +Ticket: OMN-2143 +""" + +from __future__ import annotations + +from pathlib import Path + + +def attempt_number(path: Path) -> int: + """Extract numeric attempt from filename like ``phase_1_implement_a3.yaml``. + + Returns ``0`` when no ``_a`` suffix is found. This is intentional: + returning 0 ensures non-attempt-suffixed files sort before any numbered + attempt, which is the desired ordering for checkpoint discovery. + """ + stem = path.stem + after_a = stem.rsplit("_a", maxsplit=1) + if len(after_a) == 2 and after_a[1].isdigit(): + return int(after_a[1]) + return 0 + + +__all__: list[str] = ["attempt_number"] diff --git a/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/handler_checkpoint_validate.py b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/handler_checkpoint_validate.py index 01689e44e5..0e1af7ab6c 100644 --- a/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/handler_checkpoint_validate.py +++ b/src/omnibase_infra/nodes/node_checkpoint_validate_compute/handlers/handler_checkpoint_validate.py @@ -79,9 +79,7 @@ async def execute( """ correlation_id_raw = envelope.get("correlation_id") corr_id = ( - correlation_id_raw - if isinstance(correlation_id_raw, UUID) - else UUID("00000000-0000-0000-0000-000000000000") + correlation_id_raw if isinstance(correlation_id_raw, UUID) else uuid4() ) input_envelope_id = uuid4() diff --git a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_write.py b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_write.py index eee1ec447b..c7477747bc 100644 --- a/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_write.py +++ b/tests/unit/nodes/test_node_checkpoint_effect/test_handler_checkpoint_write.py @@ -129,13 +129,15 @@ async def test_write_rejects_absolute_artifact_paths( """Write rejects checkpoints with absolute artifact paths.""" await handler.initialize({}) - cp = ModelCheckpoint( + cp = ModelCheckpoint.model_construct( run_id=uuid4(), ticket_id="OMN-2143", phase=EnumCheckpointPhase.IMPLEMENT, timestamp_utc=datetime.now(UTC), artifact_paths=("/absolute/path/bad.py",), attempt_number=1, + schema_version="1.0.0", + repo_commit_map={}, phase_payload=ModelPhasePayloadImplement( branch_name="branch", commit_sha="abc1234", diff --git a/tests/unit/nodes/test_node_checkpoint_validate_compute/test_handler_checkpoint_validate.py b/tests/unit/nodes/test_node_checkpoint_validate_compute/test_handler_checkpoint_validate.py index ae28025f32..f07d7e5382 100644 --- a/tests/unit/nodes/test_node_checkpoint_validate_compute/test_handler_checkpoint_validate.py +++ b/tests/unit/nodes/test_node_checkpoint_validate_compute/test_handler_checkpoint_validate.py @@ -55,6 +55,26 @@ def _valid_checkpoint(**overrides: object) -> ModelCheckpoint: return ModelCheckpoint(**defaults) # type: ignore[arg-type] +def _invalid_checkpoint(**overrides: object) -> ModelCheckpoint: + """Create a checkpoint bypassing model validators for handler-level testing.""" + defaults: dict[str, object] = { + "run_id": uuid4(), + "ticket_id": "OMN-2143", + "phase": EnumCheckpointPhase.IMPLEMENT, + "timestamp_utc": datetime.now(UTC), + "repo_commit_map": {"omnibase_infra": "abc1234"}, + "artifact_paths": ("src/foo.py",), + "attempt_number": 1, + "schema_version": "1.0.0", + "phase_payload": ModelPhasePayloadImplement( + branch_name="feature-branch", + commit_sha="abc1234", + ), + } + defaults.update(overrides) + return ModelCheckpoint.model_construct(**defaults) # type: ignore[arg-type] + + class TestHandlerCheckpointValidate: """Tests for HandlerCheckpointValidate.""" @@ -82,7 +102,7 @@ async def test_absolute_artifact_path_is_error( """Absolute paths in artifact_paths produce an error.""" await handler.initialize({}) - cp = _valid_checkpoint(artifact_paths=("/absolute/path.py",)) + cp = _invalid_checkpoint(artifact_paths=("/absolute/path.py",)) env: dict[str, object] = { "checkpoint": cp, @@ -98,10 +118,25 @@ async def test_invalid_commit_sha_is_error( self, handler: HandlerCheckpointValidate, ) -> None: - """Non-hex commit SHA produces an error.""" + """Non-hex commit SHA produces an error. + + Uses ``model_construct`` to bypass Pydantic field validation so the + handler's own SHA check is exercised. The model-level + ``_validate_commit_shas`` validator would reject this value at + construction time. + """ await handler.initialize({}) - cp = _valid_checkpoint(repo_commit_map={"repo": "not-a-sha!!"}) + # Build a valid checkpoint, then reconstruct with bad repo_commit_map. + # We must preserve the original field objects (not serialized dicts) + # so that model_construct produces a valid-looking instance. + base = _valid_checkpoint() + fields = { + field_name: getattr(base, field_name) + for field_name in ModelCheckpoint.model_fields + } + fields["repo_commit_map"] = {"repo": "not-a-sha!!"} + cp = ModelCheckpoint.model_construct(**fields) env: dict[str, object] = { "checkpoint": cp, @@ -214,7 +249,7 @@ async def test_bool_reflects_validity( assert bool(valid_result.result) is True invalid_env: dict[str, object] = { - "checkpoint": _valid_checkpoint(artifact_paths=("/abs/path",)), + "checkpoint": _invalid_checkpoint(artifact_paths=("/abs/path",)), "correlation_id": uuid4(), } invalid_result = await handler.execute(invalid_env) From e0e9bb5267432db7e05b223f1f17fd7e69214956 Mon Sep 17 00:00:00 2001 From: Jonah Date: Wed, 11 Feb 2026 15:20:29 -0500 Subject: [PATCH 8/9] fix(review): [minor] fix mismatch test to actually test mismatch, remove unused filename property - test_phase_payload_mismatch_is_error now creates a real mismatch (header=IMPLEMENT, payload.phase=local_review) and asserts Phase mismatch error - Remove unused EnumCheckpointPhase.filename property (misleading, no attempt suffix) --- .../enums/enum_checkpoint_phase.py | 5 ----- .../test_handler_checkpoint_validate.py | 22 +++++++++++-------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/omnibase_infra/enums/enum_checkpoint_phase.py b/src/omnibase_infra/enums/enum_checkpoint_phase.py index 42db80a6a0..2aa3829688 100644 --- a/src/omnibase_infra/enums/enum_checkpoint_phase.py +++ b/src/omnibase_infra/enums/enum_checkpoint_phase.py @@ -54,11 +54,6 @@ def phase_number(self) -> int: """ return _PHASE_ORDINALS[self] - @property - def filename(self) -> str: - """Return the canonical checkpoint filename for this phase.""" - return f"phase_{self.phase_number}_{self.value}.yaml" - # Explicit mapping — add new phases at the end with the next available ordinal. _PHASE_ORDINALS: dict[EnumCheckpointPhase, int] = { diff --git a/tests/unit/nodes/test_node_checkpoint_validate_compute/test_handler_checkpoint_validate.py b/tests/unit/nodes/test_node_checkpoint_validate_compute/test_handler_checkpoint_validate.py index f07d7e5382..9e43effa2b 100644 --- a/tests/unit/nodes/test_node_checkpoint_validate_compute/test_handler_checkpoint_validate.py +++ b/tests/unit/nodes/test_node_checkpoint_validate_compute/test_handler_checkpoint_validate.py @@ -155,22 +155,26 @@ async def test_phase_payload_mismatch_is_error( """Phase payload phase != header phase produces an error.""" await handler.initialize({}) - # Create a checkpoint where header says LOCAL_REVIEW but payload is IMPLEMENT - cp = _valid_checkpoint( - phase=EnumCheckpointPhase.LOCAL_REVIEW, - phase_payload=ModelPhasePayloadLocalReview( - iteration_count=1, - last_clean_sha="abc1234", - ), + # Header says IMPLEMENT, but payload says "local_review" + mismatched_payload = ModelPhasePayloadLocalReview.model_construct( + phase="local_review", + iteration_count=1, + last_clean_sha="abc1234", + ) + cp = _invalid_checkpoint( + phase=EnumCheckpointPhase.IMPLEMENT, + phase_payload=mismatched_payload, ) - # This should be valid — payload matches header env: dict[str, object] = { "checkpoint": cp, "correlation_id": uuid4(), } result = await handler.execute(env) - assert result.result.is_valid is True + output = result.result + + assert output.is_valid is False + assert any("Phase mismatch" in e for e in output.errors) async def test_schema_version_mismatch_is_warning( self, From 20625bd455a7309bfb7fa3d813ae85de3a1f1f3f Mon Sep 17 00:00:00 2001 From: Jonah Date: Wed, 11 Feb 2026 16:04:12 -0500 Subject: [PATCH 9/9] =?UTF-8?q?fix(OMN-2143):=20sync=20test=20assertion=20?= =?UTF-8?q?with=20merge-resolved=20union=20count=20(121=E2=86=92123)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/unit/validation/test_validator_defaults.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/unit/validation/test_validator_defaults.py b/tests/unit/validation/test_validator_defaults.py index 5ad6016387..80f7ffa37b 100644 --- a/tests/unit/validation/test_validator_defaults.py +++ b/tests/unit/validation/test_validator_defaults.py @@ -107,11 +107,14 @@ def test_infra_max_unions_constant(self) -> None: - ModelRunContext.metadata: dict[str, Any] union typing - 121 (2026-02-11): OMN-2117 atomic read-modify-write handler (+1 union) - Callable[[ModelSessionIndex], ModelSessionIndex] transform parameter + - 123 (2026-02-11): OMN-2143 checkpoint nodes (+2 unions) + - PhasePayload: 5-type discriminated Union in ModelCheckpoint + - register_handler(): Write | Read | List handler type param - Current: 121 (as of OMN-2117). Target: Keep below 150 - if this grows, consider typed patterns from omnibase_core. + Current: 123 (as of OMN-2143). Target: Keep below 150 - if this grows, consider typed patterns from omnibase_core. """ - assert INFRA_MAX_UNIONS == 121, ( - "INFRA_MAX_UNIONS should be 121 (non-optional unions only, X | None excluded)" + assert INFRA_MAX_UNIONS == 123, ( + "INFRA_MAX_UNIONS should be 123 (non-optional unions only, X | None excluded)" ) def test_infra_max_violations_constant(self) -> None: