From 6f6022151af6b450cea8c7704680dfec0f2b2957 Mon Sep 17 00:00:00 2001 From: Serhan Date: Tue, 7 Jul 2026 16:31:02 -0700 Subject: [PATCH 01/64] fix(sync): enforce prompt-declared as the public-surface contract (#1900) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The public-surface gate compared OLD-vs-NEW generated code, so it could not distinguish an intended interface change from generator drift, and its repair directive had no stable target ("restore compatible signatures" — compatible with the very code being regenerated). This dead-ended `pdd change -> pdd sync` whenever a module's declared interface legitimately evolved (live case pdd_cloud#2971: an added symbol plus unrelated `list_secret_metadata` drift). Make `_verify_public_surface_regression` declaration-aware, per-symbol: - Symbols declared in the prompt's (`type: module`) are validated against the DECLARED signature (a stable target) via `signature_entries_compatible`, with defaults resolved in the generated namespace (#1558). Binding-kind/async — which cannot express — stay anchored to the prior generation so async<->sync / function<->class drift is still caught; `BREAKING-CHANGE: change signature` remains the escape hatch for those. - Undeclared symbols keep the old-code baseline (+ `BREAKING-CHANGE`), so backward-compat protection for helpers/re-exports is unchanged (no regression). - `PublicSurfaceRegressionError` carries structured `signature_detail:` lines (full expected-vs-actual, no truncation); the agentic subprocess parser recovers them into the rebuilt repair directive and hard-failure block. `removed:` / `signature_changed:` lines stay byte-identical (cloud parser + tests depend on them). Scoped to `type: module` function declarations; cli/command names and dotted `Class.__init__`/new-class presence stay owned by the conformance gate. Adds tests/test_issue_1900_surface_contract.py (18 gate-level tests). Co-Authored-By: Claude Opus 4.8 --- pdd/agentic_sync_runner.py | 121 +++++- pdd/code_generator_main.py | 307 ++++++++++++++- tests/test_issue_1900_surface_contract.py | 451 ++++++++++++++++++++++ 3 files changed, 845 insertions(+), 34 deletions(-) create mode 100644 tests/test_issue_1900_surface_contract.py diff --git a/pdd/agentic_sync_runner.py b/pdd/agentic_sync_runner.py index f42a2ad0e1..690edc0236 100644 --- a/pdd/agentic_sync_runner.py +++ b/pdd/agentic_sync_runner.py @@ -601,9 +601,67 @@ def build_conformance_hard_failure_from_error( return "\n".join(block_lines) +def _parse_signature_detail_lines(combined: str) -> List[Tuple[str, str, str, str]]: + """Parse ``signature_detail:`` lines into ``(symbol, expected, actual, source)``. + + A ``signature_detail:`` line carries the full expected-vs-actual contract for + one declared signature mismatch (issue #1900). The subprocess path rebuilds + the repair directive from stdout, so it must recover these lines to keep the + DECLARED expected signature — the stable repair target — in the directive and + the hard-failure block. Emitted by ``PublicSurfaceRegressionError`` as: + ``signature_detail: | expected: | actual: | source: `` + + The signature entries themselves routinely contain ` | ` (PEP-604 unions such + as ``int | str``), so the fields are split from the RIGHT off their qualified + ``| : `` delimiters rather than matched with a non-greedy regex that + would truncate at the first ` | ` (codex review finding 3). ``symbol`` is a + Python identifier and ``source`` is a fixed tag, so the outer delimiters are + unambiguous. De-duplicated, preserving first-seen order. + """ + details: List[Tuple[str, str, str, str]] = [] + seen: set = set() + for raw_line in combined.splitlines(): + line = raw_line.strip() + if not line.startswith("signature_detail:"): + continue + body = line[len("signature_detail:"):].strip() + # The three field delimiters must be present AND in order + # (``| expected:`` before ``| actual:`` before ``| source:``). A missing + # or out-of-order line is malformed and must be SKIPPED, never raise + # (codex round-2 finding 3: an out-of-order line previously reached the + # rsplit chain and raised ValueError on unpack). ``find`` uses the first + # occurrence, which still admits a valid line whose expected signature + # contains a ``| actual:`` substring — the rsplit chain below anchors on + # the trailing (real) delimiters. + exp_i = body.find(" | expected: ") + act_i = body.find(" | actual: ") + src_i = body.find(" | source: ") + if not (0 <= exp_i < act_i < src_i): + continue + try: + left, source = body.rsplit(" | source: ", 1) + left, actual = left.rsplit(" | actual: ", 1) + symbol, expected = left.split(" | expected: ", 1) + except ValueError: + continue + detail = (symbol.strip(), expected.strip(), actual.strip(), source.strip()) + if detail in seen: + continue + seen.add(detail) + details.append(detail) + return details + + def _parse_public_surface_failure_fields( stdout: str, stderr: str -) -> Optional[Tuple[str, Tuple[str, ...], Tuple[str, ...]]]: +) -> Optional[ + Tuple[ + str, + Tuple[str, ...], + Tuple[str, ...], + Tuple[Tuple[str, str, str, str], ...], + ] +]: """Detect a public-surface regression and keep removals/signatures separate.""" combined = (stdout or "") + "\n" + (stderr or "") if _PUBLIC_SURFACE_PREFIX not in combined: @@ -637,6 +695,9 @@ def _parse_public_surface_failure_fields( } ) ) + # Preserve declaration source order (de-duped) so the directive lists the + # declared repair targets deterministically. + details_tuple = tuple(_parse_signature_detail_lines(combined)) if not removed and not changed: return None lines = ["Public surface regression repair required."] @@ -644,16 +705,32 @@ def _parse_public_surface_failure_fields( lines.append("Restore these public symbols from the existing module:") for sym in removed: lines.append(f"- {sym}") - if changed: + # Prefer the DECLARED signature as the repair target: it is a stable target, + # unlike "restore compatible signatures" (compatible with the code being + # regenerated), which dead-ended the change->sync loop (issue #1900). + declared_details = [d for d in details_tuple if d[3] == "pdd-interface"] + if declared_details: + lines.append( + "Restore these public symbols to their declared " + " signatures:" + ) + for symbol, expected_entry, actual_entry, _ in declared_details: + lines.append( + f"- Restore `{symbol}` to its declared signature " + f"`{expected_entry}` (found `{actual_entry}`)." + ) + declared_changed = {d[0] for d in declared_details} + remaining_changed = [sym for sym in changed if sym not in declared_changed] + if remaining_changed: lines.append("Restore compatible signatures for these public symbols:") - for sym in changed: + for sym in remaining_changed: lines.append(f"- {sym}") lines.append( "Preserve backward-compatible public helpers unless the prompt lists " "the intended changes with scoped BREAKING-CHANGE: remove " "or BREAKING-CHANGE: change signature markers." ) - return "\n".join(lines), removed, changed + return "\n".join(lines), removed, changed, details_tuple def _parse_public_surface_failure( @@ -663,7 +740,7 @@ def _parse_public_surface_failure( parsed = _parse_public_surface_failure_fields(stdout, stderr) if parsed is None: return None - directive, removed, changed = parsed + directive, removed, changed, _details = parsed signature = tuple( [f"removed:{symbol}" for symbol in removed] + [f"signature_changed:{symbol}" for symbol in changed] @@ -2518,6 +2595,7 @@ def _build_public_surface_hard_failure( parsed = _parse_public_surface_failure_fields(stdout, stderr) removed = parsed[1] if parsed else tuple() changed = parsed[2] if parsed else tuple() + details = parsed[3] if parsed else tuple() combined = (stdout or "") + "\n" + (stderr or "") prompt_field = "" @@ -2555,18 +2633,28 @@ def _extract_field(label: str, default: str = "") -> str: return default return match.group(1).strip().rstrip(".").strip() or default - return "\n".join( + block_lines = [ + failure_summary or "Public surface regression", + "", + "=== public surface regression ===", + f"prompt: {prompt_field}", + f"output: {_extract_field('Output')}", + "removed: " + (", ".join(removed) if removed else ""), + "signature_changed: " + + (", ".join(changed) if changed else ""), + f"pre surface size: {_extract_field('Pre surface size')}", + f"post surface size: {_extract_field('Post surface size')}", + ] + # Carry the full expected-vs-actual contract for each declared signature + # mismatch so criterion 4 (no truncation) holds on the agentic path too + # (issue #1900). Byte-identical to the ``signature_detail:`` message line. + for symbol, expected_entry, actual_entry, source in details: + block_lines.append( + f"signature_detail: {symbol} | expected: {expected_entry} | " + f"actual: {actual_entry} | source: {source}" + ) + block_lines.extend( [ - failure_summary or "Public surface regression", - "", - "=== public surface regression ===", - f"prompt: {prompt_field}", - f"output: {_extract_field('Output')}", - "removed: " + (", ".join(removed) if removed else ""), - "signature_changed: " - + (", ".join(changed) if changed else ""), - f"pre surface size: {_extract_field('Pre surface size')}", - f"post surface size: {_extract_field('Post surface size')}", "", "To allow this surface change, add a `BREAKING-CHANGE:` directive to", "the prompt body. Example: `BREAKING-CHANGE: remove ` (or", @@ -2577,6 +2665,7 @@ def _extract_field(label: str, default: str = "") -> str: _env_fingerprint(), ] ) + return "\n".join(block_lines) def _build_test_churn_hard_failure( self, diff --git a/pdd/code_generator_main.py b/pdd/code_generator_main.py index 4aef1738a1..191ecaa247 100644 --- a/pdd/code_generator_main.py +++ b/pdd/code_generator_main.py @@ -49,6 +49,7 @@ annotations_compatible, build_module_default_symbols, compare_default_sources, + parse_callable_contract, signature_entries_compatible, ) @@ -170,6 +171,7 @@ def __init__( total_cost: float = 0.0, model_name: str = "unknown", repair_directive: Optional[str] = None, + signature_details: Optional[List[Tuple[str, str, str, str]]] = None, ) -> None: self.prompt_name = prompt_name self.output_path = output_path or "" @@ -179,16 +181,32 @@ def __init__( self.post_surface_size = int(post_surface_size) self.total_cost = float(total_cost or 0.0) self.model_name = model_name or "unknown" + # Structured per-symbol detail for signature mismatches (issue #1900): + # ``(symbol, expected_entry, actual_entry, source)`` where ``source`` is + # ``"pdd-interface"`` when the expected signature came from the prompt's + # declaration. Purely additive — the ``removed:`` / ``signature_changed:`` + # message lines below stay byte-for-byte identical (the cloud parser and + # ~50 tests key on them). + self.signature_details = list(signature_details or []) self._repair_directive_override = repair_directive output_display = self.output_path or "" - super().__init__( - f"Public surface regression for {prompt_name}:\n" - f"removed: {', '.join(self.removed_symbols) if self.removed_symbols else ''}\n" - f"signature_changed: {', '.join(self.changed_signatures) if self.changed_signatures else ''}\n" - f"output: {output_display}\n" - f"pre_surface_size: {self.pre_surface_size}\n" - f"post_surface_size: {self.post_surface_size}" - ) + message_lines = [ + f"Public surface regression for {prompt_name}:", + f"removed: {', '.join(self.removed_symbols) if self.removed_symbols else ''}", + f"signature_changed: {', '.join(self.changed_signatures) if self.changed_signatures else ''}", + f"output: {output_display}", + f"pre_surface_size: {self.pre_surface_size}", + f"post_surface_size: {self.post_surface_size}", + ] + # Append (never alter the lines above) one structured detail line per + # signature mismatch so the full expected-vs-actual contract is carried + # in the message the local + cloud repair loops read. + for symbol, expected_entry, actual_entry, source in self.signature_details: + message_lines.append( + f"signature_detail: {symbol} | expected: {expected_entry} | " + f"actual: {actual_entry} | source: {source}" + ) + super().__init__("\n".join(message_lines)) @property def repair_directive(self) -> str: @@ -199,9 +217,30 @@ def repair_directive(self) -> str: lines.append("Restore these public symbols from the existing module:") for sym in self.removed_symbols: lines.append(f"- {sym}") - if self.changed_signatures: + # Prefer the DECLARED signature as the repair target when the prompt's + # is the source of truth: it is a stable target, unlike + # "restore compatible signatures" (compatible with the very code being + # regenerated) which dead-ended the change->sync loop (issue #1900). + declared_details = [ + detail for detail in self.signature_details if detail[3] == "pdd-interface" + ] + if declared_details: + lines.append( + "Restore these public symbols to their declared " + " signatures:" + ) + for symbol, expected_entry, actual_entry, _ in declared_details: + lines.append( + f"- Restore `{symbol}` to its declared signature " + f"`{expected_entry}` (found `{actual_entry}`)." + ) + declared_changed = {detail[0] for detail in declared_details} + remaining_changed = [ + sym for sym in self.changed_signatures if sym not in declared_changed + ] + if remaining_changed: lines.append("Restore compatible signatures for these public symbols:") - for sym in self.changed_signatures: + for sym in remaining_changed: lines.append(f"- {sym}") lines.append( "Preserve backward-compatible public helpers unless the prompt lists " @@ -2187,6 +2226,128 @@ def _prompt_allows_breaking_change(prompt_content: Optional[str]) -> bool: return _prompt_has_breaking_change_marker(prompt_content) +def _collect_declared_surface( + prompt_content: Optional[str], + prompt_name: str, +) -> Dict[str, Optional[str]]: + """Collect declared public symbols and raw signatures from ````. + + Returns ``{name -> raw_signature_or_None}`` for every function the prompt's + ``type: "module"`` ```` declares (``module.functions``). + Unlike :func:`_extract_pdd_interface_signatures`, a missing or non-paren + signature is KEPT (mapped to ``None``) so the surface gate can enforce + presence-only for description-only declarations (issue #1900). + + Scoped to ``type: "module"`` ONLY. ``type: "cli"`` / ``type: "command"`` + interfaces declare COMMAND strings (e.g. ``"sync-architecture"``, + ``"pdd extracts prune"`` — hyphens/spaces, not valid Python identifiers), not + module symbols; feeding those to the surface gate produced phantom + ``removed:`` diffs on valid generated code (codex review finding 1). CLI/ + command signature conformance stays owned by the conformance gate + (:func:`_extract_pdd_interface_signatures` still covers them). + + Returns ``{}`` when there is no prompt, no ``type: "module"`` + ```` block, or the JSON is malformed. The conformance gate owns + the parse-error warning, so this stays silent to avoid a duplicate warning. + """ + declared: Dict[str, Optional[str]] = {} + if not prompt_content: + return declared + tags = parse_prompt_tags(prompt_content) + if tags.get("interface_parse_error"): + return declared + interface = tags.get("interface") + if not isinstance(interface, dict) or interface.get("type") != "module": + return declared + + module_spec = interface.get("module") or {} + for item in module_spec.get("functions") or []: + if not isinstance(item, dict): + continue + name = item.get("name") + if not name or not isinstance(name, str): + continue + sig = item.get("signature") + declared[name] = sig if isinstance(sig, str) else None + return declared + + +def _declared_signature_to_entry( + raw_signature: Optional[str], + binding_kind: str, + *, + is_async: bool = False, +) -> Optional[str]: + """Normalize a declared ```` signature into a snapshot entry. + + Produces an entry shaped like :func:`_snapshot_public_signatures` values + (``[] (params) -> ret``) so :func:`signature_entries_compatible` can + compare the DECLARED contract against the generated one. The binding kind and + async marker are copied from the GENERATED symbol (``binding_kind`` / + ``is_async``) because a ```` signature cannot express + ``self`` / property / ``async``; matching them by construction means only the + parameter list and return annotation are actually compared, and no + binding-kind drift is ever invented from the declaration. + + Returns ``None`` (presence-only: the symbol must exist but its signature is + not checked) when the declared signature is not a parseable paren-list — a + missing signature, ``None``, a class header (``class Foo(Base)``), + ``dataclass ...``, ``...`` — or when the composed entry does not parse as a + callable contract. + """ + if not raw_signature or not isinstance(raw_signature, str): + return None + sig = raw_signature.strip() + # Strip a leading ``async`` and/or ``def `` prefix so a declaration + # written as ``async def foo(x)`` or ``def foo(x)`` reduces to the bare + # parameter list. The entry's async-ness is taken from the generated symbol, + # not the declaration, so any declared ``async`` is dropped here. + if sig.startswith("async "): + sig = sig[len("async "):].strip() + def_match = re.match(r"^def\s+[A-Za-z_]\w*\s*(.*)$", sig, re.DOTALL) + if def_match: + sig = def_match.group(1).strip() + if sig.startswith("async "): + sig = sig[len("async "):].strip() + if not sig.startswith("("): + return None + async_prefix = "async " if is_async else "" + entry = f"[{binding_kind}] {async_prefix}{sig}" + if parse_callable_contract(entry) is None: + return None + return entry + + +def _entry_binding_context(entry: Optional[str]) -> Optional[Tuple[str, bool]]: + """Return ``(binding_kind, is_async)`` parsed off a snapshot entry. + + Reads the ``[]`` prefix and a leading ``async `` from a + :func:`_snapshot_public_signatures` value (e.g. ``[async_function] async + (x)`` -> ``("async_function", True)``). Returns ``None`` when the entry has + no binding-kind prefix. + """ + if not entry: + return None + match = re.match(r"^\[([^\]]+)\]\s*(.*)$", entry.strip()) + if match is None: + return None + return match.group(1), match.group(2).lstrip().startswith("async ") + + +def _declared_presence_name(name: str) -> str: + """Map a declared symbol to the surface name whose presence satisfies it. + + A constructor's ABI is keyed on the CLASS symbol (``Foo``), never + ``Foo.__init__`` (see :func:`_snapshot_public_surface`), so a prompt that + declares ``Foo.__init__`` is present as long as ``Foo`` is — flagging it as + a removed symbol on valid code was a false positive (codex review finding 2). + Every other declared name maps to itself. + """ + if name.endswith(".__init__"): + return name[: -len(".__init__")] + return name + + def _verify_public_surface_regression( existing_code: Optional[str], generated_code: str, @@ -2265,11 +2426,40 @@ def _verify_public_surface_regression( for sym in before: if sym.startswith(prefix): expanded_allowed.add(sym) - removed = [ + # The prompt's ```` declaration is the stable surface contract + # (issue #1900): a legit ``pdd change`` that adds a symbol while regeneration + # drifts an unrelated DECLARED signature used to dead-end the change->sync + # loop, because the old-vs-new comparison had no stable target. Per-symbol + # hybrid — DECLARED symbols are validated against the declaration; UNDECLARED + # symbols keep the old-code baseline. Empty when there is no parseable + # ```` (also on a JSON parse error — the conformance gate owns + # that warning), so undeclared modules behave exactly as before. + declared = _collect_declared_surface(prompt_content, prompt_name) + declared_names = set(declared) + + # Removal, per-symbol. UNDECLARED: a public name dropped between before and + # after regresses unless BREAKING-CHANGE opts it out (today's behavior). + # DECLARED: a still-declared symbol absent from the generated surface + # regresses regardless — the declaration is authoritative, so a + # BREAKING-CHANGE: remove does NOT excuse it, and its absence counts even if + # it was never in ``before``. + undeclared_removed = [ symbol for symbol in _diff_public_surface(before, after) - if symbol not in expanded_allowed + if symbol not in declared_names and symbol not in expanded_allowed + ] + # ``Foo.__init__`` is present when the ``Foo`` class symbol is (the + # constructor ABI is keyed on the class, not ``Class.__init__``), so map each + # declared name through its presence-name before the surface membership test + # (codex review finding 2). The presence-name is also what gets reported when + # a declared symbol is genuinely missing. + declared_missing = [ + presence + for symbol in declared_names + for presence in (_declared_presence_name(symbol),) + if presence not in after ] + removed = sorted(set(undeclared_removed) | set(declared_missing)) before_signatures = _snapshot_public_signatures( existing_code, language or "python", @@ -2282,17 +2472,95 @@ def _verify_public_surface_regression( ) # Per-side module default-symbol tables (issue #1558): a parameter default # written as a same-module constant (``max_chars=_LIMIT`` where - # ``_LIMIT = 25000``) resolves to the literal it stands for. Each side is - # resolved against its OWN module — the existing code for the ``before`` - # signature and the generated code for the ``after`` — so a literal <-> - # same-module-constant refactor of a default is not flagged as a regression, - # while the same constant name resolving to a different value across the two - # versions still is. + # ``_LIMIT = 25000``) resolves to the literal it stands for. For the + # UNDECLARED old-vs-new comparison each side is resolved against its OWN + # module — the existing code for the ``before`` signature and the generated + # code for the ``after`` — so a literal <-> same-module-constant refactor of a + # default is not flagged as a regression, while the same constant name + # resolving to a different value across the two versions still is. before_default_symbols = build_module_default_symbols(existing_code) after_default_symbols = build_module_default_symbols(generated_code) allowed_signature_changes = _prompt_breaking_change_signature_symbols(prompt_content) changed_set: Set[str] = set() + signature_details: List[Tuple[str, str, str, str]] = [] + + # DECLARED symbols: validate the generated signature against the DECLARED + # PARAM/return contract (a stable target), NEVER re-comparing params against + # the old code. Defaults on BOTH sides resolve in the GENERATED module + # namespace — the prompt describes the generated module (issue #1558's + # declared-vs-generated resolution). + for symbol in sorted(declared_names): + if symbol.endswith(".__init__"): + # A declared ``Class.__init__`` is presence-only here: the snapshot + # keys the constructor ABI on the CLASS entry (receiver-stripped), + # never ``Class.__init__``, and the conformance gate already + # validates constructor params against the declaration. Comparing the + # declared ``(self, ...)`` against the receiver-stripped ``[class]`` + # entry would need fragile self-stripping (codex review finding 2). + continue + if symbol in allowed_signature_changes: + # A ``BREAKING-CHANGE: change signature `` opt-out. Honor it for + # declared symbols too (mirroring the undeclared path): the + # ```` cannot express an intended async/binding-kind + # change, so this is the only escape hatch for one (codex round-2 + # finding 1a). + continue + actual_entry = after_signatures.get(symbol) + if actual_entry is None: + # Absent from the generated signature table (missing symbol or a + # non-callable form). Presence is enforced by the removal check + # above; there is no callable entry to compare here. + continue + actual_ctx = _entry_binding_context(actual_entry) + if actual_ctx is None: + continue + # Anchor the un-declarable structural facets (binding kind + async) to the + # PRIOR generation when the symbol already existed and was callable there: + # the ```` cannot express ``self`` / property / ``async`` / + # function-vs-class, so old code is their only stable baseline and an + # async->sync or function->class drift on a declared symbol is a real + # regression (codex round-2 finding 1a). A NEWLY-added declared symbol (or + # one whose prior form was a non-callable assignment/import) has no prior + # callable contract, so fall back to the generated kind/async — the + # declaration still governs the params either way. + before_entry = before_signatures.get(symbol) + before_ctx = ( + _entry_binding_context(before_entry) + if before_entry is not None + and parse_callable_contract(before_entry) is not None + else None + ) + expected_kind, expected_async = ( + before_ctx if before_ctx is not None else actual_ctx + ) + expected_entry = _declared_signature_to_entry( + declared[symbol], expected_kind, is_async=expected_async + ) + if expected_entry is None: + # Declared signature is not a parseable paren-list -> presence-only: + # the symbol must exist (enforced above) but its signature is not + # checked, and it is never re-compared against the old code. + continue + compatible = signature_entries_compatible( + expected_entry, + actual_entry, + old_symbols=after_default_symbols, + new_symbols=after_default_symbols, + ) + if compatible is False: + changed_set.add(symbol) + signature_details.append( + (symbol, expected_entry, actual_entry, "pdd-interface") + ) + # ``compatible is True`` -> compatible; ``None`` -> unparseable entry, + # conservative skip (presence already enforced above). + + # UNDECLARED symbols: keep the historical old-vs-new comparison EXACTLY, + # skipping any DECLARED symbol (owned by the block above so it is never + # double-checked against the old code). for symbol, signature in before_signatures.items(): + if symbol in declared_names: + continue if symbol not in after_signatures or symbol in allowed_signature_changes: continue after_signature = after_signatures[symbol] @@ -2317,6 +2585,8 @@ def _verify_public_surface_regression( continue changed_set.add(symbol) for symbol in before_signatures: + if symbol in declared_names: + continue if symbol in after_signatures or symbol in changed_set: continue if "." in symbol: @@ -2332,6 +2602,7 @@ def _verify_public_surface_regression( changed_signatures=changed_signatures, pre_surface_size=len(before), post_surface_size=len(after), + signature_details=signature_details, ) diff --git a/tests/test_issue_1900_surface_contract.py b/tests/test_issue_1900_surface_contract.py new file mode 100644 index 0000000000..362801c881 --- /dev/null +++ b/tests/test_issue_1900_surface_contract.py @@ -0,0 +1,451 @@ +"""Issue #1900: the prompt-declared ```` is the surface contract. + +The public-surface regression gate +(:func:`pdd.code_generator_main._verify_public_surface_regression`) used to +compare the OLD generated code against the NEW generated code for EVERY public +symbol. That dead-ends the ``pdd change -> pdd sync`` loop whenever a legitimate +change adds a symbol AND regeneration also drifts an unrelated *declared* public +function's signature (live case pdd_cloud#2971: ``list_secret_metadata``): the +gate hard-fails with a repair directive whose target is "compatible with the +very code being regenerated", i.e. no stable target. + +The fix makes the gate declaration-aware, per-symbol: + +* DECLARED symbols (named in the prompt's ````) are validated + against the prompt declaration — a stable target — not against the old code. +* UNDECLARED symbols keep today's old-code baseline (+ ``BREAKING-CHANGE`` + opt-out), so backward-compat protection for helpers/re-exports is unchanged. + +These are gate-level acceptance tests: they call +``_verify_public_surface_regression(before, after, prompt_name, out, "python", +prompt_content)`` directly and assert on ``PublicSurfaceRegressionError``'s +``.removed_symbols`` / ``.changed_signatures`` / ``str(exc)`` / repair directive, +plus one test that the ``signature_detail:`` lines propagate through +``pdd.agentic_sync_runner``'s subprocess parser. +""" + +import json + +import pytest + +from pdd.code_generator_main import ( + PublicSurfaceRegressionError, + _verify_public_surface_regression, +) + +PROMPT = "demo_Python.prompt" +OUT = "pdd/demo.py" + + +def _iface_prompt(functions, body="% engineer\n"): + """Build a minimal prompt carrying a ```` module decl. + + ``functions`` is an iterable of ``(name, signature)`` pairs. A ``signature`` + of ``None`` is emitted as a description-only declaration (no ``signature`` + key) so the presence-only path can be exercised. + """ + decls = [] + for name, signature in functions: + entry = {"name": name} + if signature is not None: + entry["signature"] = signature + decls.append(entry) + spec = {"type": "module", "module": {"functions": decls}} + return f"{json.dumps(spec)}\n{body}" + + +class TestIssue1900SurfaceContract: + def test_2971_shape_flags_declared_drift_not_added_symbol(self): + """The pdd_cloud#2971 shape: a legit change ADDS + ``get_secret_with_enabled_fallback`` while regeneration DRIFTS the + unrelated declared ``list_secret_metadata`` (adds a required param). + + The declared drift must be flagged against the DECLARED signature (a + stable target); the correctly-added declared symbol must NOT be flagged; + a stable declared symbol must NOT be flagged.""" + before = ( + "def list_secret_metadata(project_id, prefix=''):\n" + " return []\n" + "def get_secret(project_id, name):\n" + " return name\n" + ) + after = ( + "def list_secret_metadata(project_id, prefix='', *, region):\n" + " return []\n" + "def get_secret(project_id, name):\n" + " return name\n" + "def get_secret_with_enabled_fallback(project_id, name):\n" + " return name\n" + ) + prompt = _iface_prompt( + [ + ("list_secret_metadata", "(project_id, prefix='')"), + ("get_secret", "(project_id, name)"), + ("get_secret_with_enabled_fallback", "(project_id, name)"), + ] + ) + with pytest.raises(PublicSurfaceRegressionError) as exc: + _verify_public_surface_regression( + before, after, PROMPT, OUT, "python", prompt + ) + assert "list_secret_metadata" in exc.value.changed_signatures + assert "get_secret_with_enabled_fallback" not in exc.value.changed_signatures + assert "get_secret" not in exc.value.changed_signatures + assert "get_secret_with_enabled_fallback" not in exc.value.removed_symbols + # The directive/message must name the DECLARED expected signature, the + # stable target the old code-vs-code comparison never had. + message = str(exc.value) + "\n" + exc.value.repair_directive + assert "(project_id, prefix='')" in message + + def test_declaration_authorizes_incompatible_change(self): + """A declaration is a permit: dropping a required param that the + declaration also drops must pass, even though old-vs-new alone would + flag the removed required ``b``.""" + before = "def f(a, b):\n return a\n" + after = "def f(a):\n return a\n" + prompt = _iface_prompt([("f", "(a)")]) + _verify_public_surface_regression( + before, after, PROMPT, OUT, "python", prompt + ) + + def test_additive_only_declared_symbol_passes(self): + """A newly declared symbol added in ``after`` matching its declared + signature, with no other drift, must pass with no error.""" + before = "def f(a):\n return a\n" + after = ( + "def f(a):\n return a\n" + "def g(a, b):\n return b\n" + ) + prompt = _iface_prompt([("f", "(a)"), ("g", "(a, b)")]) + _verify_public_surface_regression( + before, after, PROMPT, OUT, "python", prompt + ) + + def test_no_declaration_falls_back_to_old_code(self): + """The same drift as the #2971 case but with NO ```` must + still raise against the old code (the fallback baseline is preserved).""" + before = ( + "def list_secret_metadata(project_id, prefix=''):\n" + " return []\n" + ) + after = ( + "def list_secret_metadata(project_id, prefix='', *, region):\n" + " return []\n" + ) + with pytest.raises(PublicSurfaceRegressionError) as exc: + _verify_public_surface_regression( + before, after, PROMPT, OUT, "python", "Just prose, no interface." + ) + assert "list_secret_metadata" in exc.value.changed_signatures + + def test_declared_symbol_missing_from_generated_is_removed(self): + """A declared symbol absent from generated code is a violation, listed in + ``removed_symbols`` — regardless of whether it was in ``before`` (the + declaration is authoritative for presence).""" + before = "def f(a):\n return a\n" + after = "def f(a):\n return a\n" + # ``g`` is declared but never generated (never in before or after). + prompt = _iface_prompt([("f", "(a)"), ("g", "(a)")]) + with pytest.raises(PublicSurfaceRegressionError) as exc: + _verify_public_surface_regression( + before, after, PROMPT, OUT, "python", prompt + ) + assert "g" in exc.value.removed_symbols + + def test_declaration_authoritative_over_breaking_change(self): + """A ``BREAKING-CHANGE: remove`` marker does NOT excuse a symbol that is + still declared in ````: the declaration wins.""" + before = "def f(a):\n return a\ndef g(a):\n return a\n" + after = "def f(a):\n return a\n" + prompt = _iface_prompt( + [("f", "(a)"), ("g", "(a)")], + body="% engineer\nBREAKING-CHANGE: remove g\n", + ) + with pytest.raises(PublicSurfaceRegressionError) as exc: + _verify_public_surface_regression( + before, after, PROMPT, OUT, "python", prompt + ) + assert "g" in exc.value.removed_symbols + + def test_undeclared_symbol_still_protected(self): + """Per-symbol hybrid: an UNDECLARED public helper dropped by + regeneration must still raise (old-code protection intact), even though + another symbol IS declared.""" + before = ( + "def f(a):\n return a\n" + "def helper():\n return 1\n" + ) + after = "def f(a):\n return a\n" + prompt = _iface_prompt([("f", "(a)")]) + with pytest.raises(PublicSurfaceRegressionError) as exc: + _verify_public_surface_regression( + before, after, PROMPT, OUT, "python", prompt + ) + assert "helper" in exc.value.removed_symbols + + def test_output_carries_full_expected_and_actual_signatures(self): + """Criterion 4: the error output carries the FULL declared-expected AND + the actual generated signature text (no truncation).""" + before = "def f(a, b='x'):\n return a\n" + after = "def f(a, b='x', *, c):\n return a\n" + prompt = _iface_prompt([("f", "(a, b='x')")]) + with pytest.raises(PublicSurfaceRegressionError) as exc: + _verify_public_surface_regression( + before, after, PROMPT, OUT, "python", prompt + ) + text = str(exc.value) + "\n" + exc.value.repair_directive + assert "(a, b='x')" in text # declared expected + assert "*, c" in text # actual generated drift + + def test_presence_only_declared_signature(self): + """A declared signature that is not a parseable paren-list (here: + omitted) is presence-only: the symbol must exist but its signature is + NOT checked, so an arbitrary signature change passes; its ABSENCE is + still a violation.""" + prompt = _iface_prompt([("f", None)]) + # Present with a wildly different signature -> no signature false-positive. + _verify_public_surface_regression( + "def f(a):\n return a\n", + "def f(a, b, c, d):\n return a\n", + PROMPT, + OUT, + "python", + prompt, + ) + # Absent from generated code -> violation. + with pytest.raises(PublicSurfaceRegressionError) as exc: + _verify_public_surface_regression( + "def f(a):\n return a\n", + "def other():\n return 1\n", + PROMPT, + OUT, + "python", + prompt, + ) + assert "f" in exc.value.removed_symbols + + def test_default_resolution_uses_generated_namespace(self): + """Issue #1558: for the declared-vs-generated comparison BOTH sides + resolve in the GENERATED module namespace. A declared default written as + a bare constant name that the generated module binds must resolve to its + literal and NOT be flagged (it would fail closed if the declared side + were resolved against the OLD/empty namespace).""" + before = "def f(max_chars=999):\n return max_chars\n" + after = ( + "_LIMIT = 25000\n" + "def f(max_chars=_LIMIT):\n return max_chars\n" + ) + prompt = _iface_prompt([("f", "(max_chars=_LIMIT)")]) + _verify_public_surface_regression( + before, after, PROMPT, OUT, "python", prompt + ) + + def test_cli_and_command_names_are_not_surface_symbols(self): + """Codex finding 1: ``type: cli`` / ``type: command`` interfaces declare + COMMAND strings (``sync-architecture``, ``pdd extracts prune`` — hyphens + and spaces, not valid Python identifiers), NOT module symbols. They must + never drive ``removed:`` diffs; only ``type: module`` declares Python + symbols the surface gate can own. CLI/command signature conformance stays + with the conformance gate.""" + before = "def run():\n return 1\n" + after = "def run():\n return 1\n" + cli_spec = { + "type": "cli", + "cli": {"commands": [{"name": "sync-architecture", "signature": "(--flag)"}]}, + } + cli_prompt = ( + f"{json.dumps(cli_spec)}\n% engineer\n" + ) + _verify_public_surface_regression( + before, after, PROMPT, OUT, "python", cli_prompt + ) + command_spec = { + "type": "command", + "command": {"name": "pdd extracts prune", "signature": "(target)"}, + } + command_prompt = ( + f"{json.dumps(command_spec)}\n% engineer\n" + ) + _verify_public_surface_regression( + before, after, PROMPT, OUT, "python", command_prompt + ) + + def test_dotted_init_presence_satisfied_by_class_symbol(self): + """Codex finding 2: a constructor's ABI is keyed on the CLASS symbol + (``Foo``), never ``Foo.__init__``. A real ``type: module`` prompt may + declare ``Foo.__init__``; its presence must be satisfied by the class + symbol and its signature left to the conformance gate — so valid code + must NOT raise. The class going missing is still a violation.""" + prompt = _iface_prompt([("Foo.__init__", "(self, value)")]) + before = ( + "class Foo:\n" + " def __init__(self, value):\n" + " self.value = value\n" + ) + # Constructor signature drift (added optional param) is owned by the + # conformance gate; the surface gate must not raise here. + after = ( + "class Foo:\n" + " def __init__(self, value, extra=None):\n" + " self.value = value\n" + ) + _verify_public_surface_regression( + before, after, PROMPT, OUT, "python", prompt + ) + # The class itself absent -> still a violation naming the class. + with pytest.raises(PublicSurfaceRegressionError) as exc: + _verify_public_surface_regression( + before, + "def other():\n return 1\n", + PROMPT, + OUT, + "python", + prompt, + ) + assert "Foo" in exc.value.removed_symbols + + def test_declared_async_or_kind_drift_uses_old_code_baseline(self): + """Codex round-2 finding 1a: the ```` cannot express + ``async`` or a function/class binding kind, so those un-declarable + structural facets are anchored to the PRIOR generation for a declared + symbol. An ``async``->sync (and sync->``async``) drift on a declared + function must therefore still raise, even though the declaration's + parameter list is unchanged.""" + prompt = _iface_prompt([("f", "(x)")]) + # async -> sync. + with pytest.raises(PublicSurfaceRegressionError) as exc: + _verify_public_surface_regression( + "async def f(x):\n return x\n", + "def f(x):\n return x\n", + PROMPT, + OUT, + "python", + prompt, + ) + assert "f" in exc.value.changed_signatures + # sync -> async. + with pytest.raises(PublicSurfaceRegressionError) as exc: + _verify_public_surface_regression( + "def f(x):\n return x\n", + "async def f(x):\n return x\n", + PROMPT, + OUT, + "python", + prompt, + ) + assert "f" in exc.value.changed_signatures + + def test_declared_async_regenerated_async_passes(self): + """The anchor must not create a false positive: a declared async function + regenerated async — even with an added OPTIONAL parameter — is + backward-compatible and must NOT raise.""" + prompt = _iface_prompt([("f", "(x)")]) + _verify_public_surface_regression( + "async def f(x):\n return x\n", + "async def f(x, y=None):\n return x\n", + PROMPT, + OUT, + "python", + prompt, + ) + + def test_breaking_change_opts_out_declared_kind_change(self): + """A rare INTENDED async/kind change on a declared symbol still has an + escape hatch: ``BREAKING-CHANGE: change signature f`` opts the declared + symbol out of the declared-signature check (mirroring the undeclared + path), because the declaration cannot express the change.""" + prompt = _iface_prompt( + [("f", "(x)")], + body="% engineer\nBREAKING-CHANGE: change signature f\n", + ) + _verify_public_surface_regression( + "async def f(x):\n return x\n", + "def f(x):\n return x\n", + PROMPT, + OUT, + "python", + prompt, + ) + + +class TestIssue1900AgenticPropagation: + def test_signature_detail_lines_reach_agentic_directive(self): + """The subprocess/agentic path rebuilds the repair directive from stdout, + so the new ``signature_detail:`` lines must be parsed and carried into + the rebuilt directive (criterion 4 on the agentic path).""" + from pdd.agentic_sync_runner import _parse_public_surface_failure + + stderr = ( + "Public surface regression for demo_Python.prompt:\n" + "removed: \n" + "signature_changed: list_secret_metadata\n" + "output: pdd/demo.py\n" + "pre_surface_size: 2\n" + "post_surface_size: 3\n" + "signature_detail: list_secret_metadata | expected: " + "[function] (project_id, prefix='') | actual: " + "[function] (project_id, prefix='', *, region) | source: pdd-interface\n" + ) + parsed = _parse_public_surface_failure("", stderr) + assert parsed is not None + directive, signature = parsed + # The declared expected signature (the stable target) must ride along in + # the rebuilt directive. + assert "(project_id, prefix='')" in directive + assert "list_secret_metadata" in directive + # Existing signature tuple contract is unchanged (removed + changed). + assert signature == ("signature_changed:list_secret_metadata",) + + def test_signature_detail_union_types_survive_parsing(self): + """Codex finding 3: a ``signature_detail:`` line whose expected AND actual + entries contain PEP-604 ` | ` union types must round-trip through the + parser with the FULL signatures intact (no truncation at the ` | ` inside + the signature). Right-anchored field splitting keeps the trailing + ``| actual:`` / ``| source:`` field delimiters unambiguous.""" + from pdd.agentic_sync_runner import _parse_public_surface_failure + + expected_sig = "[function] (x: int | str) -> bool | None" + actual_sig = "[function] (x: int | str, y: int) -> bool | None" + stderr = ( + "Public surface regression for demo_Python.prompt:\n" + "removed: \n" + "signature_changed: f\n" + "output: pdd/demo.py\n" + "pre_surface_size: 1\n" + "post_surface_size: 1\n" + f"signature_detail: f | expected: {expected_sig} | " + f"actual: {actual_sig} | source: pdd-interface\n" + ) + parsed = _parse_public_surface_failure("", stderr) + assert parsed is not None + directive, _signature = parsed + assert expected_sig in directive + assert actual_sig in directive + + def test_malformed_signature_detail_line_is_skipped_not_crash(self): + """Codex round-2 finding 3: a malformed ``signature_detail:`` line (fields + out of order) must be SKIPPED, never raise. A well-formed line in the same + payload must still parse.""" + from pdd.agentic_sync_runner import _parse_public_surface_failure + + stderr = ( + "Public surface regression for demo_Python.prompt:\n" + "removed: \n" + "signature_changed: good, bad\n" + "output: pdd/demo.py\n" + "pre_surface_size: 2\n" + "post_surface_size: 2\n" + # Well-formed. + "signature_detail: good | expected: [function] (a) | " + "actual: [function] (a, b) | source: pdd-interface\n" + # Malformed: fields out of order -> must be skipped, not crash. + "signature_detail: bad | source: pdd-interface | " + "actual: [function] (a) | expected: [function] (b)\n" + ) + parsed = _parse_public_surface_failure("", stderr) + assert parsed is not None + directive, _signature = parsed + # The well-formed detail survives with its declared target line. + assert "Restore `good` to its declared signature `[function] (a)`" in directive + # The malformed line contributed no declared-target line. + assert "Restore `bad`" not in directive From 3632d6a2e6c0e29064cd503f91fd8bd5ef197523 Mon Sep 17 00:00:00 2001 From: "prompt-driven-github-staging[bot]" Date: Tue, 7 Jul 2026 23:37:31 +0000 Subject: [PATCH 02/64] chore: auto-heal prompt/example drift for agentic_sync_runner, code_generator_main PDD-Auto-Heal-Checkpoint: success --- .pdd/meta/agentic_sync_runner_python.json | 20 +- .pdd/meta/agentic_sync_runner_python_run.json | 11 - .pdd/meta/code_generator_main_python.json | 26 +- architecture.json | 60 ++--- context/agentic_sync_runner_example.py | 244 ++++++++---------- context/code_generator_main_example.py | 213 +++++++-------- 6 files changed, 278 insertions(+), 296 deletions(-) delete mode 100644 .pdd/meta/agentic_sync_runner_python_run.json diff --git a/.pdd/meta/agentic_sync_runner_python.json b/.pdd/meta/agentic_sync_runner_python.json index 9a568d6680..5eb2852d6e 100644 --- a/.pdd/meta/agentic_sync_runner_python.json +++ b/.pdd/meta/agentic_sync_runner_python.json @@ -1,21 +1,21 @@ { - "pdd_version": "0.0.275.dev3", - "timestamp": "2026-06-16T01:31:18.161464+00:00", - "command": "fix", - "prompt_hash": "0784d6ff7f749b74e6938bfdd945a1cf97aed961ebea4bf3849fe512bf7163f9", - "code_hash": "587d51aaed8470de39bbefbb0882f481f269431cca8b50e45b22e90dbd2eb695", - "example_hash": "fcb36209abb5ab882ad551754fb574d62509c968f8c564e18163be7f2b7ef35a", - "test_hash": "781deb083113a3c96b990501f24cdf41e0742190be5cfbe85553639c25e62e22", + "pdd_version": "0.0.297", + "timestamp": "2026-07-07T23:36:35.744781+00:00", + "command": "example", + "prompt_hash": "8ae27e94b72b3297e48b5ab0a8d688421c345261dc2c63e841936e7dde2fb2a6", + "code_hash": "761da3c140761897bd34db51adb43c05f77e97a9d4aa152cc9811c213a766e8b", + "example_hash": "2c4972be9247a4f5db6653838fd3047b9f3730e3640bf8e0152fc97c55ce590b", + "test_hash": "06af82e118266ab4a61ae055da174f1597041accd7d8f4b479030170182954c6", "test_files": { - "test_agentic_sync_runner.py": "781deb083113a3c96b990501f24cdf41e0742190be5cfbe85553639c25e62e22" + "test_agentic_sync_runner.py": "06af82e118266ab4a61ae055da174f1597041accd7d8f4b479030170182954c6" }, "include_deps": { "context/agentic_change_example.py": "a8308b2af708c804d626a4a96012c5a67f5bc6469e33857cafc07627213828d7", - "context/agentic_common_example.py": "f9cdb30a932973d341b795faf510056b95573610af5ac48e3849cf42c205998e", + "context/agentic_common_example.py": "c1538a115a3a33da228e7f6cc048f2a99023cd2c83a0efef382e49c477b24ad3", "context/agentic_langtest_example.py": "dbc5c87dc1b75d3299dbfb67b17df1838f9ba2c9bdb457097df0832cea112c3f", "context/agentic_test_orchestrator_example.py": "473901f6f62844c81a047fa69387c4116d91a522c62d5c6ad30188e2e99dfaa5", "context/architecture_sync_example.py": "b6158dbc7ca3fd32665528562225f6739ec521a071d845acab0f7caaea221ae0", "context/python_preamble.prompt": "0388ed131bf986f8752e1bc4c81e4da0460cfe2908ec8c60b1314edbab768254", "context/sync_order_example.py": "b4eb1e167e7604c7dbac703bdcba7287188fa9d2d5ef6d490216d003cdc542fa" } -} +} \ No newline at end of file diff --git a/.pdd/meta/agentic_sync_runner_python_run.json b/.pdd/meta/agentic_sync_runner_python_run.json deleted file mode 100644 index 7ebd1c4535..0000000000 --- a/.pdd/meta/agentic_sync_runner_python_run.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "timestamp": "2026-06-16T01:31:18.161464+00:00", - "exit_code": 0, - "tests_passed": 180, - "tests_failed": 0, - "coverage": 81.25, - "test_hash": "781deb083113a3c96b990501f24cdf41e0742190be5cfbe85553639c25e62e22", - "test_files": { - "test_agentic_sync_runner.py": "781deb083113a3c96b990501f24cdf41e0742190be5cfbe85553639c25e62e22" - } -} diff --git a/.pdd/meta/code_generator_main_python.json b/.pdd/meta/code_generator_main_python.json index 0f74e423fd..d3399aa657 100644 --- a/.pdd/meta/code_generator_main_python.json +++ b/.pdd/meta/code_generator_main_python.json @@ -1,29 +1,29 @@ { - "pdd_version": "0.0.268", - "timestamp": "2026-06-09T23:30:32.073262+00:00", + "pdd_version": "0.0.297", + "timestamp": "2026-07-07T23:37:30.225239+00:00", "command": "example", - "prompt_hash": "9c646810b3e55fa6ba5c0bb1d8210e459446ffa6826c565d7012ea6bd55fb6d5", - "code_hash": "1293a25723fdcb7870711b4759248fe4681d5107cf2106feff61df57cb56ee96", - "example_hash": "25bb0d4b5fe4f31d0d4d3795d24e8e9bb2cd346c74e4fc916d0a9ba8590141be", - "test_hash": "3cc05a05384f982254fdb38d8a156fc7507a1dfec200fc91705d01fbbfad134d", + "prompt_hash": "2f34372fa6e5f4a7bc560fac698606b925e11d89c4e9bad76a501b40221e5fb0", + "code_hash": "f72c7dd444e88cdb0a77fe3cdf9b99e51aeb1c0371561f4a3c0f3204360be262", + "example_hash": "48d9a2e1435aec9cfd7e8071620407974e28df6dd1ceb7224caf94eec698ecf2", + "test_hash": "21a0833e886cc5aa31973c709c518145421bac0fe5f45115bc852268e859096c", "test_files": { - "test_code_generator_main.py": "3cc05a05384f982254fdb38d8a156fc7507a1dfec200fc91705d01fbbfad134d" + "test_code_generator_main.py": "21a0833e886cc5aa31973c709c518145421bac0fe5f45115bc852268e859096c" }, "include_deps": { "context/__init__example.py": "84208a445d03a336e465709ec0687eb969c8d865e6739bf8b79f2c8a8aad351a", "context/_keyring_timeout_example.py": "41f83d5ef117caba86b788f69b519f7e88f3d7dc2a641a017d74a7cee9be6ed1", "context/agentic_langtest_example.py": "dbc5c87dc1b75d3299dbfb67b17df1838f9ba2c9bdb457097df0832cea112c3f", "context/api_key_scanner_example.py": "4b9f880a30445f25edffc394857ae0c218097726c41ab996a03fcbc4350b5482", - "context/architecture_sync_example.py": "6e1f65de35fcf1dbd0def81710da6f378495b37187eb01cd01ef43f662d9f1ba", + "context/architecture_sync_example.py": "b6158dbc7ca3fd32665528562225f6739ec521a071d845acab0f7caaea221ae0", "context/auth_service_example.py": "4b6d2c8c9e13d1edc0d9805ee0c07657494cbf31dffe2bb0029b72975674a8cf", "context/auto_include_example.py": "0c278ed716298149777bc7082827eb8aa6d80dac264c66b6cded6871232f2df7", "pdd/code_generator.py": "33235364b431edbb0fb1c4d8eb03fa21a3b776e07ea585530c19212da20c486d", - "pdd/code_generator_main.py": "1293a25723fdcb7870711b4759248fe4681d5107cf2106feff61df57cb56ee96", - "pdd/construct_paths.py": "832d478849bfe83c12d4a473c270902d0af0be577ae3276b07ef8a48c148d35f", + "pdd/code_generator_main.py": "f72c7dd444e88cdb0a77fe3cdf9b99e51aeb1c0371561f4a3c0f3204360be262", + "pdd/construct_paths.py": "7b42a002e4ecc27be4d517cbc07f1fc9425e795d4918714f382d2ed5ddd00eab", "pdd/core/cloud.py": "0487c0b989996af144df3af1c818a6eeafe52fd209710e5a54eb43990c89ae97", - "pdd/incremental_code_generator.py": "266a4bed6ea41519a1b1c3c33057cb33cffecd2a3075ee0c2f790798bfff4834", - "pdd/preprocess.py": "a6c86dd348d0028a0638114338013e4633fb66fa51f756bc30458f4dce833703", + "pdd/incremental_code_generator.py": "8dd5dd7f3016b8fae86056484fdc49b824405cdcd6911e8b8657d3e043b3b06d", + "pdd/preprocess.py": "d04a6cfc261c1e88076398a34d0bf96c4285061668866c5ad305e4b605ffdf56", "pdd/python_env_detector.py": "cbe4044a83cd88a683f36d6ecfca245fef6a03ea2abc7f5c407636fccc051f35", - "prompts/context_snapshot_python.prompt": "d8e129b55b27814d16e7566f07dfdaae0cc6950352f351fd78cfeeab125a7cbf" + "prompts/context_snapshot_python.prompt": "05f46fa9adba7ed3c9c6088d150ad821c723e9d0a3efed9db2fe8a461386da9d" } } \ No newline at end of file diff --git a/architecture.json b/architecture.json index 8f9930723c..7077a62f9a 100644 --- a/architecture.json +++ b/architecture.json @@ -697,7 +697,7 @@ } }, { - "reason": "Daily Rising Stars resurface check \u2014 Cloud Scheduler function scanning contribution thresholds to identify high-potential nurtured candidates and detect churned candidates", + "reason": "Daily Rising Stars resurface check — Cloud Scheduler function scanning contribution thresholds to identify high-potential nurtured candidates and detect churned candidates", "description": "Cloud Function entry point recruiting_resurface_check triggered by Cloud Scheduler (daily). Scans all recruiting/candidates with crmFunnelStage in ('nurture', 'contributing'). For each candidate, evaluates four resurface criteria from config thresholds: (1) merged_prs >= threshold (e.g., 3 merged PRs), (2) hackathon_score >= threshold (e.g., top 25%), (3) platform_usage_days >= threshold (power user), (4) marketplace_examples >= threshold (marketplace contributor). Candidates meeting any criterion are marked resurfaced=true with appropriate ResurfaceReason, CrmFunnelStage updated to 'resurfaced', and CRM GitHub Issue label updated from recruiting-nurture to recruiting-resurfaced. Also detects churned candidates: if last_activity_at > 60 days ago and no contributions, sets crmFunnelStage='churned' and updates CRM label to recruiting-churned. Verifies merged PR counts via GitHub API to prevent gaming. Logs all resurface and churn transitions. Returns summary counts.", "dependencies": [], "priority": 18, @@ -2218,7 +2218,7 @@ }, { "reason": "Primary orchestration engine for transitioning .prompt files to source code, handling path resolution, incremental generation, and architectural conformance.", - "description": "Command-line interface for code generator. Parses arguments, orchestrates the workflow, and formats output. Raises structured ArchitectureConformanceError failures with output/expected/found/missing fields plus failed-attempt total_cost/model_name (constructor accepts an optional repair_directive override for the signature check whose source of truth is the prompt, not architecture.json), and injects a non-empty PDD_REPAIR_DIRECTIVE into the generation prompt inside an block so sync retry attempts receive concrete missing-export instructions. Architecture conformance now also enforces the prompt's signature across module/cli/command interface types in three categories: (1) missing function/method \u2014 declared callables (including dotted methods like ContentSelector.select) absent from the generated code surface as bare names in missing_symbols; (2) missing parameter \u2014 declared parameter names absent from a matching function/method signature surface as dotted funcname.paramname entries; (3) signature drift \u2014 annotation drift is conservative (raises only when both sides specify and differ) while default drift is strict (raises when the prompt declares a default and the generated code drops or changes it, since callers omitting an optional kwarg would otherwise break with TypeError). Each category emits a distinct error sentence so the agentic_sync_runner repair loop can build a targeted directive (add missing function/method, add missing parameter, or update parameter to match the prompt's annotation/default). Issue #1012 adds two additional repair-loop gates: (a) PublicSurfaceRegressionError \u2014 for mature modules (pre-existing non-empty code file), snapshots top-level functions, classes, recursively-walked nested classes/methods (so a method on `Outer.Inner` is captured as `Outer.Inner.method`), and module-level public `ast.Assign` / bound `ast.AnnAssign` (only when `node.value is not None`; bare annotations like `PUBLIC_NAME: int` don't bind a runtime attribute and are ignored) targets plus `ast.Import`/`ast.ImportFrom` aliases (so removing a re-export like `import git` or a public constant like `PUBLIC_SETTING = ...` surfaces as a regression). When the module declares `__all__` as a clean list/tuple of string constants, that list is AUTHORITATIVE per Python semantics: a name is public iff it appears in `__all__`, even if underscore-prefixed; names NOT in `__all__` are NOT public. Classes listed in `__all__` ALSO contribute their recursively-walked members (methods, nested classes, methods on nested classes at every depth) regardless of underscore prefix \u2014 so `__all__ = [\"Service\"]` protects `Service.run`, `Service.Inner`, and `Service.Inner.method`. Computed `__all__` (`sorted(...)`, `X + Y`) or AugAssign extension (`__all__ += [...]`) falls back to the heuristic. Without `__all__`, underscore-prefixed names (including dunders) are excluded at every level; `from X import *` contributes no fixed name and is ignored. The same `__all__` precedence rule (including the class-member recursion) is applied to the parallel `_snapshot_public_signatures` so signature-drift detection sees the same public set. `_snapshot_public_signatures` entries carry a leading kind prefix (`[function]`/`[async_function]`/`[class]` at top level; `[instance]`/`[staticmethod]`/`[classmethod]`/`[property:]` inside classes; `[assignment]` for module-level rebindings; and `[import]`/`[import:]`/`[import:from ]`/`[import:from :]` for re-exports) so a binding-kind flip (e.g. a re-exported `pathlib.Path` replaced by a same-named `def`) diffs even when the parameter list happens to match. For `@dataclass` classes without an explicit `__init__`, `_snapshot_public_signatures` synthesises the constructor signature from class-body `AnnAssign` fields in source order: `@dataclass(kw_only=True)` emits a leading `*` so every field is kw-only; an in-body `_: KW_ONLY` sentinel (`KW_ONLY` Name or `dataclasses.KW_ONLY` Attribute) splits positional vs kw-only fields and is recognised before the underscore-prefix skip; when both decorator and sentinel are present only ONE `*` is emitted (decorator wins); `InitVar[...]` fields ARE included (they are runtime constructor params even though not stored as instance attributes); `ClassVar[...]` fields and `field(init=False, ...)` defaults are excluded; an explicit `__init__` still wins over synthesis. Inherited dataclass fields are collected in REVERSE-MRO order (matching Python's `@dataclass`: `__dataclass_fields__` is populated by walking bases right-to-left so later bases overwrite earlier ones in dict-insertion order) \u2014 `class C(A, B)` synthesises `(b, a, c)`. A base decorated with `@dataclass(init=False)` STILL contributes its annotated fields to a derived `@dataclass`'s synth; `init=False` only suppresses the base's OWN `__init__` synthesis. Cross-module bases that resolve to imports rather than same-module `ClassDef`s are marked `[inherited_unresolved]`. When a derived class redeclares a base's field name, the derived annotation/default wins but the field's POSITION matches the base's original slot (insertion-order dict semantics). The gate raises when public symbols are removed/renamed or their callable signature changes unless the prompt body contains an anchored `BREAKING-CHANGE:` directive (regex `^\\s*BREAKING-CHANGE:\\s*` with `re.MULTILINE`) whose tail starts with a `remove`/`delete`/`drop`/`rename` action verb (followed by a comma-separated symbol list) for removals or a `change signature`/`change api`/`change contract` verb pair for signature drift. Buried mid-line marker mentions and bare `BREAKING-CHANGE:` markers do NOT disable the gate. First-time generation is exempt. (b) TestChurnError \u2014 when overwriting an existing non-empty test file through code_generator_main or cmd_test_main, computes the stdlib `difflib.unified_diff` churn ratio between pre and post test bodies and raises when the ratio exceeds `PDD_TEST_CHURN_THRESHOLD` (default 0.40, accepts both decimal `0.40` and percent `40%` strings; defensive fallback to default on unparseable values) without an anchored `BREAKING-CHANGE:` directive that pairs an opt-out verb (`rewrite`/`replace`/`regenerate`/`overwrite`/`churn`/`remove`/`drop`) with `tests` as the verb's direct object. Both new exceptions share the existing PDD_REPAIR_DIRECTIVE plumbing so sync_main, sync_orchestration, and agentic_sync_runner route them through the same repair loop. The conformance and public-surface / test-churn gate-call sites in `code_generator_main` gate on `generated_code_content is not None` (NOT a truthy check) so an empty provider response over an existing module / test file still trips PublicSurfaceRegressionError / TestChurnError instead of silently truncating to 0 bytes; for non-Python artifacts the gates cannot inspect (JSON, YAML, prompts, etc.) `code_generator_main` additionally raises `click.UsageError(\"Refusing to overwrite ...\")` immediately before the write when existing content is non-empty and the generated content is empty/whitespace-only. `PDD_ALLOW_EMPTY_GENERATION=1` is the explicit escape hatch for the rare intentional empty-output case.", + "description": "Command-line interface for code generator. Parses arguments, orchestrates the workflow, and formats output. Raises structured ArchitectureConformanceError failures with output/expected/found/missing fields plus failed-attempt total_cost/model_name (constructor accepts an optional repair_directive override for the signature check whose source of truth is the prompt, not architecture.json), and injects a non-empty PDD_REPAIR_DIRECTIVE into the generation prompt inside an block so sync retry attempts receive concrete missing-export instructions. Architecture conformance now also enforces the prompt's signature across module/cli/command interface types in three categories: (1) missing function/method — declared callables (including dotted methods like ContentSelector.select) absent from the generated code surface as bare names in missing_symbols; (2) missing parameter — declared parameter names absent from a matching function/method signature surface as dotted funcname.paramname entries; (3) signature drift — annotation drift is conservative (raises only when both sides specify and differ) while default drift is strict (raises when the prompt declares a default and the generated code drops or changes it, since callers omitting an optional kwarg would otherwise break with TypeError). Each category emits a distinct error sentence so the agentic_sync_runner repair loop can build a targeted directive (add missing function/method, add missing parameter, or update parameter to match the prompt's annotation/default). Issue #1012 adds two additional repair-loop gates: (a) PublicSurfaceRegressionError — for mature modules (pre-existing non-empty code file), snapshots top-level functions, classes, recursively-walked nested classes/methods (so a method on `Outer.Inner` is captured as `Outer.Inner.method`), and module-level public `ast.Assign` / bound `ast.AnnAssign` (only when `node.value is not None`; bare annotations like `PUBLIC_NAME: int` don't bind a runtime attribute and are ignored) targets plus `ast.Import`/`ast.ImportFrom` aliases (so removing a re-export like `import git` or a public constant like `PUBLIC_SETTING = ...` surfaces as a regression). When the module declares `__all__` as a clean list/tuple of string constants, that list is AUTHORITATIVE per Python semantics: a name is public iff it appears in `__all__`, even if underscore-prefixed; names NOT in `__all__` are NOT public. Classes listed in `__all__` ALSO contribute their recursively-walked members (methods, nested classes, methods on nested classes at every depth) regardless of underscore prefix — so `__all__ = [\"Service\"]` protects `Service.run`, `Service.Inner`, and `Service.Inner.method`. Computed `__all__` (`sorted(...)`, `X + Y`) or AugAssign extension (`__all__ += [...]`) falls back to the heuristic. Without `__all__`, underscore-prefixed names (including dunders) are excluded at every level; `from X import *` contributes no fixed name and is ignored. The same `__all__` precedence rule (including the class-member recursion) is applied to the parallel `_snapshot_public_signatures` so signature-drift detection sees the same public set. `_snapshot_public_signatures` entries carry a leading kind prefix (`[function]`/`[async_function]`/`[class]` at top level; `[instance]`/`[staticmethod]`/`[classmethod]`/`[property:]` inside classes; `[assignment]` for module-level rebindings; and `[import]`/`[import:]`/`[import:from ]`/`[import:from :]` for re-exports) so a binding-kind flip (e.g. a re-exported `pathlib.Path` replaced by a same-named `def`) diffs even when the parameter list happens to match. For `@dataclass` classes without an explicit `__init__`, `_snapshot_public_signatures` synthesises the constructor signature from class-body `AnnAssign` fields in source order: `@dataclass(kw_only=True)` emits a leading `*` so every field is kw-only; an in-body `_: KW_ONLY` sentinel (`KW_ONLY` Name or `dataclasses.KW_ONLY` Attribute) splits positional vs kw-only fields and is recognised before the underscore-prefix skip; when both decorator and sentinel are present only ONE `*` is emitted (decorator wins); `InitVar[...]` fields ARE included (they are runtime constructor params even though not stored as instance attributes); `ClassVar[...]` fields and `field(init=False, ...)` defaults are excluded; an explicit `__init__` still wins over synthesis. Inherited dataclass fields are collected in REVERSE-MRO order (matching Python's `@dataclass`: `__dataclass_fields__` is populated by walking bases right-to-left so later bases overwrite earlier ones in dict-insertion order) — `class C(A, B)` synthesises `(b, a, c)`. A base decorated with `@dataclass(init=False)` STILL contributes its annotated fields to a derived `@dataclass`'s synth; `init=False` only suppresses the base's OWN `__init__` synthesis. Cross-module bases that resolve to imports rather than same-module `ClassDef`s are marked `[inherited_unresolved]`. When a derived class redeclares a base's field name, the derived annotation/default wins but the field's POSITION matches the base's original slot (insertion-order dict semantics). The gate raises when public symbols are removed/renamed or their callable signature changes unless the prompt body contains an anchored `BREAKING-CHANGE:` directive (regex `^\\s*BREAKING-CHANGE:\\s*` with `re.MULTILINE`) whose tail starts with a `remove`/`delete`/`drop`/`rename` action verb (followed by a comma-separated symbol list) for removals or a `change signature`/`change api`/`change contract` verb pair for signature drift. Buried mid-line marker mentions and bare `BREAKING-CHANGE:` markers do NOT disable the gate. First-time generation is exempt. (b) TestChurnError — when overwriting an existing non-empty test file through code_generator_main or cmd_test_main, computes the stdlib `difflib.unified_diff` churn ratio between pre and post test bodies and raises when the ratio exceeds `PDD_TEST_CHURN_THRESHOLD` (default 0.40, accepts both decimal `0.40` and percent `40%` strings; defensive fallback to default on unparseable values) without an anchored `BREAKING-CHANGE:` directive that pairs an opt-out verb (`rewrite`/`replace`/`regenerate`/`overwrite`/`churn`/`remove`/`drop`) with `tests` as the verb's direct object. Both new exceptions share the existing PDD_REPAIR_DIRECTIVE plumbing so sync_main, sync_orchestration, and agentic_sync_runner route them through the same repair loop. The conformance and public-surface / test-churn gate-call sites in `code_generator_main` gate on `generated_code_content is not None` (NOT a truthy check) so an empty provider response over an existing module / test file still trips PublicSurfaceRegressionError / TestChurnError instead of silently truncating to 0 bytes; for non-Python artifacts the gates cannot inspect (JSON, YAML, prompts, etc.) `code_generator_main` additionally raises `click.UsageError(\"Refusing to overwrite ...\")` immediately before the write when existing content is non-empty and the generated content is empty/whitespace-only. `PDD_ALLOW_EMPTY_GENERATION=1` is the explicit escape hatch for the rare intentional empty-output case.", "dependencies": [ "construct_paths_python.prompt", "preprocess_python.prompt", @@ -4180,8 +4180,8 @@ } }, { - "reason": "Generates the task_routing.csv skeleton: per-task static routing table for the E[pass]\u2212\u03bb\u00b7cost router in llm_invoke.", - "description": "Defines the static per-task config routing table (task_class, model, temperature, effort, shots, pass_rate, avg_cost_usd) consumed by llm_invoke()'s E[pass]\u2212\u03bb\u00b7cost router when PDD_ENABLE_TASK_ROUTING=1.", + "reason": "Generates the task_routing.csv skeleton: per-task static routing table for the E[pass]−λ·cost router in llm_invoke.", + "description": "Defines the static per-task config routing table (task_class, model, temperature, effort, shots, pass_rate, avg_cost_usd) consumed by llm_invoke()'s E[pass]−λ·cost router when PDD_ENABLE_TASK_ROUTING=1.", "dependencies": [ "llm_invoke_python.prompt" ], @@ -5979,7 +5979,7 @@ }, { "reason": "Orchestrates the 13-step agentic change workflow for implementing GitHub issues, with optional clean restart.", - "description": "Orchestrates the 13-step agentic change workflow. Supports `clean_restart=True` to ignore any persisted solving state for the issue and start a fresh full pdd-issue flow from the target base branch (issue #1149). Includes Step 8.5 (pre-flight drift heal) \u2014 detects prompts whose code has drifted and runs `pdd update` per module inside the worktree before Step 9 rewrites the prompts. Includes Step 10.5 (doc-sync contract verifier) \u2014 before Step 10, calls pdd.sync_order.discover_associated_documents to populate the LLM's associated_documents context, using the authoritative changed-file set so Step 9's worktree fallback path cannot bypass discovery when FILES_* markers are missing; after Step 10, enforces that every discovered doc appears in exactly one of ASSOCIATED_DOCS_MODIFIED / ASSOCIATED_DOCS_CONFLICTS / ASSOCIATED_DOCS_UNCHANGED. Silent drops and bucket overlaps are appended as ORCHESTRATOR_POSTCHECK_WARNINGS and routed to Step 11 via step10_output; PDD_STRICT_DOC_SYNC=1 turns violations into hard workflow aborts (issue #739).", + "description": "Orchestrates the 13-step agentic change workflow. Supports `clean_restart=True` to ignore any persisted solving state for the issue and start a fresh full pdd-issue flow from the target base branch (issue #1149). Includes Step 8.5 (pre-flight drift heal) — detects prompts whose code has drifted and runs `pdd update` per module inside the worktree before Step 9 rewrites the prompts. Includes Step 10.5 (doc-sync contract verifier) — before Step 10, calls pdd.sync_order.discover_associated_documents to populate the LLM's associated_documents context, using the authoritative changed-file set so Step 9's worktree fallback path cannot bypass discovery when FILES_* markers are missing; after Step 10, enforces that every discovered doc appears in exactly one of ASSOCIATED_DOCS_MODIFIED / ASSOCIATED_DOCS_CONFLICTS / ASSOCIATED_DOCS_UNCHANGED. Silent drops and bucket overlaps are appended as ORCHESTRATOR_POSTCHECK_WARNINGS and routed to Step 11 via step10_output; PDD_STRICT_DOC_SYNC=1 turns violations into hard workflow aborts (issue #739).", "dependencies": [ "architecture_sync_python.prompt", "agentic_common_python.prompt", @@ -7526,7 +7526,7 @@ }, { "reason": "Global and GitHub issue-driven module identification plus parallel sync orchestration.", - "description": "Entry point for no-argument global sync and agentic issue sync. Global sync scans architecture.json for stale/missing modules and dispatches AsyncSyncRunner in dependency order; issue sync parses a GitHub issue URL, identifies modules, validates dependencies, and dispatches AsyncSyncRunner \u2014 or DurableSyncRunner when invoked with durable=True. Issue sync uses build_dep_graph_from_architecture_data against in-memory combined architecture so nested-architecture edges are preserved (Phase 0 of #1328).", + "description": "Entry point for no-argument global sync and agentic issue sync. Global sync scans architecture.json for stale/missing modules and dispatches AsyncSyncRunner in dependency order; issue sync parses a GitHub issue URL, identifies modules, validates dependencies, and dispatches AsyncSyncRunner — or DurableSyncRunner when invoked with durable=True. Issue sync uses build_dep_graph_from_architecture_data against in-memory combined architecture so nested-architecture edges are preserved (Phase 0 of #1328).", "dependencies": [ "architecture_sync_python.prompt", "auto_deps_main_python.prompt", @@ -7587,8 +7587,8 @@ } }, { - "reason": "Parallel dependency-aware sync runner that forwards compressed-context sync settings.", - "description": "Manages pdd sync subprocesses using a ThreadPoolExecutor whose worker count is read by _read_sync_max_workers() from the PDD_SYNC_MAX_WORKERS env var at construction (default 4, non-integer falls back to 4, clamped to 1-4, forced to 1 when total_budget is set; MAX_WORKERS stays a backwards-compatible module constant), preserves architecture subdirectory basenames, strips architecture filename/target language suffixes via PDD's known-language catalog so compound languages such as TypeScriptReact and JavaScriptReact resolve correctly, respects dependency ordering from architecture.json, enforces total-budget sequential execution when requested, shrinks each child/retry budget by persisted module costs plus current-module in-flight retry cost, posts live progress to GitHub issue comments with a total body cap below GitHub's comment limit, keeps unrelated ready modules running after a module failure while blocking transitive failed-dependency dependents, and stops all new scheduling when total budget is exhausted. Detects three repairable generation failures from per-module sync subprocesses \u2014 architecture-conformance ('Architecture conformance error for ' line prefix, parsed by _parse_conformance_failure), public-surface regression ('Public surface regression for ' line prefix, parsed by _parse_public_surface_failure, issue #1012), and test-churn ('Test churn threshold exceeded for ' line prefix, parsed by _parse_test_churn_failure, issue #1012) \u2014 and retries each up to MAX_CONFORMANCE_ATTEMPTS=3 with a PDD_REPAIR_DIRECTIVE env var that names the exact offending symbols / churn ratio and forbids editing architecture.json or rewriting unrelated tests. Every child/retry forwards the parent's resolved .pddrc context, --strength, and --temperature on the retry command so a flaky regeneration cannot silently switch model or context between attempts. Stops retrying before launching another repair child if the in-flight repair cost exhausts total_budget, short-circuits when the failure signature (sorted missing/removed symbols, or (rounded churn_ratio, pre_line_count)) repeats, and on hard failure records the matching structured block \u2014 '=== architecture conformance failure ===', '=== public surface regression ===', or '=== test churn threshold exceeded ===' (built by build_conformance_hard_failure_from_error / build_public_surface_hard_failure_from_error / build_test_churn_hard_failure_from_error, all importable from pdd.agentic_sync_runner so sync_main and sync_orchestration reuse them verbatim) \u2014 plus a 'Reproduce locally:' line and an '--- env ---' fingerprint (pdd.__file__, pdd --version, git SHA, dirty flag, repo-vs-site-packages) so the GitHub App surface is actionable without exceeding GitHub comment size limits. Bounds per-attempt child stdout/stderr capture with a _BoundedTextCapture tail (capped by STDOUT_CAPTURE_LINE_LIMIT=5000 lines and STDOUT_CAPTURE_BYTE_LIMIT=1 MiB per stream, allocated fresh each attempt) so verbose repair retries cannot grow memory unbounded and trigger SIGKILL on Cloud Run; reads child pipes as bytes and decodes UTF-8 incrementally; tracks dropped line/byte counts surfaced via _log_dropped_output() and appended to failure/timeout summaries without injecting diagnostics into captured child output. The 'Overall status: ... Failed' verdict is recorded as the stdout line streams in (sticky), so a failure marker evicted from the bounded tail by trailing output still yields a failed verdict and a correct failure reason.", + "reason": "Parallel dependency-aware sync runner that forwards compressed-context sync settings and resolves each child's --context against its own module cwd's .pddrc.", + "description": "Manages pdd sync subprocesses using a ThreadPoolExecutor whose worker count is read by _read_sync_max_workers() from the PDD_SYNC_MAX_WORKERS env var at construction (default 4, non-integer falls back to 4, clamped to 1-4, forced to 1 when total_budget is set; MAX_WORKERS stays a backwards-compatible module constant), preserves architecture subdirectory basenames, strips architecture filename/target language suffixes via PDD's known-language catalog so compound languages such as TypeScriptReact and JavaScriptReact resolve correctly, respects dependency ordering from architecture.json, enforces total-budget sequential execution when requested, shrinks each child/retry budget by persisted module costs plus current-module in-flight retry cost, posts live progress to GitHub issue comments with a total body cap below GitHub's comment limit, keeps unrelated ready modules running after a module failure while blocking transitive failed-dependency dependents, and stops all new scheduling when total budget is exhausted. Detects three repairable generation failures from per-module sync subprocesses — architecture-conformance ('Architecture conformance error for ' line prefix, parsed by _parse_conformance_failure), public-surface regression ('Public surface regression for ' line prefix, parsed by _parse_public_surface_failure, issue #1012), and test-churn ('Test churn threshold exceeded for ' line prefix, parsed by _parse_test_churn_failure, issue #1012) — and retries each up to MAX_CONFORMANCE_ATTEMPTS=3 with a PDD_REPAIR_DIRECTIVE env var that names the exact offending symbols / churn ratio and forbids editing architecture.json or rewriting unrelated tests. Every child/retry forwards the parent's resolved .pddrc context, --strength, and --temperature on the retry command so a flaky regeneration cannot silently switch model or context between attempts. Stops retrying before launching another repair child if the in-flight repair cost exhausts total_budget, short-circuits when the failure signature (sorted missing/removed symbols, or (rounded churn_ratio, pre_line_count)) repeats, and on hard failure records the matching structured block — '=== architecture conformance failure ===', '=== public surface regression ===', or '=== test churn threshold exceeded ===' (built by build_conformance_hard_failure_from_error / build_public_surface_hard_failure_from_error / build_test_churn_hard_failure_from_error, all importable from pdd.agentic_sync_runner so sync_main and sync_orchestration reuse them verbatim) — plus a 'Reproduce locally:' line and an '--- env ---' fingerprint (pdd.__file__, pdd --version, git SHA, dirty flag, repo-vs-site-packages) so the GitHub App surface is actionable without exceeding GitHub comment size limits. Bounds per-attempt child stdout/stderr capture with a _BoundedTextCapture tail (capped by STDOUT_CAPTURE_LINE_LIMIT=5000 lines and STDOUT_CAPTURE_BYTE_LIMIT=1 MiB per stream, allocated fresh each attempt) so verbose repair retries cannot grow memory unbounded and trigger SIGKILL on Cloud Run; reads child pipes as bytes and decodes UTF-8 incrementally; tracks dropped line/byte counts surfaced via _log_dropped_output() and appended to failure/timeout summaries without injecting diagnostics into captured child output. The 'Overall status: ... Failed' verdict is recorded as the stdout line streams in (sticky), so a failure marker evicted from the bounded tail by trailing output still yields a failed verdict and a correct failure reason.", "dependencies": [ "architecture_sync_python.prompt", "agentic_langtest_python.prompt", @@ -7821,20 +7821,20 @@ "When a budget cap is crossed during a successful fixer turn, still commits and pushes the completed fixes before stopping prior to verifier execution", "Feeds fixer rejections back to the primary reviewer and keeps reviewer-rejected findings open until fixed or max rounds", "Issue #1088 SHA-backed verification trust boundary: captures the post-push HEAD SHA via _git_rev_parse_head (never inferred from fixer prose), populates FixResult.push_status (pushed | push_failed | not_attempted), FixResult.local_fixer_commit_sha, and FixResult.pushed_head_sha after every fix turn, and rewrites the per-round fix findings.json artifact so the on-disk audit trail carries those fields. The verifier only runs when push_status == pushed AND a non-empty pushed_head_sha was observed; otherwise the round is marked skipped/unverified and the loop stops without claiming the findings as fixed. A clean verify pass pins state.verified_head_sha to that pushed SHA", - "Issue #1088 final stale-head re-fetch: _finalize calls _fetch_pr_metadata exactly once at render to read the live remote head_sha whenever there is anything to verify \u2014 state.verified_head_sha is set (a verifier pinned a SHA), state.fresh_final_status == clean (a reviewer cleared the actual worktree HEAD without a subsequent fixer push), any fix was pushed, or any finding is marked fixed. The comparison target is state.verified_head_sha when a verifier ran clean, otherwise the most recent FixResult.pushed_head_sha for partial verifier acceptance, otherwise state.reviewed_head_sha captured from git rev-parse HEAD in the worktree the reviewer inspected (never from later PR metadata). If the remote head differs from the comparison target, the comparison target was never observed, or the re-fetch returned no head_sha, downgrades fresh_final_status to missing, downgrades every verification_status_by_round entry from verified to stale, and reverts every findings_by_key entry from fixed to open so final-state.json cannot present a stale verdict. state.final_refetch_attempted is set so the render layer can distinguish a failed re-fetch (remote-pr-head-sha: unknown) from no re-fetch at all (remote-pr-head-sha: none)", + "Issue #1088 final stale-head re-fetch: _finalize calls _fetch_pr_metadata exactly once at render to read the live remote head_sha whenever there is anything to verify — state.verified_head_sha is set (a verifier pinned a SHA), state.fresh_final_status == clean (a reviewer cleared the actual worktree HEAD without a subsequent fixer push), any fix was pushed, or any finding is marked fixed. The comparison target is state.verified_head_sha when a verifier ran clean, otherwise the most recent FixResult.pushed_head_sha for partial verifier acceptance, otherwise state.reviewed_head_sha captured from git rev-parse HEAD in the worktree the reviewer inspected (never from later PR metadata). If the remote head differs from the comparison target, the comparison target was never observed, or the re-fetch returned no head_sha, downgrades fresh_final_status to missing, downgrades every verification_status_by_round entry from verified to stale, and reverts every findings_by_key entry from fixed to open so final-state.json cannot present a stale verdict. state.final_refetch_attempted is set so the render layer can distinguish a failed re-fetch (remote-pr-head-sha: unknown) from no re-fetch at all (remote-pr-head-sha: none)", "Qualifies all unverified fixer rationale prose in the final report as fixer= fixer_disposition= / fixer_rationale= and appends verification=unverified, so bare fixer claims such as 'claude: fixed - ...' never appear as verifier evidence", "Writes per-round prompt/output/normalized-findings/dedup-state artifacts and a final-state.json under .pdd/checkup-review-loop/issue-{N}-pr-{M}/. final-state.json includes same_role_review_fix, mode, verified_head_sha, remote_pr_head_sha, reviewed_head_sha, source_of_truth, gates, and verification_status_by_round so downstream consumers can re-verify the trust boundary and distinguish independent reviewer/fixer runs from explicit single-role review/fix runs", "Posts the final report to the source issue and PR only when use_github_state=True (writes-only suppression flag)", "Issue #1092: at every clean-exit site (round-start LLM clean, review-only clean, post-verify clean, pending-findings-empty early exit, and fallback-reviewer clean), invokes _enforce_gates_before_clean to run pdd.checkup_gates over the loop-owned worktree; any failing deterministic gate (PR-range git diff --check, prettier --check, a non-mutating Python syntax check via builtin compile(), optional ruff/black/mypy/`npx --no-install tsc --noEmit`) is converted to a synthetic blocker ReviewFinding (reviewer prefix `gate:`) that the loop feeds back into the fixer rather than declaring clean", "Issue #1092 base-ref refresh: when config.enable_gates is True the loop calls pdd.agentic_checkup_orchestrator._refresh_pr_base_ref after _fetch_pr_metadata, fetching the PR's base branch into the dedicated tracking ref refs/remotes/pdd-checkup/pr-{N}/base (NEVER refs/remotes/origin/, which would mutate the operator's tracking refs when their origin is a fork) and populating pr_metadata['base_local_ref']. The fetch is bounded by a 60s subprocess timeout so a stalled transport cannot hold the review loop forever. The refresh helper resolves a trusted absolute git binary through pdd.checkup_gates and uses the sanitized gate env for its git-root, remote, and fetch subprocesses, so PATH=.:$PATH cannot execute a PR-shipped ./git before gate discovery. The review-loop short-circuit that skipped _fetch_pr_metadata in review-only mode is suppressed when gates are enabled so review-only on non-main-base PRs still gets the PR-range git-diff-check guarantee. discover_gates resolves base_ref from pr_metadata['base_local_ref'] first and falls back to the raw base_ref name when the refresh helper succeeded but did not land the ref", - "Issue #1092 fail-closed on refresh error: when _refresh_pr_base_ref populates pr_metadata['base_ref_fetch_error'] (subprocess CalledProcessError, TimeoutExpired, or other failure), _enforce_gates_before_clean appends a crash-row to state.gate_runs (phase='base-ref-refresh') and returns a single synthetic gate:base-ref blocker ReviewFinding. The loop refuses the clean verdict the same way it does for a crashed gate runner \u2014 never silently falls back to origin/main or worktree-only git diff --check, both of which would let an LLM clean verdict ride over a check we cannot prove ran against the right base", - "Issue #1092 fail-closed on metadata fetch: when _fetch_pr_metadata returns no usable base_ref (gh API outage, auth error, invalid JSON), the loop sets pr_metadata['base_ref_fetch_error'] BEFORE the refresh-call site so the same gate:base-ref fail-closed path engages \u2014 the pre-fix guard (if pr_metadata.get('base_ref')) silently skipped the refresh and let gates run with base_ref=None", - "Issue #1092 fail-closed on changed-files fallback: _pr_changed_files_all walks the base-candidate list using a trusted absolute git binary plus the sanitized gate env, then falls back to HEAD~1...HEAD only when every authoritative base diff failed. When the caller passed a real pr_metadata (base_ref or base_local_ref set) and the scanner had to fall back, the scanner records pr_metadata['changed_files_fallback']. _enforce_gates_before_clean then emits a single gate:changed-files blocker (phase='changed-files-resolution') so iter-30 node_modules and iter-27/iter-32 config-touched skips cannot be silently bypassed by a truncated inventory missing earlier-commit poisoning (iter-34 Finding 1). The same trusted git path is used by the sibling _pr_changed_python_files scanner so static drift detection cannot execute a PR-shipped ./git either. The same sentinel is also set \u2014 and same blocker emitted \u2014 when _pr_changed_files_all RAISES; the previous 'swallow into []' path proceeded with an empty inventory and let the safety skips no-op (iter-35 Finding 1). Exception text is scrubbed before storage. The fallback path stays valid when no base_ref was expected (unit-test paths)", - "Issue #1433 Bug #1 (pre-flight base-merge probe): immediately after _setup_pr_worktree and _refresh_pr_base_ref but BEFORE the first reviewer round, calls _detect_pr_base_merge_conflict(worktree, pr_metadata['base_local_ref']). The helper verifies the base ref with git rev-parse --verify, then runs `git merge-tree --write-tree --no-messages HEAD` (requires git >= 2.38; exit 0 = clean, exit 1 = conflict, other = cannot decide). Fail-OPEN on cannot-decide so non-PR worktrees, old git, or unresolvable refs do not become a new spurious block. When a real conflict is detected the loop creates a synthetic blocker ReviewFinding (reviewer='preflight:base-merge', area='pr-merge-conflict', round_number=0), persists preflight-base-merge-conflict.txt under artifacts_dir, sets reviewer_status[reviewer]='findings' + stop_reason, renders the final report via _finalize, posts via _post_review_loop_report, and returns with zero LLM spend. Codex review pass-2 finding F5: pre-flight only probes against base_local_ref freshly fetched by _refresh_pr_base_ref \u2014 a stale origin/ fallback would risk false positives on fork-origin operators. When base_ref_fetch_error is set or base_local_ref is unavailable the pre-flight is skipped and the existing gate:base-ref blocker handles the base problem", - "Issue #1433 Bug #3 (push fixer-subprocess commits): _commit_and_push_if_changed accepts a pre_fix_sha kwarg the loop captures via git rev-parse HEAD before invoking the fixer. When the fixer subprocess (typically codex autoheal) already committed inside the worktree, _git_changed_files returns empty (working tree is clean) but HEAD has moved past pre_fix_sha. The pre-fix function returned 'No changes to push' and the autoheal commit silently rotted. The helper now treats current_head != pre_fix_sha as has_unpushed_local_commits INDEPENDENTLY of the dirty flag (codex pass-2 F2 \u2014 the previous `not changed and ...` gate failed when the worktree had only filtered .pdd/checkup-context / .pdd/meta artifacts) and pushes the existing commits AS-IS, preserving the original author/message (e.g., 'Codex Local Autoheal') without creating a redundant PDD-Bot commit on top", + "Issue #1092 fail-closed on refresh error: when _refresh_pr_base_ref populates pr_metadata['base_ref_fetch_error'] (subprocess CalledProcessError, TimeoutExpired, or other failure), _enforce_gates_before_clean appends a crash-row to state.gate_runs (phase='base-ref-refresh') and returns a single synthetic gate:base-ref blocker ReviewFinding. The loop refuses the clean verdict the same way it does for a crashed gate runner — never silently falls back to origin/main or worktree-only git diff --check, both of which would let an LLM clean verdict ride over a check we cannot prove ran against the right base", + "Issue #1092 fail-closed on metadata fetch: when _fetch_pr_metadata returns no usable base_ref (gh API outage, auth error, invalid JSON), the loop sets pr_metadata['base_ref_fetch_error'] BEFORE the refresh-call site so the same gate:base-ref fail-closed path engages — the pre-fix guard (if pr_metadata.get('base_ref')) silently skipped the refresh and let gates run with base_ref=None", + "Issue #1092 fail-closed on changed-files fallback: _pr_changed_files_all walks the base-candidate list using a trusted absolute git binary plus the sanitized gate env, then falls back to HEAD~1...HEAD only when every authoritative base diff failed. When the caller passed a real pr_metadata (base_ref or base_local_ref set) and the scanner had to fall back, the scanner records pr_metadata['changed_files_fallback']. _enforce_gates_before_clean then emits a single gate:changed-files blocker (phase='changed-files-resolution') so iter-30 node_modules and iter-27/iter-32 config-touched skips cannot be silently bypassed by a truncated inventory missing earlier-commit poisoning (iter-34 Finding 1). The same trusted git path is used by the sibling _pr_changed_python_files scanner so static drift detection cannot execute a PR-shipped ./git either. The same sentinel is also set — and same blocker emitted — when _pr_changed_files_all RAISES; the previous 'swallow into []' path proceeded with an empty inventory and let the safety skips no-op (iter-35 Finding 1). Exception text is scrubbed before storage. The fallback path stays valid when no base_ref was expected (unit-test paths)", + "Issue #1433 Bug #1 (pre-flight base-merge probe): immediately after _setup_pr_worktree and _refresh_pr_base_ref but BEFORE the first reviewer round, calls _detect_pr_base_merge_conflict(worktree, pr_metadata['base_local_ref']). The helper verifies the base ref with git rev-parse --verify, then runs `git merge-tree --write-tree --no-messages HEAD` (requires git >= 2.38; exit 0 = clean, exit 1 = conflict, other = cannot decide). Fail-OPEN on cannot-decide so non-PR worktrees, old git, or unresolvable refs do not become a new spurious block. When a real conflict is detected the loop creates a synthetic blocker ReviewFinding (reviewer='preflight:base-merge', area='pr-merge-conflict', round_number=0), persists preflight-base-merge-conflict.txt under artifacts_dir, sets reviewer_status[reviewer]='findings' + stop_reason, renders the final report via _finalize, posts via _post_review_loop_report, and returns with zero LLM spend. Codex review pass-2 finding F5: pre-flight only probes against base_local_ref freshly fetched by _refresh_pr_base_ref — a stale origin/ fallback would risk false positives on fork-origin operators. When base_ref_fetch_error is set or base_local_ref is unavailable the pre-flight is skipped and the existing gate:base-ref blocker handles the base problem", + "Issue #1433 Bug #3 (push fixer-subprocess commits): _commit_and_push_if_changed accepts a pre_fix_sha kwarg the loop captures via git rev-parse HEAD before invoking the fixer. When the fixer subprocess (typically codex autoheal) already committed inside the worktree, _git_changed_files returns empty (working tree is clean) but HEAD has moved past pre_fix_sha. The pre-fix function returned 'No changes to push' and the autoheal commit silently rotted. The helper now treats current_head != pre_fix_sha as has_unpushed_local_commits INDEPENDENTLY of the dirty flag (codex pass-2 F2 — the previous `not changed and ...` gate failed when the worktree had only filtered .pdd/checkup-context / .pdd/meta artifacts) and pushes the existing commits AS-IS, preserving the original author/message (e.g., 'Codex Local Autoheal') without creating a redundant PDD-Bot commit on top", "Issue #1433 Bug #3 + codex pass-2 F3 (rebase full local range): _rebase_onto_updated_pr_head accepts a local_base_sha kwarg (typically pre_fix_sha). When supplied, the rebase upstream is `` instead of the hard-coded `HEAD~1`, so a multi-commit local range (typical for agentic autoheal that lands two or more commits per round) replays in full onto FETCH_HEAD. The pre-F3 single-commit HEAD~1..HEAD form is preserved when local_base_sha is None or equal to fixer_sha so legacy/test callers see no behaviour change", - "Issue #1433 Bug #3 (extended guard attention): the loop body unions _git_changed_files(worktree) with _files_changed_since(worktree, pre_fix_sha) before invoking _check_architecture_registry_edit_guard and _check_prompt_source_guard. _files_changed_since parses `git diff --name-status -z --find-renames ..HEAD` and surfaces BOTH old and new paths for R records (matching the _git_changed_files contract via git_porcelain) and the destination only for C records (codex pass-2 F4 \u2014 the previous --name-only form let a committed rename of a prompt-owned code file out of pdd/ slip past the guard). The union ensures the #1063 prompt-source and #1081 registry-edit guards still fire on changes the fixer subprocess COMMITTED rather than leaving in the working tree", - "Issue #1433 Bug #3 + codex pass-2 F1 (guard baseline pinned to pre_fix_sha): _check_prompt_source_guard, _check_architecture_registry_edit_guard, _load_prompt_source_map, and _path_exists_at_head all accept a head_ref kwarg (default 'HEAD' preserves backward compat for non-loop callers including the reviewer-prompt companion-files collector). The loop pins head_ref=pre_fix_sha when computing the guard baseline so the comparison is against the pre-fixer snapshot. Without this, a fixer subprocess that COMMITTED a registry repoint/removal would compare the post-fix registry against itself \u2014 zero diff \u2014 and bypass both guards entirely (defeating the protections #1063 and #1081 originally added). _path_exists_at_head's 'did this path exist at baseline?' check also threads head_ref so the unregistered-new-code scan judges additions against the pre-fix tree", + "Issue #1433 Bug #3 (extended guard attention): the loop body unions _git_changed_files(worktree) with _files_changed_since(worktree, pre_fix_sha) before invoking _check_architecture_registry_edit_guard and _check_prompt_source_guard. _files_changed_since parses `git diff --name-status -z --find-renames ..HEAD` and surfaces BOTH old and new paths for R records (matching the _git_changed_files contract via git_porcelain) and the destination only for C records (codex pass-2 F4 — the previous --name-only form let a committed rename of a prompt-owned code file out of pdd/ slip past the guard). The union ensures the #1063 prompt-source and #1081 registry-edit guards still fire on changes the fixer subprocess COMMITTED rather than leaving in the working tree", + "Issue #1433 Bug #3 + codex pass-2 F1 (guard baseline pinned to pre_fix_sha): _check_prompt_source_guard, _check_architecture_registry_edit_guard, _load_prompt_source_map, and _path_exists_at_head all accept a head_ref kwarg (default 'HEAD' preserves backward compat for non-loop callers including the reviewer-prompt companion-files collector). The loop pins head_ref=pre_fix_sha when computing the guard baseline so the comparison is against the pre-fixer snapshot. Without this, a fixer subprocess that COMMITTED a registry repoint/removal would compare the post-fix registry against itself — zero diff — and bypass both guards entirely (defeating the protections #1063 and #1081 originally added). _path_exists_at_head's 'did this path exist at baseline?' check also threads head_ref so the unregistered-new-code scan judges additions against the pre-fix tree", "Issue #1433 Bug #4 (reviewer companion source-of-truth attention): _run_review computes _collect_companion_source_of_truth_files(worktree, pr_metadata) and threads it into _review_prompt via the companion_source_of_truth kwarg. _format_companion_source_of_truth renders a `## Companion Source-Of-Truth Files To Inspect` section listing {code_path, prompt_path, prompt_in_diff, architecture_in_diff} for every code file in the PR diff. Codex pass-6 finding 1: the skip condition is `prompt_in_diff AND arch_entry_in_diff` where arch_entry_in_diff is a PER-ENTRY signal computed via _arch_entries_changed_set (canonical sorted-keys JSON comparison of base vs HEAD architecture.json entries per code_path); pass-5's prompt-only skip rule missed the case where module A's prompt was co-edited but A's architecture entry was unchanged (an arch.json edit for unrelated module B set the global signal True and hid A's at-risk arch surface). Reuses _load_prompt_source_map (which reads `git show HEAD:architecture.json` to derive the canonical code <-> prompt mapping) and _pr_changed_files_all. Always fail-open: any git/IO/JSON error returns [] so the section just disappears from the prompt rather than breaking the round. Codex pass-4 finding 2 + pass-5 finding 2 + pass-6 finding 2: when the eligible set exceeds max_entries (default 200) the collector appends a synthetic marker dict {`__truncated__`: True, `total_eligible`, `shown`, `omitted_code_paths` bounded at `max_entries // 2`, `omitted_paths_remaining`}; _run_review additionally persists the FULL eligible list as a sidecar JSON artifact (`round-{N}-{mode}-companion-source-of-truth.json` under artifacts_dir) and passes the artifact path to the formatter so the truncation note instructs the reviewer to either confirm artifact-based coverage or REPORT a `severity=blocker` finding (reviewer='companion-scope', area='workflow'). The bounded prompt section + sidecar artifact together keep the prompt context budget predictable while giving the reviewer (and post-hoc audits) the complete coverage record" ] }, @@ -7929,14 +7929,14 @@ "sideEffects": [ "Reads package.json, pyproject.toml, tsconfig.json, and .git markers from the loop-owned worktree (read-only)", "Probes PATH for ruff/black/mypy/npx via shutil.which (read-only)", - "Stats node_modules/typescript/bin/tsc before emitting the tsc-noemit gate; the argv is `npx --no-install tsc --noEmit --incremental false --composite false --tsBuildInfoFile /dev/null` so the gate runs the local binary (no registry install) AND cannot dirty the worktree with .tsbuildinfo even when tsconfig sets incremental: true, AND cannot trip TS6379 when the tsconfig extends chain sets composite: true. Also skips the gate when the PR diff modified tsconfig*.json (PR-controlled compilerOptions/paths/plugins are RCE-equivalent) OR any path under node_modules/ (iter-30 Finding 1 generalises iter-29 Finding 3 \u2014 every path under node_modules/ is a potential plugin/transformer/binary the gate would execute)", - "Skips npm-script gates (format:check / prettier:check / lint:check / typecheck / tsc / tsc:noemit) when the PR diff modified the tool config the script would load (prettier.config.* / .prettierrc* / eslint.config.* / .eslintrc* / tsconfig*.json) OR when the PR diff modified any package.json (root or nested) \u2014 corepack reads the top-level packageManager field and fetches+execs the PR-selected version on first invocation, turning the local gate into a registry download (iter-40 Finding 2). Those config files are loaded as executable JavaScript or interpreted compiler options by the corresponding tool, so a fork PR could otherwise achieve RCE through the gate (iter-27 Finding 3). Prettier/eslint script gates additionally skip on ANY PR-modified .cjs/.mjs/.js file because their configs can require() arbitrary local JS plugins anywhere in the worktree and we cannot statically follow the require chain (iter-29 Finding 2). tsc-style script gates additionally skip when ANY tsconfig in the extends chain sets incremental: true or composite: true \u2014 the script body cannot inject --incremental false like the direct gate, so running it would write tsbuildinfo (iter-29 Finding 1; walked with bounded depth to defend against cyclic configs). The emit-signal walk starts not only from the worktree-root tsconfig.json but from EVERY custom -p/--project target referenced by a recognised script (file or directory shape \u2014 the directory form resolves to /tsconfig.json). Without that extension a stable `tsc -p config/build.json --noEmit` whose config/build.json (or its extends chain) sets incremental: true would slip past the root-only walk and write .tsbuildinfo (iter-38 Finding 2). The chain walk treats package-name extends (e.g. '@demo/tsconfig') as potentially emit-signalling because the resolved config lives under node_modules where the gate layer cannot read it without parsing the resolver, AND a fork PR can install/modify the package (iter-30 Finding 2). ALL npm script gates (and the direct tsc-noemit gate) additionally skip when the PR diff modified anything under node_modules/ \u2014 every script invokes a binary or transitively loads code from there and a fork PR can land a poisoned shim at e.g. node_modules/.bin/prettier (iter-30 Finding 1; the package-lock.json / yarn.lock files live OUTSIDE node_modules/ so the common 'add a dependency' PR is unaffected). The ENTIRE npm-family gate path (script gates AND direct tsc-noemit) short-circuits when a package-manager config in the worktree can redirect script execution: .npmrc/.pnpmrc declaring script-shell, .yarnrc declaring script-shell or yarn-path, .yarnrc.yml declaring yarnPath, the presence of .yarn/releases/, OR a PR diff touching .npmrc/.pnpmrc/.yarnrc/.yarnrc.yml or anything under .yarn/. Confirmed against npm/pnpm/yarn 1 and npx 10.x: each honours script-shell so a PR-supplied ./evil-sh wraps the validated argv before it runs (iter-38 Finding 1)", - "Uses tokenized `--noEmit` detection in script acceptance so `tsc --noEmitOnError` (different flag, still emits) and `tsc --noEmit false` (explicit disable) cannot bypass the substring check (iter-27 Finding 2). Also rejects tsc-flavoured scripts that carry --watch/-w (would hang the gate), --incremental (writes tsbuildinfo regardless of --noEmit), --generateTrace, --generateCpuProfile, --listFiles, --listEmittedFiles, --diagnostics, --extendedDiagnostics, --traceResolution \u2014 the script-based path cannot inject overrides like the direct gate can (iter-33). The custom-config scan uses shlex tokenisation (handles quoted paths with embedded spaces, iter-36 Finding 2) and a single-prefix `./` strip (so hidden-directory paths like `.config/lint.json` survive comparison, iter-36 Finding 1). For tsc the scan also walks the local extends chain of `tsconfig.json` AND of every CUSTOM tsconfig referenced by a script via `-p` / `--project` (file or directory form \u2014 the directory form resolves to `/tsconfig.json`), bounded by the depth limit, refusing to leave the worktree, and deduped by resolved path. A PR-modified custom-named base (e.g. `base.json`) skips the script gate even when reached only through a non-root tsconfig chain (iter-36 Finding 3 + iter-37 Findings 1 + 2). Path comparison normalises both sides via `os.path.normpath` so non-canonical paths like `--config config/../config/lint.json` still match the PR diff (iter-37 Finding 3)", + "Stats node_modules/typescript/bin/tsc before emitting the tsc-noemit gate; the argv is `npx --no-install tsc --noEmit --incremental false --composite false --tsBuildInfoFile /dev/null` so the gate runs the local binary (no registry install) AND cannot dirty the worktree with .tsbuildinfo even when tsconfig sets incremental: true, AND cannot trip TS6379 when the tsconfig extends chain sets composite: true. Also skips the gate when the PR diff modified tsconfig*.json (PR-controlled compilerOptions/paths/plugins are RCE-equivalent) OR any path under node_modules/ (iter-30 Finding 1 generalises iter-29 Finding 3 — every path under node_modules/ is a potential plugin/transformer/binary the gate would execute)", + "Skips npm-script gates (format:check / prettier:check / lint:check / typecheck / tsc / tsc:noemit) when the PR diff modified the tool config the script would load (prettier.config.* / .prettierrc* / eslint.config.* / .eslintrc* / tsconfig*.json) OR when the PR diff modified any package.json (root or nested) — corepack reads the top-level packageManager field and fetches+execs the PR-selected version on first invocation, turning the local gate into a registry download (iter-40 Finding 2). Those config files are loaded as executable JavaScript or interpreted compiler options by the corresponding tool, so a fork PR could otherwise achieve RCE through the gate (iter-27 Finding 3). Prettier/eslint script gates additionally skip on ANY PR-modified .cjs/.mjs/.js file because their configs can require() arbitrary local JS plugins anywhere in the worktree and we cannot statically follow the require chain (iter-29 Finding 2). tsc-style script gates additionally skip when ANY tsconfig in the extends chain sets incremental: true or composite: true — the script body cannot inject --incremental false like the direct gate, so running it would write tsbuildinfo (iter-29 Finding 1; walked with bounded depth to defend against cyclic configs). The emit-signal walk starts not only from the worktree-root tsconfig.json but from EVERY custom -p/--project target referenced by a recognised script (file or directory shape — the directory form resolves to /tsconfig.json). Without that extension a stable `tsc -p config/build.json --noEmit` whose config/build.json (or its extends chain) sets incremental: true would slip past the root-only walk and write .tsbuildinfo (iter-38 Finding 2). The chain walk treats package-name extends (e.g. '@demo/tsconfig') as potentially emit-signalling because the resolved config lives under node_modules where the gate layer cannot read it without parsing the resolver, AND a fork PR can install/modify the package (iter-30 Finding 2). ALL npm script gates (and the direct tsc-noemit gate) additionally skip when the PR diff modified anything under node_modules/ — every script invokes a binary or transitively loads code from there and a fork PR can land a poisoned shim at e.g. node_modules/.bin/prettier (iter-30 Finding 1; the package-lock.json / yarn.lock files live OUTSIDE node_modules/ so the common 'add a dependency' PR is unaffected). The ENTIRE npm-family gate path (script gates AND direct tsc-noemit) short-circuits when a package-manager config in the worktree can redirect script execution: .npmrc/.pnpmrc declaring script-shell, .yarnrc declaring script-shell or yarn-path, .yarnrc.yml declaring yarnPath, the presence of .yarn/releases/, OR a PR diff touching .npmrc/.pnpmrc/.yarnrc/.yarnrc.yml or anything under .yarn/. Confirmed against npm/pnpm/yarn 1 and npx 10.x: each honours script-shell so a PR-supplied ./evil-sh wraps the validated argv before it runs (iter-38 Finding 1)", + "Uses tokenized `--noEmit` detection in script acceptance so `tsc --noEmitOnError` (different flag, still emits) and `tsc --noEmit false` (explicit disable) cannot bypass the substring check (iter-27 Finding 2). Also rejects tsc-flavoured scripts that carry --watch/-w (would hang the gate), --incremental (writes tsbuildinfo regardless of --noEmit), --generateTrace, --generateCpuProfile, --listFiles, --listEmittedFiles, --diagnostics, --extendedDiagnostics, --traceResolution — the script-based path cannot inject overrides like the direct gate can (iter-33). The custom-config scan uses shlex tokenisation (handles quoted paths with embedded spaces, iter-36 Finding 2) and a single-prefix `./` strip (so hidden-directory paths like `.config/lint.json` survive comparison, iter-36 Finding 1). For tsc the scan also walks the local extends chain of `tsconfig.json` AND of every CUSTOM tsconfig referenced by a script via `-p` / `--project` (file or directory form — the directory form resolves to `/tsconfig.json`), bounded by the depth limit, refusing to leave the worktree, and deduped by resolved path. A PR-modified custom-named base (e.g. `base.json`) skips the script gate even when reached only through a non-root tsconfig chain (iter-36 Finding 3 + iter-37 Findings 1 + 2). Path comparison normalises both sides via `os.path.normpath` so non-canonical paths like `--config config/../config/lint.json` still match the PR diff (iter-37 Finding 3)", "_script_is_acceptable rejects npm scripts whose body starts with `npx ` UNLESS `--no-install` is also present; bare `npx ` would otherwise let `npm run