fix(pi): hide detached process windows on Windows

[MarsSall]

🔗 Linked Issue

Closes #599

---

🏷️ PR Type

• type:bug — Bug fix
• type:feature — New feature
• type:docs — Documentation only
• type:refactor — Code refactoring (no behavior change)
• type:chore — Maintenance, dependencies, tooling
• type:breaking-change — Breaking change

---

📝 Summary

• Hide detached Engram subprocess windows on Windows.
• Add a source regression test that protects the windowsHide: true spawn option.

📂 Changes

File	Change
plugin/pi/index.ts	Set windowsHide: true when spawning detached Engram processes.
plugin/pi/test/index-source.test.mjs	Verify detached process spawning keeps Windows console windows hidden.

🧪 Test Plan

• Unit tests pass locally: go test ./...
• E2E tests pass locally: go test -tags e2e ./internal/server/...
• Manually tested the affected functionality
• Regression assertion passes: cd plugin/pi && node --test test/index-source.test.mjs (spawnDetached hides detached process windows)

The targeted regression assertion passes. The test file still reports two unrelated, pre-existing Cannot use import statement outside a module failures that reproduce on upstream/main.

---

🤖 Automated Checks

These run automatically and all must pass before merge:

Check	What it verifies	Status
Check Issue Reference	PR body contains Closes/Fixes/Resolves #N	✅ Linked
Check Issue Has status:approved	Linked issue has status:approved label	⏳ Maintainer action required
Check PR Has type: Label*	PR has exactly one type:* label	⏳ Maintainer action required
Unit Tests	go test ./... passes	⏳ Workflow approval required
E2E Tests	go test -tags e2e ./internal/server/... passes	⏳ Workflow approval required

---

✅ Contributor Checklist

• I linked an approved issue above (Closes #N) (linked, awaiting maintainer approval)
• I added exactly one type:* label to this PR (awaiting maintainer permissions)
• I ran unit tests locally: go test ./...
• I ran e2e tests locally: go test -tags e2e ./internal/server/...
• Documentation is not required for this internal process-launch fix
• Commits follow conventional commits format
• No Co-Authored-By trailers in commits

---

💬 Notes for Reviewers

Issue #599 is not yet labeled status:approved, and this fork contributor cannot add repository labels. A maintainer needs to:

1. decide whether fix(windows): Engram Git subprocesses open transient terminal windows #599 should remain separate from the related Windows: console flash when Pi auto-starts engram serve (startup and subagents) #536;
2. add status:approved to fix(windows): Engram Git subprocesses open transient terminal windows #599 and type:bug to this PR;
3. approve the pending fork workflows so CI and PR Validation can run.

Summary by CodeRabbit

• Bug Fixes

  • Prevented console windows from appearing when background processes start on Windows.
  • Improved the Windows experience when detecting projects through Git commands.

• Tests

  • Added platform-specific coverage to verify hidden process windows and preserve existing command behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a9a9693e-93cf-4094-b269-943c0b4c5eef

📥 Commits

Reviewing files that changed from the base of the PR and between 6211fcb and 67f66ee.

📒 Files selected for processing (7)

• internal/project/command_other.go
• internal/project/command_other_test.go
• internal/project/command_usage_test.go
• internal/project/command_windows.go
• internal/project/command_windows_test.go
• internal/project/detect.go
• plugin/pi/index.ts

---

📝 Walkthrough

Walkthrough

Changes

Windows process visibility

Layer / File(s)	Summary
Detached plugin spawn configuration plugin/pi/index.ts	spawnDetached now passes windowsHide: true when starting the detached Engram server.
Platform-specific Git command helper internal/project/command_other.go, internal/project/command_windows.go, internal/project/command_other_test.go, internal/project/command_windows_test.go	Platform-specific helpers create context-aware commands; Windows commands set SysProcAttr.HideWindow, with tests covering both platforms.
Git detection command wiring and validation internal/project/detect.go, internal/project/command_usage_test.go	Both Git detection paths use the platform helper, and an AST test verifies each function calls it once.

Estimated code review effort: 2 (Simple) | ~15 minutes

Suggested reviewers: gentleman-programming

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately describes the Windows console-hiding fix.
Linked Issues check	✅ Passed	The changes add windowsHide/HideWindow handling and route Git subprocesses through it, matching #599's silent-background requirement.
Out of Scope Changes check	✅ Passed	All changes support the Windows window-hiding fix and related regression tests; no unrelated edits are evident.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Alan-TheGentleman]

