Skip to content

Commit bf411d9

Browse files
committed
feat(registry): extract _validate_module_id + raise MAX_MODULE_ID_LENGTH=192 (0.18.0)
Folded into the in-flight 0.18.0 release. All items now in CHANGELOG.md [0.18.0] - 2026-04-08. Changed: - MAX_MODULE_ID_LENGTH raised from 128 to 192 (apcore.registry.registry). Tracks PROTOCOL_SPEC §2.7 EBNF constraint #1 update. Module IDs valid before this change remain valid; only the upper bound moved. Forward- compatible relaxation: older 0.17.x/0.18.x readers will reject IDs in the 129-192 range emitted by this version. - Registry.register() and Registry.register_internal() now share a _validate_module_id() helper that runs validation in canonical order: empty → EBNF pattern → length → reserved word per-segment. The reserved-word check is the only step register_internal() skips (so sys modules can use the system.* prefix); empty/pattern/length/duplicate now apply uniformly. Aligned cross-language with apcore-typescript and apcore-rust. - register_internal() now enforces empty/pattern/length/duplicate checks. Previously bypassed every validation step. Production callers (apcore.sys_modules.*) all use canonical-shape IDs so no in-tree caller is broken; external adapters that used register_internal as a generic escape hatch should review. - Duplicate registration error message canonicalized to "Module ID '<id>' is already registered" (was "Module already exists: <id>" for register_internal). Both register() and register_internal() now emit the same message. Byte-aligned with apcore-rust and apcore-typescript. Added: - Pipeline preset builders re-exported at package root: build_standard_strategy, build_internal_strategy, build_testing_strategy, build_performance_strategy, build_minimal_strategy. These functions existed in apcore.builtin_steps but were not in apcore.__all__. Parity with apcore-typescript (buildXxxStrategy) and apcore-rust (build_xxx_strategy at crate root). - TestRegisterInternalValidation test class in tests/registry/test_registry.py (6 parity tests) + two new test_pipeline_preset_builders_* tests in tests/test_public_api.py + tests/test_public_api.py EXPECTED_NAMES updated. - tests/sys_modules/test_control.py: test_reload_module_calls_safe_unregister now also mocks register_internal since the re-registration path now enforces the duplicate-ID check (the test mocks safe_unregister so the slot is never actually cleared). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: tercel <tercel.yi@gmail.com>
1 parent a05c515 commit bf411d9

6 files changed

Lines changed: 191 additions & 19 deletions

File tree

CHANGELOG.md

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,19 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8-
## [0.18.0] - 2026-04-07
8+
## [0.18.0] - 2026-04-08
9+
10+
### Added
11+
12+
- **Pipeline preset builders re-exported at package root**`build_standard_strategy`, `build_internal_strategy`, `build_testing_strategy`, `build_performance_strategy`, `build_minimal_strategy` are now importable directly from `apcore`. These functions existed in `apcore.builtin_steps` but were not previously in `apcore.__all__`. Parity with apcore-typescript (`buildXxxStrategy`) and apcore-rust (`build_xxx_strategy` at the crate root).
13+
- **`TestRegisterInternalValidation`** test class in `tests/registry/test_registry.py` (6 parity tests covering empty rejection, pattern rejection, over-length rejection, reserved-word bypass, duplicate rejection, accept-at-max-length) plus `test_pipeline_preset_builders_*` in `tests/test_public_api.py`.
14+
15+
### Changed
16+
17+
- **`MAX_MODULE_ID_LENGTH` raised from 128 to 192** (`apcore.registry.registry`). Tracks PROTOCOL_SPEC §2.7 EBNF constraint #1 update — accommodates Java/.NET deep-namespace FQN-derived IDs while remaining filesystem-safe (`192 + len('.binding.yaml') = 205 < 255`-byte filename limit on ext4/xfs/NTFS/APFS/btrfs). Module IDs valid before this change remain valid; only the upper bound moved. **Forward-compatible relaxation:** older 0.17.x/0.18.x readers will reject IDs in the 129–192 range emitted by this version.
18+
- **`Registry.register()` and `Registry.register_internal()` now share a `_validate_module_id()` helper** that runs validation in canonical order (empty → EBNF pattern → length → reserved word per-segment). The reserved-word check is the only step `register_internal()` skips (so sys modules can use the `system.*` prefix); empty/pattern/length/duplicate now apply uniformly. Aligned cross-language with apcore-typescript and apcore-rust.
19+
- **`register_internal()` now enforces empty / pattern / length / duplicate checks.** Previously bypassed every validation step. Production callers (`apcore.sys_modules.*`) all use canonical-shape IDs so no in-tree caller is broken; external adapters that used `register_internal` as a generic escape hatch should review.
20+
- **Duplicate registration error message canonicalized** to `"Module ID '<id>' is already registered"` (was `"Module already exists: <id>"` for `register_internal`). Both `register()` and `register_internal()` now emit the same message via the shared error path. Aligned with apcore-rust and apcore-typescript byte-for-byte.
921

