Skip to content

fix(desktop): stop portable launchers from persisting bind overrides into config.json - #1836

Open
fengyue-xve wants to merge 1 commit into
TencentCloud:developfrom
fengyue-xve:fix/desktop-no-bind-persist
Open

fengyue-xve wants to merge 1 commit into
TencentCloud:developfrom
fengyue-xve:fix/desktop-no-bind-persist

Conversation

@fengyue-xve

Copy link
Copy Markdown

Summary

Fixes #1816. octop run --host/--port persists the CLI overrides into config.json by design, but the desktop exe and the portable launchers passed their own defaults as flags on every start, so a hand-edited bind_host reverted to 127.0.0.1 the moment the app restarted.

  • desktop/src/process.go: the spawned launch.py run no longer carries --host 127.0.0.1 --port <port> flags. The same values are passed as OCTOP_BIND_HOST / OCTOP_PORT env entries, which resolve_bind honors (env > config.json) and never writes back. The desktop still binds loopback exactly as before, so no behavior changes beyond persistence.
  • desktop/portable/templates/start.bat / start.sh: --host/--port are now forwarded only when the user passed them explicitly (documented octop run flag semantics preserved). A plain launch passes neither, so the bind comes from config.json and the file is left untouched. The status line now says when values come from config.json instead of printing a possibly-wrong default URL.
  • The help text of both launchers reflects that config.json is the default source.

Target branch

  • Base is develop (feature / fix — default)

Type of change

  • Bug fix

Test plan

  • No local Go toolchain is available in this environment (go is not installed / cannot be executed), so the process.go change is verified by inspection + CI/manual; it only replaces argv flags with env entries on the same exec.Command.
  • uv run pytest tests/unit/cli/test_run_bind_resolution.py — passed, including the new test_env_overrides_do_not_touch_config_file which runs octop run with OCTOP_BIND_HOST/OCTOP_PORT set and asserts config.json keeps its original bytes (no "Saved config", uvicorn gets the env values).
  • uv run pytest tests/unit/desktop/test_portable_launcher_argv.py — 3 passed. The tests run the real start.sh with a stubbed portable Python: a default launch executes launch.py run with no --host/--port at all, while explicit flags are still forwarded. Red→green: on the unpatched script the first and third cases fail because the defaults are always appended.
  • ruff check / ruff format --check on the changed test files — clean.
  • Added/updated tests

Checklist

  • Updated CHANGELOG.md (if user-facing)

octop run writes --host/--port CLI overrides into config.json, so the
desktop exe and the start.bat/start.sh portable launchers rewrote a
hand-edited bind_host on every start (issue TencentCloud#1816).

- process.go passes OCTOP_BIND_HOST/OCTOP_PORT env instead of flags;
  env is honored by resolve_bind and never persisted
- start.bat/start.sh forward --host/--port only when the user passed
  them explicitly, so plain launches read config.json untouched

Fixes TencentCloud#1816
@fengyue-xve

Copy link
Copy Markdown
Author

Heads-up: develop currently has two unrelated repo-level CI failures that also surface on this PR. (1) The Linux job flakes on test_cold_target_install_budget_preserves_regular_timeout — fixed by #1826. (2) The Windows job fails on two plan-path tests introduced by #1825 — fixed by #1841. Neither comes from this change.

This branch has not been deployed

No deployments
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.

1 participant