Thanks for chasing this down — the bug in #599 is real and worth fixing.

The problem is that this fixes the wrong layer. The evidence chain in #599 is engram serve → git.exe → conhost.exe → WindowsTerminal.exe, i.e. the windows come from Go-side subprocesses, not from the Node spawn. Those live at internal/project/detect.go:282 (git -C … rev-parse --show-toplevel) and :381 (git … remote get-url origin) — the exact commands in your report. rg "HideWindow|CreationFlags|SysProcAttr" returns nothing across the repo, so no Go call site currently suppresses a console.

windowsHide on the Node spawn only suppresses the engram serve window. A console-subsystem child like git.exe launched from a console-less parent still allocates its own console, so the transient windows you're seeing would persist.

• Move the fix into Go: set SysProcAttr{HideWindow: true} on detect.go:282 and :381 behind a //go:build windows helper. This also matches the thin-adapter rule — plugins are shims, logic belongs in core.
• Keep windowsHide: true as a complementary plugin change if you like, but replace the source-regex test: asserting windowsHide: true, appears before detached: true, breaks on any key reorder without a behavior change.
• Rebase — #640 just landed and rewrote plugin/pi/index.ts substantially, including moving spawnDetached.

I'll get #599 labelled status:approved so this isn't blocked on process.

[Alan-TheGentleman]

This is exactly the change I was asking for, and then some. Approving.

You moved the fix to the layer where the bug actually lives: newProjectCommandContext behind //go:build windows setting SysProcAttr{HideWindow: true}, with a plain passthrough for every other platform, wired into detectGitRootDir and detectFromGitRemote — the two git.exe call sites named in #599's engram serve → git.exe → conhost.exe chain. Keeping windowsHide: true on the Node spawn as a complementary change is right too, and you dropped the brittle source-regex test I flagged.

What I verified locally on your branch, rebased onto current main:

• GOOS=windows GOARCH=amd64 go build ./... passes. Worth calling out because command_windows.go and its syscall.SysProcAttr never compile on the maintainers' macOS machines or in the default CI job, so this file could have been broken without anyone noticing. It isn't.
• go build ./... and the full go test ./... pass, 22 packages.
• plugin/pi suite passes 42/42, so the index.ts change composes cleanly with #640 which rewrote that file.
• TestNewProjectCommandContextPreservesDefaultProcessAttributes and TestProjectDetectionGitCommandsUsePlatformHelper both pass.

The AST-based test is a nice touch. Asserting that each detection function calls the helper exactly once means a future contributor who reaches for a raw exec.CommandContext there gets a failing test instead of silently reintroducing the console flash. That's the right way to guard a platform behavior you can't assert on directly from CI.

Follow-up, not blocking this PR

Two other subprocess call sites are in the same bug class but outside #599's reported path, so I don't want to hold this fix for them:

• internal/llm/claude.go:140 (defaultRunCLI) spawns the agent CLI and will flash a console on Windows the same way.
• internal/setup/setup.go:36 (runCommand) does too, though it runs during interactive engram setup where a console window is far less surprising.

Happy to open a separate issue for those, or you can if you want to carry the work forward — the helper you built here is the obvious thing to reuse, probably lifted somewhere both packages can import.

[Alan-TheGentleman]

Merged. Opened #641 to track the remaining call sites (internal/llm/claude.go and internal/setup/setup.go) and pre-approved it, so it's open if you want to carry the helper forward — lifting it out of internal/project into something all three packages can import is probably the cleaner shape. Thanks for reworking this into the right layer.