1022
### Removed (BREAKING)
1123

src/apcore/__init__.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,15 @@
206206
unregister_step_type,
207207
)
208208

209+
# Pipeline Preset Builders (parity with apcore-typescript and apcore-rust)
210+
from apcore.builtin_steps import (
211+
build_internal_strategy,
212+
build_minimal_strategy,
213+
build_performance_strategy,
214+
build_standard_strategy,
215+
build_testing_strategy,
216+
)
217+
209218
# Trace Context
210219
from apcore.trace_context import TraceContext, TraceParent
211220

@@ -557,6 +566,12 @@ def enable(module_id: str, reason: str = "Enabled via APCore client") -> dict[st
557566
"unregister_step_type",
558567
"registered_step_types",
559568
"build_strategy_from_config",
569+
# Pipeline Preset Builders (parity with apcore-typescript / apcore-rust)
570+
"build_standard_strategy",
571+
"build_internal_strategy",
572+
"build_testing_strategy",
573+
"build_performance_strategy",
574+
"build_minimal_strategy",
560575
# System Modules
561576
"register_sys_modules",
562577
"register_subscriber_type",

src/apcore/registry/registry.py

Lines changed: 72 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ def validate(self, module: Any) -> list[str]:
103103
...
104104

105105

106-
MAX_MODULE_ID_LENGTH = 128
106+
MAX_MODULE_ID_LENGTH = 192
107107

108108
RESERVED_WORDS = frozenset({"system", "internal", "core", "apcore", "plugin", "schema", "acl"})
109109

@@ -118,6 +118,48 @@ def validate(self, module: Any) -> list[str]:
118118
]
119119

120120

121+
def _validate_module_id(module_id: str, *, allow_reserved: bool = False) -> None:
122+
"""Validate a module ID against PROTOCOL_SPEC §2.7 in canonical order.
123+
124+
Order: empty → pattern → length → reserved (per-segment).
125+
Duplicate detection is the caller's responsibility (it requires registry
126+
state).
127+
128+
Args:
129+
module_id: Candidate module ID to validate.
130+
allow_reserved: When True, the per-segment reserved word check is
131+
skipped — used by ``Registry.register_internal`` to allow sys
132+
modules to use the ``system.*`` prefix. All other validations
133+
(empty, pattern, length) still apply.
134+
135+
Raises:
136+
InvalidInputError: On any validation failure.
137+
138+
Aligned with ``apcore-typescript._validateModuleId`` and
139+
``apcore::registry::registry::validate_module_id``.
140+
"""
141+
# 1. empty check
142+
if not module_id:
143+
raise InvalidInputError(message="module_id must be a non-empty string")
144+
145+
# 2. EBNF pattern check
146+
if not MODULE_ID_PATTERN.match(module_id):
147+
raise InvalidInputError(
148+
f"Invalid module ID: '{module_id}'. Must match pattern: "
149+
f"{MODULE_ID_PATTERN.pattern} (lowercase, digits, underscores, dots only; no hyphens)"
150+
)
151+
152+
# 3. length check
153+
if len(module_id) > MAX_MODULE_ID_LENGTH:
154+
raise InvalidInputError(f"Module ID exceeds maximum length of {MAX_MODULE_ID_LENGTH}: {len(module_id)}")
155+
156+
# 4. reserved word per-segment check (skipped for register_internal)
157+
if not allow_reserved:
158+
for segment in module_id.split("."):
159+
if segment in RESERVED_WORDS:
160+
raise InvalidInputError(f"Module ID contains reserved word: '{segment}'")
161+
162+
121163
class Registry:
122164
"""Central module registry for discovering, registering, and querying modules."""
123165

