From 696a45c3c0437d4f9592314ae634403c608a8adf Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 14 Aug 2026 14:42:58 +0700 Subject: [PATCH] fix: bound recursive authoring schema traversal --- .../task-1-report.md | 33 +++++++++++++++++++ src/wf_api/authoring_contracts.py | 22 +++++++++++++ tests/wf_api/test_authoring_contracts.py | 22 +++++++++++++ 3 files changed, 77 insertions(+) diff --git a/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-1-report.md b/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-1-report.md index c658ab57..2a883401 100644 --- a/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-1-report.md +++ b/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-1-report.md @@ -58,3 +58,36 @@ uv run ruff format --check src/wf_api/authoring_contracts.py src/wf_api/models/a None for the Task 1 scope. Runtime-context analysis and persisted workspace or capability loading remain intentionally deferred to Tasks 2 and 3. + +## Round 1 Fix + +The review identified that recursive schema walking was not bounded even +though individual `$ref` chains were bounded. A self-referential local +definition repeatedly expanded `_append_schema_options` until +`RecursionError`. + +Added the regression test +`test_schema_path_options_stops_expanding_recursive_local_definition` before +changing production code. The RED run failed with `RecursionError` in +`_append_schema_options` after repeatedly traversing the self-reference. + +The fix tracks active local `$ref` definitions per traversal branch. A repeated +definition is emitted as a selectable path but is not expanded again. Distinct +active references are also capped using the existing +`_MAX_LOCAL_SCHEMA_REFERENCE_DEPTH` limit from `schema_projection`. + +Round 1 verification: + +```text +uv run pytest tests/wf_api/test_authoring_contracts.py tests/wf_api/test_schema_projection.py -q +49 passed + +uv run basedpyright --level error src/wf_api/authoring_contracts.py src/wf_api/models/authoring_contracts.py +0 errors, 0 warnings, 0 notes + +uv run ruff check src/wf_api/authoring_contracts.py src/wf_api/models/authoring_contracts.py src/wf_api/models/__init__.py tests/wf_api/test_authoring_contracts.py +All checks passed! + +uv run ruff format --check src/wf_api/authoring_contracts.py src/wf_api/models/authoring_contracts.py src/wf_api/models/__init__.py tests/wf_api/test_authoring_contracts.py +4 files already formatted +``` diff --git a/src/wf_api/authoring_contracts.py b/src/wf_api/authoring_contracts.py index 61d7aa6e..47fd9b29 100644 --- a/src/wf_api/authoring_contracts.py +++ b/src/wf_api/authoring_contracts.py @@ -54,6 +54,7 @@ def schema_path_options( origin=origin, uses=normalized_uses, options=options, + active_references=frozenset(), ) return options @@ -127,6 +128,7 @@ def _append_schema_options( origin: AuthoringPathOrigin, uses: list[AuthoringPathUse], options: list[AuthoringPathOptionPayload], + active_references: frozenset[str], ) -> None: fragment = schema_fragment_at_location(schema, location) resolved = _resolve_local_reference(schema, fragment) @@ -164,6 +166,16 @@ def _append_schema_options( if _is_array_schema(resolved_child): continue + reference = _direct_local_reference(child_fragment) + next_active_references = active_references + if reference is not None: + # A repeated definition means this branch is recursive. Keep the + # repeated path as a selectable option, but do not expand it again. + if reference in active_references: + continue + next_active_references = active_references | {reference} + if len(next_active_references) > _MAX_LOCAL_SCHEMA_REFERENCE_DEPTH: + continue _append_schema_options( schema, location=child_location, @@ -171,6 +183,7 @@ def _append_schema_options( origin=origin, uses=uses, options=options, + active_references=next_active_references, ) @@ -229,3 +242,12 @@ def _description_for( def _is_array_schema(schema: Mapping[str, Any]) -> bool: schema_type = schema.get("type") return schema_type == "array" + + +def _direct_local_reference(schema: Mapping[str, Any]) -> str | None: + reference = schema.get("$ref") + if isinstance(reference, str) and ( + reference.startswith("#/$defs/") or reference.startswith("#/definitions/") + ): + return reference + return None diff --git a/tests/wf_api/test_authoring_contracts.py b/tests/wf_api/test_authoring_contracts.py index 814d5af6..7be42b7b 100644 --- a/tests/wf_api/test_authoring_contracts.py +++ b/tests/wf_api/test_authoring_contracts.py @@ -194,6 +194,28 @@ def test_schema_path_options_resolves_local_definition_metadata() -> None: ] +def test_schema_path_options_stops_expanding_recursive_local_definition() -> None: + options = schema_path_options( + { + "type": "object", + "properties": {"node": {"$ref": "#/$defs/Node"}}, + "$defs": { + "Node": { + "type": "object", + "properties": {"child": {"$ref": "#/$defs/Node"}}, + } + }, + }, + root="input", + uses=["step_input"], + ) + + assert [option["path"] for option in options] == [ + "input.node", + "input.node.child", + ] + + def test_schema_path_options_returns_empty_schema_for_unconstrained_property() -> None: options = schema_path_options( {