Skip to content

More no-window flags#7122

Merged
jamadeo merged 6 commits intomainfrom
jackamadeo/more-create-no-window-flag
Feb 11, 2026
Merged

More no-window flags#7122
jamadeo merged 6 commits intomainfrom
jackamadeo/more-create-no-window-flag

Conversation

@jamadeo
Copy link
Collaborator

@jamadeo jamadeo commented Feb 10, 2026

We were missing these on a few subprocess creations

Copilot AI review requested due to automatic review settings February 10, 2026 16:31
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces unwanted console window pop-ups on Windows by setting CREATE_NO_WINDOW on various subprocess invocations across the core goose crate, CLI, and MCP server components.

Changes:

  • Add Windows-only creation_flags(CREATE_NO_WINDOW) to multiple Command invocations (tokio + std).
  • Refactor several command constructions to let mut cmd = ...; cmd.args(...); ...; cmd.output()/status() to enable setting flags before execution.
  • Add Windows CommandExt imports where needed for std::process::Command.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
crates/goose/src/providers/azureauth.rs Prevents Azure CLI token fetch from spawning a visible window on Windows.
crates/goose/src/posthog.rs Prevents cmd /C ver telemetry platform detection from opening a window on Windows.
crates/goose/src/agents/retry.rs Prevents retry success-check shell commands from opening a window on Windows.
crates/goose-mcp/src/developer/shell.rs Prevents developer shell commands and taskkill from opening windows on Windows.
crates/goose-mcp/src/developer/paths.rs Prevents PATH discovery shell calls from opening a window on Windows.
crates/goose-mcp/src/computercontroller/platform/windows.rs Prevents PowerShell automation script execution from opening a window on Windows.
crates/goose-mcp/src/computercontroller/mod.rs Prevents automation script execution paths from opening a window on Windows.
crates/goose-cli/src/session/output.rs Prevents status hook execution via cmd /C ... from opening a window on Windows.
crates/goose-cli/src/recipes/github_recipe.rs Prevents gh/git helper commands used for GitHub recipes from opening windows on Windows.

Comment on lines 137 to 139
#[cfg(windows)]
cmd.creation_flags(0x08000000 /* CREATE_NO_WINDOW */);
let output = cmd
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The Windows CREATE_NO_WINDOW flag is currently an inline magic number; consider reusing a shared helper (e.g., crate::subprocess::configure_subprocess) or a local const to avoid duplicating 0x08000000 and keep subprocess configuration consistent.

Copilot uses AI. Check for mistakes.
Comment on lines 107 to 113
let mut cmd = Command::new("gh");
cmd.args(["auth", "status"]);
#[cfg(windows)]
cmd.creation_flags(0x08000000 /* CREATE_NO_WINDOW */);
let status = cmd.status().map_err(|_| {
anyhow::anyhow!("Failed to run `gh auth status`. Make sure you have `gh` installed.")
})?;
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

This repeats the Windows CREATE_NO_WINDOW value in several places in this module; consider introducing a module-level const for 0x08000000 (or a small helper) to reduce duplication and make the intent clearer.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 does using a const make sense?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@katzdave yeah, it does, but it means you have to either make that also conditionally compile, and/or tell the linter it can be unused (in non-windows mode), with similar flags everywhere you import it if it's in a use

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

replaced it with an extension trait which makes this all much cleaner

Comment on lines 107 to 113
let mut cmd = Command::new("gh");
cmd.args(["auth", "status"]);
#[cfg(windows)]
cmd.creation_flags(0x08000000 /* CREATE_NO_WINDOW */);
let status = cmd.status().map_err(|_| {
anyhow::anyhow!("Failed to run `gh auth status`. Make sure you have `gh` installed.")
})?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 does using a const make sense?

Copilot AI review requested due to automatic review settings February 11, 2026 16:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

@jamadeo jamadeo added this pull request to the merge queue Feb 11, 2026
Merged via the queue into main with commit fb47728 Feb 11, 2026
25 checks passed
@jamadeo jamadeo deleted the jackamadeo/more-create-no-window-flag branch February 11, 2026 17:53
zanesq added a commit to Abhijay007/goose that referenced this pull request Feb 11, 2026
* upstream/main: (109 commits)
  [docs] Skills Marketplace UI Improvements (block#7158)
  More no-window flags (block#7122)
  feat: Allow overriding default bat themes using environment variables (block#7140)
  Make the system prompt smaller (block#6991)
  Pre release script (block#7145)
  Spelling (block#7137)
  feat(mcp): upgrade rmcp to 0.15.0 and advertise MCP Apps UI extension capability (block#6927)
  fix: ensure assistant messages with tool_calls include content field (block#7076)
  fix(canonical): handle gcp_vertex_ai model mapping correctly (block#6836)
  Group dependencies in root Cargo.toml (block#6948)
  refactor: updated elevenLabs API module and `remove button` UX (block#6781)
  fix: we were missing content from langfuse traces (block#7135)
  docs: update username in authors.yml (block#7132)
  fix extension selector syncing issues (block#7133)
  fix(acp): per-session Agent for model isolation and load_session restore (block#7115)
  fix(claude-code): defensive coding improvements for model switching (block#7131)
  feat(claude-code): dynamic model listing and mid-session model switching (block#7120)
  Inline worklet source (block#7128)
  [docs] One shot prompting is dead - Blog Post (block#7113)
  fix: correct spelling of Debbie O'Brien's name in authors.yml (block#7127)
  ...
jh-block added a commit that referenced this pull request Feb 12, 2026
* origin/main: (33 commits)
  fix: replace panic with proper error handling in get_tokenizer (#7175)
  Lifei/smoke test for developer (#7174)
  fix text editor view broken (#7167)
  docs: White label guide (#6857)
  Add PATH detection back to developer extension (#7161)
  docs: pin version in ci/cd (#7168)
  Desktop: - No Custom Headers field for custom OpenAI-compatible providers  (#6681)
  feat: edit model and extensions of a recipe from GUI (#6804)
  feat: MCP support for agentic CLI providers (#6972)
  docs: keyring fallback to secrets.yaml (#7165)
  feat: load provider/model specified inside the recipe config (#6884)
  fix ask-ai bot hitting tool call limits (#7162)
  fix flatpak icon (#7154)
  [docs] Skills Marketplace UI Improvements (#7158)
  More no-window flags (#7122)
  feat: Allow overriding default bat themes using environment variables (#7140)
  Make the system prompt smaller (#6991)
  Pre release script (#7145)
  Spelling (#7137)
  feat(mcp): upgrade rmcp to 0.15.0 and advertise MCP Apps UI extension capability (#6927)
  ...
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