feat(OMN-2117): Canonical state nodes — session state effect + lifecycle FSM reducer

[jonahgabriel]

Summary

• Implement node_session_state_effect (EFFECT node) with 5 handlers for filesystem I/O on ~/.claude/state/
• Implement node_session_lifecycle_reducer (REDUCER node) with pure FSM: idle → run_created → run_active → run_ended → idle
• Add EnumSessionLifecycleState enum and 4 Pydantic models: ModelSessionIndex, ModelRunContext, ModelSessionLifecycleState, ModelSessionStateResult
• 49 new unit tests covering all handlers, models, FSM transitions, concurrent pipeline isolation, and stale run GC

Key Design Decisions

• Atomic writes: write-tmp → fsync → rename (POSIX guarantees)
• Concurrent safety: flock on session.json, no lock needed for per-run documents (single-writer)
• Stale run GC: 4hr TTL, also cleans malformed files
• run_id is str, not UUID: Used in filesystem paths (runs/{run_id}.json) and for human readability

Test plan

• 49 new unit tests all passing
• Full unit test suite (11,068 tests) passing
• All 18 pre-commit hooks passing (including ONEX pattern, naming, architecture, union, any-type, circular import, declarative node validators)
• ruff check clean
• mypy clean

Closes OMN-2117

Summary by CodeRabbit

• New Features

  • Session lifecycle FSM, enum and immutable models for idle → created → active → ended; session-state effect node, registries and factories to wire nodes.

• Reliability

  • Atomic, crash‑safe, concurrency‑safe read/write with path‑validation; configurable GC for stale runs; validation threshold raised.

• Tests

  • Extensive unit tests for models, handlers, lifecycle transitions, concurrency and end‑to‑end scenarios.

• Chores

  • Validation exemptions added for run_id fields.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a session lifecycle enum and reducer plus a contract-driven session-state filesystem effect node: new immutable models, handlers for atomic I/O, locking and GC, DI registries, contracts, validation exemptions and INFRA_MAX_UNIONS bump, with extensive unit tests.

Changes

