Skip to content

fix: hide detached broker window on Windows - #451

Open
e345ee wants to merge 3 commits into
openai:mainfrom
e345ee:fix-broker-windows-hide
Open

fix: hide detached broker window on Windows#451
e345ee wants to merge 3 commits into
openai:mainfrom
e345ee:fix-broker-windows-hide

Conversation

@e345ee

@e345ee e345ee commented Jul 8, 2026

Copy link
Copy Markdown

Summary

  • Add windowsHide: true to the detached broker spawn path.
  • Add a focused unit test covering the broker spawn options.

Fixes #440.

Test plan

  • node --test tests/broker-lifecycle.test.mjs
  • git diff --check
  • npm test
  • npm run build

@e345ee
e345ee requested a review from a team July 8, 2026 07:14
@Wintersta7e

Copy link
Copy Markdown

The spawnImpl seam plus asserting the captured options is a better test than scanning source text — it proves the option actually reaches spawn, rather than that the string appears somewhere in the file.

Worth folding in while you're in here: there's a third spawn site with the same problem that neither #440 nor #441 covers. plugins/codex/scripts/stop-review-gate-hook.mjs spawns the companion with

spawnSync(process.execPath, [scriptPath, "task", "--json", prompt], {
  cwd, env: childEnv, encoding: "utf8", timeout: STOP_REVIEW_TIMEOUT_MS
});

and no windowsHide. Unlike the broker, that one runs at every turn end while the review gate is enabled, so on Windows it's a recurring console window rather than a one-off. lib/process.mjs already sets windowsHide: true on its own spawnSync, which makes the hook the odd one out.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d028ce9c5f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

process.stderr.write(`${message}\n`);
process.exitCode = 1;
function isMainModule() {
return process.argv[1] ? import.meta.url === pathToFileURL(path.resolve(process.argv[1])).href : false;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Resolve symlinks when identifying the hook entrypoint

When stop-review-gate-hook.mjs is invoked through a symlinked ${CLAUDE_PLUGIN_ROOT}—for example, a locally linked plugin checkout—Node resolves import.meta.url to the real target while path.resolve(process.argv[1]) retains the symlink path. This comparison then returns false, causing the hook to exit successfully without consuming its input or running an enabled stop review, silently bypassing the gate. Compare canonical real paths or keep the executable entry module unconditional.

Useful? React with 👍 / 👎.

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.

spawnBrokerProcess() missing windowsHide: true — leftover spawn site from #67

2 participants