feat(cmd): add detached background mode and document usage#2927
feat(cmd): add detached background mode and document usage#2927forlornMWS wants to merge 4 commits intorouter-for-me:devfrom
Conversation
|
This pull request targeted The base branch has been automatically changed to |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 519ceb145a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a --background flag for the Windows version of the CLI proxy, allowing it to run as a detached process. Documentation has been updated across multiple languages to reflect this change. The feedback suggests using syscall constants instead of magic numbers for process creation flags, utilizing os.Executable() for more reliable process relaunching, and removing a redundant check for the child process object.
- Added a note in `README.md`, `README_CN.md`, and `README_JA.md` to specify that the `--background` flag should not be combined with interactive/login/import/TUI modes. - Updated the implementation in `main.go` to enforce this restriction and log an error if violated. - Refactored the detached process creation logic in `background_windows.go` for improved clarity and error handling.
luispater
left a comment
luispater
left a comment
There was a problem hiding this comment.
Summary
This PR adds a Windows-only --background flag to relaunch cli-proxy-api.exe as a detached child process so it keeps running after the launching terminal is closed. It also documents the behavior in README (EN/CN/JA) and blocks combining --background with interactive/login/import/TUI modes.
Blocking
-
The parent process filters out
--background/-backgroundand--background=.../-background=..., but it does not handle the valid Goflagform--background true(or-background true). In that case, the standalonetruetoken will be forwarded to the detached child, which can causeflag.Parse()to stop early and ignore subsequent flags like--config.Suggestion: when filtering, if you see
--backgroundor-background, also skip the next arg if it looks like a boolean value (true/false/1/0, etc.), or otherwise adjust the approach to ensure no stray value is forwarded.
Non-blocking notes / suggestions
- Non-Windows currently accepts
--backgroundbut performs no relaunch. Consider warning/erroring (or explicitly documenting that it is a no-op off Windows). internal/cmd/background_windows.gouses a magic0x00000008for detached process creation. Consider using a named constant (if available) or add a short comment, and confirm whether you wantCREATE_NO_WINDOW/HideWindowbehavior.- The detached child redirects stdin/stdout/stderr to
NUL, so logs are dropped from the console. Consider documenting log expectations (or where logs should go) for background mode.
Test plan (recommended)
- Windows:
go build -o cli-proxy-api.exe ./cmd/server - Run:
cli-proxy-api.exe --background --config <path>and confirm:- parent exits quickly and prints detached PID
- child stays running after terminal is closed
- service is reachable (health/API endpoint)
- Regression for the blocking issue after fix:
cli-proxy-api.exe --background true --config <path>should behave equivalently to--backgroundand still honor--config.
Ensure detached relaunch strips standalone boolean values after --background so child parsing keeps subsequent flags like --config, and add unit tests to lock behavior. Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ccf3c1b97
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Reject --background true/false style before flag parsing to prevent parse-short-circuit bypasses, and adjust background arg filtering/tests to keep detached relaunch safe for interactive flags. Made-with: Cursor
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
2 participants
Summary
This PR adds a Windows-only detached background startup mode so
cli-proxy-api.execan keep running after the launching terminal is closed.Previously, when starting the server from a terminal on Windows, closing the terminal would terminate the server process. This change introduces a
--backgroundflag that relaunches the process in detached mode and exits the parent process immediately after successful spawn. It also keeps behavior unchanged on non-Windows platforms and updates multilingual docs for discoverability.Key Changes
cmd/server/main.go:--background/-background--background,-background,--background=true,-background=true)cmd.StartDetachedIfRequested(...)and return early in the parent processinternal/cmd/background_windows.goos.DevNullinternal/cmd/background_nonwindows.goREADME.md: add Windows Background Mode section with usage and behaviorREADME_CN.md: add Windows 后台运行 sectionREADME_JA.md: add Windows バックグラウンド実行 section and align feature wording with Codex supportFiles Changed
cmd/server/main.gointernal/cmd/background_windows.go(new)internal/cmd/background_nonwindows.go(new)README.mdREADME_CN.mdREADME_JA.mdBehavior Notes
--backgroundis effective on Windows (detached relaunch).stdin/stdout/stderr-> null device).--config) are preserved and passed through to the detached child process.Test Plan
go build -o cli-proxy-api.exe ./cmd/servercli-proxy-api.exe --background--background) to confirm no regression