Cohort / File(s)	Summary
Enums src/omnibase_infra/enums/enum_session_lifecycle_state.py, src/omnibase_infra/enums/__init__.py	Add EnumSessionLifecycleState and export it from the enums package.
Session Lifecycle Reducer src/omnibase_infra/nodes/node_session_lifecycle_reducer/... src/omnibase_infra/nodes/node_session_lifecycle_reducer/node.py, .../models/*, .../registry/*, .../contract.yaml, .../__init__.py	New reducer package: ModelSessionLifecycleState (immutable FSM), NodeSessionLifecycleReducer, registry, and declarative contract describing states, transitions, and intent emissions.
Session State Effect Node & Registry src/omnibase_infra/nodes/node_session_state_effect/node.py, .../registry/*, .../__init__.py, .../contract.yaml	New effect node for filesystem session state (~/.claude/state/), registry factory, and contract metadata.
Session State Handlers src/omnibase_infra/nodes/node_session_state_effect/handlers/*	Add handlers: HandlerSessionIndexRead/Write, HandlerRunContextRead/Write, HandlerStaleRunGC — atomic write patterns, flock locking (POSIX), path-traversal defenses, parsing/validation, and GC logic.
Session State Models src/omnibase_infra/nodes/node_session_state_effect/models/*	Add ModelRunContext, ModelSessionIndex, ModelSessionStateResult, RUN_ID_PATTERN, validate_run_id — frozen Pydantic models with validators, pure-transition helpers, and staleness checks.
Package Exports src/omnibase_infra/nodes/__init__.py, various __init__.py files	Re-export new enums, models, reducers, effects, handlers, and registries at package level for public API surface.
Validation updates src/omnibase_infra/validation/infra_validators.py, src/omnibase_infra/validation/validation_exemptions.yaml	Bump INFRA_MAX_UNIONS 119→121 and add validation exemptions for run_id fields (explicitly non-UUID human-readable IDs).
Tests tests/unit/nodes/node_session_lifecycle_reducer/*, tests/unit/nodes/node_session_state_effect/*, tests/unit/validation/*	Extensive unit tests covering FSM transitions, model invariants, handler I/O behavior (atomicity, concurrency), path-traversal defenses, stale-run GC, and updated validator expectations.

Sequence Diagram(s)

sequenceDiagram
    participant Caller as Pipeline Caller
    participant Reducer as NodeSessionLifecycleReducer
    participant State as ModelSessionLifecycleState

    Caller->>Reducer: create_run(run_id, event_id)
    Reducer->>State: can_create_run()
    State-->>Reducer: allowed/denied
    alt allowed
        Reducer->>State: with_run_created(run_id, event_id)
        State-->>Reducer: new state RUN_CREATED
        Reducer-->>Caller: updated state
    else denied
        Reducer-->>Caller: error
    end

    Caller->>Reducer: activate_run(event_id)
    Reducer->>State: can_activate_run()
    State-->>Reducer: allowed/denied
    alt allowed
        Reducer->>State: with_run_activated(event_id)
        State-->>Reducer: new state RUN_ACTIVE
        Reducer-->>Caller: updated state
    else denied
        Reducer-->>Caller: error
    end

Loading

sequenceDiagram
    participant Caller as Pipeline Caller
    participant Effect as NodeSessionStateEffect
    participant Handler as HandlerSessionIndexRead
    participant FS as Filesystem (~/.claude/state/)

    Caller->>Effect: session.index.read(correlation_id)
    Effect->>Handler: handle(correlation_id)
    Handler->>FS: stat/read session.json
    alt file exists
        FS-->>Handler: file contents
        Handler->>Handler: parse & validate -> ModelSessionIndex
        Handler-->>Effect: ModelSessionIndex + success
    else missing
        Handler-->>Effect: default ModelSessionIndex + success
    end
    Effect-->>Caller: ModelSessionStateResult

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~55 minutes

Possibly related PRs

• fix(ci): resolve 49 test failures on main branch #77 — Modifies the same INFRA_MAX_UNIONS constant.
• feat(OMN-1653): Implement contract registry reducer with Postgres pro… #212 — Similar validation bump and exemptions for session-state nodes.
• feat(services): add capability-based node discovery service [OMN-1135] #139 — Adds new enum exports to enums/__init__.py, overlapping enum export changes.

Poem

🐇 I hopped through enums, state, and file,

I fenced each path and wrote with temp-and-while.
I locked and fsync'd so nothing would fall,
GC swept the crumbs and answered the call.
Hooray — FSM and state now snug in my hall!

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the two main additions: session state effect node and lifecycle FSM reducer, with explicit reference to the ticket OMN-2117.
Docstring Coverage	✅ Passed	Docstring coverage is 96.12% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch jonah/omn-2117-omnibase_infra-p1-canonical-state-nodes-session-state-effect

---

No actionable comments were generated in the recent review. 🎉

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 10

🤖 Fix all issues with AI agents

In
`@src/omnibase_infra/nodes/node_session_state_effect/handlers/handler_run_context_read.py`:
- Around line 24-34: The HandlerRunContextRead class is missing the required
handler_type and handler_category properties; import EnumHandlerType and
EnumHandlerTypeCategory and add two `@property` methods on HandlerRunContextRead
named handler_type and handler_category returning the appropriate enums (follow
the same pattern as other handlers such as handler_ledger_projection.py and
handler_postgres_cleanup_topics.py); typically implement handler_type to return
EnumHandlerType.READ and handler_category to return the corresponding category
used for session/state handlers (e.g., EnumHandlerTypeCategory.STATE or the
category used by other read handlers in the codebase).

In
`@src/omnibase_infra/nodes/node_session_state_effect/handlers/handler_run_context_write.py`:
- Around line 27-32: The class HandlerRunContextWrite is missing the mandatory
handler_type and handler_category properties required by the handler contract;
add two `@property` methods on HandlerRunContextWrite named handler_type(self) ->
EnumHandlerType and handler_category(self) -> EnumHandlerTypeCategory that
return the appropriate EnumHandlerType and EnumHandlerTypeCategory values for
this handler (use the enum members that represent run-context write behavior in
your codebase), so registration/routing can discover this handler.
- Around line 94-101: In the OSError except block in handler_run_context_write,
don't embed raw exception text into the returned ModelSessionStateResult.error
or logs; instead import and call the sanitization utility from
omnibase_infra.utils.util_error_sanitization (e.g., sanitize_error_message) on
str(e) and use the sanitized string when populating
ModelSessionStateResult(error=...) and the logger.warning message (keep
context.run_id and correlation_id as before). Ensure you replace direct uses of
e with the sanitized message so no filesystem paths or sensitive data are
returned or logged.
- Around line 56-69: The code uses context.run_id directly to build filesystem
names (run_path and the tempfile prefix in handler_run_context_write), which
allows path traversal; validate or sanitize run_id before using it by enforcing
a strict safe pattern (e.g., UUID or slug regex like ^[A-Za-z0-9_-]+$) or
reject/raise if it fails, and only then construct run_path = runs_dir /
f"{safe_run_id}.json" and use safe_run_id in tempfile.mkstemp prefix;
alternatively, if you must accept arbitrary IDs, hex-encode or base64-url-safe
encode context.run_id into a filesystem-safe token and use that token for both
run_path and tmp file prefixes (apply this check/sanitation in the handler where
run_path and tempfile.mkstemp are created).

In
`@src/omnibase_infra/nodes/node_session_state_effect/handlers/handler_session_index_read.py`:
- Around line 24-29: Add the required handler properties to the
HandlerSessionIndexRead class: implement a handler_type property that returns
EnumHandlerType.READ and a handler_category property that returns
EnumHandlerTypeCategory.STATE (import both EnumHandlerType and
EnumHandlerTypeCategory if not already imported); ensure the properties are
simple `@property` methods on HandlerSessionIndexRead so the handler contract can
register/route the class correctly.
- Around line 80-102: The exception handlers in handler_session_index_read.py
currently embed raw exception text (variable e) into logger messages and the
ModelSessionStateResult.error field; sanitize these strings by calling
omnibase_infra.utils.util_error_sanitization on str(e) (or the error message)
before using them. Specifically update the except blocks catching
(json.JSONDecodeError, ValueError) and OSError: call
util_error_sanitization(e_msg) to produce a safe_error and use safe_error in
logger.warning and in the ModelSessionStateResult(error=...), leaving other
fields (success, operation, correlation_id, error_code, files_affected)
unchanged.

In
`@src/omnibase_infra/nodes/node_session_state_effect/handlers/handler_session_index_write.py`:
- Around line 32-38: HandlerSessionIndexWrite is missing the required
handler_type and handler_category properties required by the handler contract;
add two read-only properties on the class named handler_type and
handler_category that return the appropriate EnumHandlerType and
EnumHandlerTypeCategory values (import them if necessary) so the class exposes
its type and category for registration/routing; ensure the properties are
implemented as simple getters on HandlerSessionIndexWrite and return the correct
enum members consistent with other handlers.
- Around line 109-116: The exception handler in the session index write path
currently embeds the raw OSError text into the ModelSessionStateResult.error,
potentially leaking sensitive paths; update the except OSError as e block in
handler_session_index_write to pass e through the sanitizer in
omnibase_infra.utils.util_error_sanitization (e.g. call the provided sanitize
function) and use the sanitized string when constructing
ModelSessionStateResult(error=...), leaving error_code and other fields
unchanged; ensure you import the sanitizer and apply it to the error message
before populating the error field (you may keep the existing logger.warning but
do not return the raw e).

In
`@src/omnibase_infra/nodes/node_session_state_effect/handlers/handler_stale_run_gc.py`:
- Around line 28-50: The HandlerStaleRunGC class is missing the required
handler_type and handler_category properties used by the handler
registration/routing system; add two read-only `@property` methods on
HandlerStaleRunGC: handler_type() -> EnumHandlerType and handler_category() ->
EnumHandlerTypeCategory that return the correct enum members for this
garbage-collect stale-run handler (choose the appropriate EnumHandlerType and
EnumHandlerTypeCategory values used elsewhere in the codebase), and import
EnumHandlerType and EnumHandlerTypeCategory if not already imported so the class
exposes these properties for the handler contract.

In `@tests/unit/nodes/node_session_lifecycle_reducer/test_state_transitions.py`:
- Around line 8-17: Add a module-level pytest marker so the file is classified
as a unit test: declare pytestmark = pytest.mark.unit at module scope (after the
existing imports) in tests referencing EnumSessionLifecycleState and
ModelSessionLifecycleState; this uses the existing pytest import and ensures all
tests in the module are marked as unit tests.

🧹 Nitpick comments (6)

tests/unit/nodes/node_session_state_effect/test_handlers.py (2)

16-16: Unused import: timezone.

The timezone import is not used anywhere in this file; only UTC is used.

🧹 Proposed fix

-from datetime import UTC, datetime, timezone
+from datetime import UTC, datetime

---

49-50: Consider adding @pytest.mark.unit marker to test classes.

Per coding guidelines, test files should use pytest markers for classification. Adding @pytest.mark.unit to test classes ensures proper test categorization and filtering.

🏷️ Proposed fix example

+@pytest.mark.unit
 class TestHandlerSessionIndexRead:
     """Tests for HandlerSessionIndexRead."""

Apply similarly to other test classes: TestHandlerSessionIndexWrite, TestHandlerRunContextRead, TestHandlerRunContextWrite, TestHandlerStaleRunGC, TestConcurrentPipelineIsolation.

As per coding guidelines: "Use pytest markers: @pytest.mark.unit, @pytest.mark.integration, @pytest.mark.slow, @pytest.mark.chaos, @pytest.mark.serial for test classification".

src/omnibase_infra/nodes/node_session_state_effect/models/model_session_index.py (1)

18-18: Unused import: timezone.

The timezone import is not used; only UTC is referenced throughout the file.

🧹 Proposed cleanup

-from datetime import UTC, datetime, timezone
+from datetime import UTC, datetime

src/omnibase_infra/nodes/node_session_state_effect/models/model_run_context.py (2)

12-12: Unused import: timezone.

The timezone import is not used; only UTC is referenced.

🧹 Proposed cleanup

-from datetime import UTC, datetime, timezone
+from datetime import UTC, datetime

---

49-52: Consider immutability implications for metadata field.

The model is frozen, but dict[str, object] is inherently mutable. While Pydantic prevents field reassignment, external code could still mutate the dict's contents if a reference is held. The with_metadata method correctly creates a shallow copy, but the copied dict could still contain mutable values.

For strict immutability, consider using MappingProxyType for read-only access or documenting that callers must not mutate the dict. Given that the transition helpers follow the pure pattern, the current approach is acceptable for pragmatic use.

As per coding guidelines: "use tuple for immutable collections in frozen models."

src/omnibase_infra/nodes/node_session_state_effect/contract.yaml (1)

32-39: Input model may not accurately represent all operations.

The input_model is declared as ModelSessionIndex, but this node handles multiple operations with different input requirements:

• session.index.read requires no input
• session.run.read requires run_id
• session.gc requires no input (or optional TTL)

Consider whether the contract schema supports operation-specific input models, or add a clarifying comment that the actual input varies per operation and is documented in io_operations.

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In
`@src/omnibase_infra/nodes/node_session_lifecycle_reducer/models/model_session_lifecycle_state.py`:
- Around line 89-160: Replace plain ValueError raises in the lifecycle
transition methods (e.g., with_run_created, with_run_activated, with_run_ended,
with_reset) with exceptions created via
ModelInfraErrorContext.with_correlation(...). Use the incoming correlation_id if
available or uuid4() when missing, pass transport_type and operation (use a
clear transport_type like "internal" or the node name and operation equal to the
method name, e.g., "with_run_activated"), and ensure the message is sanitized
via omnibase_infra.utils.util_error_sanitization (sanitize_message) before
putting it into the error context; then raise/return the context-created
exception so the node-layer receives correlation-aware, sanitized errors.
- Around line 167-211: The can_activate_run and can_end_run predicates currently
only check status and should also guard that a valid run_id exists; update
ModelSessionLifecycleState.can_activate_run and can_end_run to return True only
when status == EnumSessionLifecycleState.RUN_CREATED (or RUN_ACTIVE
respectively) AND self.run_id is not None (or use a more strict validation if
run_id must be a UUID), so malformed states with run_id=None cannot transition
to active/ended.

In
`@src/omnibase_infra/nodes/node_session_state_effect/handlers/handler_run_context_read.py`:
- Around line 115-139: Import the sanitizer from
omnibase_infra.utils.util_error_sanitization (e.g., sanitize_error_message) and
use it in the exception handlers in handler_run_context_read.py: call sanitized
= sanitize_error_message(e) and replace uses of the raw exception text in
logger.warning and the ModelSessionStateResult.error fields (the blocks that
create ModelSessionStateResult on JSONDecodeError/ValueError and on OSError) to
include sanitized instead of f"{e}"; keep run_id and correlation_id as-is.

🧹 Nitpick comments (7)

tests/unit/nodes/node_session_state_effect/test_models.py (3)

23-106: Missing @pytest.mark.unit marker on test class.

Per coding guidelines, test files should use pytest markers (@pytest.mark.unit, @pytest.mark.integration, etc.) for test classification. Add @pytest.mark.unit to TestModelSessionIndex and other test classes in this file.

🔧 Suggested fix

+@pytest.mark.unit
 class TestModelSessionIndex:
     """Tests for ModelSessionIndex."""

---

113-170: Test class TestModelRunContext also needs @pytest.mark.unit marker.

Same issue as above - add the pytest marker for proper test classification.

---

178-203: Test class TestModelSessionStateResult needs @pytest.mark.unit marker.

Additionally, the uuid4 import on lines 183 and 194 could be moved to the module-level imports for consistency.

🔧 Suggested fix for imports

 from datetime import UTC, datetime
+from uuid import uuid4
 
 import pytest

Then remove the inline from uuid import uuid4 statements inside the test methods.

tests/unit/nodes/node_session_state_effect/test_handlers.py (2)

49-86: Missing @pytest.mark.unit marker on test classes.

All test classes in this file (TestHandlerSessionIndexRead, TestHandlerSessionIndexWrite, etc.) should have @pytest.mark.unit decorator per coding guidelines for test classification.

🔧 Suggested fix example

+@pytest.mark.unit
 class TestHandlerSessionIndexRead:
     """Tests for HandlerSessionIndexRead."""

---

133-148: Good concurrent safety test, but consider adding assertion for data integrity.

The test verifies that JSON is valid after concurrent writes, but doesn't assert that the final state is consistent (one complete write won). Consider adding an assertion that recent_run_ids has exactly one entry matching one of the written values.

🔧 Optional enhancement

         # File should be valid JSON (one of the writes won)
         data = json.loads((state_dir / "session.json").read_text())
         assert "recent_run_ids" in data
+        # Verify the final state has exactly one run from one writer
+        assert len(data["recent_run_ids"]) == 1
+        assert data["recent_run_ids"][0] in [f"run-{i}" for i in range(10)]

src/omnibase_infra/nodes/node_session_state_effect/handlers/handler_stale_run_gc.py (1)

120-128: Consider logging the file age at debug level for non-stale files.

Currently, only stale files get logged. For debugging GC behavior, it might be helpful to log at debug level when files are checked but not deleted.

src/omnibase_infra/nodes/node_session_state_effect/models/model_run_context.py (1)

55-61: Consider documenting the defensive copy behavior for metadata.

The with_status method creates a defensive copy of metadata ({**self.metadata}), which is good practice. However, external code could still mutate the dict after construction if they hold a reference to the original. Since the model is frozen, this is a minor concern, but you might consider using MappingProxyType or documenting this limitation.

Also applies to: 104-110

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In
`@src/omnibase_infra/nodes/node_session_state_effect/handlers/handler_session_index_write.py`:
- Around line 66-68: Replace the raw RuntimeError in the
HandlerSessionIndexWrite constructor with a raise that uses
ModelInfraErrorContext.with_correlation(...): call
ModelInfraErrorContext.with_correlation(transport_type=<appropriate transport>,
operation=<appropriate operation>, correlation_id=<generate new id if none>) to
build the context and pass it into the exception so the RuntimeError includes
the required context; specifically update the branch where fcntl is None to
raise using ModelInfraErrorContext.with_correlation and include transport_type
and operation values and an autogenerated correlation id when not provided.

In `@tests/unit/nodes/node_session_state_effect/test_handlers.py`:
- Around line 20-34: This test module is missing the pytest unit marker; add a
module-level pytestmark to classify the tests as unit tests by importing pytest
(if not already) and defining pytestmark = pytest.mark.unit near the top of
tests/unit/nodes/node_session_state_effect/test_handlers.py so the
handlers/models tests (HandlerRunContextRead, HandlerRunContextWrite,
HandlerSessionIndexRead, HandlerSessionIndexWrite, HandlerStaleRunGC,
ModelRunContext, ModelSessionIndex) are marked as unit tests.

In `@tests/unit/nodes/node_session_state_effect/test_models.py`:
- Around line 7-16: The test module is missing a unit test marker; add a
module-level pytest mark by defining pytestmark = pytest.mark.unit at the top of
the file (import pytest if not present) so this test file is classified as unit
tests—update tests/unit/nodes/node_session_state_effect/test_models.py to
include the pytest import and set pytestmark for the module.

🧹 Nitpick comments (1)

src/omnibase_infra/validation/infra_validators.py (1)

455-456: Documentation gap: missing history entry for 121st union.

The comment on line 455 documents the 120th union (ModelRunContext.metadata), but INFRA_MAX_UNIONS is set to 121. The test file (test_validator_defaults.py) documents both additions:

• 120: ModelRunContext.metadata
• 121: Callable[[ModelSessionIndex], ModelSessionIndex] transform parameter

Consider adding a second history note for completeness:

📝 Suggested documentation fix

 # Note: OMN-2117 ModelRunContext.metadata uses StrictJsonPrimitive from omnibase_core (120th)
+# Note: OMN-2117 SessionIndexRead/Write Callable transform parameter (121st)
 INFRA_MAX_UNIONS = 121

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@src/omnibase_infra/nodes/node_session_state_effect/contract.yaml`:
- Around line 31-40: The contract's input_model currently names
ModelSessionIndex but handlers accept varied input shapes (e.g., run_id: str,
ModelRunContext, RMW transform); update the contract to match actual handler
signatures by either (a) defining a union/envelope type (e.g.,
ModelSessionStateInput) that includes ModelSessionIndex, run_id,
ModelRunContext, and transform payloads and set input_model to that union, or
(b) add per-operation input_model entries mapping each operation to its correct
model and remove the TODO; reference the existing input_model declaration and
the handler types (ModelSessionIndex, ModelRunContext, run_id, RMW transform)
when making the change so validation and consumer docs align.

In
`@src/omnibase_infra/nodes/node_session_state_effect/handlers/handler_run_context_read.py`:
- Around line 137-191: The code currently treats a FileNotFoundError from
run_path.read_text() as a generic OSError and returns failure, but the contract
for run_context_read requires missing files to return success=True and
files_affected=0; modify the exception handling around the read/parse block in
handler_run_context_read so you explicitly catch FileNotFoundError (from
run_path.read_text) and return (None, ModelSessionStateResult(success=True,
operation="run_context_read", correlation_id=correlation_id, files_affected=0));
keep the existing JSON/ValidationError handling for parse errors and leave the
broader OSError handler for other I/O errors (ensure FileNotFoundError is
handled before the OSError except block).