Skip to content

fix(pi): hide detached process windows on Windows#604

Open
MarsSall wants to merge 1 commit into
Gentleman-Programming:mainfrom
MarsSall:fix/windows-hide-engram-processes
Open

fix(pi): hide detached process windows on Windows#604
MarsSall wants to merge 1 commit into
Gentleman-Programming:mainfrom
MarsSall:fix/windows-hide-engram-processes

Conversation

@MarsSall

@MarsSall MarsSall commented Jul 12, 2026

Copy link
Copy Markdown

🔗 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 #N / Fixes #N / Resolves #N
Check Issue Has status:approved Linked issue has status:approved label
Check PR Has type: Label* PR has exactly one type:* label
Unit Tests go test ./... passes
E2E Tests go test -tags e2e ./internal/server/... passes

✅ Contributor Checklist

  • I linked an approved issue above (Closes #N)
  • I added exactly one type:* label to this PR
  • I ran unit tests locally: go test ./...
  • I ran e2e tests locally: go test -tags e2e ./internal/server/...
  • Docs updated (if behavior changed)
  • Commits follow conventional commits format
  • No Co-Authored-By trailers in commits

💬 Notes for Reviewers

Issue #599 is not yet labeled status:approved, so the approval check is expected to remain pending/blocked until maintainers approve it.

Summary by CodeRabbit

  • Bug Fixes
    • Prevented an unwanted console window from appearing on Windows when starting the Engram server in detached mode.
  • Tests
    • Added coverage to verify that detached server processes use the appropriate Windows launch settings.

@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The detached Engram server process now sets windowsHide: true when spawned, and a source-level test verifies this option is used with detached execution.

Changes

Windows process visibility

Layer / File(s) Summary
Detached spawn configuration and validation
plugin/pi/index.ts, plugin/pi/test/index-source.test.mjs
spawnDetached adds windowsHide: true to the detached spawn options, and the source test verifies both windowsHide: true and detached: true.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related issues

  • #536 — Addresses the Windows console flash during Pi’s automatic Engram startup through the detached process spawn configuration.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The code adds windowsHide:true to the detached spawn and a regression test, matching issue #599's silent-background requirement.
Out of Scope Changes check ✅ Passed No unrelated code changes are present; the edit and test both support the Windows console-window fix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the WindowsHide fix for detached Pi process spawning on Windows.
✨ 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

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

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.

fix(windows): Engram Git subprocesses open transient terminal windows

1 participant