fix: fix child process spawning attached

[guemidiborhane]

Fixes #1083

Summary by CodeRabbit

• Bug Fixes
  • Improved process launching on Linux and other non-Windows platforms: external commands now start in their own session and detach from the terminal. This prevents the app from hanging, suppresses unintended console output, and keeps the app responsive after opening files, links, or external tools. Windows and macOS behavior remains unchanged.

[lazysegtree]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Walkthrough

Non-Windows process spawning in executeOpenCommand is updated to start child processes in a new session (Setsid: true) with detached stdio (stdin/stdout/stderr nil) using SysProcAttr. Imports now include syscall. Windows and Darwin code paths are unchanged.

Changes

Cohort / File(s)	Summary
Process spawning and detachment src/internal/handle_panel_movement.go	Added syscall import. For non-Windows platforms in executeOpenCommand, set Cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true} and nil out stdin/stdout/stderr to detach child processes from the terminal. Windows/Darwin logic remains as before.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant Superfile
  participant OS
  participant ChildProc

  User->>Superfile: Trigger open (executeOpenCommand)
  Superfile->>OS: Build command for file opener
  alt Windows or Darwin
    OS->>ChildProc: Start process (default session/stdio)
  else Other platforms (Linux, etc.)
    note over Superfile,OS: Configure SysProcAttr{ Setsid: true }<br/>stdin=stdout=stderr=nil
    OS->>ChildProc: Start process detached (new session)
  end
  Superfile-->>User: Return control to TUI
  note over ChildProc: Continues independently of terminal

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I tap my paws, process hops away,
No string attached to terminal’s sway.
In a new session, free and bright,
mpv dances into the night.
Thump-thump—detached delight,
A bunny ships the fix just right. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "fix: fix child process spawning attached" accurately describes the primary intent of the changeset — preventing child processes from remaining attached to the terminal — and directly relates to the modifications in src/internal/handle_panel_movement.go. The code changes implement process detachment (SysProcAttr.Setsid = true and stdio redirection) for non-Windows platforms, which matches the title's focus. The title is slightly redundant with the repeated word "fix" but remains clear and relevant for a quick scan.
Linked Issues Check	✅ Passed	For linked issue #1083 (child processes closing when the terminal running superfile is closed), the PR implements detachment by setting SysProcAttr.Setsid = true and assigning nil to Stdin/Stdout/Stderr for spawned processes on non-Windows platforms, which prevents them from inheriting the terminal session and addresses the core objective. The change is narrowly targeted to executeOpenCommand and therefore meets the coding-related requirement to avoid child processes being tied to the terminal on Unix-like systems. Note that Windows and Darwin code paths are left unchanged, so if the issue needs coverage on macOS or Windows those platforms may require additional platform-specific handling or verification.
Out of Scope Changes Check	✅ Passed	The raw summary shows a single-file change (src/internal/handle_panel_movement.go) limited to importing syscall and adjusting how child processes are spawned; there are no modifications to unrelated files, public API signatures, or other unrelated functionality. These edits are directly related to the linked issue's objective and therefore there is no evidence of out-of-scope changes in this PR.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

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 and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/internal/handle_panel_movement.go (1)

108-111: Reap the child to avoid zombies; start a non‑blocking Wait goroutine.

We don’t intend to wait synchronously, but never calling Wait can leave defunct processes until superfile exits. Start a goroutine to reap quickly. Do the same in the Windows branch below.

Apply:

-err := cmd.Start()
-if err != nil {
+err := cmd.Start()
+if err != nil {
   slog.Error("Error while open file with", "error", err)
-}
+  return
+}
+go func() { _ = cmd.Wait() }()

And in the Windows block above:

-err := cmd.Start()
+err := cmd.Start()
 if err != nil {
   slog.Error("Error while open file with", "error", err)
+} else {
+  go func() { _ = cmd.Wait() }()
 }

♻️ Duplicate comments (2)

src/internal/handle_panel_movement.go (2)

101-101: Use constants for GOOS comparisons consistently.

If you keep any GOOS string comparisons elsewhere in this file, replace "windows" with utils.OsWindows to satisfy goconst and match existing style.

---

101-107: Replace inline GOOS check + SysProcAttr with portable helper; remove redundant stdio lines; also avoid bare "windows" string.

• The runtime check doesn’t help the Windows compiler; use the helper above.
• The Stdin/Stdout/Stderr assignments to nil are redundant; nil already maps to os.DevNull per os/exec docs.
• If any GOOS string checks remain, prefer utils.OsWindows (goconst). This was flagged previously; same guidance applies here.

Apply:

 cmd := exec.Command(openCommand, panel.element[panel.cursor].location)
-if runtime.GOOS != "windows" {
-  cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
-  // Optionally, redirect stdio to avoid terminal hangups
-  cmd.Stdin = nil
-  cmd.Stdout = nil
-  cmd.Stderr = nil
-}
+detachFromTerminal(cmd)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bd79f4d and f72d1c4.

📒 Files selected for processing (1)

• src/internal/handle_panel_movement.go (2 hunks)

🧰 Additional context used

🪛 GitHub Check: Build and Test (windows-latest)

src/internal/handle_panel_movement.go

[failure] 102-102:
unknown field Setsid in struct literal of type syscall.SysProcAttr

🪛 GitHub Actions: Go CI

src/internal/handle_panel_movement.go

[error] 102-102: Build failed: unknown field Setsid in struct literal of type syscall.SysProcAttr

[lazysegtree]

Looks good to me. Although I am not entire sure of the bug.

[yorukot]

LGTM, although I do not fully understand why this can help the app detach from superfile.

[guemidiborhane]

By using setsid to true, we essentially tell the OS to make the child process (eg: document viewer) a child of PID 1, which prevents terminal closing from sending SIGHUP to the child process and closing it.