diff --git a/CHANGELOG.md b/CHANGELOG.md index 16c9c365..0a96e2a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- DeepSchema PostToolUse hook (`deepschema_write`) no longer reports `File is not valid JSON` for YAML files whose name has no extension (e.g. `.deepreview`). The hook now parses target files and the referenced JSON Schema as YAML, which is a superset of JSON, so both formats are accepted regardless of file extension. DW-REQ-011.7.3 updated to match. (Mirrors the fix shipped in #338 for the workflow quality gate.) + ### Removed ## [0.13.1] - 2026-04-06 diff --git a/specs/deepwork/DW-REQ-011-deepschema.md b/specs/deepwork/DW-REQ-011-deepschema.md index 3bcfd00d..79bd7781 100644 --- a/specs/deepwork/DW-REQ-011-deepschema.md +++ b/specs/deepwork/DW-REQ-011-deepschema.md @@ -50,7 +50,7 @@ The DeepSchema system provides rich, file-level schemas with automatic validatio 1. The write hook MUST fire on PostToolUse events for Write and Edit tools. 2. For each applicable schema, the hook MUST inject a conformance note: "Note: this file must conform to the DeepSchema at ``". -3. If `json_schema_path` is set, the hook MUST validate the written file against the JSON Schema, parsing YAML files (`.yml`/`.yaml`) as YAML before validation. +3. If `json_schema_path` is set, the hook MUST validate the written file against the JSON Schema. The file content MUST be parsed as YAML (which is a superset of JSON), so both YAML and JSON formats are accepted regardless of file extension. 4. If `verification_bash_command` is set, the hook MUST execute each command with the file path as `$1`, with a 30-second timeout. 5. Validation failures MUST be reported via `hookSpecificOutput.additionalContext` so the agent can act on them. 6. The hook MUST NOT use `systemMessage` for validation output — that route is user-visible only. diff --git a/src/deepwork/hooks/deepschema_write.py b/src/deepwork/hooks/deepschema_write.py index 3044052d..24fc18c9 100644 --- a/src/deepwork/hooks/deepschema_write.py +++ b/src/deepwork/hooks/deepschema_write.py @@ -7,11 +7,13 @@ from __future__ import annotations -import json import os import subprocess import sys from pathlib import Path +from typing import Any, cast + +import yaml from deepwork.hooks.wrapper import ( HookInput, @@ -114,34 +116,38 @@ def _relative_path(path: Path, project_root: Path) -> str: def _validate_json_schema(filepath: Path, schema_path: Path) -> str | None: """Validate a file against a JSON Schema. - Parses the file as YAML if it has a .yml/.yaml extension, otherwise as JSON. - Returns error message or None on success. + Parses the file as YAML, which is a superset of JSON, so both YAML and + JSON formats are accepted regardless of file extension. Returns error + message or None on success. """ if not schema_path.exists(): return f"JSON Schema file not found: {schema_path}" try: - content = filepath.read_text(encoding="utf-8") - if filepath.suffix in (".yml", ".yaml"): - import yaml - - parsed = yaml.safe_load(content) - else: - parsed = json.loads(content) - except json.JSONDecodeError as e: - return f"File is not valid JSON: {e}" - except Exception as e: + parsed = yaml.safe_load(filepath.read_text(encoding="utf-8")) + except (yaml.YAMLError, UnicodeDecodeError) as e: return f"Cannot parse file: {e}" try: - schema_data = json.loads(schema_path.read_text(encoding="utf-8")) - except (json.JSONDecodeError, OSError) as e: + schema_data = yaml.safe_load(schema_path.read_text(encoding="utf-8")) + except (yaml.YAMLError, UnicodeDecodeError, OSError) as e: return f"Cannot read JSON Schema: {e}" + # JSON Schema must be an object or a boolean (per JSON Schema spec). + # Anything else (e.g., a bare string from yaml.safe_load on free-form + # text) would crash the validator with a SchemaError. + if not isinstance(schema_data, (dict, bool)): + return ( + f"Cannot read JSON Schema: not a JSON Schema object (got {type(schema_data).__name__})" + ) + try: from deepwork.utils.validation import ValidationError, validate_against_schema - validate_against_schema(parsed, schema_data) + # validate_against_schema's signature only types dict, but the + # underlying jsonschema.validate also accepts bool schemas per spec + # (true = accept all, false = reject all). Cast to satisfy mypy. + validate_against_schema(parsed, cast("dict[str, Any]", schema_data)) except ValidationError as e: return f"JSON Schema validation failed: {e}" diff --git a/tests/unit/deepschema/test_write_hook.py b/tests/unit/deepschema/test_write_hook.py index fc23a074..4b8383d2 100644 --- a/tests/unit/deepschema/test_write_hook.py +++ b/tests/unit/deepschema/test_write_hook.py @@ -24,6 +24,32 @@ def _make_hook_input( ) +def _write_name_required_schema(tmp_path: Path, slug: str, matcher: str) -> None: + """Create a named DeepSchema under `.deepwork/schemas//` whose + `json_schema_path` requires a top-level `name` string. Used by the + _validate_json_schema exercise tests below, which all share the same + schema shape and differ only in the matcher glob. + """ + schema_dir = tmp_path / ".deepwork" / "schemas" / slug + schema_dir.mkdir(parents=True) + (schema_dir / "test.schema.json").write_text( + json.dumps( + { + "type": "object", + "required": ["name"], + "properties": {"name": {"type": "string"}}, + } + ), + encoding="utf-8", + ) + (schema_dir / "deepschema.yml").write_text( + f"matchers:\n - '{matcher}'\n" + "json_schema_path: 'test.schema.json'\n" + "requirements:\n r1: 'MUST conform'\n", + encoding="utf-8", + ) + + class TestDeepschemaWriteHook: def test_no_schemas_returns_empty(self, tmp_path: Path) -> None: # THIS TEST VALIDATES A HARD REQUIREMENT (DW-REQ-011.7.1). @@ -70,25 +96,7 @@ def test_anonymous_schema_conformance_note(self, tmp_path: Path) -> None: def test_json_schema_validation_failure(self, tmp_path: Path) -> None: # THIS TEST VALIDATES A HARD REQUIREMENT (DW-REQ-011.7.3, DW-REQ-011.7.5). # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES - # Create a JSON Schema - json_schema = tmp_path / ".deepwork" / "schemas" / "json_test" / "test.schema.json" - json_schema.parent.mkdir(parents=True) - json_schema.write_text( - json.dumps( - { - "type": "object", - "required": ["name"], - "properties": {"name": {"type": "string"}}, - } - ), - encoding="utf-8", - ) - # Create schema referencing the JSON Schema - (json_schema.parent / "deepschema.yml").write_text( - "matchers:\n - '**/*.json'\njson_schema_path: 'test.schema.json'\nrequirements:\n r1: 'MUST conform'\n", - encoding="utf-8", - ) - # Write invalid JSON + _write_name_required_schema(tmp_path, slug="json_test", matcher="**/*.json") target = tmp_path / "data.json" target.write_text('{"other": "field"}', encoding="utf-8") @@ -100,22 +108,7 @@ def test_json_schema_validation_failure(self, tmp_path: Path) -> None: def test_json_schema_validation_success(self, tmp_path: Path) -> None: # THIS TEST VALIDATES A HARD REQUIREMENT (DW-REQ-011.7.3). # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES - json_schema = tmp_path / ".deepwork" / "schemas" / "json_test" / "test.schema.json" - json_schema.parent.mkdir(parents=True) - json_schema.write_text( - json.dumps( - { - "type": "object", - "required": ["name"], - "properties": {"name": {"type": "string"}}, - } - ), - encoding="utf-8", - ) - (json_schema.parent / "deepschema.yml").write_text( - "matchers:\n - '**/*.json'\njson_schema_path: 'test.schema.json'\nrequirements:\n r1: 'MUST conform'\n", - encoding="utf-8", - ) + _write_name_required_schema(tmp_path, slug="json_test", matcher="**/*.json") target = tmp_path / "data.json" target.write_text('{"name": "valid"}', encoding="utf-8") @@ -127,23 +120,8 @@ def test_json_schema_validation_success(self, tmp_path: Path) -> None: def test_json_schema_validation_of_yaml_file(self, tmp_path: Path) -> None: # THIS TEST VALIDATES A HARD REQUIREMENT (DW-REQ-011.7.3). # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES - """YAML files validated against a JSON Schema should be parsed as YAML, not JSON.""" - json_schema = tmp_path / ".deepwork" / "schemas" / "yml_test" / "test.schema.json" - json_schema.parent.mkdir(parents=True) - json_schema.write_text( - json.dumps( - { - "type": "object", - "required": ["name"], - "properties": {"name": {"type": "string"}}, - } - ), - encoding="utf-8", - ) - (json_schema.parent / "deepschema.yml").write_text( - "matchers:\n - '**/*.yml'\njson_schema_path: 'test.schema.json'\nrequirements:\n r1: 'MUST conform'\n", - encoding="utf-8", - ) + # YAML files validated against a JSON Schema should be parsed as YAML, not JSON. + _write_name_required_schema(tmp_path, slug="yml_test", matcher="**/*.yml") target = tmp_path / "data.yml" target.write_text("name: valid\nother: field\n", encoding="utf-8") @@ -152,26 +130,34 @@ def test_json_schema_validation_of_yaml_file(self, tmp_path: Path) -> None: assert "must conform to the DeepSchema" in result.context assert "CRITICAL" not in result.context + def test_json_schema_validation_of_yaml_file_with_no_extension(self, tmp_path: Path) -> None: + # THIS TEST VALIDATES A HARD REQUIREMENT (DW-REQ-011.7.3). + # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES + # Files like `.deepreview` whose `Path.suffix` is empty (the leading + # dot is treated as a hidden-file marker, not an extension separator) + # MUST still be parsed as YAML — not as JSON. Regression for the bug + # where `.deepreview` files reported `File is not valid JSON: + # Expecting value: line 1 column 1 (char 0)`. + _write_name_required_schema(tmp_path, slug="dotfile_test", matcher="**/.myappconfig") + target = tmp_path / ".myappconfig" + target.write_text("name: valid\n", encoding="utf-8") + # Sanity check: this is the case the old extension-based dispatch + # missed — Path.suffix is empty for any dot-prefixed filename, so + # the old code routed `.myappconfig`, `.deepreview`, etc. to + # json.loads() and produced false-positive parse errors. + assert target.suffix == "" + + hook_input = _make_hook_input(str(target), str(tmp_path)) + result = deepschema_write_hook(hook_input) + assert "must conform to the DeepSchema" in result.context + assert "CRITICAL" not in result.context + assert "File is not valid JSON" not in result.context + def test_json_schema_validation_of_invalid_yaml_file(self, tmp_path: Path) -> None: # THIS TEST VALIDATES A HARD REQUIREMENT (DW-REQ-011.7.3, DW-REQ-011.7.5). # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES - """YAML files that fail JSON Schema validation should report errors.""" - json_schema = tmp_path / ".deepwork" / "schemas" / "yml_test" / "test.schema.json" - json_schema.parent.mkdir(parents=True) - json_schema.write_text( - json.dumps( - { - "type": "object", - "required": ["name"], - "properties": {"name": {"type": "string"}}, - } - ), - encoding="utf-8", - ) - (json_schema.parent / "deepschema.yml").write_text( - "matchers:\n - '**/*.yml'\njson_schema_path: 'test.schema.json'\nrequirements:\n r1: 'MUST conform'\n", - encoding="utf-8", - ) + # YAML files that fail JSON Schema validation should report errors. + _write_name_required_schema(tmp_path, slug="yml_test", matcher="**/*.yml") target = tmp_path / "data.yml" target.write_text("other: field\n", encoding="utf-8") diff --git a/tests/unit/test_deepschema_write_hook.py b/tests/unit/test_deepschema_write_hook.py index f3f0b2a8..8ff85e09 100644 --- a/tests/unit/test_deepschema_write_hook.py +++ b/tests/unit/test_deepschema_write_hook.py @@ -246,15 +246,20 @@ def test_schema_file_not_found(self, tmp_path: Path) -> None: assert result is not None assert "not found" in result - def test_invalid_json_file(self, tmp_path: Path) -> None: - """Returns an error string when the data file is not valid JSON.""" - data_file = tmp_path / "data.json" - data_file.write_text("not json") + def test_invalid_yaml_file(self, tmp_path: Path) -> None: + """Returns an error string when the data file cannot be parsed as YAML. + + Files are now parsed via yaml.safe_load (a JSON superset), so the + previous "not json" content is actually valid YAML (a string scalar). + Use a syntactically broken construct that YAML cannot parse. + """ + data_file = tmp_path / "data.yml" + data_file.write_text("key: [unclosed\n") schema_file = tmp_path / "schema.json" schema_file.write_text("{}") result = _validate_json_schema(data_file, schema_file) assert result is not None - assert "not valid JSON" in result + assert "Cannot parse file" in result def test_generic_parse_error(self, tmp_path: Path) -> None: """Returns a 'Cannot parse file' error when reading the data file raises a decode error.""" @@ -269,8 +274,15 @@ def test_generic_parse_error(self, tmp_path: Path) -> None: assert result is not None assert "Cannot parse file" in result - def test_invalid_schema_file_json(self, tmp_path: Path) -> None: - """Returns a 'Cannot read JSON Schema' error when the schema file is not valid JSON.""" + def test_invalid_schema_file_not_an_object(self, tmp_path: Path) -> None: + """Returns a 'Cannot read JSON Schema' error when the schema file + parses but is not a JSON Schema (i.e., not an object or boolean). + + Schema files are now parsed via yaml.safe_load, which turns + "not json schema" into the bare string "not json schema". That is + not a valid JSON Schema, so we reject it before invoking the + validator (which would otherwise raise SchemaError). + """ data_file = tmp_path / "data.json" data_file.write_text('{"key": "value"}') schema_file = tmp_path / "schema.json" @@ -278,6 +290,18 @@ def test_invalid_schema_file_json(self, tmp_path: Path) -> None: result = _validate_json_schema(data_file, schema_file) assert result is not None assert "Cannot read JSON Schema" in result + assert "not a JSON Schema object" in result + + def test_invalid_schema_file_unparseable(self, tmp_path: Path) -> None: + """Returns a 'Cannot read JSON Schema' error when the schema file + cannot be parsed at all (broken YAML/JSON).""" + data_file = tmp_path / "data.json" + data_file.write_text('{"key": "value"}') + schema_file = tmp_path / "schema.json" + schema_file.write_text("key: [unclosed\n") + result = _validate_json_schema(data_file, schema_file) + assert result is not None + assert "Cannot read JSON Schema" in result def test_schema_file_oserror(self, tmp_path: Path) -> None: """Returns a 'Cannot read JSON Schema' error when an OSError occurs reading the schema file."""