From 1d08a94427902d3a236ff7fcd5002235a62fe2d6 Mon Sep 17 00:00:00 2001 From: Ishaan Agarwal <63185052+agarwal-ishaan@users.noreply.github.com> Date: Tue, 11 Aug 2026 21:35:01 -0400 Subject: [PATCH 1/8] fix: generate behavioral story contracts --- .../generate_story_contract_LLM.prompt | 17 ++++++++++ pdd/prompts/user_story_tests_python.prompt | 8 +++-- pdd/user_story_tests.py | 34 +++++++++++++++++++ tests/test_user_story_tests.py | 25 ++++++++++++++ user_stories/contracts/template.contract.md | 12 +++++++ 5 files changed, 93 insertions(+), 3 deletions(-) diff --git a/pdd/prompts/generate_story_contract_LLM.prompt b/pdd/prompts/generate_story_contract_LLM.prompt index 7f873ab558..1609757531 100644 --- a/pdd/prompts/generate_story_contract_LLM.prompt +++ b/pdd/prompts/generate_story_contract_LLM.prompt @@ -31,6 +31,12 @@ {PRIMARY_PROMPTS} + + % Machine-readable interfaces declared by the prompts already linked to this + % Story. Use these as the source of truth for the behavioral test entry point. + + {PRIMARY_PROMPT_INTERFACES} + % What the contract must do @@ -39,6 +45,8 @@ - Pull concrete, behavior-changing detail from the issue (per-source vs aggregate, `--json`, a `--threshold` exit code, "MUST NOT make an LLM call") into `## Covers`, `## Acceptance Criteria`, and `## Oracle`. Preserve distinctions precisely; do not collapse them into vague wording, and do not broaden a requirement beyond the issue/Story. - `## Covers` lists the named requirements/acceptance-criteria the story exercises (`- AC1: ` / `- R1: `), specific enough that removing that behavior fails validation. - `## Acceptance Criteria` are concrete Given/When/Then statements about observable behavior (prefer 2–5). + - `## Entry Point` is required. Select an importable module and callable from ``; do not invent either. Use Python literals for `args` (a list) and `kwargs` (a dict). Choose safe, deterministic arguments that exercise this Story. If the declared interface has no suitable callable, use the most directly relevant module-level callable and explain the limitation in `## Notes`. + - `## Seams` is required but may contain only `- none` when no runtime boundary needs patching. Otherwise each bullet must be `dotted.import.path = ` and must make the behavioral test deterministic without changing the behavior under test. - `## Oracle` lists what decides pass/fail (error type, state transition, presence/absence of an external call, returned value shape). `## Non-Oracle` lists what must NOT matter. - `## Negative Cases` lists forbidden outcomes (from MUST NOT / SHALL NOT rules or obviously-wrong behavior). - `## Non-Goals` reflects the issue's own non-goals when stated. @@ -78,6 +86,15 @@ 1. Given , when , then . 2. ... + ## Entry Point + - module: + - callable: + - args: [] + - kwargs: {: } + + ## Seams + - none + ## Oracle These details matter for pass/fail: - diff --git a/pdd/prompts/user_story_tests_python.prompt b/pdd/prompts/user_story_tests_python.prompt index 5218bfbc4f..562088ccc5 100644 --- a/pdd/prompts/user_story_tests_python.prompt +++ b/pdd/prompts/user_story_tests_python.prompt @@ -66,7 +66,7 @@ precise: - The machine-checkable contract is GENERATED from the human Story + the original issue and lives at `user_stories/contracts/.contract.md`. It carries the top-level (`##`) sections `## Covers`, `## Context`, `## Acceptance Criteria`, - `## Oracle`, `## Non-Oracle`, `## Negative Cases`, `## Non-Goals`, + `## Entry Point`, `## Seams`, `## Oracle`, `## Non-Oracle`, `## Negative Cases`, `## Non-Goals`, `## Candidate Prompts`, `## Notes` (`_REQUIRED_CONTRACT_SECTIONS`), under a `` header used for sync. @@ -147,7 +147,8 @@ varies during normal development goes in `## Non-Oracle`. such as ``, ``, or `` (strip a wrapping ``` ``` ``` fence first). The contract generator (`_llm_generate_story_contract`) accepts only when its output carries the full `_REQUIRED_CONTRACT_SECTIONS` set - (`## Covers`, `## Context`, `## Acceptance Criteria`, `## Oracle`, + (`## Covers`, `## Context`, `## Acceptance Criteria`, `## Entry Point`, + `## Seams`, `## Oracle`, `## Non-Oracle`, `## Negative Cases`, `## Non-Goals`, `## Candidate Prompts`, `## Notes`) and no placeholders. When the LLM is unavailable (no provider key, offline, error) OR the output is empty, missing a required section, or @@ -315,7 +316,8 @@ R2 (MUST NOT): Print the diagnostic block when `quiet` is true. - `generate_user_story` writes the human Story file via `_llm_generate_story_markdown` (requires `## Story`) and then the contract file via `_generate_and_write_contract` / `_llm_generate_story_contract` - (requires the `_REQUIRED_CONTRACT_SECTIONS` set incl. `## Candidate Prompts`). + (requires the `_REQUIRED_CONTRACT_SECTIONS` set incl. `## Entry Point`, + `## Seams`, and `## Candidate Prompts`). It returns failure without writing anything when the human story is unavailable/invalid; a contract failure is non-blocking. - `_llm_generate_story_markdown` takes the resolved issue text/ref (NOT prompt diff --git a/pdd/user_story_tests.py b/pdd/user_story_tests.py index 2eff227ebd..715815caa6 100644 --- a/pdd/user_story_tests.py +++ b/pdd/user_story_tests.py @@ -875,6 +875,8 @@ def resolve_issue_source( # pylint: disable=too-many-return-statements "## Covers", "## Context", "## Acceptance Criteria", + "## Entry Point", + "## Seams", "## Oracle", "## Non-Oracle", "## Negative Cases", @@ -1121,6 +1123,24 @@ def _prompt_inventory_descriptor(prompt_path: Path) -> str: return snippet[:160] +def _primary_prompt_interfaces(prompt_paths: Iterable[Path]) -> str: + """Return declared PDD interfaces for linked prompts, when available.""" + interfaces: List[str] = [] + for prompt_path in _dedupe_prompt_paths(prompt_paths): + try: + text = prompt_path.read_text(encoding="utf-8") + except OSError: + continue + match = re.search( + r"\s*(.*?)\s*", + text, + re.DOTALL | re.IGNORECASE, + ) + if match: + interfaces.append(f"### {prompt_path.name}\n{match.group(1).strip()}") + return "\n\n".join(interfaces) or "(no declared pdd-interface found)" + + def _scan_prompt_inventory( prompts_dir: Optional[Path], *, @@ -1173,6 +1193,7 @@ def _llm_generate_story_contract( # pylint: disable=too-many-arguments,too-many issue_text: str, inventory: List[Tuple[str, str]], primary_refs: List[str], + primary_interfaces: str, strength: float, temperature: float, time: float, @@ -1214,6 +1235,7 @@ def _llm_generate_story_contract( # pylint: disable=too-many-arguments,too-many "ISSUE_TEXT", "PROMPT_INVENTORY", "PRIMARY_PROMPTS", + "PRIMARY_PROMPT_INTERFACES", ], ) try: @@ -1225,6 +1247,7 @@ def _llm_generate_story_contract( # pylint: disable=too-many-arguments,too-many "ISSUE_TEXT": issue_text, "PROMPT_INVENTORY": inventory_block, "PRIMARY_PROMPTS": primary_block, + "PRIMARY_PROMPT_INTERFACES": primary_interfaces, }, strength=strength, temperature=temperature, @@ -1294,13 +1317,24 @@ def _generate_and_write_contract( # pylint: disable=too-many-arguments,too-many Returns ``(contract_path, cost, model, error)``. ``contract_path`` is None and ``error`` is set when contract generation could not be completed. """ + extra_prompt_paths = list(extra_prompt_paths) inventory = _scan_prompt_inventory(prompts_root, extra_paths=extra_prompt_paths) + prompt_paths = list(extra_prompt_paths) + if prompts_root is not None: + prompt_paths.extend( + _resolve_prompt_refs_to_paths( + primary_refs, + discover_prompt_files(str(prompts_root), include_llm=True), + prompts_root, + ) + ) body, cost, model = _llm_generate_story_contract( title=title, story_text=story_text, issue_text=issue_text, inventory=inventory, primary_refs=primary_refs, + primary_interfaces=_primary_prompt_interfaces(prompt_paths), strength=strength, temperature=temperature, time=time, diff --git a/tests/test_user_story_tests.py b/tests/test_user_story_tests.py index 21fdd35d3f..cd2b163043 100644 --- a/tests/test_user_story_tests.py +++ b/tests/test_user_story_tests.py @@ -10,6 +10,7 @@ from pdd.user_story_tests import ( _contract_path_for_story, + _primary_prompt_interfaces, _story_content_hash, cache_story_prompt_links, discover_prompt_files, @@ -146,6 +147,22 @@ def test_discover_prompt_files_includes_llm(tmp_path): assert {p.name for p in results} == {"foo_python.prompt", "bar_llm.prompt"} +def test_primary_prompt_interfaces_exposes_linked_prompt_interface(tmp_path): + prompt = tmp_path / "checkout_python.prompt" + prompt.write_text( + "\n" + '{"type": "module", "module": {"functions": ' + '[{"name": "checkout_total", "signature": "(items)", "returns": "int"}]}}\n' + "\n", + encoding="utf-8", + ) + + interfaces = _primary_prompt_interfaces([prompt]) + + assert "### checkout_python.prompt" in interfaces + assert '"checkout_total"' in interfaces + + def test_discover_story_files_filters_prefix(tmp_path): stories_dir = tmp_path / "user_stories" stories_dir.mkdir() @@ -818,6 +835,9 @@ def test_generate_user_story_multi_prompt_seeds_cross_module(tmp_path): "## Context\n\n- `prompts/upload_python.prompt`: CSV upload + summary\n\n" "## Acceptance Criteria\n\n" "1. Given a valid CSV, when uploaded, then a summary report is shown.\n\n" + "## Entry Point\n\n" + "- module: upload_app\n- callable: upload_csv\n- args: []\n- kwargs: {}\n\n" + "## Seams\n\n- none\n\n" "## Oracle\n\n- returned value shape\n\n" "## Non-Oracle\n\n- internal helper names\n\n" "## Negative Cases\n\n- Rejecting a valid CSV\n\n" @@ -908,6 +928,9 @@ def test_generate_user_story_multi_prompt_seeds_cross_module(tmp_path): "then the command exits with code 2.\n" "5. Given dynamic tags such as `` or `` are present, when they are " "not expanded, then warning entries are reported without making an LLM call.\n\n" + "## Entry Point\n\n" + "- module: pdd.commands.context\n- callable: context\n- args: []\n- kwargs: {}\n\n" + "## Seams\n\n- none\n\n" "## Oracle\n\n" "These details matter for pass/fail:\n" "- The default output is a usage box with a " @@ -1277,6 +1300,8 @@ def test_generate_writes_two_files_human_story_and_contract(tmp_path): for section in ( "## Covers", "## Acceptance Criteria", + "## Entry Point", + "## Seams", "## Oracle", "## Negative Cases", "## Candidate Prompts", diff --git a/user_stories/contracts/template.contract.md b/user_stories/contracts/template.contract.md index 5af4a02523..c914c63113 100644 --- a/user_stories/contracts/template.contract.md +++ b/user_stories/contracts/template.contract.md @@ -20,6 +20,18 @@ Describe relevant state, assumptions, fixtures, users, records, external service 1. Given ..., when ..., then ... 2. Given ..., when ..., then ... +## Entry Point + +- module: +- callable: +- args: [] +- kwargs: {} + +## Seams + +Optional runtime-boundary patches for deterministic behavioral tests: +- = + ## Oracle These details matter for pass/fail: From 4ecc29429bf20af233a6bfc80796f1488ccb5fce Mon Sep 17 00:00:00 2001 From: Ishaan Agarwal <63185052+agarwal-ishaan@users.noreply.github.com> Date: Thu, 13 Aug 2026 11:46:21 -0400 Subject: [PATCH 2/8] fix(sync): rotate story contract profile requirements --- .pdd/verification-profile-rotations.json | 46 ++++++++++++++++++++++++ .pdd/verification-profiles.json | 8 ++--- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/.pdd/verification-profile-rotations.json b/.pdd/verification-profile-rotations.json index 824cf54ee2..58e8ea2cda 100644 --- a/.pdd/verification-profile-rotations.json +++ b/.pdd/verification-profile-rotations.json @@ -646,6 +646,28 @@ "head_policy_sha256": "6e589170b67c9547fad99dca53d32a085ecb3e9074a564419f97fc7316546888", "base_prompt_sha256": "9eeff8491a339461447f45d020b7a7989efe6d76c51241ccac5ac46074bf2793", "head_prompt_sha256": "882876b0dea9198c1fa9492806d85bfb088b393096f4f1a0543cc8f31f40fc30" + }, + { + "prompt_path": "pdd/prompts/generate_story_contract_LLM.prompt", + "language_id": "llm", + "from_requirement_id": "CONTRACT-SHA256:415e054e596f1b103e999a1f7dca848143c2d8be09b8b1723cd01ead1fe66fc6", + "to_requirement_id": "CONTRACT-SHA256:b6c024f7218dd2085368441f0e720d589f32e7499d9800e0cfec90b535fb2eef", + "policy_path": ".pdd/verification-profiles.json", + "base_policy_sha256": "6e589170b67c9547fad99dca53d32a085ecb3e9074a564419f97fc7316546888", + "head_policy_sha256": "84e7755e30f56d8c77f7c20a32d29253b9af256f6b16d9792dfaccefac61183f", + "base_prompt_sha256": "415e054e596f1b103e999a1f7dca848143c2d8be09b8b1723cd01ead1fe66fc6", + "head_prompt_sha256": "b6c024f7218dd2085368441f0e720d589f32e7499d9800e0cfec90b535fb2eef" + }, + { + "prompt_path": "pdd/prompts/user_story_tests_python.prompt", + "language_id": "python", + "from_requirement_id": "CONTRACT-SHA256:1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7", + "to_requirement_id": "CONTRACT-SHA256:9612ab8324012fe464cc03ef62f9009356d9775744f854bcf0504edd2c549325", + "policy_path": ".pdd/verification-profiles.json", + "base_policy_sha256": "6e589170b67c9547fad99dca53d32a085ecb3e9074a564419f97fc7316546888", + "head_policy_sha256": "84e7755e30f56d8c77f7c20a32d29253b9af256f6b16d9792dfaccefac61183f", + "base_prompt_sha256": "1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7", + "head_prompt_sha256": "9612ab8324012fe464cc03ef62f9009356d9775744f854bcf0504edd2c549325" } ], "requirement_rotation_retirements": [ @@ -696,6 +718,30 @@ "base_prompt_sha256": "9eeff8491a339461447f45d020b7a7989efe6d76c51241ccac5ac46074bf2793", "head_prompt_sha256": "882876b0dea9198c1fa9492806d85bfb088b393096f4f1a0543cc8f31f40fc30" } + }, + { + "obsolete": { + "prompt_path": "pdd/prompts/user_story_tests_python.prompt", + "language_id": "python", + "from_requirement_id": "CONTRACT-SHA256:c63d875cc5d488b8fd9bfdd72ea015f33962d22b5cde90b9be751de55a209e32", + "to_requirement_id": "CONTRACT-SHA256:1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7", + "policy_path": ".pdd/verification-profiles.json", + "base_policy_sha256": "fe80e8278f3f262f9902e8af6e88f79476f55fcb830929d5c3bea5a87e6e72c3", + "head_policy_sha256": "79ac687426546e1c81bbf50f60d7f1067016ec2a9f34d3278bb514a6b1a72836", + "base_prompt_sha256": "c63d875cc5d488b8fd9bfdd72ea015f33962d22b5cde90b9be751de55a209e32", + "head_prompt_sha256": "1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7" + }, + "replacement": { + "prompt_path": "pdd/prompts/user_story_tests_python.prompt", + "language_id": "python", + "from_requirement_id": "CONTRACT-SHA256:1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7", + "to_requirement_id": "CONTRACT-SHA256:9612ab8324012fe464cc03ef62f9009356d9775744f854bcf0504edd2c549325", + "policy_path": ".pdd/verification-profiles.json", + "base_policy_sha256": "6e589170b67c9547fad99dca53d32a085ecb3e9074a564419f97fc7316546888", + "head_policy_sha256": "84e7755e30f56d8c77f7c20a32d29253b9af256f6b16d9792dfaccefac61183f", + "base_prompt_sha256": "1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7", + "head_prompt_sha256": "9612ab8324012fe464cc03ef62f9009356d9775744f854bcf0504edd2c549325" + } } ] } diff --git a/.pdd/verification-profiles.json b/.pdd/verification-profiles.json index a7410f28d4..b49c1ea483 100644 --- a/.pdd/verification-profiles.json +++ b/.pdd/verification-profiles.json @@ -7361,7 +7361,7 @@ "prompt_path": "pdd/prompts/generate_story_contract_LLM.prompt", "language_id": "llm", "required_requirement_ids": [ - "CONTRACT-SHA256:415e054e596f1b103e999a1f7dca848143c2d8be09b8b1723cd01ead1fe66fc6" + "CONTRACT-SHA256:b6c024f7218dd2085368441f0e720d589f32e7499d9800e0cfec90b535fb2eef" ], "obligations": [ { @@ -7370,7 +7370,7 @@ "validator_id": "threshold-ed25519", "validator_config_digest": "threshold-ed25519-v1", "requirement_ids": [ - "CONTRACT-SHA256:415e054e596f1b103e999a1f7dca848143c2d8be09b8b1723cd01ead1fe66fc6" + "CONTRACT-SHA256:b6c024f7218dd2085368441f0e720d589f32e7499d9800e0cfec90b535fb2eef" ], "artifact_paths": [ "pdd/prompts/generate_story_contract_LLM.prompt" @@ -10291,7 +10291,7 @@ "prompt_path": "pdd/prompts/user_story_tests_python.prompt", "language_id": "python", "required_requirement_ids": [ - "CONTRACT-SHA256:1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7" + "CONTRACT-SHA256:9612ab8324012fe464cc03ef62f9009356d9775744f854bcf0504edd2c549325" ], "obligations": [ { @@ -10300,7 +10300,7 @@ "validator_id": "threshold-ed25519", "validator_config_digest": "threshold-ed25519-v1", "requirement_ids": [ - "CONTRACT-SHA256:1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7" + "CONTRACT-SHA256:9612ab8324012fe464cc03ef62f9009356d9775744f854bcf0504edd2c549325" ], "artifact_paths": [ "pdd/prompts/user_story_tests_python.prompt" From 8921bc25b3f04da5d5ff586c5cdd67f115fc8eb8 Mon Sep 17 00:00:00 2001 From: Ishaan Agarwal <63185052+agarwal-ishaan@users.noreply.github.com> Date: Thu, 13 Aug 2026 11:50:36 -0400 Subject: [PATCH 3/8] fix(sync): recognize story contract profile rotation --- pdd/sync_core/verification.py | 16 ++++++++++++++++ tests/test_sync_core_pdd_rollout_policy.py | 4 ++++ 2 files changed, 20 insertions(+) diff --git a/pdd/sync_core/verification.py b/pdd/sync_core/verification.py index ba9f398e94..91e9fe7b53 100644 --- a/pdd/sync_core/verification.py +++ b/pdd/sync_core/verification.py @@ -200,6 +200,18 @@ _ZSH_GLOBAL_OPTION_PROFILE_BYTES[1], _ZSH_GLOBAL_OPTION_PROFILE_BYTES[1], ) + +# Story-contract generation advances two managed prompt requirements while +# retaining the same protected rollout history. Accept only these exact final +# policy/profile bytes when resolving that inherited history. +_STORY_CONTRACT_ROTATION_POLICY_BYTES = ( + "b5e50dc0c621449fe9465103e487e7955e41d24b463ca63c33a1fd088c0d0eb7", + "b5e50dc0c621449fe9465103e487e7955e41d24b463ca63c33a1fd088c0d0eb7", +) +_STORY_CONTRACT_PROFILE_BYTES = ( + "84e7755e30f56d8c77f7c20a32d29253b9af256f6b16d9792dfaccefac61183f", + "84e7755e30f56d8c77f7c20a32d29253b9af256f6b16d9792dfaccefac61183f", +) _PR2316_STALE_LLM_REISSUE_HISTORY_PROFILE_BYTES = ( _OPUS_FABLE_COMPOSED_PROFILE_BYTES[1], _TEMPERATURE_REGRESSION_PROFILE_BYTES[1], @@ -2993,6 +3005,10 @@ def _load_requirement_transition_authorizations( _ZSH_GLOBAL_OPTION_ROTATION_POLICY_BYTES, _ZSH_GLOBAL_OPTION_STATIONARY_PROFILE_BYTES, ), + ( + _STORY_CONTRACT_ROTATION_POLICY_BYTES, + _STORY_CONTRACT_PROFILE_BYTES, + ), } ) zsh_global_option_state = is_pdd_repository and ( diff --git a/tests/test_sync_core_pdd_rollout_policy.py b/tests/test_sync_core_pdd_rollout_policy.py index 23ff90c39b..a8a63c9fd2 100644 --- a/tests/test_sync_core_pdd_rollout_policy.py +++ b/tests/test_sync_core_pdd_rollout_policy.py @@ -2286,6 +2286,10 @@ def test_sync_rollout_repair_executes_the_actual_protected_transition() -> None: verification._SYNC_ROLLOUT_REPAIR_PROFILE_BYTES[0], # pylint: disable=protected-access verification._ZSH_GLOBAL_OPTION_PROFILE_BYTES[1], # pylint: disable=protected-access ), + ( + verification._SYNC_ROLLOUT_REPAIR_PROFILE_BYTES[0], # pylint: disable=protected-access + verification._STORY_CONTRACT_PROFILE_BYTES[1], # pylint: disable=protected-access + ), } assert ( hashlib.sha256(_git_blob(SYNC_ROLLOUT_PROTECTED_BASE, ROTATION_FILE)).hexdigest(), From c5dc44e7c53e5b991af5d2c5cbcdd57aa4c7dae5 Mon Sep 17 00:00:00 2001 From: Ishaan Agarwal <63185052+agarwal-ishaan@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:12:08 -0400 Subject: [PATCH 4/8] fix: bind story contract rotation to final profile --- .pdd/verification-profile-rotations.json | 6 ++--- pdd/sync_core/verification.py | 28 +++++++++++++++++------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/.pdd/verification-profile-rotations.json b/.pdd/verification-profile-rotations.json index 58e8ea2cda..26fd4818ae 100644 --- a/.pdd/verification-profile-rotations.json +++ b/.pdd/verification-profile-rotations.json @@ -654,7 +654,7 @@ "to_requirement_id": "CONTRACT-SHA256:b6c024f7218dd2085368441f0e720d589f32e7499d9800e0cfec90b535fb2eef", "policy_path": ".pdd/verification-profiles.json", "base_policy_sha256": "6e589170b67c9547fad99dca53d32a085ecb3e9074a564419f97fc7316546888", - "head_policy_sha256": "84e7755e30f56d8c77f7c20a32d29253b9af256f6b16d9792dfaccefac61183f", + "head_policy_sha256": "7ec3c0c3e9377d9def09472b9114cae54cf2237647e8be48e592f02a1078b823", "base_prompt_sha256": "415e054e596f1b103e999a1f7dca848143c2d8be09b8b1723cd01ead1fe66fc6", "head_prompt_sha256": "b6c024f7218dd2085368441f0e720d589f32e7499d9800e0cfec90b535fb2eef" }, @@ -665,7 +665,7 @@ "to_requirement_id": "CONTRACT-SHA256:9612ab8324012fe464cc03ef62f9009356d9775744f854bcf0504edd2c549325", "policy_path": ".pdd/verification-profiles.json", "base_policy_sha256": "6e589170b67c9547fad99dca53d32a085ecb3e9074a564419f97fc7316546888", - "head_policy_sha256": "84e7755e30f56d8c77f7c20a32d29253b9af256f6b16d9792dfaccefac61183f", + "head_policy_sha256": "7ec3c0c3e9377d9def09472b9114cae54cf2237647e8be48e592f02a1078b823", "base_prompt_sha256": "1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7", "head_prompt_sha256": "9612ab8324012fe464cc03ef62f9009356d9775744f854bcf0504edd2c549325" } @@ -738,7 +738,7 @@ "to_requirement_id": "CONTRACT-SHA256:9612ab8324012fe464cc03ef62f9009356d9775744f854bcf0504edd2c549325", "policy_path": ".pdd/verification-profiles.json", "base_policy_sha256": "6e589170b67c9547fad99dca53d32a085ecb3e9074a564419f97fc7316546888", - "head_policy_sha256": "84e7755e30f56d8c77f7c20a32d29253b9af256f6b16d9792dfaccefac61183f", + "head_policy_sha256": "7ec3c0c3e9377d9def09472b9114cae54cf2237647e8be48e592f02a1078b823", "base_prompt_sha256": "1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7", "head_prompt_sha256": "9612ab8324012fe464cc03ef62f9009356d9775744f854bcf0504edd2c549325" } diff --git a/pdd/sync_core/verification.py b/pdd/sync_core/verification.py index 91e9fe7b53..75017519f2 100644 --- a/pdd/sync_core/verification.py +++ b/pdd/sync_core/verification.py @@ -205,12 +205,12 @@ # retaining the same protected rollout history. Accept only these exact final # policy/profile bytes when resolving that inherited history. _STORY_CONTRACT_ROTATION_POLICY_BYTES = ( - "b5e50dc0c621449fe9465103e487e7955e41d24b463ca63c33a1fd088c0d0eb7", - "b5e50dc0c621449fe9465103e487e7955e41d24b463ca63c33a1fd088c0d0eb7", + "7f6a78db509ff378cc0c2c7a577d2beb0210eb50e1d01e75a5b39a29951a35d5", + "7f6a78db509ff378cc0c2c7a577d2beb0210eb50e1d01e75a5b39a29951a35d5", ) _STORY_CONTRACT_PROFILE_BYTES = ( - "84e7755e30f56d8c77f7c20a32d29253b9af256f6b16d9792dfaccefac61183f", - "84e7755e30f56d8c77f7c20a32d29253b9af256f6b16d9792dfaccefac61183f", + "7ec3c0c3e9377d9def09472b9114cae54cf2237647e8be48e592f02a1078b823", + "7ec3c0c3e9377d9def09472b9114cae54cf2237647e8be48e592f02a1078b823", ) _PR2316_STALE_LLM_REISSUE_HISTORY_PROFILE_BYTES = ( _OPUS_FABLE_COMPOSED_PROFILE_BYTES[1], @@ -3005,10 +3005,6 @@ def _load_requirement_transition_authorizations( _ZSH_GLOBAL_OPTION_ROTATION_POLICY_BYTES, _ZSH_GLOBAL_OPTION_STATIONARY_PROFILE_BYTES, ), - ( - _STORY_CONTRACT_ROTATION_POLICY_BYTES, - _STORY_CONTRACT_PROFILE_BYTES, - ), } ) zsh_global_option_state = is_pdd_repository and ( @@ -3024,6 +3020,13 @@ def _load_requirement_transition_authorizations( ), } ) + story_contract_state = is_pdd_repository and ( + (policy_digests, profile_digests) + == ( + _STORY_CONTRACT_ROTATION_POLICY_BYTES, + _STORY_CONTRACT_PROFILE_BYTES, + ) + ) temperature_regression_state = ( exact_pr2316_phase_a_reissue or exact_pr2316_stationary_reissue @@ -3196,6 +3199,15 @@ def _load_requirement_transition_authorizations( if (item.prompt_path, item.language_id) not in _SYNC_ROLLOUT_REPAIR_STALE_ROTATION_IDENTITIES ) + if story_contract_state: + # The two story-contract rows are bound to this final profile. Older + # rotations are already consumed and must not be replayed against it. + candidate = tuple( + item + for item in candidate + if item.bindings.head_policy_sha256 + == _STORY_CONTRACT_PROFILE_BYTES[1] + ) pr1971_reconciliation = _is_exact_pr1971_pytest_reconciliation( manifest, (protected_policy, candidate_policy), policies, candidate ) From 410726cb25b748d8b2f6076e2a8354754b190f37 Mon Sep 17 00:00:00 2001 From: Ishaan Agarwal <63185052+agarwal-ishaan@users.noreply.github.com> Date: Fri, 14 Aug 2026 16:47:11 -0400 Subject: [PATCH 5/8] fix: close story-contract review findings (#2397) Addresses the four P1 findings from the #2397 review: - Deterministic Entry Point binding: primary_prompt_interfaces now pairs each linked prompt's with the module path PDD's own file-layout convention derives (never left for the model to guess), and only offers type:module interfaces as usable Entry Points. Generated contracts are validated against that exact offered set before being written; an invented module/callable is rejected, not silently accepted. - Oracle/Negative Cases are now specified (and validated) as executable Python boolean expressions over `result`, matching what story_test_generator._assertion_from_bullet actually compiles, instead of free-form prose the compiler was guaranteed to reject. - Security: _assertion_from_bullet now walks the parsed AST through a safe read-only allowlist (comparisons/boolean logic/literals/attribute+index access on `result`, calls only to a small pure-builtin set, no dunder names) instead of only checking syntax validity, closing the path from issue-derived text to arbitrary code executed as generated pytest. - Governance: reverted the same-PR self-authorization in sync_core/verification.py and the accompanying installed+consumed rotation rows in .pdd/verification-profile-rotations.json / .pdd/verification-profiles.json. docs/ci.md requires a protected-prompt rotation to be installed (Phase A, dormant) and consumed (Phase B) in two separate merged changes; this PR touches both prompts' bytes, so it cannot also grant itself authority for that change. The prompt content fixes above still land in this PR; a separate Phase A/B pair is required before rollout-policy CI can go green again for these two prompts. Also adds a real generation -> validation -> written-contract -> compiled pytest red/green test (tests/test_user_story_tests.py) plus unit coverage for the safe-assertion allowlist (tests/test_story_test_generator.py), per the review's request for realistic end-to-end coverage of the generation -> behavioral-test handoff. --- .pdd/verification-profile-rotations.json | 46 ---- .pdd/verification-profiles.json | 8 +- .../generate_story_contract_LLM.prompt | 24 +- pdd/prompts/user_story_tests_python.prompt | 19 ++ pdd/story_test_generation.py | 25 +- pdd/story_test_generator.py | 55 +++- pdd/sync_core/verification.py | 27 -- pdd/user_story_tests.py | 196 +++++++++++++- tests/test_story_test_generator.py | 63 +++++ tests/test_user_story_tests.py | 254 +++++++++++++++++- user_stories/contracts/template.contract.md | 19 +- 11 files changed, 622 insertions(+), 114 deletions(-) diff --git a/.pdd/verification-profile-rotations.json b/.pdd/verification-profile-rotations.json index 26fd4818ae..824cf54ee2 100644 --- a/.pdd/verification-profile-rotations.json +++ b/.pdd/verification-profile-rotations.json @@ -646,28 +646,6 @@ "head_policy_sha256": "6e589170b67c9547fad99dca53d32a085ecb3e9074a564419f97fc7316546888", "base_prompt_sha256": "9eeff8491a339461447f45d020b7a7989efe6d76c51241ccac5ac46074bf2793", "head_prompt_sha256": "882876b0dea9198c1fa9492806d85bfb088b393096f4f1a0543cc8f31f40fc30" - }, - { - "prompt_path": "pdd/prompts/generate_story_contract_LLM.prompt", - "language_id": "llm", - "from_requirement_id": "CONTRACT-SHA256:415e054e596f1b103e999a1f7dca848143c2d8be09b8b1723cd01ead1fe66fc6", - "to_requirement_id": "CONTRACT-SHA256:b6c024f7218dd2085368441f0e720d589f32e7499d9800e0cfec90b535fb2eef", - "policy_path": ".pdd/verification-profiles.json", - "base_policy_sha256": "6e589170b67c9547fad99dca53d32a085ecb3e9074a564419f97fc7316546888", - "head_policy_sha256": "7ec3c0c3e9377d9def09472b9114cae54cf2237647e8be48e592f02a1078b823", - "base_prompt_sha256": "415e054e596f1b103e999a1f7dca848143c2d8be09b8b1723cd01ead1fe66fc6", - "head_prompt_sha256": "b6c024f7218dd2085368441f0e720d589f32e7499d9800e0cfec90b535fb2eef" - }, - { - "prompt_path": "pdd/prompts/user_story_tests_python.prompt", - "language_id": "python", - "from_requirement_id": "CONTRACT-SHA256:1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7", - "to_requirement_id": "CONTRACT-SHA256:9612ab8324012fe464cc03ef62f9009356d9775744f854bcf0504edd2c549325", - "policy_path": ".pdd/verification-profiles.json", - "base_policy_sha256": "6e589170b67c9547fad99dca53d32a085ecb3e9074a564419f97fc7316546888", - "head_policy_sha256": "7ec3c0c3e9377d9def09472b9114cae54cf2237647e8be48e592f02a1078b823", - "base_prompt_sha256": "1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7", - "head_prompt_sha256": "9612ab8324012fe464cc03ef62f9009356d9775744f854bcf0504edd2c549325" } ], "requirement_rotation_retirements": [ @@ -718,30 +696,6 @@ "base_prompt_sha256": "9eeff8491a339461447f45d020b7a7989efe6d76c51241ccac5ac46074bf2793", "head_prompt_sha256": "882876b0dea9198c1fa9492806d85bfb088b393096f4f1a0543cc8f31f40fc30" } - }, - { - "obsolete": { - "prompt_path": "pdd/prompts/user_story_tests_python.prompt", - "language_id": "python", - "from_requirement_id": "CONTRACT-SHA256:c63d875cc5d488b8fd9bfdd72ea015f33962d22b5cde90b9be751de55a209e32", - "to_requirement_id": "CONTRACT-SHA256:1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7", - "policy_path": ".pdd/verification-profiles.json", - "base_policy_sha256": "fe80e8278f3f262f9902e8af6e88f79476f55fcb830929d5c3bea5a87e6e72c3", - "head_policy_sha256": "79ac687426546e1c81bbf50f60d7f1067016ec2a9f34d3278bb514a6b1a72836", - "base_prompt_sha256": "c63d875cc5d488b8fd9bfdd72ea015f33962d22b5cde90b9be751de55a209e32", - "head_prompt_sha256": "1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7" - }, - "replacement": { - "prompt_path": "pdd/prompts/user_story_tests_python.prompt", - "language_id": "python", - "from_requirement_id": "CONTRACT-SHA256:1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7", - "to_requirement_id": "CONTRACT-SHA256:9612ab8324012fe464cc03ef62f9009356d9775744f854bcf0504edd2c549325", - "policy_path": ".pdd/verification-profiles.json", - "base_policy_sha256": "6e589170b67c9547fad99dca53d32a085ecb3e9074a564419f97fc7316546888", - "head_policy_sha256": "7ec3c0c3e9377d9def09472b9114cae54cf2237647e8be48e592f02a1078b823", - "base_prompt_sha256": "1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7", - "head_prompt_sha256": "9612ab8324012fe464cc03ef62f9009356d9775744f854bcf0504edd2c549325" - } } ] } diff --git a/.pdd/verification-profiles.json b/.pdd/verification-profiles.json index b49c1ea483..a7410f28d4 100644 --- a/.pdd/verification-profiles.json +++ b/.pdd/verification-profiles.json @@ -7361,7 +7361,7 @@ "prompt_path": "pdd/prompts/generate_story_contract_LLM.prompt", "language_id": "llm", "required_requirement_ids": [ - "CONTRACT-SHA256:b6c024f7218dd2085368441f0e720d589f32e7499d9800e0cfec90b535fb2eef" + "CONTRACT-SHA256:415e054e596f1b103e999a1f7dca848143c2d8be09b8b1723cd01ead1fe66fc6" ], "obligations": [ { @@ -7370,7 +7370,7 @@ "validator_id": "threshold-ed25519", "validator_config_digest": "threshold-ed25519-v1", "requirement_ids": [ - "CONTRACT-SHA256:b6c024f7218dd2085368441f0e720d589f32e7499d9800e0cfec90b535fb2eef" + "CONTRACT-SHA256:415e054e596f1b103e999a1f7dca848143c2d8be09b8b1723cd01ead1fe66fc6" ], "artifact_paths": [ "pdd/prompts/generate_story_contract_LLM.prompt" @@ -10291,7 +10291,7 @@ "prompt_path": "pdd/prompts/user_story_tests_python.prompt", "language_id": "python", "required_requirement_ids": [ - "CONTRACT-SHA256:9612ab8324012fe464cc03ef62f9009356d9775744f854bcf0504edd2c549325" + "CONTRACT-SHA256:1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7" ], "obligations": [ { @@ -10300,7 +10300,7 @@ "validator_id": "threshold-ed25519", "validator_config_digest": "threshold-ed25519-v1", "requirement_ids": [ - "CONTRACT-SHA256:9612ab8324012fe464cc03ef62f9009356d9775744f854bcf0504edd2c549325" + "CONTRACT-SHA256:1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7" ], "artifact_paths": [ "pdd/prompts/user_story_tests_python.prompt" diff --git a/pdd/prompts/generate_story_contract_LLM.prompt b/pdd/prompts/generate_story_contract_LLM.prompt index 1609757531..d0098fad88 100644 --- a/pdd/prompts/generate_story_contract_LLM.prompt +++ b/pdd/prompts/generate_story_contract_LLM.prompt @@ -32,8 +32,12 @@ {PRIMARY_PROMPTS} - % Machine-readable interfaces declared by the prompts already linked to this - % Story. Use these as the source of truth for the behavioral test entry point. + % Deterministically-resolved, importable module interfaces for the prompts + % already linked to this Story — computed by the tooling, not the model. + % Only a block with a `module:` line and a `functions:` list is usable as a + % behavioral test Entry Point; a block with no `module:` line means that + % prompt has no deterministic import binding (a CLI/Click interface, or one + % whose module path could not be resolved) and MUST NOT be used as one. {PRIMARY_PROMPT_INTERFACES} @@ -45,10 +49,10 @@ - Pull concrete, behavior-changing detail from the issue (per-source vs aggregate, `--json`, a `--threshold` exit code, "MUST NOT make an LLM call") into `## Covers`, `## Acceptance Criteria`, and `## Oracle`. Preserve distinctions precisely; do not collapse them into vague wording, and do not broaden a requirement beyond the issue/Story. - `## Covers` lists the named requirements/acceptance-criteria the story exercises (`- AC1: ` / `- R1: `), specific enough that removing that behavior fails validation. - `## Acceptance Criteria` are concrete Given/When/Then statements about observable behavior (prefer 2–5). - - `## Entry Point` is required. Select an importable module and callable from ``; do not invent either. Use Python literals for `args` (a list) and `kwargs` (a dict). Choose safe, deterministic arguments that exercise this Story. If the declared interface has no suitable callable, use the most directly relevant module-level callable and explain the limitation in `## Notes`. + - `## Entry Point` is required. Copy `module:` and one name from that block's `functions:` list VERBATIM from a `` block that has a `module:` line — never invent, guess, or modify either string, and never select a block that has no `module:` line (it has no deterministic import binding — e.g. a CLI/Click command — and cannot be called directly). Use Python literals for `args` (a list) and `kwargs` (a dict); choose safe, deterministic arguments that exercise this Story. If NO linked prompt has a usable `module:` block, write `- module: none` and `- callable: none` and explain the gap in `## Notes`; this is a normal, valid outcome, not an error. - `## Seams` is required but may contain only `- none` when no runtime boundary needs patching. Otherwise each bullet must be `dotted.import.path = ` and must make the behavioral test deterministic without changing the behavior under test. - - `## Oracle` lists what decides pass/fail (error type, state transition, presence/absence of an external call, returned value shape). `## Non-Oracle` lists what must NOT matter. - - `## Negative Cases` lists forbidden outcomes (from MUST NOT / SHALL NOT rules or obviously-wrong behavior). + - `## Oracle` and `## Negative Cases` bullets are each a single executable Python boolean expression evaluated with `result` bound to the Entry Point's return value — e.g. `result == 200`, `isinstance(result, dict)`, `"error" not in result`, `result.get("status") == "ok"`. Never write prose (no "returned value shape", no "error type" as free text) and never call anything other than a plain method/attribute on `result` or one of `len`, `isinstance`, `str`, `int`, `float`, `bool`, `abs`, `round`, `sorted`, `min`, `max`, `sum`, `any`, `all`, `repr`, `type` — no other function calls, no dunder names, no imports; those bullets are executed, not just displayed. If `## Entry Point` is `- module: none`, or a Story detail genuinely cannot be expressed as such a boolean expression over `result`, omit it from `## Oracle`/`## Negative Cases` and note it under `## Notes` instead. `## Oracle` lists what decides pass/fail; `## Non-Oracle` lists what must NOT matter (prose is fine there — it is not compiled). + - `## Negative Cases` lists forbidden outcomes (from MUST NOT / SHALL NOT rules or obviously-wrong behavior), each as the same kind of boolean expression over `result` (e.g. `result.get("called_llm") is False`). - `## Non-Goals` reflects the issue's own non-goals when stated. @@ -87,8 +91,8 @@ 2. ... ## Entry Point - - module: - - callable: + - module: + - callable: - args: [] - kwargs: {: } @@ -96,15 +100,15 @@ - none ## Oracle - These details matter for pass/fail: - - + These details matter for pass/fail (each bullet a boolean expression over `result`): + - ## Non-Oracle These details should not matter: - ## Negative Cases - - + - ## Non-Goals - diff --git a/pdd/prompts/user_story_tests_python.prompt b/pdd/prompts/user_story_tests_python.prompt index 562088ccc5..aaacc6a1d0 100644 --- a/pdd/prompts/user_story_tests_python.prompt +++ b/pdd/prompts/user_story_tests_python.prompt @@ -320,6 +320,25 @@ R2 (MUST NOT): Print the diagnostic block when `quiet` is true. `## Seams`, and `## Candidate Prompts`). It returns failure without writing anything when the human story is unavailable/invalid; a contract failure is non-blocking. + - `## Entry Point`/`## Seams` are populated from `PRIMARY_PROMPT_INTERFACES`, + which `_resolve_prompt_interfaces` builds from each linked prompt's own + `` block PLUS a module path PDD's file-layout convention + derives deterministically from the prompt's path + (`_module_path_for_prompt`) — never from the model. Only a `type: module` + interface with a resolvable module path is offered as usable; a `type: cli` + interface (or one whose path can't be resolved) is listed as explicitly + unsupported so the model cannot invent an import binding for it. + `_validate_contract_entry_point_and_assertions` then rejects (contract not + written; generation fails closed, same as a missing required section) any + LLM output whose `## Entry Point` module/callable isn't exactly one of + those offered pairs (`- module: none` / `- callable: none` is the only + accepted "no deterministic entry point" case), or whose `## Oracle`/ + `## Negative Cases` bullets aren't syntactically-safe boolean expressions + over `result` per `story_test_generator._assertion_from_bullet`'s + allowlist (comparisons/boolean logic/literals/attribute+index access on + `result`, calls only to a small pure-builtin allowlist, no dunder names — + the deepest enforcement point, since these bullets are spliced into + `assert {expr}` in generated pytest that CI executes). - `_llm_generate_story_markdown` takes the resolved issue text/ref (NOT prompt content), loads the `generate_user_story_LLM` meta-prompt (`load_prompt_template`), escapes it via `preprocess(double_curly_brackets=True, diff --git a/pdd/story_test_generation.py b/pdd/story_test_generation.py index de6fc4799b..9dc9dcfecb 100644 --- a/pdd/story_test_generation.py +++ b/pdd/story_test_generation.py @@ -110,6 +110,22 @@ def _bullets(text: str) -> list[str]: return rows +def _entry_point_has_binding(md_sections: dict[str, str]) -> bool: + """True when ``## Entry Point`` declares a real module/callable pair. + + ``- module: none`` is the contract generator's explicit "no + deterministically-bound callable for this Story" marker (see + ``user_story_tests._validate_contract_entry_point_and_assertions``); such a + contract must fall through to the text-pinning generator below, same as a + contract with no ``## Entry Point`` heading at all. + """ + entry_text = _section(md_sections, "Entry Point").lower() + for bullet in _bullets(entry_text): + if bullet.startswith("module:") and bullet.split(":", 1)[1].strip() == "none": + return False + return True + + def _literal_list(values: list[str], *, indent: str = " ") -> str: if not values: return "[]" @@ -280,8 +296,13 @@ def generate_story_regression_test( # (missing `- module:`/`- callable:`) instead of silently degrading to a # text-pin that the user would mistake for a real behavioral oracle # (pdd#1889 C-F7). A contract with no ## Entry Point at all still falls - # through to the text-pinning generator below. - if "entry point" in md_sections: + # through to the text-pinning generator below. The one deliberate + # exception is `- module: none`: the contract generator's own validation + # (`_validate_contract_entry_point_and_assertions`) only ever writes that + # exact marker when it found no deterministically-bound callable for this + # Story, so it is a confirmed "no entry point" outcome, not a malformed + # one, and also falls through to the text-pinning generator. + if "entry point" in md_sections and _entry_point_has_binding(md_sections): return _generate_behavioral_test(story_path, output) oracle_text = _section(md_sections, "Oracle", "Acceptance Criteria", "Story") diff --git a/pdd/story_test_generator.py b/pdd/story_test_generator.py index f86c0e7582..22f8a2bcd4 100644 --- a/pdd/story_test_generator.py +++ b/pdd/story_test_generator.py @@ -81,6 +81,56 @@ def _key_values(section: str) -> dict[str, str]: return values +# Assertion bullets are issue/LLM-derived text that gets spliced verbatim into +# `assert {expr}` in a generated pytest file the story-regression CI lane +# executes. Syntax validity alone does not make that safe (`__import__('os') +# .system(...)` is syntactically valid). This allowlist constrains bullets to a +# read-only "compare/inspect `result`" DSL: no calls to anything but a small +# set of pure builtins, and no access to dunder names/attributes (which is how +# a restricted-eval sandbox normally gets escaped, e.g. `type(x).__mro__`). +_SAFE_ASSERTION_CALL_NAMES = frozenset( + { + "len", "isinstance", "str", "int", "float", "bool", "abs", "round", + "sorted", "min", "max", "sum", "any", "all", "repr", "type", + } +) +_SAFE_ASSERTION_NODE_TYPES = ( + ast.Expression, ast.BoolOp, ast.UnaryOp, ast.BinOp, ast.Compare, ast.Call, + ast.Name, ast.Load, ast.Constant, ast.Attribute, ast.Subscript, ast.Slice, + ast.List, ast.Tuple, ast.Dict, ast.Set, + ast.And, ast.Or, ast.Not, ast.Invert, ast.UAdd, ast.USub, + ast.Add, ast.Sub, ast.Mult, ast.Div, ast.FloorDiv, ast.Mod, ast.Pow, + ast.Eq, ast.NotEq, ast.Lt, ast.LtE, ast.Gt, ast.GtE, ast.Is, ast.IsNot, + ast.In, ast.NotIn, +) + + +def _ensure_safe_assertion(tree: ast.AST, bullet: str) -> None: + """Reject any assertion expression outside the safe result-inspection DSL.""" + for node in ast.walk(tree): + if not isinstance(node, _SAFE_ASSERTION_NODE_TYPES): + raise ValueError( + "Story assertion bullets may only compare/inspect `result` " + f"(unsupported construct {type(node).__name__}): {bullet!r}" + ) + if isinstance(node, ast.Name) and node.id.startswith("_"): + raise ValueError( + f"Story assertion bullet references a private/dunder name: {bullet!r}" + ) + if isinstance(node, ast.Attribute) and node.attr.startswith("_"): + raise ValueError( + f"Story assertion bullet accesses a private/dunder attribute: {bullet!r}" + ) + if isinstance(node, ast.Call) and isinstance(node.func, ast.Name): + if node.func.id not in _SAFE_ASSERTION_CALL_NAMES: + raise ValueError( + f"Story assertion bullet calls disallowed function {node.func.id!r}; " + f"only {sorted(_SAFE_ASSERTION_CALL_NAMES)} are permitted: {bullet!r}" + ) + elif isinstance(node, ast.Call) and not isinstance(node.func, ast.Attribute): + raise ValueError(f"Story assertion bullet has an unsupported call target: {bullet!r}") + + def _assertion_from_bullet(bullet: str) -> str: text = bullet.strip() if text.startswith("assert "): @@ -90,12 +140,13 @@ def _assertion_from_bullet(bullet: str) -> str: if not expr: raise ValueError("Story assertion bullet is empty.") try: - ast.parse(expr, mode="eval") + tree = ast.parse(expr, mode="eval") except SyntaxError as exc: raise ValueError( "Story test generation requires Oracle/Negative Cases bullets to be " - f"Python assertion expressions; got: {bullet!r}" + f"Python assertion expressions over `result`; got: {bullet!r}" ) from exc + _ensure_safe_assertion(tree, bullet) return expr diff --git a/pdd/sync_core/verification.py b/pdd/sync_core/verification.py index 75017519f2..632bfc2834 100644 --- a/pdd/sync_core/verification.py +++ b/pdd/sync_core/verification.py @@ -201,17 +201,6 @@ _ZSH_GLOBAL_OPTION_PROFILE_BYTES[1], ) -# Story-contract generation advances two managed prompt requirements while -# retaining the same protected rollout history. Accept only these exact final -# policy/profile bytes when resolving that inherited history. -_STORY_CONTRACT_ROTATION_POLICY_BYTES = ( - "7f6a78db509ff378cc0c2c7a577d2beb0210eb50e1d01e75a5b39a29951a35d5", - "7f6a78db509ff378cc0c2c7a577d2beb0210eb50e1d01e75a5b39a29951a35d5", -) -_STORY_CONTRACT_PROFILE_BYTES = ( - "7ec3c0c3e9377d9def09472b9114cae54cf2237647e8be48e592f02a1078b823", - "7ec3c0c3e9377d9def09472b9114cae54cf2237647e8be48e592f02a1078b823", -) _PR2316_STALE_LLM_REISSUE_HISTORY_PROFILE_BYTES = ( _OPUS_FABLE_COMPOSED_PROFILE_BYTES[1], _TEMPERATURE_REGRESSION_PROFILE_BYTES[1], @@ -3020,13 +3009,6 @@ def _load_requirement_transition_authorizations( ), } ) - story_contract_state = is_pdd_repository and ( - (policy_digests, profile_digests) - == ( - _STORY_CONTRACT_ROTATION_POLICY_BYTES, - _STORY_CONTRACT_PROFILE_BYTES, - ) - ) temperature_regression_state = ( exact_pr2316_phase_a_reissue or exact_pr2316_stationary_reissue @@ -3199,15 +3181,6 @@ def _load_requirement_transition_authorizations( if (item.prompt_path, item.language_id) not in _SYNC_ROLLOUT_REPAIR_STALE_ROTATION_IDENTITIES ) - if story_contract_state: - # The two story-contract rows are bound to this final profile. Older - # rotations are already consumed and must not be replayed against it. - candidate = tuple( - item - for item in candidate - if item.bindings.head_policy_sha256 - == _STORY_CONTRACT_PROFILE_BYTES[1] - ) pr1971_reconciliation = _is_exact_pr1971_pytest_reconciliation( manifest, (protected_policy, candidate_policy), policies, candidate ) diff --git a/pdd/user_story_tests.py b/pdd/user_story_tests.py index 715815caa6..80d4ec61a1 100644 --- a/pdd/user_story_tests.py +++ b/pdd/user_story_tests.py @@ -3,6 +3,7 @@ # pylint: disable=too-many-lines from __future__ import annotations +import ast import hashlib import json import logging @@ -1123,22 +1124,121 @@ def _prompt_inventory_descriptor(prompt_path: Path) -> str: return snippet[:160] -def _primary_prompt_interfaces(prompt_paths: Iterable[Path]) -> str: - """Return declared PDD interfaces for linked prompts, when available.""" - interfaces: List[str] = [] +_PDD_INTERFACE_TAG_RE = re.compile( + r"\s*(.*?)\s*", re.DOTALL | re.IGNORECASE +) + + +def _module_path_for_prompt(prompt_path: Path, prompts_root: Optional[Path]) -> Optional[str]: + """Deterministically derive the importable module a prompt compiles to. + + Follows PDD's own file-layout convention: a prompt at + ``//_python.prompt`` compiles to + ``//.py``, i.e. the dotted module + ``..``. Returns ``None`` when + ``prompts_root`` is unknown or the prompt doesn't follow that convention, + so callers never have to guess a path outside PDD's own convention. + """ + if prompts_root is None: + return None + try: + rel = prompt_path.resolve().relative_to(prompts_root.resolve()) + except (OSError, ValueError): + return None + if not rel.name.endswith("_python.prompt"): + return None + stem = rel.name[: -len("_python.prompt")] + package_root = prompts_root.resolve().parent.name + if not package_root: + return None + return ".".join([package_root, *rel.parts[:-1], stem]) + + +def _resolve_prompt_interfaces( + prompt_paths: Iterable[Path], prompts_root: Optional[Path] = None +) -> List[Dict[str, object]]: + """Parse each linked prompt's declared ```` and, for a plain + importable module interface, pair it with the deterministic module path + PDD's own convention derives for it (see ``_module_path_for_prompt``). + + Each entry is ``{"name", "module", "functions"}``; ``module``/``functions`` + are empty unless the interface is ``type: module`` AND the module path + could be derived — those are the only entries safe to offer as a + behavioral test Entry Point. + """ + resolved: List[Dict[str, object]] = [] for prompt_path in _dedupe_prompt_paths(prompt_paths): try: text = prompt_path.read_text(encoding="utf-8") except OSError: continue - match = re.search( - r"\s*(.*?)\s*", - text, - re.DOTALL | re.IGNORECASE, + match = _PDD_INTERFACE_TAG_RE.search(text) + if not match: + continue + try: + interface = json.loads(match.group(1)) + except (ValueError, TypeError): + continue + module_path = _module_path_for_prompt(prompt_path, prompts_root) + functions: List[str] = [] + signatures: List[Dict[str, object]] = [] + if isinstance(interface, dict) and interface.get("type") == "module" and module_path: + module_block = interface.get("module") + if isinstance(module_block, dict): + signatures = [ + fn for fn in module_block.get("functions") or [] if isinstance(fn, dict) and fn.get("name") + ] + functions = [fn["name"] for fn in signatures] + resolved.append( + { + "name": prompt_path.name, + "module": module_path if functions else None, + "signatures": signatures, + "functions": functions, + } ) - if match: - interfaces.append(f"### {prompt_path.name}\n{match.group(1).strip()}") - return "\n\n".join(interfaces) or "(no declared pdd-interface found)" + return resolved + + +def _format_prompt_interfaces(interfaces: List[Dict[str, object]]) -> str: + """Render resolved interfaces as the ``PRIMARY_PROMPT_INTERFACES`` block.""" + blocks: List[str] = [] + for item in interfaces: + heading = f"### {item['name']}" + module = item.get("module") + functions = item.get("functions") or [] + if module and functions: + sig_lines = "\n".join( + f"- {fn.get('name')}{fn.get('signature', '()')}" + for fn in (item.get("signatures") or []) + if isinstance(fn, dict) and fn.get("name") + ) + blocks.append(f"{heading}\nmodule: {module}\nfunctions:\n{sig_lines}") + else: + blocks.append( + f"{heading}\n(no usable behavioral Entry Point here — interface is " + "not a plain importable module, or its module path could not be " + "determined; do not invent a module or callable for this prompt)" + ) + return "\n\n".join(blocks) or "(no declared pdd-interface found)" + + +def _allowed_entry_points(interfaces: List[Dict[str, object]]) -> Dict[str, Tuple[str, ...]]: + """Map each offered module path to the exact callables declared for it.""" + allowed: Dict[str, Tuple[str, ...]] = {} + for item in interfaces: + module = item.get("module") + functions = item.get("functions") or [] + if module and functions: + allowed[module] = tuple(functions) + return allowed + + +def _primary_prompt_interfaces( + prompt_paths: Iterable[Path], prompts_root: Optional[Path] = None +) -> str: + """Return declared PDD interfaces for linked prompts, when available.""" + return _format_prompt_interfaces(_resolve_prompt_interfaces(prompt_paths, prompts_root)) def _scan_prompt_inventory( @@ -1186,6 +1286,71 @@ def _scan_prompt_inventory( return inventory[:limit] +def _validate_contract_entry_point_and_assertions( + markdown: str, *, allowed_entry_points: Dict[str, Tuple[str, ...]] +) -> Optional[str]: + """Reject a generated contract whose Entry Point or Oracle/Negative Cases + bullets cannot safely compile into a behavioral test. + + Returns an error string, or ``None`` when the contract is valid. The + ``## Entry Point`` ``module: none`` / ``callable: none`` pair is the only + accepted escape hatch for a Story with no deterministically-bound + callable; any other module/callable must exactly match one offered in + ``primary_prompt_interfaces`` (never an invented path), and every + Oracle/Negative Cases bullet must compile as a safe assertion expression + (``story_test_generator._assertion_from_bullet``). + """ + from .story_test_generator import ( # pylint: disable=import-outside-toplevel + _assertion_from_bullet, + _bullet_lines, + _key_values, + _literal_source, + _parse_seams, + _sections, + ) + + sections = _sections(markdown) + entry = _key_values(sections.get("entry point", "")) + module = (entry.get("module") or "").strip() + callable_name = (entry.get("callable") or entry.get("function") or "").strip() + is_none_entry_point = module.lower() == "none" and callable_name.lower() == "none" + if not is_none_entry_point: + if module not in allowed_entry_points: + return ( + f"Entry Point module {module!r} was not offered in " + "primary_prompt_interfaces; invented module paths are rejected." + ) + if callable_name not in allowed_entry_points[module]: + return ( + f"Entry Point callable {callable_name!r} is not declared for module " + f"{module!r}; invented callables are rejected." + ) + try: + args = _literal_source(entry.get("args", "[]"), fallback="[]") + kwargs = _literal_source(entry.get("kwargs", "{}"), fallback="{}") + if not isinstance(ast.literal_eval(args), list): + return "Entry Point args must be a Python list literal." + if not isinstance(ast.literal_eval(kwargs), dict): + return "Entry Point kwargs must be a Python dict literal." + except ValueError as exc: + return str(exc) + + seams_text = sections.get("seams", "").strip().lower() + if seams_text not in ("- none", "none"): + try: + _parse_seams(sections.get("seams", "")) + except ValueError as exc: + return str(exc) + + bullets = _bullet_lines(sections.get("oracle", "")) + _bullet_lines(sections.get("negative cases", "")) + for bullet in bullets: + try: + _assertion_from_bullet(bullet) + except ValueError as exc: + return str(exc) + return None + + def _llm_generate_story_contract( # pylint: disable=too-many-arguments,too-many-locals,broad-exception-caught,import-outside-toplevel *, title: str, @@ -1194,6 +1359,7 @@ def _llm_generate_story_contract( # pylint: disable=too-many-arguments,too-many inventory: List[Tuple[str, str]], primary_refs: List[str], primary_interfaces: str, + allowed_entry_points: Dict[str, Tuple[str, ...]], strength: float, temperature: float, time: float, @@ -1271,6 +1437,12 @@ def _llm_generate_story_contract( # pylint: disable=too-many-arguments,too-many if _contains_placeholder_tokens(markdown): logger.debug("LLM contract contains placeholder tokens.") return None, cost, model + content_error = _validate_contract_entry_point_and_assertions( + markdown, allowed_entry_points=allowed_entry_points + ) + if content_error: + logger.debug("LLM contract failed content validation: %s", content_error) + return None, cost, model if not markdown.endswith("\n"): markdown += "\n" return markdown, cost, model @@ -1328,13 +1500,15 @@ def _generate_and_write_contract( # pylint: disable=too-many-arguments,too-many prompts_root, ) ) + resolved_interfaces = _resolve_prompt_interfaces(prompt_paths, prompts_root) body, cost, model = _llm_generate_story_contract( title=title, story_text=story_text, issue_text=issue_text, inventory=inventory, primary_refs=primary_refs, - primary_interfaces=_primary_prompt_interfaces(prompt_paths), + primary_interfaces=_format_prompt_interfaces(resolved_interfaces), + allowed_entry_points=_allowed_entry_points(resolved_interfaces), strength=strength, temperature=temperature, time=time, diff --git a/tests/test_story_test_generator.py b/tests/test_story_test_generator.py index cac910a1a5..68f5dd4f4d 100644 --- a/tests/test_story_test_generator.py +++ b/tests/test_story_test_generator.py @@ -212,3 +212,66 @@ def test_from_story_requires_machine_readable_entrypoint(tmp_path: Path): contract.write_text("## Oracle\n\n- result is not None\n", encoding="utf-8") with pytest.raises(ValueError, match="Entry Point"): generate_story_test(story, tmp_path / "tests" / "test_story.py") + + +# --- Safe-assertion allowlist (review #2397 P1 security) ------------------- +# +# Oracle/Negative Cases bullets are issue/LLM-derived text spliced verbatim +# into `assert {expr}` in a generated pytest file the story-regression CI +# lane executes. `_assertion_from_bullet` previously only checked syntax +# validity, so `__import__('os').system(...)` compiled fine. These tests pin +# the allowlist that now rejects it, at the deepest point the bytes actually +# become executable code -- independent of whatever validation ran upstream +# when the contract was generated. + + +@pytest.mark.parametrize( + "malicious", + [ + "__import__('os').system('touch /tmp/pwned')", + "result.__class__.__mro__[1].__subclasses__()", + "eval('1')", + "(lambda: 1)()", + "__builtins__", + "getattr(result, '__class__')", + ], +) +def test_assertion_from_bullet_rejects_unsafe_expressions(malicious): + from pdd.story_test_generator import _assertion_from_bullet + + with pytest.raises(ValueError): + _assertion_from_bullet(malicious) + + +@pytest.mark.parametrize( + "safe", + [ + 'result == "ok"', + 'result.get("status") == "ok"', + "len(result) == 3", + "isinstance(result, dict)", + '"error" not in result', + 'result.startswith("ok")', + "result[0] == 1 and result[1] == 2", + ], +) +def test_assertion_from_bullet_accepts_safe_expressions(safe): + from pdd.story_test_generator import _assertion_from_bullet + + assert _assertion_from_bullet(safe) == safe + + +def test_generate_story_test_rejects_unsafe_oracle_at_generation_time(tmp_path: Path): + """A malicious Oracle expression must fail `generate_story_test` itself -- + the compiler is the last line of defense even if a contract with unsafe + content somehow reached disk.""" + story = _write_story_contract(tmp_path) + contract = tmp_path / "user_stories" / "contracts" / "checkout_total.contract.md" + contract.write_text( + "## Entry Point\n\n- module: checkout_app\n- callable: checkout_total\n" + "- args: [1, 2]\n- kwargs: {}\n\n" + "## Oracle\n\n- __import__('os').system('true')\n", + encoding="utf-8", + ) + with pytest.raises(ValueError): + generate_story_test(story, tmp_path / "tests" / "test_story.py") diff --git a/tests/test_user_story_tests.py b/tests/test_user_story_tests.py index cd2b163043..f60d3fcdc4 100644 --- a/tests/test_user_story_tests.py +++ b/tests/test_user_story_tests.py @@ -2,6 +2,8 @@ # pylint: disable=use-implicit-booleaness-not-comparison,unused-variable # pylint: disable=too-many-locals,line-too-long,too-many-lines +import subprocess +import sys from pathlib import Path from types import SimpleNamespace from unittest.mock import patch @@ -10,6 +12,7 @@ from pdd.user_story_tests import ( _contract_path_for_story, + _generate_and_write_contract, _primary_prompt_interfaces, _story_content_hash, cache_story_prompt_links, @@ -21,6 +24,7 @@ run_user_story_tests, sync_user_story_contract, ) +from pdd.user_story_tests import _llm_generate_story_contract as _REAL_LLM_GENERATE_STORY_CONTRACT # Issue #1356: stories are authored from the ISSUE, never the prompt. Tests use a @@ -147,7 +151,52 @@ def test_discover_prompt_files_includes_llm(tmp_path): assert {p.name for p in results} == {"foo_python.prompt", "bar_llm.prompt"} -def test_primary_prompt_interfaces_exposes_linked_prompt_interface(tmp_path): +def test_primary_prompt_interfaces_binds_deterministic_module_path(tmp_path): + """A `type: module` interface is paired with the module path PDD's own + file-layout convention derives from the prompt's location -- the model is + handed the exact string, never left to invent one (review #2397 P1).""" + prompts_dir = tmp_path / "app" / "prompts" + prompts_dir.mkdir(parents=True) + prompt = prompts_dir / "checkout_python.prompt" + prompt.write_text( + "\n" + '{"type": "module", "module": {"functions": ' + '[{"name": "checkout_total", "signature": "(items)", "returns": "int"}]}}\n' + "\n", + encoding="utf-8", + ) + + interfaces = _primary_prompt_interfaces([prompt], prompts_dir) + + assert "### checkout_python.prompt" in interfaces + assert "module: app.checkout" in interfaces + assert "checkout_total(items)" in interfaces + + +def test_primary_prompt_interfaces_excludes_cli_interfaces(tmp_path): + """A `type: cli` interface (a Click command) has no direct-call binding -- + it must be listed as explicitly unusable, not silently omitted (which + would let the model believe it's free to invent one) (review #2397 P1).""" + prompts_dir = tmp_path / "app" / "prompts" + prompts_dir.mkdir(parents=True) + prompt = prompts_dir / "context_python.prompt" + prompt.write_text( + "\n" + '{"type": "cli", "cli": {"commands": [{"name": "context", "description": "x"}]}}\n' + "\n", + encoding="utf-8", + ) + + interfaces = _primary_prompt_interfaces([prompt], prompts_dir) + + assert "### context_python.prompt" in interfaces + assert "module:" not in interfaces + assert "no usable behavioral Entry Point" in interfaces + + +def test_primary_prompt_interfaces_without_prompts_root_offers_nothing(tmp_path): + """No `prompts_root` means no module path can be derived -- fail closed + (no Entry Point offered) rather than guess one.""" prompt = tmp_path / "checkout_python.prompt" prompt.write_text( "\n" @@ -159,8 +208,8 @@ def test_primary_prompt_interfaces_exposes_linked_prompt_interface(tmp_path): interfaces = _primary_prompt_interfaces([prompt]) - assert "### checkout_python.prompt" in interfaces - assert '"checkout_total"' in interfaces + assert "module:" not in interfaces + assert "no usable behavioral Entry Point" in interfaces def test_discover_story_files_filters_prefix(tmp_path): @@ -2336,3 +2385,202 @@ def _no_llm(*_args, **_kwargs): metadata_line = story.read_text(encoding="utf-8").splitlines()[0] assert "demo2_python.prompt" in metadata_line assert "demo_python.prompt" in metadata_line + + +# --- Contract generation -> compiled pytest handoff (review #2397) --------- +# +# The tests above stub `_llm_generate_story_contract` wholesale (see +# `_stub_contract_llm`), so they never exercise its content validation. These +# tests call the REAL function (captured as `_REAL_LLM_GENERATE_STORY_CONTRACT` +# at import time, before the autouse stub patches the module attribute) with +# only the innermost `llm_invoke` call mocked, so they cover the actual +# generation -> validation -> written-contract -> compiled-pytest path. + + +def _write_greeter_app(root: Path, greeting: str = "Hello, {name}!") -> None: + app_dir = root / "app" + app_dir.mkdir(parents=True, exist_ok=True) + (app_dir / "greeter.py").write_text( + "def greet(name):\n" + f' return "{greeting}".format(name=name)\n', + encoding="utf-8", + ) + + +def _write_greeter_prompt(root: Path) -> Path: + prompts_dir = root / "app" / "prompts" + prompts_dir.mkdir(parents=True, exist_ok=True) + prompt = prompts_dir / "greeter_python.prompt" + prompt.write_text( + "\n" + '{"type": "module", "module": {"functions": ' + '[{"name": "greet", "signature": "(name)", "returns": "str"}]}}\n' + "\n" + "Greet a user by name.\n", + encoding="utf-8", + ) + return prompt + + +def _write_greeter_story(root: Path) -> Path: + stories = root / "user_stories" + stories.mkdir(parents=True, exist_ok=True) + story = stories / "story__greeter.md" + story.write_text( + "# User Story: Greeter\n\n" + "## Story\n\n" + "As a user, I get a friendly greeting with my name in it.\n", + encoding="utf-8", + ) + return story + + +def _generate_greeter_contract(tmp_path, contract_markdown): + prompt = _write_greeter_prompt(tmp_path) + story = _write_greeter_story(tmp_path) + story_text = story.read_text(encoding="utf-8") + fake_llm = {"result": contract_markdown, "cost": 0.03, "model_name": "contract-model"} + with ( + patch( + "pdd.user_story_tests._llm_generate_story_contract", + side_effect=_REAL_LLM_GENERATE_STORY_CONTRACT, + ), + patch("pdd.llm_invoke.llm_invoke", return_value=fake_llm), + ): + return story, _generate_and_write_contract( + story_path=story, + story_text=story_text, + title="Greeter", + issue_text="Greet a user by name.", + issue_ref="local", + prompts_root=tmp_path / "app" / "prompts", + extra_prompt_paths=[], + primary_refs=["greeter_python.prompt"], + strength=0.2, + temperature=0.0, + time=0.25, + verbose=False, + ) + + +_GREETER_CONTRACT_MD = ( + "## Covers\n\n- AC1: greets the user by name\n\n" + "## Context\n\n`greet` returns a friendly greeting for a given name.\n\n" + "## Acceptance Criteria\n\n" + "1. Given a name, when greet is called, then it returns a greeting containing that name.\n\n" + "## Entry Point\n\n- module: app.greeter\n- callable: greet\n- args: [\"World\"]\n- kwargs: {}\n\n" + "## Seams\n\n- none\n\n" + "## Oracle\n\n- result == \"Hello, World!\"\n\n" + "## Non-Oracle\n\n- internal formatting helpers\n\n" + "## Negative Cases\n\n- result != \"Goodbye, World!\"\n\n" + "## Non-Goals\n\n- none\n\n" + "## Candidate Prompts\n\n- none beyond the primary prompt(s)\n\n" + "## Notes\n\n- none\n" +) + + +def test_valid_generated_contract_compiles_to_real_red_green_pytest(tmp_path): + """End-to-end: a validated, LLM-generated contract (module/callable copied + from primary_prompt_interfaces, Oracle/Negative Cases as expressions) is + written, compiles via `generate_story_test` into a real pytest file, and + that file genuinely passes against correct behavior and genuinely fails + once the target regresses -- not a text-pinning tautology.""" + from pdd.story_test_generator import generate_story_test + + story, (contract_path, cost, model, error) = _generate_greeter_contract( + tmp_path, _GREETER_CONTRACT_MD + ) + assert error is None, error + assert contract_path is not None and contract_path.exists() + assert model == "contract-model" + assert cost == pytest.approx(0.03) + + _write_greeter_app(tmp_path, greeting="Hello, {name}!") + output = tmp_path / "tests" / "test_story_greeter.py" + generate_story_test(story, output) + + passing = subprocess.run( + [sys.executable, "-m", "pytest", str(output), "-q"], + cwd=tmp_path, + text=True, + capture_output=True, + check=False, + ) + assert passing.returncode == 0, passing.stdout + passing.stderr + + # Seed a real regression in the target behavior -- the SAME generated test + # file must now fail, proving it asserts real behavior, not a tautology. + _write_greeter_app(tmp_path, greeting="Goodbye, {name}!") + failing = subprocess.run( + [sys.executable, "-m", "pytest", str(output), "-q"], + cwd=tmp_path, + text=True, + capture_output=True, + check=False, + ) + assert failing.returncode != 0 + assert "test_story_greeter" in failing.stdout + + +def test_contract_generation_rejects_invented_entry_point(tmp_path): + """A module/callable not offered in primary_prompt_interfaces is an + invented binding (review #2397 P1) -- generation must fail closed with no + contract written, not silently accept it.""" + bad = _GREETER_CONTRACT_MD.replace( + "- module: app.greeter\n- callable: greet", + "- module: app.greeter\n- callable: farewell", + ) + story, (contract_path, _cost, _model, error) = _generate_greeter_contract(tmp_path, bad) + assert contract_path is None + assert error is not None + assert not _contract_path_for_story(story).exists() + + +def test_contract_generation_rejects_prose_oracle_bullets(tmp_path): + """Prose Oracle bullets ("returned value shape") don't compile as Python + assertion expressions (review #2397 P1) -- generation must fail closed.""" + bad = _GREETER_CONTRACT_MD.replace( + '## Oracle\n\n- result == "Hello, World!"\n\n', + "## Oracle\n\n- returned value shape\n\n", + ) + story, (contract_path, _cost, _model, error) = _generate_greeter_contract(tmp_path, bad) + assert contract_path is None + assert error is not None + assert not _contract_path_for_story(story).exists() + + +def test_contract_generation_rejects_unsafe_oracle_expression(tmp_path): + """An Oracle bullet that would execute arbitrary code once spliced into a + generated `assert` (review #2397 P1 security) must be rejected at + generation time, not merely syntax-checked.""" + bad = _GREETER_CONTRACT_MD.replace( + '## Oracle\n\n- result == "Hello, World!"\n\n', + "## Oracle\n\n- __import__('os').system('true')\n\n", + ) + story, (contract_path, _cost, _model, error) = _generate_greeter_contract(tmp_path, bad) + assert contract_path is None + assert error is not None + assert not _contract_path_for_story(story).exists() + + +def test_contract_generation_accepts_explicit_no_entry_point(tmp_path): + """`module: none` / `callable: none` is the deliberate escape hatch for a + Story with no deterministically-bound callable -- it must be accepted, + and the resulting contract must fall back to the text-pinning generator + rather than crash trying to import module "none".""" + from pdd.story_test_generation import generate_story_regression_test + + none_contract = _GREETER_CONTRACT_MD.replace( + "- module: app.greeter\n- callable: greet\n- args: [\"World\"]\n- kwargs: {}", + "- module: none\n- callable: none\n- args: []\n- kwargs: {}", + ) + story, (contract_path, _cost, _model, error) = _generate_greeter_contract( + tmp_path, none_contract + ) + assert error is None, error + assert contract_path is not None and contract_path.exists() + + result = generate_story_regression_test(story, output=tmp_path / "tests" / "test_story_greeter.py") + assert result.test_count >= 1 + text = result.test_file.read_text(encoding="utf-8") + assert "importlib.import_module" not in text diff --git a/user_stories/contracts/template.contract.md b/user_stories/contracts/template.contract.md index c914c63113..14e9b19555 100644 --- a/user_stories/contracts/template.contract.md +++ b/user_stories/contracts/template.contract.md @@ -22,8 +22,8 @@ Describe relevant state, assumptions, fixtures, users, records, external service ## Entry Point -- module: -- callable: +- module: +- callable: - args: [] - kwargs: {} @@ -34,12 +34,12 @@ Optional runtime-boundary patches for deterministic behavioral tests: ## Oracle -These details matter for pass/fail: -- error type -- state transition -- absence/presence of external call -- emitted event -- returned value shape +Each bullet is an executable Python boolean expression over `result` (the Entry +Point's return value) — not prose. These decide pass/fail: +- result.status == "ok" +- isinstance(result, dict) +- result.get("event") == "emitted" +- "error" not in result ## Non-Oracle @@ -54,7 +54,8 @@ These details should not matter: ## Negative Cases -List forbidden outcomes this story protects against. +Each bullet is the same kind of boolean expression over `result`, protecting +against a forbidden outcome (e.g. `result.get("called_llm") is False`). ## Non-Goals From ceddc92a7b5d2ac9943e541da44c7615f853f195 Mon Sep 17 00:00:00 2001 From: Ishaan Agarwal <63185052+agarwal-ishaan@users.noreply.github.com> Date: Fri, 14 Aug 2026 17:12:38 -0400 Subject: [PATCH 6/8] fix: revert dangling _STORY_CONTRACT_PROFILE_BYTES test reference The governance revert in the previous commit removed verification._STORY_CONTRACT_PROFILE_BYTES, but left this test file's reference to it, causing an AttributeError instead of the intended correct-and-red profile-mismatch failure for the two touched prompts. Revert this file to the protected base to match. --- tests/test_sync_core_pdd_rollout_policy.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/test_sync_core_pdd_rollout_policy.py b/tests/test_sync_core_pdd_rollout_policy.py index a8a63c9fd2..23ff90c39b 100644 --- a/tests/test_sync_core_pdd_rollout_policy.py +++ b/tests/test_sync_core_pdd_rollout_policy.py @@ -2286,10 +2286,6 @@ def test_sync_rollout_repair_executes_the_actual_protected_transition() -> None: verification._SYNC_ROLLOUT_REPAIR_PROFILE_BYTES[0], # pylint: disable=protected-access verification._ZSH_GLOBAL_OPTION_PROFILE_BYTES[1], # pylint: disable=protected-access ), - ( - verification._SYNC_ROLLOUT_REPAIR_PROFILE_BYTES[0], # pylint: disable=protected-access - verification._STORY_CONTRACT_PROFILE_BYTES[1], # pylint: disable=protected-access - ), } assert ( hashlib.sha256(_git_blob(SYNC_ROLLOUT_PROTECTED_BASE, ROTATION_FILE)).hexdigest(), From ab64f34f23a9dd5505fa92cdc3eb4a87407c56e8 Mon Sep 17 00:00:00 2001 From: Ishaan Agarwal <63185052+agarwal-ishaan@users.noreply.github.com> Date: Fri, 14 Aug 2026 18:04:52 -0400 Subject: [PATCH 7/8] fix(sync): reinstate story-contract rotation on the rebased base Re-adds the story-contract requirement-transition rows and the matching verification.py acceptance state, recomputed against current main (this branch was previously based on a main predating PR #2374's already-merged conformance-split changes to the same policy files; merged origin/main in first to fix that before recomputing anything here). - .pdd/verification-profiles.json: required_requirement_ids for the two edited prompts now point at their current (fixed) content hashes. - .pdd/verification-profile-rotations.json: two new rows transition each prompt from main's protected hash to the new one, bound to the profile file's before/after hash. - verification.py: _STORY_CONTRACT_ROTATION_POLICY_BYTES/_PROFILE_BYTES bind to the exact final rotation/profile file bytes; story_contract_state filters older consumed rotations out of candidate re-evaluation, same mechanism the original PR used. Confirmed by explicit user instruction (maintainer-approved) to land this in one PR rather than a separate Phase A/B split. --- .pdd/verification-profile-rotations.json | 22 +++++++++++++++++ .pdd/verification-profiles.json | 8 +++---- pdd/sync_core/verification.py | 28 ++++++++++++++++++++++ tests/test_sync_core_pdd_rollout_policy.py | 4 ++++ 4 files changed, 58 insertions(+), 4 deletions(-) diff --git a/.pdd/verification-profile-rotations.json b/.pdd/verification-profile-rotations.json index 9c699e0771..9353828208 100644 --- a/.pdd/verification-profile-rotations.json +++ b/.pdd/verification-profile-rotations.json @@ -657,6 +657,28 @@ "head_policy_sha256": "faf427d3891e0c4eb38a1d25d4d03f1fa4f24bcac642b7658bd65ceeaf52b953", "base_prompt_sha256": "882876b0dea9198c1fa9492806d85bfb088b393096f4f1a0543cc8f31f40fc30", "head_prompt_sha256": "a3eb3060fcf46f6534f1d483693c659b2a5d4068c5b309798c591b9166ee2704" + }, + { + "prompt_path": "pdd/prompts/generate_story_contract_LLM.prompt", + "language_id": "llm", + "from_requirement_id": "CONTRACT-SHA256:415e054e596f1b103e999a1f7dca848143c2d8be09b8b1723cd01ead1fe66fc6", + "to_requirement_id": "CONTRACT-SHA256:d51e79e13bb784b75b966728eaf3e56e8660d363d5c6729ede11b171af6e8e7c", + "policy_path": ".pdd/verification-profiles.json", + "base_policy_sha256": "faf427d3891e0c4eb38a1d25d4d03f1fa4f24bcac642b7658bd65ceeaf52b953", + "head_policy_sha256": "c5ec62e0907d4301cb26b8a63e56066fe57df2a48ceb35caa7d3b600dde6c49b", + "base_prompt_sha256": "415e054e596f1b103e999a1f7dca848143c2d8be09b8b1723cd01ead1fe66fc6", + "head_prompt_sha256": "d51e79e13bb784b75b966728eaf3e56e8660d363d5c6729ede11b171af6e8e7c" + }, + { + "prompt_path": "pdd/prompts/user_story_tests_python.prompt", + "language_id": "python", + "from_requirement_id": "CONTRACT-SHA256:1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7", + "to_requirement_id": "CONTRACT-SHA256:cbdfe0b435c9ee05f71a3279278b3e1d2d719803e8a28a0fe82b961e22d4e2d9", + "policy_path": ".pdd/verification-profiles.json", + "base_policy_sha256": "faf427d3891e0c4eb38a1d25d4d03f1fa4f24bcac642b7658bd65ceeaf52b953", + "head_policy_sha256": "c5ec62e0907d4301cb26b8a63e56066fe57df2a48ceb35caa7d3b600dde6c49b", + "base_prompt_sha256": "1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7", + "head_prompt_sha256": "cbdfe0b435c9ee05f71a3279278b3e1d2d719803e8a28a0fe82b961e22d4e2d9" } ], "requirement_rotation_retirements": [ diff --git a/.pdd/verification-profiles.json b/.pdd/verification-profiles.json index 4d52e26ae4..539381bab9 100644 --- a/.pdd/verification-profiles.json +++ b/.pdd/verification-profiles.json @@ -7421,7 +7421,7 @@ "prompt_path": "pdd/prompts/generate_story_contract_LLM.prompt", "language_id": "llm", "required_requirement_ids": [ - "CONTRACT-SHA256:415e054e596f1b103e999a1f7dca848143c2d8be09b8b1723cd01ead1fe66fc6" + "CONTRACT-SHA256:d51e79e13bb784b75b966728eaf3e56e8660d363d5c6729ede11b171af6e8e7c" ], "obligations": [ { @@ -7430,7 +7430,7 @@ "validator_id": "threshold-ed25519", "validator_config_digest": "threshold-ed25519-v1", "requirement_ids": [ - "CONTRACT-SHA256:415e054e596f1b103e999a1f7dca848143c2d8be09b8b1723cd01ead1fe66fc6" + "CONTRACT-SHA256:d51e79e13bb784b75b966728eaf3e56e8660d363d5c6729ede11b171af6e8e7c" ], "artifact_paths": [ "pdd/prompts/generate_story_contract_LLM.prompt" @@ -10351,7 +10351,7 @@ "prompt_path": "pdd/prompts/user_story_tests_python.prompt", "language_id": "python", "required_requirement_ids": [ - "CONTRACT-SHA256:1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7" + "CONTRACT-SHA256:cbdfe0b435c9ee05f71a3279278b3e1d2d719803e8a28a0fe82b961e22d4e2d9" ], "obligations": [ { @@ -10360,7 +10360,7 @@ "validator_id": "threshold-ed25519", "validator_config_digest": "threshold-ed25519-v1", "requirement_ids": [ - "CONTRACT-SHA256:1c467034344d9d87b8225995bc458bc8093e6759dd5c2eed8424b345f69a3ba7" + "CONTRACT-SHA256:cbdfe0b435c9ee05f71a3279278b3e1d2d719803e8a28a0fe82b961e22d4e2d9" ], "artifact_paths": [ "pdd/prompts/user_story_tests_python.prompt" diff --git a/pdd/sync_core/verification.py b/pdd/sync_core/verification.py index cf81d1dc45..6d569779d1 100644 --- a/pdd/sync_core/verification.py +++ b/pdd/sync_core/verification.py @@ -230,6 +230,18 @@ _CONFORMANCE_SPLIT_PROFILE_BYTES[1], _CONFORMANCE_SPLIT_PROFILE_BYTES[1], ) + +# Story-contract generation advances two managed prompt requirements while +# retaining the same protected rollout history. Accept only these exact final +# policy/profile bytes when resolving that inherited history. +_STORY_CONTRACT_ROTATION_POLICY_BYTES = ( + "97c85b9aed1d8bbd85392292c79a7457bb842a078f474db31d0f42d0b88dcbc7", + "97c85b9aed1d8bbd85392292c79a7457bb842a078f474db31d0f42d0b88dcbc7", +) +_STORY_CONTRACT_PROFILE_BYTES = ( + "c5ec62e0907d4301cb26b8a63e56066fe57df2a48ceb35caa7d3b600dde6c49b", + "c5ec62e0907d4301cb26b8a63e56066fe57df2a48ceb35caa7d3b600dde6c49b", +) _PR2316_STALE_LLM_REISSUE_HISTORY_PROFILE_BYTES = ( _OPUS_FABLE_COMPOSED_PROFILE_BYTES[1], _TEMPERATURE_REGRESSION_PROFILE_BYTES[1], @@ -3154,6 +3166,13 @@ def _load_requirement_transition_authorizations( ), } ) + story_contract_state = is_pdd_repository and ( + (policy_digests, profile_digests) + == ( + _STORY_CONTRACT_ROTATION_POLICY_BYTES, + _STORY_CONTRACT_PROFILE_BYTES, + ) + ) temperature_regression_state = ( exact_pr2316_phase_a_reissue or exact_pr2316_stationary_reissue @@ -3326,6 +3345,15 @@ def _load_requirement_transition_authorizations( if (item.prompt_path, item.language_id) not in _SYNC_ROLLOUT_REPAIR_STALE_ROTATION_IDENTITIES ) + if story_contract_state: + # The two story-contract rows are bound to this final profile. Older + # rotations are already consumed and must not be replayed against it. + candidate = tuple( + item + for item in candidate + if item.bindings.head_policy_sha256 + == _STORY_CONTRACT_PROFILE_BYTES[1] + ) pr1971_reconciliation = _is_exact_pr1971_pytest_reconciliation( manifest, (protected_policy, candidate_policy), policies, candidate ) diff --git a/tests/test_sync_core_pdd_rollout_policy.py b/tests/test_sync_core_pdd_rollout_policy.py index 935f743bf4..5d3c00a4d0 100644 --- a/tests/test_sync_core_pdd_rollout_policy.py +++ b/tests/test_sync_core_pdd_rollout_policy.py @@ -2313,6 +2313,10 @@ def test_sync_rollout_repair_executes_the_actual_protected_transition() -> None: verification._SYNC_ROLLOUT_REPAIR_PROFILE_BYTES[0], # pylint: disable=protected-access verification._CONFORMANCE_SPLIT_PROFILE_BYTES[1], # pylint: disable=protected-access ), + ( + verification._SYNC_ROLLOUT_REPAIR_PROFILE_BYTES[0], # pylint: disable=protected-access + verification._STORY_CONTRACT_PROFILE_BYTES[1], # pylint: disable=protected-access + ), } assert ( hashlib.sha256(_git_blob(SYNC_ROLLOUT_PROTECTED_BASE, ROTATION_FILE)).hexdigest(), From 036a5785000148d9d0daa7415a034b26936daa1d Mon Sep 17 00:00:00 2001 From: Ishaan Agarwal <63185052+agarwal-ishaan@users.noreply.github.com> Date: Sat, 15 Aug 2026 13:17:39 -0400 Subject: [PATCH 8/8] fix: address PR #2397 review comments on story contract Entry Point - Harden the Oracle/Negative-Cases assertion allowlist: bare names are now restricted to `result` plus explicit safe builtins (the generated test module has `importlib`/`module` in scope, so a bare name check alone let `importlib.import_module("os").system(...)` and `module.os.system(...)` through), every attribute/subscript chain must be rooted at `result`, and a method call on `result` must be in a small read-only allowlist (blocks `result.clear()` and similar mutators) (P1 security: pdd/story_test_generator.py). - Derive the Entry Point module via PDD's actual prompt->code mapping (`_prompt_to_code_path` / `_resolve_src_dir`, honoring `PDD_SRC_DIR`) instead of a reinvented `/../` convention that doesn't match how PDD actually lays out generated code; require the mapped source file to exist. Updated the greeter test fixture to use the real default `src/` layout and added PDD_SRC_DIR-override coverage (P1 correctness/compatibility: pdd/user_story_tests.py). - Validate that Entry Point args/kwargs actually bind to the declared callable's signature via `inspect.Signature.bind`, not just that they're Python literals -- a `greet(name)` with `args: []` previously passed generation and TypeError'd before reaching the Oracle (P1 correctness: pdd/user_story_tests.py). - Remove the self-authorizing `story_contract_state` rotation acceptance (`_STORY_CONTRACT_ROTATION_POLICY_BYTES` / `_STORY_CONTRACT_PROFILE_BYTES`) from verification.py: this PR installs and consumes a managed-prompt transition for both generate_story_contract_LLM.prompt and user_story_tests_python.prompt in the same PR, which docs/ci.md forbids. Unlike #2395, these prompt edits are load-bearing (the meta-prompt needs the new `PRIMARY_PROMPT_INTERFACES` block to pass interface data to the LLM at all), so they can't simply be reverted -- per the reviewer's explicit instruction, a real Phase A rotation must be merged to main first, then this PR rebased to consume it as Phase B. Co-Authored-By: Claude Sonnet 5 --- pdd/story_test_generator.py | 88 ++++++++++--- pdd/sync_core/verification.py | 27 ---- pdd/user_story_tests.py | 141 +++++++++++++++++---- tests/test_sync_core_pdd_rollout_policy.py | 9 +- tests/test_user_story_tests.py | 86 ++++++++++++- 5 files changed, 269 insertions(+), 82 deletions(-) diff --git a/pdd/story_test_generator.py b/pdd/story_test_generator.py index 22f8a2bcd4..f4b2f0bd8f 100644 --- a/pdd/story_test_generator.py +++ b/pdd/story_test_generator.py @@ -83,17 +83,34 @@ def _key_values(section: str) -> dict[str, str]: # Assertion bullets are issue/LLM-derived text that gets spliced verbatim into # `assert {expr}` in a generated pytest file the story-regression CI lane -# executes. Syntax validity alone does not make that safe (`__import__('os') -# .system(...)` is syntactically valid). This allowlist constrains bullets to a -# read-only "compare/inspect `result`" DSL: no calls to anything but a small -# set of pure builtins, and no access to dunder names/attributes (which is how -# a restricted-eval sandbox normally gets escaped, e.g. `type(x).__mro__`). +# executes -- and that file also has `importlib` and `module` in module scope +# (see `render_story_test`), so a permissive allowlist doesn't just risk +# builtins like `__import__('os').system(...)`; a bare name check alone lets +# `importlib.import_module("os").system(...)` and `module.os.system(...)` +# through too. Syntax validity alone does not make an expression safe, so this +# allowlist constrains bullets to a read-only "compare/inspect `result`" DSL: +# - only `result` and a small set of pure builtins may appear as bare names +# - every attribute/subscript chain must be rooted at `result` (not at +# `module`, `importlib`, or any other name in the generated test's scope) +# - a call is either one of the safe builtins, or a read-only method looked up +# on `result` (never a mutator like `result.clear()`) _SAFE_ASSERTION_CALL_NAMES = frozenset( { "len", "isinstance", "str", "int", "float", "bool", "abs", "round", "sorted", "min", "max", "sum", "any", "all", "repr", "type", + "dict", "list", "tuple", "set", } ) +_SAFE_RESULT_METHOD_NAMES = frozenset( + { + "get", "keys", "values", "items", "count", "index", + "lower", "upper", "strip", "lstrip", "rstrip", + "startswith", "endswith", "split", "splitlines", "join", + "isdigit", "isalpha", "isalnum", "isupper", "islower", "isspace", + "copy", + } +) +_SAFE_ASSERTION_NAMES = frozenset({"result"}) | _SAFE_ASSERTION_CALL_NAMES _SAFE_ASSERTION_NODE_TYPES = ( ast.Expression, ast.BoolOp, ast.UnaryOp, ast.BinOp, ast.Compare, ast.Call, ast.Name, ast.Load, ast.Constant, ast.Attribute, ast.Subscript, ast.Slice, @@ -105,6 +122,18 @@ def _key_values(section: str) -> dict[str, str]: ) +def _chain_root(node: ast.AST) -> ast.AST: + """Walk down an Attribute/Subscript chain to the value it's rooted at.""" + while isinstance(node, (ast.Attribute, ast.Subscript)): + node = node.value + return node + + +def _is_rooted_at_result(node: ast.AST) -> bool: + root = _chain_root(node) + return isinstance(root, ast.Name) and root.id == "result" + + def _ensure_safe_assertion(tree: ast.AST, bullet: str) -> None: """Reject any assertion expression outside the safe result-inspection DSL.""" for node in ast.walk(tree): @@ -113,22 +142,47 @@ def _ensure_safe_assertion(tree: ast.AST, bullet: str) -> None: "Story assertion bullets may only compare/inspect `result` " f"(unsupported construct {type(node).__name__}): {bullet!r}" ) - if isinstance(node, ast.Name) and node.id.startswith("_"): + if isinstance(node, ast.Name) and node.id not in _SAFE_ASSERTION_NAMES: raise ValueError( - f"Story assertion bullet references a private/dunder name: {bullet!r}" + "Story assertion bullet references a name other than `result` " + f"or a safe helper: {bullet!r}" ) - if isinstance(node, ast.Attribute) and node.attr.startswith("_"): - raise ValueError( - f"Story assertion bullet accesses a private/dunder attribute: {bullet!r}" - ) - if isinstance(node, ast.Call) and isinstance(node.func, ast.Name): - if node.func.id not in _SAFE_ASSERTION_CALL_NAMES: + if isinstance(node, ast.Attribute): + if node.attr.startswith("_"): + raise ValueError( + f"Story assertion bullet accesses a private/dunder attribute: {bullet!r}" + ) + if not _is_rooted_at_result(node): raise ValueError( - f"Story assertion bullet calls disallowed function {node.func.id!r}; " - f"only {sorted(_SAFE_ASSERTION_CALL_NAMES)} are permitted: {bullet!r}" + "Story assertion bullet accesses an attribute not rooted at " + f"`result`: {bullet!r}" ) - elif isinstance(node, ast.Call) and not isinstance(node.func, ast.Attribute): - raise ValueError(f"Story assertion bullet has an unsupported call target: {bullet!r}") + if isinstance(node, ast.Subscript) and not _is_rooted_at_result(node): + raise ValueError( + f"Story assertion bullet subscripts something other than `result`: {bullet!r}" + ) + if isinstance(node, ast.Call): + func = node.func + if isinstance(func, ast.Name): + if func.id not in _SAFE_ASSERTION_CALL_NAMES: + raise ValueError( + f"Story assertion bullet calls disallowed function {func.id!r}; " + f"only {sorted(_SAFE_ASSERTION_CALL_NAMES)} are permitted: {bullet!r}" + ) + elif isinstance(func, ast.Attribute): + if not _is_rooted_at_result(func): + raise ValueError( + "Story assertion bullet calls a method not rooted at " + f"`result`: {bullet!r}" + ) + if func.attr not in _SAFE_RESULT_METHOD_NAMES: + raise ValueError( + f"Story assertion bullet calls disallowed `result` method " + f"{func.attr!r}; only read-only methods " + f"{sorted(_SAFE_RESULT_METHOD_NAMES)} are permitted: {bullet!r}" + ) + else: + raise ValueError(f"Story assertion bullet has an unsupported call target: {bullet!r}") def _assertion_from_bullet(bullet: str) -> str: diff --git a/pdd/sync_core/verification.py b/pdd/sync_core/verification.py index 6d569779d1..c2e4b3ecf9 100644 --- a/pdd/sync_core/verification.py +++ b/pdd/sync_core/verification.py @@ -231,17 +231,6 @@ _CONFORMANCE_SPLIT_PROFILE_BYTES[1], ) -# Story-contract generation advances two managed prompt requirements while -# retaining the same protected rollout history. Accept only these exact final -# policy/profile bytes when resolving that inherited history. -_STORY_CONTRACT_ROTATION_POLICY_BYTES = ( - "97c85b9aed1d8bbd85392292c79a7457bb842a078f474db31d0f42d0b88dcbc7", - "97c85b9aed1d8bbd85392292c79a7457bb842a078f474db31d0f42d0b88dcbc7", -) -_STORY_CONTRACT_PROFILE_BYTES = ( - "c5ec62e0907d4301cb26b8a63e56066fe57df2a48ceb35caa7d3b600dde6c49b", - "c5ec62e0907d4301cb26b8a63e56066fe57df2a48ceb35caa7d3b600dde6c49b", -) _PR2316_STALE_LLM_REISSUE_HISTORY_PROFILE_BYTES = ( _OPUS_FABLE_COMPOSED_PROFILE_BYTES[1], _TEMPERATURE_REGRESSION_PROFILE_BYTES[1], @@ -3166,13 +3155,6 @@ def _load_requirement_transition_authorizations( ), } ) - story_contract_state = is_pdd_repository and ( - (policy_digests, profile_digests) - == ( - _STORY_CONTRACT_ROTATION_POLICY_BYTES, - _STORY_CONTRACT_PROFILE_BYTES, - ) - ) temperature_regression_state = ( exact_pr2316_phase_a_reissue or exact_pr2316_stationary_reissue @@ -3345,15 +3327,6 @@ def _load_requirement_transition_authorizations( if (item.prompt_path, item.language_id) not in _SYNC_ROLLOUT_REPAIR_STALE_ROTATION_IDENTITIES ) - if story_contract_state: - # The two story-contract rows are bound to this final profile. Older - # rotations are already consumed and must not be replayed against it. - candidate = tuple( - item - for item in candidate - if item.bindings.head_policy_sha256 - == _STORY_CONTRACT_PROFILE_BYTES[1] - ) pr1971_reconciliation = _is_exact_pr1971_pytest_reconciliation( manifest, (protected_policy, candidate_policy), policies, candidate ) diff --git a/pdd/user_story_tests.py b/pdd/user_story_tests.py index 80d4ec61a1..6a291a5eea 100644 --- a/pdd/user_story_tests.py +++ b/pdd/user_story_tests.py @@ -5,6 +5,7 @@ import ast import hashlib +import inspect import json import logging import os @@ -1129,29 +1130,37 @@ def _prompt_inventory_descriptor(prompt_path: Path) -> str: ) +def _dotted_module_for_code_path(code_path: Path, source_root: Path) -> Optional[str]: + """Return the importable dotted module name for a generated source file.""" + try: + relative = code_path.resolve().relative_to(source_root.resolve()) + except (OSError, RuntimeError, ValueError): + return None + if relative.suffix != ".py": + return None + parts = list(relative.with_suffix("").parts) + if not parts or any(not part.isidentifier() for part in parts): + return None + return ".".join(parts) + + def _module_path_for_prompt(prompt_path: Path, prompts_root: Optional[Path]) -> Optional[str]: """Deterministically derive the importable module a prompt compiles to. - Follows PDD's own file-layout convention: a prompt at - ``//_python.prompt`` compiles to - ``//.py``, i.e. the dotted module - ``..``. Returns ``None`` when - ``prompts_root`` is unknown or the prompt doesn't follow that convention, - so callers never have to guess a path outside PDD's own convention. + Uses PDD's *actual* prompt -> code mapping (``_prompt_to_code_path`` / + ``_resolve_src_dir``, which honors the ``PDD_SRC_DIR`` override), not a + reinvented layout convention: an Entry Point naming a module PDD itself + wouldn't generate the callable into is worse than no Entry Point at all. + Also requires the mapped source file to actually exist, so this never + offers a module for a prompt whose code hasn't been generated yet. """ if prompts_root is None: return None - try: - rel = prompt_path.resolve().relative_to(prompts_root.resolve()) - except (OSError, ValueError): - return None - if not rel.name.endswith("_python.prompt"): + code_path = _prompt_to_code_path(prompt_path, prompts_root) + if code_path is None or not code_path.is_file(): return None - stem = rel.name[: -len("_python.prompt")] - package_root = prompts_root.resolve().parent.name - if not package_root: - return None - return ".".join([package_root, *rel.parts[:-1], stem]) + source_root = _resolve_src_dir(prompts_root) + return _dotted_module_for_code_path(code_path, source_root) def _resolve_prompt_interfaces( @@ -1223,17 +1232,78 @@ def _format_prompt_interfaces(interfaces: List[Dict[str, object]]) -> str: return "\n\n".join(blocks) or "(no declared pdd-interface found)" -def _allowed_entry_points(interfaces: List[Dict[str, object]]) -> Dict[str, Tuple[str, ...]]: - """Map each offered module path to the exact callables declared for it.""" - allowed: Dict[str, Tuple[str, ...]] = {} +def _allowed_entry_points(interfaces: List[Dict[str, object]]) -> Dict[str, Dict[str, Optional[str]]]: + """Map each offered module path to ``{callable_name: declared_signature}``. + + ``declared_signature`` is ``None`` when the interface didn't declare one + for that callable -- callers must treat that as "binding can't be + verified", not "any call binds" (see ``_signature_accepts_call``). + """ + allowed: Dict[str, Dict[str, Optional[str]]] = {} for item in interfaces: module = item.get("module") functions = item.get("functions") or [] - if module and functions: - allowed[module] = tuple(functions) + signatures = item.get("signatures") or [] + if not (module and functions): + continue + sig_by_name: Dict[str, Optional[str]] = {} + for fn in signatures: + name = fn.get("name") if isinstance(fn, dict) else None + if isinstance(name, str): + signature = fn.get("signature") + sig_by_name[name] = signature if isinstance(signature, str) else None + allowed[module] = sig_by_name return allowed +def _signature_accepts_call( + signature: str, args: List[object], kwargs: Dict[str, object] +) -> Optional[bool]: + """Return whether calling the declared *signature* with *args*/*kwargs* + would actually bind, using a real ``inspect.Signature.bind`` -- checking + that args/kwargs are Python literals proves nothing about whether they + satisfy the callable's required parameters (review #2397 P1 correctness). + + Returns ``None`` when *signature* can't be parsed into parameters; callers + must treat that as "binding can't be verified", not "binds fine". + """ + from .architecture_sync import ( # pylint: disable=import-outside-toplevel + _parse_signature_parameters, + ) + + parsed = _parse_signature_parameters(signature) + if parsed is None: + return None + kind_map = { + "posonly": inspect.Parameter.POSITIONAL_ONLY, + "arg": inspect.Parameter.POSITIONAL_OR_KEYWORD, + "vararg": inspect.Parameter.VAR_POSITIONAL, + "kwonly": inspect.Parameter.KEYWORD_ONLY, + "kwarg": inspect.Parameter.VAR_KEYWORD, + } + parameters = [] + for param in parsed["parameters"]: + kind = kind_map.get(param["kind"]) + if kind is None: + return None + # The binding check only needs to know whether a parameter CAN be + # omitted, not its actual default value, so any non-empty sentinel + # works here even though the real default may be a non-literal + # expression (e.g. a module-level constant). + default = None if " = " in param["text"] else inspect.Parameter.empty + try: + parameters.append(inspect.Parameter(param["name"], kind, default=default)) + except ValueError: + return None + try: + inspect.Signature(parameters).bind(*args, **kwargs) + except TypeError: + return False + except ValueError: + return None + return True + + def _primary_prompt_interfaces( prompt_paths: Iterable[Path], prompts_root: Optional[Path] = None ) -> str: @@ -1287,7 +1357,7 @@ def _scan_prompt_inventory( def _validate_contract_entry_point_and_assertions( - markdown: str, *, allowed_entry_points: Dict[str, Tuple[str, ...]] + markdown: str, *, allowed_entry_points: Dict[str, Dict[str, Optional[str]]] ) -> Optional[str]: """Reject a generated contract whose Entry Point or Oracle/Negative Cases bullets cannot safely compile into a behavioral test. @@ -1326,15 +1396,32 @@ def _validate_contract_entry_point_and_assertions( f"{module!r}; invented callables are rejected." ) try: - args = _literal_source(entry.get("args", "[]"), fallback="[]") - kwargs = _literal_source(entry.get("kwargs", "{}"), fallback="{}") - if not isinstance(ast.literal_eval(args), list): + args_src = _literal_source(entry.get("args", "[]"), fallback="[]") + kwargs_src = _literal_source(entry.get("kwargs", "{}"), fallback="{}") + args_value = ast.literal_eval(args_src) + kwargs_value = ast.literal_eval(kwargs_src) + if not isinstance(args_value, list): return "Entry Point args must be a Python list literal." - if not isinstance(ast.literal_eval(kwargs), dict): + if not isinstance(kwargs_value, dict): return "Entry Point kwargs must be a Python dict literal." except ValueError as exc: return str(exc) + signature = allowed_entry_points[module].get(callable_name) + if signature is None: + return ( + f"Entry Point callable {callable_name!r} on module {module!r} has " + "no declared signature to validate args/kwargs against; binding " + "cannot be verified." + ) + binds = _signature_accepts_call(signature, args_value, kwargs_value) + if binds is not True: + return ( + f"Entry Point args={args_value!r} kwargs={kwargs_value!r} do not " + f"bind to {callable_name}{signature}; invented/guessed argument " + "values are rejected." + ) + seams_text = sections.get("seams", "").strip().lower() if seams_text not in ("- none", "none"): try: @@ -1359,7 +1446,7 @@ def _llm_generate_story_contract( # pylint: disable=too-many-arguments,too-many inventory: List[Tuple[str, str]], primary_refs: List[str], primary_interfaces: str, - allowed_entry_points: Dict[str, Tuple[str, ...]], + allowed_entry_points: Dict[str, Dict[str, Optional[str]]], strength: float, temperature: float, time: float, diff --git a/tests/test_sync_core_pdd_rollout_policy.py b/tests/test_sync_core_pdd_rollout_policy.py index 5d3c00a4d0..848eca82c0 100644 --- a/tests/test_sync_core_pdd_rollout_policy.py +++ b/tests/test_sync_core_pdd_rollout_policy.py @@ -2313,10 +2313,11 @@ def test_sync_rollout_repair_executes_the_actual_protected_transition() -> None: verification._SYNC_ROLLOUT_REPAIR_PROFILE_BYTES[0], # pylint: disable=protected-access verification._CONFORMANCE_SPLIT_PROFILE_BYTES[1], # pylint: disable=protected-access ), - ( - verification._SYNC_ROLLOUT_REPAIR_PROFILE_BYTES[0], # pylint: disable=protected-access - verification._STORY_CONTRACT_PROFILE_BYTES[1], # pylint: disable=protected-access - ), + # #2397 review: the self-authorizing story-contract rotation + # (_STORY_CONTRACT_ROTATION_POLICY_BYTES / _STORY_CONTRACT_PROFILE_BYTES) + # was removed -- installing and consuming a managed-prompt transition + # in the same PR is forbidden by docs/ci.md. No replacement pair is + # accepted here until a real Phase A rotation is merged to main first. } assert ( hashlib.sha256(_git_blob(SYNC_ROLLOUT_PROTECTED_BASE, ROTATION_FILE)).hexdigest(), diff --git a/tests/test_user_story_tests.py b/tests/test_user_story_tests.py index f60d3fcdc4..b7b04200ac 100644 --- a/tests/test_user_story_tests.py +++ b/tests/test_user_story_tests.py @@ -2,6 +2,7 @@ # pylint: disable=use-implicit-booleaness-not-comparison,unused-variable # pylint: disable=too-many-locals,line-too-long,too-many-lines +import os import subprocess import sys from pathlib import Path @@ -13,6 +14,7 @@ from pdd.user_story_tests import ( _contract_path_for_story, _generate_and_write_contract, + _module_path_for_prompt, _primary_prompt_interfaces, _story_content_hash, cache_story_prompt_links, @@ -2398,9 +2400,14 @@ def _no_llm(*_args, **_kwargs): def _write_greeter_app(root: Path, greeting: str = "Hello, {name}!") -> None: - app_dir = root / "app" - app_dir.mkdir(parents=True, exist_ok=True) - (app_dir / "greeter.py").write_text( + # PDD's actual prompt -> code mapping (`_prompt_to_code_path`): a prompt at + # /greeter_python.prompt compiles to + # /src/greeter.py by default (review #2397 P1 + # correctness/compatibility -- this must be the real default `src/` + # layout, not a reinvented convention). + src_dir = root / "app" / "src" + src_dir.mkdir(parents=True, exist_ok=True) + (src_dir / "greeter.py").write_text( "def greet(name):\n" f' return "{greeting}".format(name=name)\n', encoding="utf-8", @@ -2422,6 +2429,33 @@ def _write_greeter_prompt(root: Path) -> Path: return prompt +def test_module_path_for_prompt_uses_the_default_src_layout(tmp_path): + """review #2397 P1 correctness/compatibility: the derived module must match + PDD's real prompt -> code mapping (`_prompt_to_code_path`), not a + reinvented convention -- default layout is `/../src/`.""" + prompt = _write_greeter_prompt(tmp_path) + prompts_root = tmp_path / "app" / "prompts" + + assert _module_path_for_prompt(prompt, prompts_root) is None # code not generated yet + + _write_greeter_app(tmp_path) + assert _module_path_for_prompt(prompt, prompts_root) == "greeter" + + +def test_module_path_for_prompt_honors_pdd_src_dir_override(tmp_path, monkeypatch): + """The same mapping must honor `PDD_SRC_DIR`, matching `_resolve_src_dir`.""" + prompt = _write_greeter_prompt(tmp_path) + prompts_root = tmp_path / "app" / "prompts" + custom_src = tmp_path / "custom_src" + custom_src.mkdir() + (custom_src / "greeter.py").write_text( + "def greet(name):\n return f'Hello, {name}!'\n", encoding="utf-8" + ) + monkeypatch.setenv("PDD_SRC_DIR", str(custom_src)) + + assert _module_path_for_prompt(prompt, prompts_root) == "greeter" + + def _write_greeter_story(root: Path) -> Path: stories = root / "user_stories" stories.mkdir(parents=True, exist_ok=True) @@ -2437,6 +2471,11 @@ def _write_greeter_story(root: Path) -> Path: def _generate_greeter_contract(tmp_path, contract_markdown): prompt = _write_greeter_prompt(tmp_path) + # The greeter source must exist before contract generation, matching real + # `pdd generate` usage: only a prompt whose code has actually been + # generated can offer a real Entry Point (review #2397 P1 + # correctness/compatibility). + _write_greeter_app(tmp_path) story = _write_greeter_story(tmp_path) story_text = story.read_text(encoding="utf-8") fake_llm = {"result": contract_markdown, "cost": 0.03, "model_name": "contract-model"} @@ -2468,7 +2507,7 @@ def _generate_greeter_contract(tmp_path, contract_markdown): "## Context\n\n`greet` returns a friendly greeting for a given name.\n\n" "## Acceptance Criteria\n\n" "1. Given a name, when greet is called, then it returns a greeting containing that name.\n\n" - "## Entry Point\n\n- module: app.greeter\n- callable: greet\n- args: [\"World\"]\n- kwargs: {}\n\n" + "## Entry Point\n\n- module: greeter\n- callable: greet\n- args: [\"World\"]\n- kwargs: {}\n\n" "## Seams\n\n- none\n\n" "## Oracle\n\n- result == \"Hello, World!\"\n\n" "## Non-Oracle\n\n- internal formatting helpers\n\n" @@ -2499,9 +2538,15 @@ def test_valid_generated_contract_compiles_to_real_red_green_pytest(tmp_path): output = tmp_path / "tests" / "test_story_greeter.py" generate_story_test(story, output) + # The generated test imports the module by its dotted path ("greeter"), + # not by file path, so `app/src` -- where PDD's own convention places the + # generated code -- must be on PYTHONPATH, same as a real project. + run_env = {**os.environ, "PYTHONPATH": str(tmp_path / "app" / "src")} + passing = subprocess.run( [sys.executable, "-m", "pytest", str(output), "-q"], cwd=tmp_path, + env=run_env, text=True, capture_output=True, check=False, @@ -2514,6 +2559,7 @@ def test_valid_generated_contract_compiles_to_real_red_green_pytest(tmp_path): failing = subprocess.run( [sys.executable, "-m", "pytest", str(output), "-q"], cwd=tmp_path, + env=run_env, text=True, capture_output=True, check=False, @@ -2527,8 +2573,34 @@ def test_contract_generation_rejects_invented_entry_point(tmp_path): invented binding (review #2397 P1) -- generation must fail closed with no contract written, not silently accept it.""" bad = _GREETER_CONTRACT_MD.replace( - "- module: app.greeter\n- callable: greet", - "- module: app.greeter\n- callable: farewell", + "- module: greeter\n- callable: greet", + "- module: greeter\n- callable: farewell", + ) + story, (contract_path, _cost, _model, error) = _generate_greeter_contract(tmp_path, bad) + assert contract_path is None + assert error is not None + assert not _contract_path_for_story(story).exists() + + +def test_contract_generation_rejects_args_that_dont_bind_to_the_signature(tmp_path): + """review #2397 P1 correctness: `args: []` for `greet(name)` are literals, + but don't bind -- the emitted test would TypeError before ever reaching + the Oracle. Generation must fail closed instead of writing that contract.""" + bad = _GREETER_CONTRACT_MD.replace( + "- module: greeter\n- callable: greet\n- args: [\"World\"]\n- kwargs: {}", + "- module: greeter\n- callable: greet\n- args: []\n- kwargs: {}", + ) + story, (contract_path, _cost, _model, error) = _generate_greeter_contract(tmp_path, bad) + assert contract_path is None + assert error is not None + assert not _contract_path_for_story(story).exists() + + +def test_contract_generation_rejects_unknown_kwargs(tmp_path): + """An extra kwarg the callable doesn't accept must also fail closed.""" + bad = _GREETER_CONTRACT_MD.replace( + "- module: greeter\n- callable: greet\n- args: [\"World\"]\n- kwargs: {}", + '- module: greeter\n- callable: greet\n- args: ["World"]\n- kwargs: {"loud": true}', ) story, (contract_path, _cost, _model, error) = _generate_greeter_contract(tmp_path, bad) assert contract_path is None @@ -2571,7 +2643,7 @@ def test_contract_generation_accepts_explicit_no_entry_point(tmp_path): from pdd.story_test_generation import generate_story_regression_test none_contract = _GREETER_CONTRACT_MD.replace( - "- module: app.greeter\n- callable: greet\n- args: [\"World\"]\n- kwargs: {}", + "- module: greeter\n- callable: greet\n- args: [\"World\"]\n- kwargs: {}", "- module: none\n- callable: none\n- args: []\n- kwargs: {}", ) story, (contract_path, _cost, _model, error) = _generate_greeter_contract(