Report actual and requested debug port in xstep JSON output#90
Conversation
When the configured debug port (9004) is already in use, DebugServer falls back to an OS-assigned ephemeral port but only mentions this via log(), which is a no-op in --json mode. A caller that only sees the JSON output had no way to know a fallback happened, or which port Xdebug actually bound to. Add debug_port (always emitted) and requested_port (emitted only when it differs from debug_port) to both JSON emission paths - outputStepRecordingResults() and outputMultipleBreakResults() - via a shared addPortInfoToResult() helper. Update the XstepJsonOutput phpstan-type and the public xstep.json schema to match.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds ChangesDebug Port Reporting
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Split out from an independent review of PR #89 (merged): a Nit noting that when the debug listener falls back from the configured port (9004) to an OS-assigned ephemeral port (see
listenOnAvailablePort()/effectiveDebugPort, introduced in #89), that fact is only surfaced throughlog(), which is a silent no-op in--jsonmode (the default forxstep). A caller that only sees the JSON output - e.g. an AI client - had no way to learn a fallback happened or which port Xdebug actually bound to.This PR adds two fields to xstep's JSON output:
debug_port- the port the listener actually bound to. Always present.requested_port- the originally configured port. Present only when it differs fromdebug_port, i.e. only when a fallback occurred.Both
outputStepRecordingResults()andoutputMultipleBreakResults()(the two JSON emission paths) now populate these via a sharedaddPortInfoToResult()helper, keeping the two paths consistent. TheXstepJsonOutputphpstan-type and the publicdocs/schemas/xstep.jsonschema are updated to match. Field naming follows the existingdebug_portprecedent ingetCurrentDebugContext().Note: this was originally planned as a stacked PR on top of PR #89's branch, but #89 merged into
1.xwhile this change was being prepared, so it is now rebased directly onto1.xas a normal PR.Test plan
vendor/bin/phpunit --no-coverage- full suite passes (312 tests, 851 assertions, 7 pre-existing skips)composer cs(phpcs) - 0 violationscomposer sa(phpstan level max,src/) - 0 errorstests/Unit/DebugServerTest.phpcovering both output methods x both cases (no fallback / fallback), using reflection to seteffectiveDebugPort/debugPortdirectly (no mocks)bin/xstepnormally and confirmeddebug_portappears withoutrequested_port; then occupied port 9004 with another process and confirmed bothdebug_port(ephemeral) andrequested_port: 9004appear correctlySummary by CodeRabbit
New Features
Documentation
Bug Fixes