@@ -422,20 +464,16 @@ def register(
422464
metadata: Optional metadata dict (may include x-compatible-versions, x-deprecation).
423465
424466
Raises:
425-
InvalidInputError: If module_id is already registered (non-versioned).
467+
InvalidInputError: If module_id is empty, malformed, exceeds the
468+
length limit, contains a reserved word, or is already
469+
registered (non-versioned).
426470
RuntimeError: If module.on_load() fails (propagated).
427-
"""
428-
if not module_id:
429-
raise InvalidInputError(message="module_id must be a non-empty string")
430-
431-
if not MODULE_ID_PATTERN.match(module_id):
432-
raise InvalidInputError(
433-
f"Invalid module ID: '{module_id}'. Must match pattern: "
434-
f"{MODULE_ID_PATTERN.pattern} (lowercase, digits, underscores, dots only; no hyphens)"
435-
)
436471
437-
if len(module_id) > MAX_MODULE_ID_LENGTH:
438-
raise InvalidInputError(f"Module ID exceeds maximum length of {MAX_MODULE_ID_LENGTH}: {len(module_id)}")
472+
Validation order (PROTOCOL_SPEC §2.7, aligned with apcore-typescript
473+
and apcore-rust): empty → pattern → length → reserved (per-segment)
474+
→ duplicate.
475+
"""
476+
_validate_module_id(module_id, allow_reserved=False)
439477

440478
_ensure_schema_adapter(module)
441479

@@ -1009,12 +1047,31 @@ def get_module_metadata(self, module_id: str) -> dict[str, Any]:
10091047
return dict(self._module_meta.get(module_id, {}))
10101048

10111049
def register_internal(self, module_id: str, module: Any) -> None:
1012-
"""Register a module bypassing reserved word validation.
1050+
"""Register a sys/internal module that bypasses **only** the reserved
1051+
word check.
1052+
1053+
All other PROTOCOL_SPEC §2.7 validations (empty, EBNF pattern, length,
1054+
duplicate) still apply. The intended use case is registering modules
1055+
under reserved prefixes like ``system.health`` or
1056+
``system.control.toggle_feature`` from ``apcore.sys_modules``.
10131057
1014-
Used by sys modules that use the reserved 'system.' prefix.
1058+
Aligned with apcore-typescript ``Registry.registerInternal`` and
1059+
apcore-rust ``Registry::register_internal``.
1060+
1061+
Raises:
1062+
InvalidInputError: If module_id is empty, malformed, exceeds the
1063+
length limit, or is already registered.
1064+
RuntimeError: If module.on_load() fails (propagated).
10151065
"""
1066+
_validate_module_id(module_id, allow_reserved=True)
1067+
10161068
_ensure_schema_adapter(module)
10171069
with self._lock:
1070+
if module_id in self._modules:
1071+
# Aligned with apcore-typescript / apcore-rust and the canonical
1072+
# message produced by detect_id_conflicts for the public
1073+
# `register()` path.
1074+
raise InvalidInputError(message=f"Module ID '{module_id}' is already registered")
10181075
self._modules[module_id] = module
10191076
self._lowercase_map[module_id.lower()] = module_id
10201077
self._trigger_event("register", module_id, module)

tests/registry/test_registry.py

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,9 +1111,13 @@ def test_custom_validator_with_default_discover(self, tmp_path: Path) -> None:
11111111

11121112

11131113
class TestRegistryConstants:
1114-
def test_max_module_id_length_is_128(self) -> None:
1115-
"""MAX_MODULE_ID_LENGTH is 128."""
1116-
assert MAX_MODULE_ID_LENGTH == 128
1114+
def test_max_module_id_length_matches_spec(self) -> None:
1115+
"""MAX_MODULE_ID_LENGTH matches PROTOCOL_SPEC §2.7 EBNF constraint #1 (192).
1116+
1117+
Filesystem-safe: 192 + len('.binding.yaml')=13 = 205 < 255-byte filename limit.
1118+
Bumped from 128 to accommodate Java/.NET deep-namespace FQN-derived IDs.
1119+
"""
1120+
assert MAX_MODULE_ID_LENGTH == 192
11171121

11181122
def test_max_module_id_length_is_int(self) -> None:
11191123
"""MAX_MODULE_ID_LENGTH is an integer."""
@@ -1155,3 +1159,42 @@ def test_module_id_at_max_length_accepted(self) -> None:
11551159
exact_id = "a" * MAX_MODULE_ID_LENGTH
11561160
reg.register(exact_id, _ValidModule())
11571161
assert reg.has(exact_id)
1162+
1163+
1164+
# ===== register_internal — bypasses ONLY reserved word check =====
1165+
# (parity with apcore-typescript and apcore-rust)
1166+
1167+
1168+
class TestRegisterInternalValidation:
1169+
def test_register_internal_accepts_reserved_first_segment(self) -> None:
1170+
reg = Registry()
1171+
reg.register_internal("system.health", _ValidModule())
1172+
assert reg.has("system.health")
1173+
1174+
def test_register_internal_accepts_reserved_any_segment(self) -> None:
1175+
reg = Registry()
1176+
reg.register_internal("myapp.system.config", _ValidModule())
1177+
assert reg.has("myapp.system.config")
1178+
1179+
def test_register_internal_still_rejects_empty(self) -> None:
1180+
reg = Registry()
1181+
with pytest.raises(InvalidInputError, match="non-empty"):
1182+
reg.register_internal("", _ValidModule())
1183+
1184+
def test_register_internal_still_rejects_invalid_pattern(self) -> None:
1185+
reg = Registry()
1186+
for bad_id in ["INVALID-ID", "1abc", "Module", "a..b", ".leading", "trailing.", "has space"]:
1187+
with pytest.raises(InvalidInputError, match="Invalid module ID|Must match pattern"):
1188+
reg.register_internal(bad_id, _ValidModule())
1189+
1190+
def test_register_internal_still_rejects_over_length(self) -> None:
1191+
reg = Registry()
1192+
overlong = "a" * (MAX_MODULE_ID_LENGTH + 1)
1193+
with pytest.raises(InvalidInputError, match="maximum length"):
1194+
reg.register_internal(overlong, _ValidModule())
1195+
1196+
def test_register_internal_rejects_duplicate(self) -> None:
1197+
reg = Registry()
1198+
reg.register_internal("system.dup", _ValidModule())
1199+
with pytest.raises(InvalidInputError, match="already registered"):
1200+
reg.register_internal("system.dup", _ValidModule())

tests/sys_modules/test_control.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,8 +321,14 @@ def test_reload_module_calls_safe_unregister(self) -> None:
321321
mod = _make_reload_module(registry=registry)
322322

323323
new_module = _FakeModuleV2()
324+
# `register_internal` now enforces the duplicate check (aligned with
325+
# apcore-typescript / apcore-rust). Because `safe_unregister` is mocked
326+
# to return True without actually removing the slot, we must also mock
327+
# `register_internal` so the re-registration doesn't trip the new
328+
# duplicate guard. This test only cares about the safe_unregister call.
324329
with (
325330
patch.object(registry, "safe_unregister", return_value=True) as mock_unreg,
331+
patch.object(registry, "register_internal"),
326332
patch.object(mod, "_rediscover_module", return_value=new_module),
327333
):
328334
mod.execute(

tests/test_public_api.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,39 @@ def test_version_is_set(self):
311311
assert isinstance(apcore.__version__, str)
312312
assert re.match(r"^\d+\.\d+\.\d+", apcore.__version__)
313313

314+
# -- Pipeline preset builders (parity with apcore-typescript / apcore-rust) --
315+
# Regression for sync finding A-006: Python previously only exported
316+
# build_strategy_from_config; the 5 named-preset builders existed in
317+
# apcore.builtin_steps but were not re-exported to the package root.
318+
319+
def test_pipeline_preset_builders_importable_from_package_root(self):
320+
from apcore import (
321+
build_internal_strategy,
322+
build_minimal_strategy,
323+
build_performance_strategy,
324+
build_standard_strategy,
325+
build_testing_strategy,
326+
)
327+
328+
for fn in (
329+
build_standard_strategy,
330+
build_internal_strategy,
331+
build_testing_strategy,
332+
build_performance_strategy,
333+
build_minimal_strategy,
334+
):
335+
assert callable(fn)
336+
337+
def test_pipeline_preset_builders_in_all(self):
338+
for name in (
339+
"build_standard_strategy",
340+
"build_internal_strategy",
341+
"build_testing_strategy",
342+
"build_performance_strategy",
343+
"build_minimal_strategy",
344+
):
345+
assert name in apcore.__all__, f"{name} missing from apcore.__all__"
346+
314347

315348
class TestPublicAPIAll:
316349
"""Verify __all__ is comprehensive and matches actual exports."""
@@ -514,6 +547,12 @@ class TestPublicAPIAll:
514547
"unregister_step_type",
515548
"registered_step_types",
516549
"build_strategy_from_config",
550+
# Pipeline Preset Builders (parity with apcore-typescript / apcore-rust)
551+
"build_standard_strategy",
552+
"build_internal_strategy",
553+
"build_testing_strategy",
554+
"build_performance_strategy",
555+
"build_minimal_strategy",
517556
# Config Bus (0.15.0)
518557
"ConfigBindError",
519558
"ConfigEnvMapConflictError",

0 commit comments

Comments
 (0)