fix: start --serve honours the manifest ui: section#167
Merged
Conversation
…oken) `start --serve` (the daemon path) ignored config.ui entirely, always defaulting --serve-host/--serve-port/--serve-token to loopback :8000 with no auth even when the manifest declared a ui: section — unlike `dashboard`, which already resolves host/port/token from ui: with flags/env overriding. A manifest configured for `dashboard` silently served on loopback with no token when launched via `start --serve` instead. Mirror `dashboard`'s resolution: the three options default to None and resolve against config.ui.* after loading the config, with the same non-loopback-without-token refusal (still enforced in _run_daemon) applying to the resolved values. A bare AppConfig() (no --config) keeps today's loopback :8000, no-auth default.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
trading-bot dashboardresolves its web settings from the manifest'sui:section (host/port/token), with CLI flags/env overriding — set once in the config, serve the same way every launch.trading-bot start --serve(the daemon path) ignoredconfig.uientirely:--serve-hostalways defaulted to127.0.0.1,--serve-portto8000,--serve-tokentoNone, regardless of what the manifest declared. A manifest configured withui: {host: 0.0.0.0, port: 8000, token: ...}would silently serve on loopback with no auth when launched viastart --serveinstead ofdashboard— this bit the maintainer in production (dashboard unreachable over the tailnet after switching tostart --serve).--serve-host/--serve-port/--serve-tokeninstartnow default toNoneand resolve againstconfig.ui.host/.port/.tokenafter loading the config — the same precedence asdashboard(explicit flag, orTRADING_BOT_UI_TOKENenv for the token, wins; otherwise the manifest)._run_daemon) now evaluates the resolved values, so it applies whether the unsafe combination came from a flag or from the manifest.AppConfig()(no--config) keeps today's behaviour: loopback:8000, no auth —UIConfig's own defaults already match.read_onlyintentionally stays out of scope: it doesn't apply to the control daemon.Test plan
python -m pytest— 1177 passed (includes 4 new tests: manifest flow-through + non-loopback-with-token accepted, explicit flag override, no-config/no-flags unchanged default, non-loopback-without-token refusal)ruff check trading_bot/— cleanruff format --check trading_bot/— cleanmypy trading_bot/— clean🤖 Generated with Claude Code