diff --git a/docs/information/python-api.md b/docs/information/python-api.md index 359af21..d316735 100644 --- a/docs/information/python-api.md +++ b/docs/information/python-api.md @@ -3,14 +3,14 @@ "schema": "wellmanifest.docs/document/v1", "id": "python-api", "kind": "information", - "version": 1, + "version": 2, "title": "Current Planfile Python API", "status": "proposed", "owner": "semcod/planfile", "created": "2026-09-09", - "updated": "2026-09-09", - "review_after": "2026-10-09", - "source_revision": "53754107b59a4457264632e1aa53aa8fc9491717", + "updated": "2026-09-10", + "review_after": "2026-10-10", + "source_revision": "cde0727646c006845278d4774c06718cf7e9d148", "affected_repositories": [ "semcod/planfile" ], @@ -18,7 +18,9 @@ "https://github.com/semcod/planfile/blob/53754107b59a4457264632e1aa53aa8fc9491717/planfile/__init__.py", "https://github.com/semcod/planfile/blob/53754107b59a4457264632e1aa53aa8fc9491717/planfile/ci.py", "https://github.com/semcod/planfile/blob/53754107b59a4457264632e1aa53aa8fc9491717/planfile/loaders/yaml_loader.py", - "https://github.com/semcod/planfile/blob/53754107b59a4457264632e1aa53aa8fc9491717/docs/API.md" + "https://github.com/semcod/planfile/blob/53754107b59a4457264632e1aa53aa8fc9491717/docs/API.md", + "https://github.com/semcod/planfile/blob/cde0727646c006845278d4774c06718cf7e9d148/planfile/strategy_input.py", + "https://github.com/semcod/planfile/blob/cde0727646c006845278d4774c06718cf7e9d148/tests/test_strategy_input.py" ] } --- @@ -85,6 +87,23 @@ errors. `save_strategy_yaml(strategy_or_dict, path)` writes YAML; it does not execute the strategy. `load_tasks_yaml(path)` reads task-pattern groups. The old `load_strategy` and `save_strategy` names are not loader exports. +## Ticket validation and TODO synchronization + +`validate_planfile_tickets(strategy_path, project_path)` and +`sync_todo_checkboxes_from_planfile(strategy_path, project_path)` require a +readable UTF-8 YAML mapping. An explicit empty mapping (`{}`) is valid; an empty +file or a YAML sequence is not. + +Both functions raise `ValueError` when loading fails, with one of these stable +messages: `strategy_input_unreadable`, `strategy_input_invalid_encoding`, +`strategy_input_invalid_yaml`, or `strategy_input_not_mapping`. These messages +omit YAML contents. Callers must surface the failure or request corrected input; +they must not treat it as a successful report with zero tickets. + +TODO synchronization validates input before processing execution results or +writing checkboxes, including when `enabled=True` is supplied explicitly. +Previously these input failures silently became an empty strategy. + ## CI runner ```python diff --git a/planfile/strategy_input.py b/planfile/strategy_input.py new file mode 100644 index 0000000..1f7c8b5 --- /dev/null +++ b/planfile/strategy_input.py @@ -0,0 +1,22 @@ +"""Load an untyped strategy mapping without hiding input failures.""" +from pathlib import Path +from typing import Any + +import yaml + + +def load_strategy_mapping(path: Path) -> dict[str, Any]: + """Raise a stable ValueError without embedding private YAML contents.""" + try: + text = path.read_text(encoding='utf-8') + except UnicodeError: + raise ValueError('strategy_input_invalid_encoding') from None + except OSError: + raise ValueError('strategy_input_unreadable') from None + try: + data = yaml.safe_load(text) + except yaml.YAMLError: + raise ValueError('strategy_input_invalid_yaml') from None + if not isinstance(data, dict): + raise ValueError('strategy_input_not_mapping') + return data diff --git a/planfile/ticket_validation.py b/planfile/ticket_validation.py index f8c91c1..82b739a 100644 --- a/planfile/ticket_validation.py +++ b/planfile/ticket_validation.py @@ -10,15 +10,7 @@ from pathlib import Path from typing import Any -import yaml - - -def _load_strategy(path: Path) -> dict[str, Any]: - try: - data = yaml.safe_load(path.read_text(encoding="utf-8")) - except Exception: - return {} - return data if isinstance(data, dict) else {} +from planfile.strategy_input import load_strategy_mapping as _load_strategy def _normalize_rule(value: Any) -> str: diff --git a/planfile/todo_sync.py b/planfile/todo_sync.py index 40f3f83..c6ff5c9 100644 --- a/planfile/todo_sync.py +++ b/planfile/todo_sync.py @@ -6,20 +6,12 @@ from pathlib import Path from typing import Any, Iterable -import yaml +from planfile.strategy_input import load_strategy_mapping as _load_strategy _DONE_STATUSES = {"success", "done", "completed", "already_fixed"} _CHECKBOX_RE = re.compile(r"^(?P\s*-\s*\[)(?P[ xX])(?P\]\s+)(?P.*)$") -def _load_strategy(path: Path) -> dict[str, Any]: - try: - data = yaml.safe_load(path.read_text(encoding="utf-8")) - except Exception: - return {} - return data if isinstance(data, dict) else {} - - def _status_done(status: Any) -> bool: return str(status or "").strip().lower() in _DONE_STATUSES diff --git a/project/ticket-065/README.md b/project/ticket-065/README.md new file mode 100644 index 0000000..5c9173f --- /dev/null +++ b/project/ticket-065/README.md @@ -0,0 +1,11 @@ +# Ticket 065: Reject invalid strategy input + +Status: IN_PROGRESS / EDIT + +Issue: https://github.com/semcod/planfile/issues/65 +Doctor evidence: subactor/doctor-agent#381 (PLF-13741) and #382 (PLF-13742). +SESSION_EXECUTION_AUTHORIZATION: repair, test, push and protected merge, continued on 2026-09-10. + +Scope: reject unreadable, malformed or non-mapping strategy input in validation and TODO synchronization. Valid mappings retain current behavior. Failed loading must not report empty success or write TODO based on result markers. No changes to queue authority or automatic diagnostic closure. + +Acceptance: regression tests demonstrate explicit load failures, unchanged TODO bytes, safe error messages, and existing valid-input behavior. diff --git a/project/ticket-065/intent.json b/project/ticket-065/intent.json new file mode 100644 index 0000000..f1f30c8 --- /dev/null +++ b/project/ticket-065/intent.json @@ -0,0 +1,29 @@ +{ + "schema": "new-project.intent/v3", + "ticket": "ticket-065", + "summary": "Reject invalid strategy input after Doctor diagnoses 381 and 382", + "workstream": "strategy-input", + "classification": { + "kind": "BUG", + "priority": "P1", + "origin": "health" + }, + "allowedPaths": [ + "planfile/strategy_input.py", + "planfile/ticket_validation.py", + "planfile/todo_sync.py", + "tests/test_strategy_input.py", + "docs/information/python-api.md", + "project/ticket-065/**" + ], + "forbiddenPaths": [ + ".env", + ".github/**" + ], + "stacks": [ + "python" + ], + "dependsOn": [], + "conflictsWith": [], + "integrationTicket": null +} diff --git a/tests/test_strategy_input.py b/tests/test_strategy_input.py new file mode 100644 index 0000000..4b82f34 --- /dev/null +++ b/tests/test_strategy_input.py @@ -0,0 +1,40 @@ +"""Invalid strategy input must not produce successful empty work or TODO writes.""" +from pathlib import Path + +import pytest + +from planfile.ticket_validation import validate_planfile_tickets +from planfile.todo_sync import sync_todo_checkboxes_from_planfile + + +@pytest.mark.parametrize('consumer', ['validate', 'sync']) +@pytest.mark.parametrize('raw,code', [ + (None, 'strategy_input_unreadable'), + (b'tasks: [PRIVATE_CONTENT', 'strategy_input_invalid_yaml'), + (b'\xff', 'strategy_input_invalid_encoding'), + (b'- not-a-mapping\n', 'strategy_input_not_mapping'), + (b'', 'strategy_input_not_mapping'), +]) +def test_invalid_strategy_cannot_look_like_empty_success(tmp_path, consumer, raw, code): + strategy = tmp_path / 'planfile.yaml' + if raw is not None: + strategy.write_bytes(raw) + todo = tmp_path / 'TODO.md' + before = '- [ ] TASK-1234\n' + todo.write_text(before) + with pytest.raises(ValueError, match=code) as error: + if consumer == 'validate': + validate_planfile_tickets(strategy, tmp_path) + else: + sync_todo_checkboxes_from_planfile( + strategy, tmp_path, enabled=True, + results=[{'id': 'TASK-1234', 'status': 'done'}]) + assert 'PRIVATE_CONTENT' not in str(error.value) + assert todo.read_text() == before + + +def test_empty_mapping_is_valid_input(tmp_path): + strategy = tmp_path / 'planfile.yaml' + strategy.write_text('{}\n') + assert validate_planfile_tickets(strategy, tmp_path)['total'] == 0 + assert sync_todo_checkboxes_from_planfile(strategy, tmp_path)['updated'] == 0