Refactor CLI Detached Spawn Helper · Entire

Review of GitHub Pull Request 1724

Gather context for the target's diff:

1. Context Command:

   gh pr view 1724 --json title,body,author,baseRefName,headRefName,state,additions,deletions,changedFiles,labels
   

2. Diff Command:

   gh pr diff 1724
   

The PR's diff is the only review scope.

Code Review Summary

• Overview:
  The pull request consolidates six platform files into one execx.SpawnDetached helper.

Code Quality and Style Analysis

• Six platform files collapsed into one portable helper:
  • From: telemetry/detached_{unix,windows,other}.go and trail_refresh_detached_{unix,windows,other}.go
  • To: execx.SpawnDetached(dir, args...)
  • No redundancy in SysProcAttr logic as it reuses detachFromTTY.
• No-op Guard:
  • Introduced a testing.Testing() no-op guard to prevent forking issues.
  • Closed a hazard where tests could have re-exec’d the entire test suite.

Suggestions for Improvements

• Ensure all related documentation is updated with the new logic flow to maintain clarity.

Potential Issues or Risks

• Removed Issue References:
  • All #450 references are stripped from various comments.
  • Re-add Fixes #450 to PR body if closure linkage is desired.
• Dropping Setpgid for Setsid changes Unix detach behavior, which could affect existing sessions.

Review Focus Areas

• Code Correctness: Solid functionality observed in the implementation.
• Project Conventions: Followed established project conventions.
• Performance Implications: Consolidation should improve performance by reducing overhead.
• Test Coverage: New tests added cover additional behaviors effectively.
• Security Considerations: Overall security posture improved with fewer points of failure.

Conclusion

The current PR appears robust with significant improvements. A minor quirk remains regarding throttle marker behavior, which should be monitored but is not critical.