Skip to content

Windows: prevent console flashes and focus stealing when loading maps - #16

Closed
arhidavid wants to merge 1 commit into
thekannen:mainfrom
arhidavid:fix/windows-token-check-console
Closed

arhidavid wants to merge 1 commit into
thekannen:mainfrom
arhidavid:fix/windows-token-check-console

Conversation

@arhidavid

Copy link
Copy Markdown
Contributor

Opening or switching maps on Windows flashes two console windows while the bridge initializes its private token. These windows can steal focus from another application and interrupt typing. Whether focus returned automatically was not established. Ordinary MCP reads and edits did not reproduce the problem.

Godot 3.4’s blocking OS.execute captures output through _wpopen, which launches cmd.exe. Hiding PowerShell alone therefore does not reliably prevent the flashes.

This change starts a bundled JScript launcher through GUI wscript.exe, using nonblocking OS.execute. The launcher starts PowerShell hidden and waits for it. The existing ownership, ACL, redirected-path, and hard-link checks still run. A failure still prevents the bridge from starting.

The bridge reads a temporary UTF-8 report instead of capturing process output. Each invocation has separate, unpredictable success and failure proofs: only a successful check emits the success proof, and unauthenticated report content is never logged. Reports use CreateNew and exclusive write access. Missing launchers, invalid reports, and a 30-second timeout leave the bridge disabled.

Validation

  • Full python server/tools/gate.py passed, with DUNGEONDRAFT_TEST_EXECUTABLE set for isolated SceneTree tests using Dungeondraft’s Godot 3.4.2 engine on Windows.
  • New engine tests cover successful and failed checks, preserved error text, missing or unresponsive launchers, forged reports, and cleanup after real checks.
  • Existing token-persistence and permission tests passed.
  • Release archive tests include the helper, with byte-for-byte payload and line-ending checks.
  • python tools/release.py check, Ruff lint, and Ruff formatting checks for release tooling passed.
  • After restarting Dungeondraft, the patched bridge’s source hash was confirmed and a separate test copy of a map reloaded successfully. The human tester watched the reload and confirmed no console-window flashes.

Compatibility and documentation

Windows Script Host must be enabled. If unavailable, the bridge refuses to start; an unresponsive launcher times out after 30 seconds. This requirement is documented in docs/developer-installation.md.

The upstream technical reference should receive the corresponding Windows setup and troubleshooting note.

The fix does not alter map content or permissions on Dungeondraft-owned folders. Non-Windows launch paths are unchanged.

AI disclosure

Code was written with OpenAI Codex. Codex ran the automated tests. The absence of console flashes during map reload was verified by human eyeballs.

@thekannen

Copy link
Copy Markdown
Owner

Thanks for this, and for the thorough write-up. The flashes are real: the bridge checks the token file's permissions through PowerShell twice on every map open, and Godot 3.4 can't start a console program without a window. You also traced the cause correctly.

I'd like to fix it a different way, though, because of what the launcher looks like from outside:

  • Antivirus. A script started through wscript.exe that runs a hidden PowerShell with -EncodedCommand is a pattern Microsoft Defender and other engines watch for. Microsoft has already flagged our launchers twice with Wacatac, and a Microsoft detection blocks a release. Shipping a .js in the mod ZIP raises that risk for the one file every user installs.
  • Machines with Windows Script Host turned off. Some organisations and hardening guides disable it. Today the bridge works there; with this change it refuses to start, and Dungeondraft freezes for up to 30 seconds on each map open while the bridge waits.
  • Timeouts leave things behind. Killing wscript.exe doesn't stop the PowerShell it started, so that process can still write its report into Dungeondraft's user folder after the bridge has removed the file.

Could you try this lower-risk change first?

  1. Check once per process, not once per map. The mod is reloaded for every map, but Engine.set_meta / get_meta survive that. Record a successful check for the token path, and skip the PowerShell call on later map opens in the same Dungeondraft session.
  2. One PowerShell call instead of two, so the first map open flashes once.

That keeps the existing OS.execute path, so there's no new file and no new way to fail. Flashing once at startup isn't ideal, but nobody is typing elsewhere at that point, and it fixes the focus-stealing on map switches you described. If it still matters after that, we can look at the wscript route again, with a VirusTotal scan of the mod ZIP and a test under Defender's ASR rules before it ships.

A note on how changes land: production is built from a separate development repository, so I don't merge PRs here directly. I port accepted work as a commit authored by you, so you show up as a contributor, and it ships in the next release.

@arhidavid

Copy link
Copy Markdown
Contributor Author

Meat disclosure :) This text was written by a human and is reproduced verbatim.

Hello! Thank you for making this fantastic Dungeondraft MCP. I hope I haven't annoyed you too much with my AI-written and AI-commented pull requests.

The fixes I've suggested are things I use in my own setup and plan to continue using. I just thought that since I've already spent the tokens/usage/evaporated water in the data center, I might as well make a pull request out of it.

I fancy myself a C++ programmer, but this web/JavaScript (?) stuff is way outside my comfort zone, so I sic my agents on those problems.

Grok and I have read your comments, and I don't mind implementing your suggestions. However, Grok here has been a bad boy and already burned through a lot of my usage allowance, so I'm almost out of tokens for the next 7 days.

After that, I can sic it on your feedback, and it will hopefully address your concerns, assuming they're still relevant by then.

@thekannen

Copy link
Copy Markdown
Owner

Not annoyed at all lol I'd be lying if I said that I wasn't AI assisted as well.

The PRs were useful: #16 found a real bug, and import_image from #17 is going in.

No need to spend your tokens on #16. The change I described is small, so I'll make it myself and credit you as co-author, since you found the cause. I'll port import_image from #17 as your commit at the same time. If you want to pick up the pack builder later as its own PR, the notes on #17 still apply whenever you get to it.

@arhidavid

Copy link
Copy Markdown
Contributor Author

Deal, I will sic Abominable Intelligence on pack builder later as its own PR once I have some extra tokens in my pockets.
And don't worry about commit authorship - I'm not submitting these PRs for recognition. As I've said, the code was already written and solves actual problems, so I might as well make a PR out of it.
Feel free to attribute the commits however you see fit - to yourself, me, or Grok/ChatGPT.

@thekannen

Copy link
Copy Markdown
Owner

Sounds good, and thank you. I'll keep you credited anyway: you found the cause on #16 and wrote import_image, so it's only fair.

Both are now in the development repo. The token check runs once per Dungeondraft session instead of on every map open, and import_image is in with two fixes from macOS testing: a clean-up of the staged images that could crash Dungeondraft, and a case where importing the same image again after switching maps saved the object without its picture.

I'll hold the next release for a few days, so if the pack builder PR arrives in that window it can ship together with these. No pressure if it takes longer; it'll go in the release after.

@thekannen

Copy link
Copy Markdown
Owner

This shipped in 1.4.2, with you as the commit's author, so you're now listed as a contributor. Thank you, David.

The token check now runs once per Dungeondraft session. On Windows we measured it on fresh processes: the first map open runs the two checks, and later opens in the same session started no console processes at all (19 opens across the test suites). Closing this PR now that it's released.

@thekannen thekannen closed this Nov 8, 2026
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.

2 participants