Skip to content

fix(packaging): dcs-serve.exe / dcs-client.exe never start — missing __main__ guard (LOT-020 #01) - #31

Merged
davidp57 merged 1 commit into
developfrom
fix/LOT-020
Jul 26, 2026
Merged

fix(packaging): dcs-serve.exe / dcs-client.exe never start — missing __main__ guard (LOT-020 #01)#31
davidp57 merged 1 commit into
developfrom
fix/LOT-020

Conversation

@davidp57

@davidp57 davidp57 commented Jul 26, 2026

Copy link
Copy Markdown
Member

Closes ticket 01 of LOT-020.

Problem

The packaged dcs-serve.exe exits immediately with code 0: nothing listens on
127.0.0.1:7777 (DCS side) or 0.0.0.0:8080 (HTTP side), and it prints nothing — a
mission maker just sees a window flash and vanish.

Both specs pass an application module to Analysis([...]), which makes that module
the frozen script. PyInstaller runs it top-to-bottom — defining _cli, main,
_serve — and exits without ever calling main(). The Poetry console-script
(dcs-serve = "dcs_bridge.serve.app:main") is what normally invokes it, and it does
not exist inside the exe, so poetry run dcs-serve always worked and hid the defect.

client/app.py had the same defect, as the ticket suspected.

Fix

An if __name__ == "__main__": main() guard in serve/app.py and client/app.py.

Regression guards

The ticket assumed a unit test could not catch a packaging defect. It can:

  • test/test_packaging_entrypoints.py parses each .spec as an AST, resolves the
    module it freezes as its script, and asserts the guard exists and calls main().
    Runs in the normal quality gate — no PyInstaller needed. Verified red without the
    guard, green with it.
  • release.yml smoke-tests the built exes before publishing: dcs-serve.exe must
    accept TCP connections on 127.0.0.1:7777 and 8080 within 30 s (both polled,
    since asyncio.gather binds them concurrently, with the server logs dumped on
    failure); dcs-client.exe --help must list its subcommands.

A subtlety worth flagging in review

The smoke test tears the server down with taskkill /F /T, not Stop-Process. A
one-file PyInstaller exe is a bootloader that re-launches itself as a child
process
, and the child owns the sockets — Windows says so explicitly
(PID 32468 (child process of PID 24908)). Killing the PID returned by
Start-Process orphans it and leaves 7777/8080 bound, so a later run would validate
a phantom listener. The step also refuses to start if either port is already bound,
because a stale listener would otherwise yield a pass that proves nothing. Both
issues were found by actually running the gate, not by reading it.

Verification

Built both exes locally with PyInstaller 6.20 and ran the workflow's smoke-test
scripts verbatim:

Case Result
Real built exes listen on 7777 + 8080, process tree killed, ports released, no leftover files
Exe that never serves fails: exited with code 1 without serving — the frozen script never called main()
Port already bound refuses to run rather than pass meaninglessly

Quality gate: ruff clean, mypy clean on 22 files, 270 tests pass.

Also in this PR

  • .gitignore for the build_pyi/ and test-mission/ build leftovers.
  • The capabilities.py scratch edit that hardcoded framework versions to one running
    mission (self-labelled DO NOT COMMIT) is reverted, not committed. Loose version
    matching stays out of scope and deserves its own lot.
  • Corrected a stale status on the already-completed LOT-019 ticket.

Not in this PR

Ticket 02 (publish the first release) is blocked by a finding recorded on the ticket:
RELEASE_NOTES.md has never been committed, yet release.yml passes
body_path: RELEASE_NOTES.md to softprops/action-gh-release — the publish step
would have failed. It will be authored via the project's own /release-notes
procedure on a release/1.0.0 branch, which is also where the version bump to
1.0.0 belongs. This PR keeps the PATCH bump (0.8.3) per CLAUDE.md § 9.

🤖 Generated with Claude Code

Summary by Sourcery

Ensure packaged dcs-serve.exe and dcs-client.exe actually start and are guarded against packaging regressions, and bump the project patch version.

Bug Fixes:

  • Add main guards to the serve and client application modules so PyInstaller-built executables invoke main() and run correctly.

Enhancements:

  • Add unit tests that parse PyInstaller spec files to verify each frozen script module contains an if name == "main": main() guard.
  • Document the packaging fix and new CI safeguards in the Unreleased CHANGELOG section and update backlog tickets to reflect LOT-019 completion and LOT-020 progress.

CI:

  • Extend the release workflow with smoke tests that validate dcs-serve.exe listens on the expected TCP ports and dcs-client.exe exposes its CLI before publishing, including proper teardown of the PyInstaller process tree.

Documentation:

  • Add LOT-020 product and ticket documentation describing the PyInstaller entry-point issue, planned first release, and its impact on downstream tooling.

Tests:

  • Introduce packaging entrypoint tests that statically inspect PyInstaller spec files and their frozen scripts to assert the presence of a main() guard.

Chores:

  • Ignore PyInstaller build artifacts and mission test leftovers via .gitignore and bump the project version from 0.8.2 to 0.8.3.

…start (LOT-020 #1)

Both PyInstaller specs pass an application module to `Analysis([...])`, which makes
that module the frozen script: PyInstaller runs it top-to-bottom and exits. Neither
`serve/app.py` nor `client/app.py` had an `if __name__ == "__main__": main()` block,
so the built executables defined everything and quit — exit code 0, no output,
nothing listening on 7777/8080. The Poetry console-scripts
(`dcs-serve = ...app:main`) call main() for `poetry run`, which masked the gap.

Two regression guards:

- `test/test_packaging_entrypoints.py` parses each `.spec` as an AST, resolves the
  module it freezes as its script, and asserts the guard is present and calls
  main(). Runs in the normal quality gate, no PyInstaller needed.
- `release.yml` smoke-tests the built exes before publishing: `dcs-serve.exe` must
  accept TCP connections on 127.0.0.1:7777 and 8080 within 30 s (both polled, since
  asyncio.gather binds them concurrently), and `dcs-client.exe --help` must list its
  subcommands.

The smoke test refuses to run if either port is already bound — a stale listener
would otherwise produce a meaningless pass — and tears the server down with
`taskkill /T`: a one-file PyInstaller exe is a bootloader that re-launches itself as
a child process which owns the sockets, so killing the PID returned by Start-Process
orphans it and leaves the ports held.

Verified against locally built executables on all three paths: the real exes pass
and release their ports, an exe that never serves fails with an actionable message,
and a pre-bound port is rejected.

Also gitignore the `build_pyi/` and `test-mission/` build leftovers, and correct the
stale status on the completed LOT-019 ticket.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Jul 26, 2026

Copy link
Copy Markdown

🧙 Sourcery is reviewing your pull request!


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@davidp57
davidp57 merged commit 7ed0af0 into develop Jul 26, 2026
2 checks passed
@davidp57
davidp57 deleted the fix/LOT-020 branch July 26, 2026 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant