Skip to content

Fix npx serve bug (port-in-use detection) - #20

Merged
skapoor8 merged 4 commits into
mainfrom
fix/npx-serve-bug
Aug 21, 2026
Merged

skapoor8 merged 4 commits into
mainfrom
fix/npx-serve-bug

Conversation

@skapoor8

Copy link
Copy Markdown
Contributor

Follow-up to merged PR #19. Adds port-in-use detection so diffwiki ui up fails fast with guidance instead of reporting a false success.

Changes:

  • fix(cli): fail ui up when port already in use (db7474d)

Notable:

  • Stops the spawned UI and exits nonzero when a foreign process owns the port
  • Distinguishes 'app responded' from 'something responded on the port'

@skapoor8 skapoor8 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Draft review of the port-in-use detection added to ui up. The main concern is that the success/failure decision is made from pid liveness (isAlive) rather than from the actual spawned child, which leaves two TOCTOU windows where the CLI reports the wrong outcome. See inline comments.

Comment thread packages/diffwiki-cli/src/cli.ts Resolved
Comment thread packages/diffwiki-cli/src/cli.ts Resolved
Comment thread packages/diffwiki-cli/src/cli.ts Resolved
@skapoor8
skapoor8 marked this pull request as ready for review August 21, 2026 07:54
Comment thread packages/diffwiki-cli/src/cli.ts Resolved
Comment thread packages/diffwiki-core/src/home.ts Resolved
startUi now exposes the spawned child; upAction decides success/failure from child.exitCode instead of isAlive(), closing the zombie TOCTOU where an unreaped dead child was reported alive. Distinguish a foreign port owner from an app that crashed after serving, and add an e2e test for the port-conflict branch.
@skapoor8

Copy link
Copy Markdown
Contributor Author

Addressed the home.ts thread: the ensureHome doc block was shortened in d3afab7 — it now describes the tree/seeding and idempotency in four lines instead of the original paragraph.

@skapoor8
skapoor8 merged commit 2678f11 into main Aug 21, 2026
4 checks passed
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 0.17.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant