Skip to content

Commit 1d376c9

Browse files
fix(states): reject unimplemented storage phases at config refresh
An accepted-but-unimplemented phase previously surfaced only on first use: request-time 500s from the API and a crash inside Temporal worker wiring. Fail at process startup instead, with the misconfigured variable named in the error. The selector keeps its NotImplementedError as defense in depth for configs constructed outside refresh().
1 parent d28a45c commit 1d376c9

4 files changed

Lines changed: 76 additions & 12 deletions

File tree

agentex/src/config/environment_variables.py

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,10 @@ class Environment(str, Enum):
8080
class StoragePhase(str, Enum):
8181
"""Which backend serves a document store (task state, task messages).
8282
83-
A future data-migration effort would define its own additional phases;
84-
new values are additive, not breaking.
83+
POSTGRES becomes selectable per store once that store's Postgres
84+
repository lands; until then refresh() rejects it at startup. A future
85+
data-migration effort would define its own additional phases; new values
86+
are additive, not breaking.
8587
"""
8688

8789
MONGODB = "mongodb"
@@ -115,6 +117,30 @@ def _parse_bool_env(key: EnvVarKeys, default: bool) -> bool:
115117
)
116118

117119

120+
def _validate_storage_phases(environment_variables: EnvironmentVariables) -> None:
121+
"""
122+
The Postgres storage path lands store-by-store; until a store's repository
123+
exists, selecting its phase must fail here — at process startup, in the
124+
API and Temporal workers alike — rather than on first use, where it would
125+
surface as request-time 500s or a crash inside worker wiring.
126+
"""
127+
for key, phase in (
128+
(
129+
EnvVarKeys.TASK_STATE_STORAGE_PHASE,
130+
environment_variables.TASK_STATE_STORAGE_PHASE,
131+
),
132+
(
133+
EnvVarKeys.TASK_MESSAGE_STORAGE_PHASE,
134+
environment_variables.TASK_MESSAGE_STORAGE_PHASE,
135+
),
136+
):
137+
if phase is not StoragePhase.MONGODB:
138+
raise ValueError(
139+
f"{key.value}={phase.value!r} is not supported yet; "
140+
"only 'mongodb' is currently available"
141+
)
142+
143+
118144
class EnvironmentVariables(BaseModel):
119145
ENVIRONMENT: str | None = Environment.DEV
120146
OPENAI_API_KEY: str | None
@@ -318,6 +344,7 @@ def refresh(cls, force_refresh: bool = False) -> EnvironmentVariables | None:
318344
EnvVarKeys.TASK_MESSAGE_STORAGE_PHASE, StoragePhase.MONGODB
319345
),
320346
)
347+
_validate_storage_phases(environment_variables)
321348
refreshed_environment_variables = environment_variables
322349
return refreshed_environment_variables
323350

