From 0cf0f0db1f99b38454144fc5d83ef629fa58c020 Mon Sep 17 00:00:00 2001 From: Andrew Bates Date: Mon, 5 Oct 2026 22:24:18 -0500 Subject: [PATCH 1/2] fix(test): strip comments before matching in the #44 and #117 sensors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tools/tests/test_runtime_lifecycle_invariants.py's regex-based source checks matched raw source text, so a commented-out call site with a decoy statement nearby still satisfied the check — mutation-tested on both test_shutdown_thread_never_touches_the_callback_registry (#44) and test_broadcaster_callbacks_record_their_owner_before_listening (#117). Add strip_comments(), the same //-and-block-comment-aware approach the justfile's N99 sensors already use, and run it over every extracted function body before the regexes see it. Fixes #123 Co-Authored-By: nevr-runtime --- .../test_runtime_lifecycle_invariants.py | 108 ++++++++++++++++-- 1 file changed, 100 insertions(+), 8 deletions(-) diff --git a/tools/tests/test_runtime_lifecycle_invariants.py b/tools/tests/test_runtime_lifecycle_invariants.py index c4200417..4f708799 100644 --- a/tools/tests/test_runtime_lifecycle_invariants.py +++ b/tools/tests/test_runtime_lifecycle_invariants.py @@ -6,6 +6,86 @@ ROOT = Path(__file__).resolve().parents[2] +def strip_comments(text: str) -> str: + """Remove // line comments and /* */ block comments from C++ source text. + + A regex over raw source cannot distinguish "the call is here" from "the + call used to be here and is now commented out" — both contain the same + substring. Strip comments first so a commented-out call site (with or + without a decoy real statement nearby) is invisible to the regex, the + same way the compiler would see it. String/char literals are tracked so + a literal containing "//" or "/*" is not treated as a comment start. + Newlines are preserved so line numbers in any future diagnostics stay + roughly aligned; comment bodies are dropped, not blanked to the same width. + """ + result = [] + i = 0 + n = len(text) + in_line_comment = False + in_block_comment = False + in_string = False + in_char = False + while i < n: + c = text[i] + two = text[i : i + 2] + if in_line_comment: + if c == "\n": + in_line_comment = False + result.append(c) + i += 1 + continue + if in_block_comment: + if two == "*/": + in_block_comment = False + i += 2 + else: + if c == "\n": + result.append("\n") + i += 1 + continue + if in_string: + result.append(c) + if c == "\\" and i + 1 < n: + result.append(text[i + 1]) + i += 2 + continue + if c == '"': + in_string = False + i += 1 + continue + if in_char: + result.append(c) + if c == "\\" and i + 1 < n: + result.append(text[i + 1]) + i += 2 + continue + if c == "'": + in_char = False + i += 1 + continue + if two == "//": + in_line_comment = True + i += 2 + continue + if two == "/*": + in_block_comment = True + i += 2 + continue + if c == '"': + in_string = True + result.append(c) + i += 1 + continue + if c == "'": + in_char = True + result.append(c) + i += 1 + continue + result.append(c) + i += 1 + return "".join(result) + + def extract_braced_function(source: str, signature: str) -> str: start = source.index(signature) opening = source.index("{", start) @@ -108,7 +188,11 @@ def test_shutdown_thread_never_touches_the_callback_registry(self): # no lock (echovr.exe 0x140f8df20). The shutdown thread now hands that work to Update() # through MainThreadHandoff; its own fallback must skip the registry. source = (ROOT / "src/runtime/server/gameserver.cpp").read_text() - shutdown = extract_braced_function(source, "void GameServerLib::BeginGracefulShutdown(") + # Comment-stripped (#123): a commented-out call site with a decoy real + # statement nearby would otherwise still satisfy these regexes — a raw + # substring match can't tell "the call is here" from "the call is + # described in a comment above the decoy". + shutdown = strip_comments(extract_braced_function(source, "void GameServerLib::BeginGracefulShutdown(")) for forbidden in (r"\bUnregister\s*\(\s*\)", r"\bUnregisterAllCallbacks\s*\(", r"\bGetCallbackRegistry\s*\(", r"\bUnregisterFromServerDb\s*\(\s*true"): @@ -118,18 +202,19 @@ def test_shutdown_thread_never_touches_the_callback_registry(self): "subject vanished: the shutdown thread no longer hands off to the game thread") self.assertRegex(shutdown, r"ShutdownUnregisterOnGameThread\s*\(") - off_thread = extract_braced_function(source, "void GameServerLib::ShutdownUnregisterOffGameThread(") + off_thread = strip_comments( + extract_braced_function(source, "void GameServerLib::ShutdownUnregisterOffGameThread(")) self.assertRegex(off_thread, r"UnregisterFromServerDb\s*\(\s*false\s*\)") for forbidden in (r"\bUnregister\s*\(\s*\)", r"\bUnregisterAllCallbacks\s*\(", r"\bGetCallbackRegistry\s*\(", r"\bUnregisterFromServerDb\s*\(\s*true"): self.assertNotRegex(off_thread, forbidden) # The skip must be real: the registry action is only built when asked for. - impl = extract_braced_function(source, "void GameServerLib::UnregisterFromServerDb(") + impl = strip_comments(extract_braced_function(source, "void GameServerLib::UnregisterFromServerDb(")) self.assertRegex(impl, r"if\s*\(\s*touchCallbackRegistry\s*\)\s*unregisterCallbacks\s*=") # And the game thread must actually service the hand-off, or every shutdown times out. - update = extract_braced_function(source, "VOID GameServerLib::Update(") + update = strip_comments(extract_braced_function(source, "VOID GameServerLib::Update(")) self.assertRegex(update, r"m_gameThreadHandoff\.Service\(\)") def test_broadcaster_callbacks_record_their_owner_before_listening(self): @@ -139,7 +224,12 @@ def test_broadcaster_callbacks_record_their_owner_before_listening(self): # the owner null every unregister silently skipped the game and only cleared the struct. No # C++ test links gameserver.cpp, so the wiring is pinned here. source = (ROOT / "src/runtime/server/gameserver.cpp").read_text() - register = extract_braced_function(source, "void GameServerLib::RegisterBroadcasterCallbacks(") + # Comment-stripped (#123): same reasoning as the #44 sensor above — a + # commented-out RecordBroadcasterOwner call with a `owner = nullptr;` + # decoy nearby still contains the substring these regexes look for + # unless comments are removed first. + register = strip_comments( + extract_braced_function(source, "void GameServerLib::RegisterBroadcasterCallbacks(")) record = re.search(r"\bGameServer::RecordBroadcasterOwner\s*\(\s*\*m_context\s*\)", register) self.assertIsNotNone(record, "RegisterBroadcasterCallbacks no longer records the callback owner") first_listen = re.search(r"\bListenForBroadcasterMessage\s*\(", register) @@ -149,12 +239,14 @@ def test_broadcaster_callbacks_record_their_owner_before_listening(self): # Registration, the recorded owner and the unregister guard must all name the same # broadcaster (the lobby's), or the guard rejects every handle again. - listen = extract_braced_function(source, "uint16_t ListenForBroadcasterMessage(") + listen = strip_comments(extract_braced_function(source, "uint16_t ListenForBroadcasterMessage(")) self.assertRegex(listen, r"BroadcasterListen\(\s*lobby->broadcaster\s*,") - unregister = extract_braced_function(source, "void GameServerLib::UnregisterAllCallbacks(") + unregister = strip_comments( + extract_braced_function(source, "void GameServerLib::UnregisterAllCallbacks(")) self.assertRegex(unregister, r"liveOwner\s*=\s*lobby\s*!=\s*nullptr\s*\?\s*lobby->broadcaster\s*:") helper_source = (ROOT / "src/runtime/server/callback_unregistration.cpp").read_text() - helper = extract_braced_function(helper_source, "EchoVR::Broadcaster* RecordBroadcasterOwner(") + helper = strip_comments( + extract_braced_function(helper_source, "EchoVR::Broadcaster* RecordBroadcasterOwner(")) self.assertRegex(helper, r"lobby\s*!=\s*nullptr\s*\?\s*lobby->broadcaster\s*:") self.assertRegex(helper, r"\.broadcasterOwner\s*=") From 19fd692cce135059d4ec5abca048bcc9048aea7a Mon Sep 17 00:00:00 2001 From: Andrew Bates Date: Mon, 5 Oct 2026 22:39:05 -0500 Subject: [PATCH 2/2] fix(test): close two #123 review gaps in strip_comments and extract_braced_function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opus review of PR #126 found two remaining issues in the strip_comments fix: 1. False negative: closing a block comment appended nothing to the result, so `return/**/Unregister();` collapsed to `returnUnregister();` — gluing two real tokens together and breaking \bUnregister word-boundary matching for a call a comment merely interrupts, not one that is commented out. The compiler treats a block comment as whitespace; strip_comments now does too, appending a single space when the `*/` closes. 2. Scope gap: the #44/#117 sensors were wrapped in strip_comments() by hand, but every other regex/substring sensor in this file (e.g. test_every_boot_detour_result_is_consumed's `if (installed) return;` check) still matched raw, unstripped source — a commented-out real line with a decoy nearby still satisfied assertIn. Moved strip_comments() into extract_braced_function() itself so every sensor in the file gets the same comment-transparency the compiler has, and removed the now- redundant per-call wraps around the #44/#117 variables. Verified both fixes with a mutation suite (scratch copies under /var/tmp/work-nevr-runtime/issue123-mutation/): 6 real-regression mutations across the #44/#117 sensors still correctly fail pre- and post-fix: the return/**/Unregister(); false negative now correctly fails post-fix (missed pre-fix); and a new mutation against initialize.cpp's `if (installed) return;` (commented out with a decoy) now also correctly fails post-fix (missed pre-fix) — proving the scope-gap fix reaches a sensor the per-call wraps never touched. strip_comments edge cases (line comments, multi-line block comments, strings/char literals containing // or /*, a / b / c division) re-verified correct. just verify: green (CMAKE_BUILD_PARALLEL_LEVEL=4 just verify -> "verify: OK (mingw-release)"), including the file's own 87 ground-truth tests under test-auth-unit. Co-Authored-By: nevr-runtime --- .../test_runtime_lifecycle_invariants.py | 49 ++++++++++++------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/tools/tests/test_runtime_lifecycle_invariants.py b/tools/tests/test_runtime_lifecycle_invariants.py index 4f708799..48dcb073 100644 --- a/tools/tests/test_runtime_lifecycle_invariants.py +++ b/tools/tests/test_runtime_lifecycle_invariants.py @@ -38,6 +38,12 @@ def strip_comments(text: str) -> str: if two == "*/": in_block_comment = False i += 2 + # The compiler treats a block comment as whitespace, so a + # closed block comment must leave a separator behind it — + # otherwise "return/**/Unregister();" collapses into + # "returnUnregister();" and \bUnregister can no longer match + # a real call that a comment merely interrupts. + result.append(" ") else: if c == "\n": result.append("\n") @@ -87,6 +93,14 @@ def strip_comments(text: str) -> str: def extract_braced_function(source: str, signature: str) -> str: + """Extract a function body by brace-matching, with comments stripped. + + Every sensor in this file matches against the returned text with regex + or substring checks, none of which can tell real code from a comment + describing it. Stripping here, once, means every sensor gets the same + comment-transparency the compiler has — not just the ones a reviewer + happened to wrap by hand (#123). + """ start = source.index(signature) opening = source.index("{", start) depth = 0 @@ -96,7 +110,7 @@ def extract_braced_function(source: str, signature: str) -> str: elif source[index] == "}": depth -= 1 if depth == 0: - return source[opening : index + 1] + return strip_comments(source[opening : index + 1]) raise AssertionError(f"unterminated function body: {signature}") @@ -188,11 +202,12 @@ def test_shutdown_thread_never_touches_the_callback_registry(self): # no lock (echovr.exe 0x140f8df20). The shutdown thread now hands that work to Update() # through MainThreadHandoff; its own fallback must skip the registry. source = (ROOT / "src/runtime/server/gameserver.cpp").read_text() - # Comment-stripped (#123): a commented-out call site with a decoy real - # statement nearby would otherwise still satisfy these regexes — a raw - # substring match can't tell "the call is here" from "the call is - # described in a comment above the decoy". - shutdown = strip_comments(extract_braced_function(source, "void GameServerLib::BeginGracefulShutdown(")) + # Comment-stripped (#123): extract_braced_function strips comments for + # every caller, so a commented-out call site with a decoy real + # statement nearby can't satisfy these regexes — a raw substring + # match can't tell "the call is here" from "the call is described in + # a comment above the decoy". + shutdown = extract_braced_function(source, "void GameServerLib::BeginGracefulShutdown(") for forbidden in (r"\bUnregister\s*\(\s*\)", r"\bUnregisterAllCallbacks\s*\(", r"\bGetCallbackRegistry\s*\(", r"\bUnregisterFromServerDb\s*\(\s*true"): @@ -202,19 +217,18 @@ def test_shutdown_thread_never_touches_the_callback_registry(self): "subject vanished: the shutdown thread no longer hands off to the game thread") self.assertRegex(shutdown, r"ShutdownUnregisterOnGameThread\s*\(") - off_thread = strip_comments( - extract_braced_function(source, "void GameServerLib::ShutdownUnregisterOffGameThread(")) + off_thread = extract_braced_function(source, "void GameServerLib::ShutdownUnregisterOffGameThread(") self.assertRegex(off_thread, r"UnregisterFromServerDb\s*\(\s*false\s*\)") for forbidden in (r"\bUnregister\s*\(\s*\)", r"\bUnregisterAllCallbacks\s*\(", r"\bGetCallbackRegistry\s*\(", r"\bUnregisterFromServerDb\s*\(\s*true"): self.assertNotRegex(off_thread, forbidden) # The skip must be real: the registry action is only built when asked for. - impl = strip_comments(extract_braced_function(source, "void GameServerLib::UnregisterFromServerDb(")) + impl = extract_braced_function(source, "void GameServerLib::UnregisterFromServerDb(") self.assertRegex(impl, r"if\s*\(\s*touchCallbackRegistry\s*\)\s*unregisterCallbacks\s*=") # And the game thread must actually service the hand-off, or every shutdown times out. - update = strip_comments(extract_braced_function(source, "VOID GameServerLib::Update(")) + update = extract_braced_function(source, "VOID GameServerLib::Update(") self.assertRegex(update, r"m_gameThreadHandoff\.Service\(\)") def test_broadcaster_callbacks_record_their_owner_before_listening(self): @@ -226,10 +240,9 @@ def test_broadcaster_callbacks_record_their_owner_before_listening(self): source = (ROOT / "src/runtime/server/gameserver.cpp").read_text() # Comment-stripped (#123): same reasoning as the #44 sensor above — a # commented-out RecordBroadcasterOwner call with a `owner = nullptr;` - # decoy nearby still contains the substring these regexes look for - # unless comments are removed first. - register = strip_comments( - extract_braced_function(source, "void GameServerLib::RegisterBroadcasterCallbacks(")) + # decoy nearby can't satisfy the substring these regexes look for, + # because extract_braced_function strips comments before returning. + register = extract_braced_function(source, "void GameServerLib::RegisterBroadcasterCallbacks(") record = re.search(r"\bGameServer::RecordBroadcasterOwner\s*\(\s*\*m_context\s*\)", register) self.assertIsNotNone(record, "RegisterBroadcasterCallbacks no longer records the callback owner") first_listen = re.search(r"\bListenForBroadcasterMessage\s*\(", register) @@ -239,14 +252,12 @@ def test_broadcaster_callbacks_record_their_owner_before_listening(self): # Registration, the recorded owner and the unregister guard must all name the same # broadcaster (the lobby's), or the guard rejects every handle again. - listen = strip_comments(extract_braced_function(source, "uint16_t ListenForBroadcasterMessage(")) + listen = extract_braced_function(source, "uint16_t ListenForBroadcasterMessage(") self.assertRegex(listen, r"BroadcasterListen\(\s*lobby->broadcaster\s*,") - unregister = strip_comments( - extract_braced_function(source, "void GameServerLib::UnregisterAllCallbacks(")) + unregister = extract_braced_function(source, "void GameServerLib::UnregisterAllCallbacks(") self.assertRegex(unregister, r"liveOwner\s*=\s*lobby\s*!=\s*nullptr\s*\?\s*lobby->broadcaster\s*:") helper_source = (ROOT / "src/runtime/server/callback_unregistration.cpp").read_text() - helper = strip_comments( - extract_braced_function(helper_source, "EchoVR::Broadcaster* RecordBroadcasterOwner(")) + helper = extract_braced_function(helper_source, "EchoVR::Broadcaster* RecordBroadcasterOwner(") self.assertRegex(helper, r"lobby\s*!=\s*nullptr\s*\?\s*lobby->broadcaster\s*:") self.assertRegex(helper, r"\.broadcasterOwner\s*=")