fix: don't report the xdebug module by default - #108

@claude review

Claude finished @carlos-granados's task in 3m 1s —— View job

Claude Code Review

• Read the PR diff
• Examine the changed files for correctness
• Check test coverage
• Post review feedback

Reviewed the four commits against origin/main. Overall the change is well‑scoped and correct — the default flip is a sensible UX fix, and the MINIT reordering to register the php_debugger.* aliases before checking settings.library.report_xdebug_module is exactly right so the flag can be set through either namespace.

What looks good

• xdebug.c MINIT reordering (xdebug.c:493‑517): REGISTER_INI_ENTRIES() populates the setting from xdebug.*, then zend_register_ini_entries(php_debugger_ini_entries, …) applies any php_debugger.* override, and only then is XG(settings.library.report_xdebug_module) read. Correct dataflow.
• PHP_INI_SYSTEM scope is the right choice — module registration only happens at MINIT, so allowing per‑request toggling would be misleading.
• src/lib/log.c:460 swap from "xdebug" → "php_debugger" is the right fix; otherwise phpinfo()'s settings table would silently show nothing whenever the alias is disabled (the default). Both name sets share module_number, so the iteration below still finds everything.
• Three‑case test matrix (default / xdebug.*=1 / php_debugger.*=1) covers the interesting branches.
• CI + Docker smoke tests, README, run-xdebug-tests.php, and src/lib/lib.h are all updated in lockstep — nothing was missed.

Suggestions (nits, non‑blocking)

1. Sync direction not verified — tests/base/report_xdebug_module-002.phpt sets xdebug.report_xdebug_module=1 but never asserts ini_get('php_debugger.report_xdebug_module'). Symmetric coverage would prove the canonical→alias direction still works for this key. Same for -003.phpt and the reverse. One extra var_dump() per file. Fix this →

2. String literal for the module name — src/lib/log.c:460 and xdebug.c:77 both hard‑code "php_debugger". A #define XDEBUG_MODULE_NAME "php_debugger" in php_xdebug.h (alongside the existing XDEBUG_NAME "PHP Debugger") would remove the duplication. Cosmetic.

3. README omits the xdebug.* form — the "Reporting the xdebug module" section only shows php_debugger.report_xdebug_module = 1, but xdebug.report_xdebug_module = 1 also works (as -002.phpt proves). One extra sentence would keep the doc consistent with the rest of the "Xdebug‑compatible names" story just above it.

4. report_xdebug_module-002.phpt title says "re‑enables the module alias" — accurate but slightly misleading if a reader hasn't seen the default‑off change; something like "enables the xdebug module alias" reads a bit cleaner. Pure wording.

5. Static‑variable lifetime — the static zend_module_entry xdebug_compat_module_entry = {0}; inside the if block is fine (only initialized once), but if MINIT ever ran twice in the same process (odd embedded SAPI cases), zend_hash_add_ptr would silently return NULL on the second insert. Pre‑existing behavior — not introduced by this PR — but worth being aware of.

Testing note

The "temporarily drop tests to isolate Windows CI failure" commit was followed by re‑adding them in 969d90e, and the working tree currently has all three test files. Confirm CI (particularly Windows) is now green on the final commit before merging, since the intermediate commit history suggests the Windows failure may have been sensitive to the test‑runner change in run-xdebug-tests.php:877‑880.

Verdict

LGTM. No correctness issues found; the items above are polish, not blockers.
• Branch: xdebug-impersonation