agentex/src/domain/repositories/task_state_repository.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,8 @@ def get_task_state_repository() -> TaskStateRepositoryProtocol:
120120
phase = EnvironmentVariables.refresh().TASK_STATE_STORAGE_PHASE
121121
if phase == StoragePhase.MONGODB:
122122
return TaskStateRepository(GlobalDependencies().mongodb_database)
123+
# Unreachable through env config (refresh() rejects unimplemented phases
124+
# at startup); defense in depth for configs constructed another way.
123125
raise NotImplementedError(
124126
f"TASK_STATE_STORAGE_PHASE={phase.value!r} is not implemented yet; "
125127
"only 'mongodb' is currently supported."

agentex/tests/unit/config/test_storage_phase_env.py

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@ def test_storage_phases_default_to_mongodb(monkeypatch):
2424

2525
@pytest.mark.unit
2626
def test_storage_phases_parse_from_environment(monkeypatch):
27-
monkeypatch.setenv("TASK_STATE_STORAGE_PHASE", "postgres")
27+
monkeypatch.setenv("TASK_STATE_STORAGE_PHASE", "mongodb")
2828
monkeypatch.setenv("TASK_MESSAGE_STORAGE_PHASE", "mongodb")
2929

3030
env = EnvironmentVariables.refresh(force_refresh=True)
3131

32-
assert env.TASK_STATE_STORAGE_PHASE == StoragePhase.POSTGRES
32+
assert env.TASK_STATE_STORAGE_PHASE == StoragePhase.MONGODB
3333
assert env.TASK_MESSAGE_STORAGE_PHASE == StoragePhase.MONGODB
3434

3535

@@ -47,6 +47,20 @@ def test_storage_phase_rejects_unknown_values(monkeypatch, raw):
4747
EnvironmentVariables.refresh(force_refresh=True)
4848

4949

50+
@pytest.mark.unit
51+
@pytest.mark.parametrize(
52+
"key", ["TASK_STATE_STORAGE_PHASE", "TASK_MESSAGE_STORAGE_PHASE"]
53+
)
54+
def test_unimplemented_phase_is_rejected_at_refresh(monkeypatch, key):
55+
"""postgres is a valid phase value but has no repository yet: the config
56+
refresh (process startup, API and Temporal workers alike) must fail with
57+
the variable named — not the first state request or worker wiring."""
58+
monkeypatch.setenv(key, "postgres")
59+
60+
with pytest.raises(ValueError, match=key):
61+
EnvironmentVariables.refresh(force_refresh=True)
62+
63+
5064
@pytest.mark.unit
5165
@pytest.mark.parametrize(
5266
("state_phase", "message_phase", "expected"),
@@ -60,9 +74,17 @@ def test_storage_phase_rejects_unknown_values(monkeypatch, raw):
6074
def test_mongodb_required_unless_every_phase_is_postgres(
6175
monkeypatch, state_phase, message_phase, expected
6276
):
63-
monkeypatch.setenv("TASK_STATE_STORAGE_PHASE", state_phase)
64-
monkeypatch.setenv("TASK_MESSAGE_STORAGE_PHASE", message_phase)
65-
77+
monkeypatch.delenv("TASK_STATE_STORAGE_PHASE", raising=False)
78+
monkeypatch.delenv("TASK_MESSAGE_STORAGE_PHASE", raising=False)
6679
env = EnvironmentVariables.refresh(force_refresh=True)
6780

68-
assert env.mongodb_required is expected
81+
# Unimplemented phases cannot come from the environment (refresh rejects
82+
# them at startup), so pin the derived property on updated copies.
83+
patched = env.model_copy(
84+
update={
85+
"TASK_STATE_STORAGE_PHASE": StoragePhase(state_phase),
86+
"TASK_MESSAGE_STORAGE_PHASE": StoragePhase(message_phase),
87+
}
88+
)
89+
90+
assert patched.mongodb_required is expected

agentex/tests/unit/repositories/test_task_state_repository_selector.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
from unittest.mock import MagicMock
44

55
import pytest
6-
from src.config.environment_variables import EnvironmentVariables
6+
from src.config import environment_variables as environment_variables_module
7+
from src.config.environment_variables import EnvironmentVariables, StoragePhase
78
from src.domain.repositories import task_state_repository as selector_module
89
from src.domain.repositories.task_state_repository import (
910
DTaskStateRepository,
@@ -27,6 +28,19 @@ def _set_phase(monkeypatch, phase: str | None):
2728
EnvironmentVariables.refresh(force_refresh=True)
2829

2930

31+
def _force_cached_phase(monkeypatch, phase: StoragePhase):
32+
"""Unimplemented phases are rejected by refresh() itself (the startup
33+
guard), so exercise the selector's own defense-in-depth branch by patching
34+
the cached config directly."""
35+
monkeypatch.delenv("TASK_STATE_STORAGE_PHASE", raising=False)
36+
env = EnvironmentVariables.refresh(force_refresh=True)
37+
monkeypatch.setattr(
38+
environment_variables_module,
39+
"refreshed_environment_variables",
40+
env.model_copy(update={"TASK_STATE_STORAGE_PHASE": phase}),
41+
)
42+
43+
3044
@pytest.mark.unit
3145
@pytest.mark.parametrize("phase", [None, "mongodb"])
3246
def test_selector_returns_mongo_repository_for_mongodb_phase(monkeypatch, phase):
@@ -48,14 +62,13 @@ def test_selector_returns_mongo_repository_for_mongodb_phase(monkeypatch, phase)
4862
def test_selector_rejects_unimplemented_phases_lazily(monkeypatch):
4963
"""Unimplemented phases fail loud, and without touching any Mongo wiring:
5064
the mongodb branch must be the only one that needs a Mongo handle."""
51-
phase = "postgres"
52-
_set_phase(monkeypatch, phase)
65+
_force_cached_phase(monkeypatch, StoragePhase.POSTGRES)
5366
global_dependencies = MagicMock(
5467
side_effect=AssertionError("selector must not touch Mongo wiring")
5568
)
5669
monkeypatch.setattr(selector_module, "GlobalDependencies", global_dependencies)
5770

58-
with pytest.raises(NotImplementedError, match=phase):
71+
with pytest.raises(NotImplementedError, match="postgres"):
5972
get_task_state_repository()
6073

6174
global_dependencies.assert_not_called()

0 commit comments

Comments
 (0)