fix(GATE-WINDOWS-WARNING-POLICY): /WX- is not /WX, and OBJCXX is not the C++ compile (#774)

.agents/roadmap_v1.md

@@ -158,6 +158,7 @@ issue is not yet placed. Keyed record: update in place, never append.
 | [#615](https://github.com/mudler/vllm.cpp/issues/615) | `GATE-PR-SIZE-BINARY` | `check-pr-size` fail-closes on every binary path with no exemption route, so no golden-bearing PR can merge: it blocks #431 and post-dates the golden precedent it rejects, spec [`gate-pr-size-binary.md`](specs/gate-pr-size-binary.md) | bug |
 | [#670](https://github.com/mudler/vllm.cpp/issues/670) | `BACKEND-TENSTORRENT-MISTRAL` | Tenstorrent: allowlist `MistralForCausalLM` and gate it on-device; goldens are `transformers`-teacher-forced because vLLM has no TT backend, spec [`tenstorrent-mistral.md`](specs/tenstorrent-mistral.md) | feature |
 | [#773](https://github.com/mudler/vllm.cpp/issues/773) | `GATE-FORK-ANCESTRY` | `check-pr-size` and `check-commit-trailers` abort on every fork PR because the base SHA stops being an ancestor once main moves, so CI has never validated an outside contributor's trailers, spec [`gate-fork-ancestry.md`](specs/gate-fork-ancestry.md) | bug |
+| [#774](https://github.com/mudler/vllm.cpp/issues/774) | `ENG-RELEASE-WINDOWS` | `check-windows-portability.py:1710` asserted the MSVC warning policy with `token in warnings`, and `"/WX" in "/WX-"` is `True` — `/WX-` is MSVC's spelling for DISABLE warnings-as-errors, so the gate was blind to its own inversion. Measured on PR #640 commit `74ba3823f`, which shipped `/WX-` on the CXX arm while the only bare `/WX` left was on `$<COMPILE_LANGUAGE:OBJCXX>` — Objective-C++, the Metal backend, which never compiles under MSVC. Two further blindnesses fell out of the same `in`: `/W44996` answers for `/W4`, and `CMakeLists.txt:30`'s `#` comment satisfies the whole policy on its own. Repaired to a token-boundary match over the flags that reach the C/C++ compile, plus the negating spellings `/WX-` `/W0` `/w`; spec [`windows-msvc-warning-policy-tokens.md`](specs/windows-msvc-warning-policy-tokens.md) | bug |
 | [#238](https://github.com/mudler/vllm.cpp/issues/238) | `SAMPLE-LOGPROB-TOKEN-IDS` | `logprobs_mode`: three of four modes are runtime-refused stubs | bug |
 | [#264](https://github.com/mudler/vllm.cpp/issues/264) | `SAMPLE-LOGPROB-TOKEN-IDS` | `logprob_token_ids`: generative scoring over an explicit token set is unported | feature |
 | [#365](https://github.com/mudler/vllm.cpp/issues/365) | `PERF-27B-DENSE-MARLIN-GATEUP` | 27B gap decomposed vs vLLM's own decode profile; dense W4A16 MLP bypassed the fused gate_up seam (spec `specs/perf-27b-dense-marlin-gateup.md`) | bug |

.agents/specs/windows-msvc-warning-policy-tokens.md

@@ -0,0 +1,222 @@
+# MSVC `/W4 /WX` policy — assert the TOKEN on the C/C++ compile, not a substring anywhere in the file
+
+Identity: `ENG-RELEASE-WINDOWS`
+
+Issue: [#774](https://github.com/mudler/vllm.cpp/issues/774)
+
+Parent specification: [windows-binary-release.md](windows-binary-release.md)
+
+Related repair: [windows-msvc-strict-build.md](windows-msvc-strict-build.md),
+which established the strict warning policy this checker line exists to hold.
+
+Status: `ACTIVE`. Base `af026e5241e16e1d2a89da001b978ffcb3e0f634`.
+
+## Scope
+
+Replace the substring assertion at `scripts/check-windows-portability.py:1710`
+with a token-boundary assertion evaluated over the flags that actually reach an
+MSVC **C/C++** compile.
+
+In scope:
+
+1. Token boundaries for `/W4` and `/WX`, so a longer flag that merely *contains*
+   the token no longer satisfies the policy.
+2. Language scoping, so an occurrence confined to a non-C/C++ `COMPILE_LANGUAGE`
+   generator expression (OBJCXX, CUDA, ...) cannot satisfy the policy.
+3. Comment stripping, so the policy cannot be satisfied by prose.
+4. Rejecting the *negating spellings of the two tokens the policy names* when
+   they reach the C/C++ compile: `/WX-`, `/W0`, `/w`.
+
+Explicitly excluded, argued in **Sibling evasions** below: the
+`COMPILE_WARNING_AS_ERROR` target/cache property, and per-warning `/wd####`
+suppressions.
+
+Nothing outside `scripts/check-windows-portability.py`,
+`tests/scripts/test_check_windows_portability.py`, this spec and the roadmap
+issue table changes. `cmake/CompilerWarnings.cmake` is **not** edited: the tree
+already satisfies the repaired policy, and a checker repair that also moves the
+thing it checks cannot show that it holds.
+
+## Observed baseline and root cause
+
+`scripts/check-windows-portability.py:1710` at base:
+
+```python
+if not all(token in warnings for token in ("/W4", "/WX")):
+    errors.append("CMakeLists.txt: MSVC /W4 /WX policy is required")
+```
+
+`warnings` is the concatenation of `CMakeLists.txt` and
+`cmake/CompilerWarnings.cmake`, unparsed. Three independent blindnesses follow
+from `in`:
+
+- **Inversion.** `"/WX" in "/WX-"` is `True`. `/WX-` is MSVC's spelling for
+  *disable* warnings-as-errors — the exact inversion of the policy.
+  `"/W4" in "/W44996"` is `True` too, and `/W44996` sets warning C4996 to level
+  4 rather than raising the warning level.
+- **Language.** The test asks only whether the bytes occur *somewhere*. A `/WX`
+  reachable only through `$<$<COMPILE_LANGUAGE:OBJCXX>:...>` is inert under
+  MSVC — Objective-C++ is the Metal backend and never compiles on Windows — yet
+  satisfies the policy.
+- **Prose.** `CMakeLists.txt:30` contains the literal text `/W4 /WX` inside a
+  `#` comment. That comment alone satisfies the base assertion, so the policy
+  survives deleting every real flag in the tree.
+
+The measured demonstration is PR
+[#640](https://github.com/mudler/vllm.cpp/pull/640) commit `74ba3823f`, whose
+`cmake/CompilerWarnings.cmake` used all three at once (reverted in `15aa963a6`
+after review):
+
+```cmake
+  if(MSVC)
+    set_property(TARGET ${target} PROPERTY COMPILE_WARNING_AS_ERROR OFF)
+    target_compile_options(${target} PRIVATE
+      $<$<COMPILE_LANGUAGE:CXX>:/W4>
+      $<$<COMPILE_LANGUAGE:CXX>:/WX->
+      $<$<COMPILE_LANGUAGE:CXX>:/utf-8>
+      $<$<COMPILE_LANGUAGE:CXX>:/wd4324>
+      $<$<COMPILE_LANGUAGE:CXX>:/wd4458>
+      $<$<COMPILE_LANGUAGE:OBJCXX>:/W4>
+      $<$<COMPILE_LANGUAGE:OBJCXX>:/WX>
+      $<$<COMPILE_LANGUAGE:CUDA>:-Werror=all-warnings>)
+  endif()
+```
+
+The only bare `/WX` left is on OBJCXX. The gate passes.
+
+## Design
+
+Two helpers next to the existing CMake-text helpers
+(`without_set_source_properties`, `source_properties`), then a rewritten
+assertion.
+
+`msvc_cxx_flag_text(text)` returns the flag text that can reach an MSVC C/C++
+translation unit:
+
+- `#` comments removed, using the same `re.sub(r"(?m)#.*$", "", ...)` idiom
+  `_has_central_msvc_nominmax` already uses.
+- Every generator expression whose `COMPILE_LANGUAGE` mentions name only
+  languages outside `{C, CXX}` blanked. Spans come from a `$<` / `>` stack scan,
+  so `$<$<COMPILE_LANGUAGE:OBJCXX>:/WX>` is blanked whole, `$<$<COMPILE_LANGUAGE:CXX>:/W4 /WX>`
+  is kept whole, and a genex naming no language at all (`$<$<CONFIG:Debug>:/WX>`)
+  is kept, because it does reach C/C++. Blanking preserves offsets, matching
+  `without_set_source_properties`.
+
+`has_msvc_flag(text, flag)` matches the flag with boundaries on both sides:
+`(?<![A-Za-z0-9_-])` before and `(?![A-Za-z0-9_-])` after. `/WX-` therefore does
+not answer for `/WX`, `/W44996` does not answer for `/W4`, and `/wd4324` does
+not answer for `/w`. Matching stays case-sensitive because `cl` is: `/w` and
+`/W4` are different flags, not two spellings of one.
+
+The assertion then reads: both `/W4` and `/WX` must be present in that text, and
+none of `/WX-`, `/W0`, `/w` may be. The first error message keeps its existing
+prefix (`CMakeLists.txt: MSVC /W4 /WX policy is required`) so nothing that greps
+for it breaks, and names which token is missing; the negation is a second,
+separately-worded error, because "the flag is absent" and "the flag is present
+and cancelled" are different repairs.
+
+Rejected alternative: a `re.search(r"/WX(?!-)")` over the raw text. It closes
+the inversion and nothing else — the OBJCXX-only occurrence and the comment
+still satisfy it, and #640 shows those are the shapes that actually got written.
+
+## Sibling evasions — decided, not silently widened
+
+**`COMPILE_WARNING_AS_ERROR OFF` (target property or `CMAKE_` cache variable):
+OUT of scope.** CMake uses this property only to decide whether *it* adds a
+warnings-as-errors flag; there is no mechanism by which setting it `OFF` removes
+a literal flag written into `target_compile_options`. Under the VS generator it
+lowers to MSBuild `TreatWarningAsError=false`, whose `/WX-` precedes
+`AdditionalOptions` on the CL command line, and MSVC takes the last of the pair
+— so our explicit `/WX` still wins. It cannot defeat the repaired policy, and
+the converse case fails closed already: a tree that dropped the literal `/WX`
+and relied on `COMPILE_WARNING_AS_ERROR ON` instead would go RED on the missing
+token, which is the correct direction. Asserting a property *value* would be a
+new policy about how the flag is spelled, which #774 does not describe.
+
+**Blanket `/wd####`: OUT of scope.** A per-warning suppression is a different
+axis from the two tokens the policy names: it does not invert `/W4` or `/WX`, it
+narrows what `/W4` reports. Refusing it wholesale would forbid the legitimate
+targeted suppression (the tree has none today, so nothing is being grandfathered
+in), and "how many suppressions are too many" is a threshold nobody has decided.
+Any future `/wd` arrives on a PR of its own and is a review judgement there.
+Recorded here as a known, deliberate non-goal rather than an oversight.
+
+**`/W0` and `/w`: IN scope**, unlike the two above, because they are the
+disable spellings of `/W4` itself. Leaving them out would close `/WX-` while
+leaving its exact twin one character away, which is the same defect #774
+reports rather than a wider one.
+
+## Risks
+
+- **Over-strictness on a legitimate tree.** A CXX-scoped genex, a bare
+  `add_compile_options(/W4 /WX)`, and a config-conditional genex must all still
+  pass. Pinned by an inverse test on each shape, and by the real tree.
+- **Genex parsing.** The stack scan handles the two forms this repository
+  writes. A `COMPILE_LANGUAGE` buried inside `$<AND:...>` is handled by
+  collecting *every* `COMPILE_LANGUAGE` mention inside a span; a span with no
+  mention is conservatively kept, so the failure direction is "counts as
+  reaching C/C++", never "silently excluded".
+- **Comment stripping.** `#` inside a CMake string would be stripped early. The
+  only consequence is a false RED on the warning policy, which is visible, and
+  the repository has no such line.
+
+## Tests
+
+`tests/scripts/test_check_windows_portability.py`, all through the existing
+`assert_rejected` / `run_checker` harness so they exercise the real checker
+process:
+
+1. `/WX-` on the CXX arm is rejected (the #774 headline).
+2. The #640 `74ba3823f` shape — `/WX-` on CXX with the only bare `/WX` on
+   OBJCXX — is rejected.
+3. `/W44996` and `/WXsomething` do not satisfy the policy.
+4. `/W4 /WX` present but cancelled by a later `/w` is rejected.
+5. Flags present only inside a `#` comment are rejected.
+6. Inverse pins: the honest `$<$<COMPILE_LANGUAGE:CXX>:/W4 /WX>` genex, the
+   bare `add_compile_options(/W4 /WX)` fixture, and a `$<$<CONFIG:Debug>:...>`
+   genex all pass.
+7. The real tree's `cmake/CompilerWarnings.cmake` satisfies the repaired policy
+   with the OBJCXX arm excluded and no negation present.
+
+Every one of 1-5 and 7 must FAIL against the base checker; that is the
+red-before evidence, and it is also what `check-pr-size.py` re-executes for a
+`governance_checker` change.
+
+## Gates
+
+- `scripts/agent-preflight.sh`
+- `python3 -m pytest tests/scripts/ --ignore=tests/scripts/test_cpu_kernel_bench.py`
+- `python3 scripts/check-windows-portability.py` against the real tree (green
+  before and after — this repair does not change the verdict on `main`).
+
+No build, no GPU, no benchmark: this is a source-contract checker.
+
+## Evidence
+
+Base checker, run over the real `CMakeLists.txt` plus a substituted
+`cmake/CompilerWarnings.cmake`, evaluating the assertion exactly as line 1710
+spells it:
+
+```
+PASS  BASE (main: honest /W4 /WX on CXX)
+PASS  PR#640@74ba3823f (/WX- on CXX; bare /WX only on OBJCXX)
+PASS  decoy: /WX- appended on CXX after the real /WX
+PASS  decoy: /w (all warnings off) appended on CXX
+PASS  substring-only tokens: /W44996 and /WXsomething
+```
+
+Four of those five are policy inversions and the gate cannot see any of them.
+The per-test red-before and green-after runs are recorded in the PR body.
+
+## Stop conditions
+
+- If closing the language scope turns the real tree RED, stop and report: the
+  tree, not the checker, would then be the finding.
+- If a shape outside `$<$<COMPILE_LANGUAGE:...>:...>` is needed to decide
+  reachability, stop and return `NEEDS_DECISION` rather than growing a CMake
+  generator-expression evaluator inside a portability checker.
+
+## Now
+
+`ENG-RELEASE-WINDOWS` does not change lifecycle state. This is a checker repair
+inside an already-`ACTIVE` row; no `STATUS`/`BENCHMARKS` projection is owed.

scripts/check-windows-portability.py

@@ -276,6 +276,72 @@ def windows_possible_lines(text: str):
             yield number, line
 
 
+C_FAMILY_COMPILE_LANGUAGES = {"C", "CXX"}
+
+
+def _generator_expression_spans(text: str) -> list[tuple[int, int]]:
+    """Return (start, end) for every balanced `$<...>` generator expression."""
+    spans: list[tuple[int, int]] = []
+    stack: list[int] = []
+    index = 0
+    while index < len(text):
+        if text.startswith("$<", index):
+            stack.append(index)
+            index += 2
+            continue
+        if text[index] == ">" and stack:
+            spans.append((stack.pop(), index + 1))
+        index += 1
+    return spans
+
+
+def msvc_cxx_flag_text(cmake_text: str) -> str:
+    """Return the flag text that can reach an MSVC C/C++ translation unit.
+
+    Two things are removed, both of which satisfied the old substring test
+    without compiling anything (#774):
+
+    * `#` comments. `CMakeLists.txt` says `/W4 /WX` in prose, and a policy
+      cannot be satisfied by prose.
+    * Generator expressions whose `COMPILE_LANGUAGE` names only languages
+      outside C/C++. `$<$<COMPILE_LANGUAGE:OBJCXX>:/WX>` is the Metal backend,
+      which never compiles under MSVC, so it answers for nothing here. A
+      generator expression naming no language at all is KEPT: it does reach
+      C/C++.
+
+    Spans are blanked rather than cut so reported offsets stay meaningful,
+    matching `without_set_source_properties`.
+    """
+    active = re.sub(r"(?m)#.*$", "", cmake_text)
+    out = list(active)
+    for start, end in _generator_expression_spans(active):
+        languages: set[str] = set()
+        for mention in re.finditer(
+            r"COMPILE_LANGUAGE\s*:\s*([^>]*)", active[start:end]
+        ):
+            languages |= {
+                name.strip().upper()
+                for name in mention.group(1).split(",")
+                if name.strip()
+            }
+        if languages and not (languages & C_FAMILY_COMPILE_LANGUAGES):
+            out[start:end] = " " * (end - start)
+    return "".join(out)
+
+
+def has_msvc_flag(text: str, flag: str) -> bool:
+    """Return whether `flag` appears as a WHOLE token, not as a substring.
+
+    `"/WX" in "/WX-"` is True and `/WX-` DISABLES warnings-as-errors; `"/W4" in
+    "/W44996"` is True and `/W44996` sets one warning to level 4 rather than
+    raising the level. Both satisfied the old test (#774). Matching stays
+    case-sensitive because `cl` is: `/w` and `/W4` are different flags.
+    """
+    return re.search(
+        rf"(?<![A-Za-z0-9_-]){re.escape(flag)}(?![A-Za-z0-9_-])", text
+    ) is not None
+
+
 def without_set_source_properties(text: str) -> str:
     """Remove balanced set_source_files_properties commands."""
     lowered = text.lower()
@@ -1708,8 +1774,30 @@ def check(root: Path, build_dir: Path | None = None,
     global_options = without_set_source_properties(cmake)
     if re.search(r"(?i)/arch\s*:\s*AVX2", global_options):
         errors.append("CMakeLists.txt: global /arch:AVX2 contaminates the portable baseline")
-    if not all(token in warnings for token in ("/W4", "/WX")):
-        errors.append("CMakeLists.txt: MSVC /W4 /WX policy is required")
+    # Asserted on the flags that reach the C/C++ compile, by TOKEN. A substring
+    # test passed a tree whose CXX arm said `/WX-` because the only bare `/WX`
+    # left was on `$<COMPILE_LANGUAGE:OBJCXX>` (#774).
+    cxx_warning_flags = msvc_cxx_flag_text(warnings)
+    missing = [
+        flag for flag in ("/W4", "/WX")
+        if not has_msvc_flag(cxx_warning_flags, flag)
+    ]
+    if missing:
+        errors.append(
+            "CMakeLists.txt: MSVC /W4 /WX policy is required on the C/C++ "
+            f"compile; missing {' '.join(missing)}"
+        )
+    # The disable spellings of the SAME two flags. Present-and-cancelled is a
+    # different repair from absent, so it is a different error.
+    negated = [
+        flag for flag in ("/WX-", "/W0", "/w")
+        if has_msvc_flag(cxx_warning_flags, flag)
+    ]
+    if negated:
+        errors.append(
+            "CMakeLists.txt: MSVC /W4 /WX policy is negated on the C/C++ "
+            f"compile by {' '.join(negated)}"
+        )
     if re.search(r"__attribute__\s*\(\(\s*target\s*\(\s*\"f16c\"", cpu_baseline):
         errors.append("cpu_matmul_elem.cpp: F16C must be isolated in a dedicated translation unit")
     f16c_properties = source_properties(cmake, "src/vt/cpu/cpu_matmul_elem_f16c.cpp")