feat: support desktop dashboard listen config#134
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request changes the default dashboard host from 127.0.0.1 to 0.0.0.0 and updates the environment configuration logic and tests. Feedback highlights a security concern regarding binding to 0.0.0.0 by default, which exposes the service to the local network and triggers OS firewall prompts, recommending to keep 127.0.0.1 as the default. Additionally, a minor code simplification is suggested to inline the default host conversion.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| "ASTRBOT_DASHBOARD_SKIP_DEFAULT_PASSWORD_AUTH"; | ||
| const DASHBOARD_SKIP_DEFAULT_PASSWORD_AUTH_ENV: &str = "DASHBOARD_SKIP_DEFAULT_PASSWORD_AUTH"; | ||
| const DEFAULT_DASHBOARD_HOST: &str = "127.0.0.1"; | ||
| const DEFAULT_DASHBOARD_HOST: &str = "0.0.0.0"; |
Contributor
There was a problem hiding this comment.
Security & Usability Concerns with 0.0.0.0 by Default
Binding the dashboard to 0.0.0.0 by default exposes the service to all network interfaces (LAN/WAN). For a desktop application, this introduces several security and usability concerns:
- Security Exposure: It increases the attack surface by allowing any device on the local network to connect to the dashboard. Even with password authentication enabled, exposing the service by default to the entire LAN can lead to unauthorized access attempts, potential brute-force attacks, or exploitation of any unpatched vulnerabilities in the backend.
- OS Firewall Prompts: On Windows and macOS, binding to
0.0.0.0typically triggers an OS firewall warning prompt (e.g., "Windows Defender Firewall has blocked some features of this app"). This can be alarming or confusing to non-technical users. - Privacy: In public or shared network environments (e.g., coffee shops, hotels), other users on the same network could discover and attempt to access the dashboard.
Recommendation
It is highly recommended to keep the default host as 127.0.0.1 (localhost) to maintain a secure-by-default posture and avoid firewall prompts. If users need LAN access, they can explicitly opt-in by setting the DASHBOARD_HOST or ASTRBOT_DASHBOARD_HOST environment variable to 0.0.0.0.
Suggested change
| const DEFAULT_DASHBOARD_HOST: &str = "0.0.0.0"; | |
| const DEFAULT_DASHBOARD_HOST: &str = "127.0.0.1"; |
Comment on lines
+54
to
+58
| let default_dashboard_host = OsStr::new(DEFAULT_DASHBOARD_HOST); | ||
| let effective_host = dashboard_host_env | ||
| .as_deref() | ||
| .or(astrbot_dashboard_host_env.as_deref()); | ||
| .or(astrbot_dashboard_host_env.as_deref()) | ||
| .or(Some(default_dashboard_host)); |
Contributor
There was a problem hiding this comment.
If the decision is made to keep 0.0.0.0 as the default host, we can simplify this block by removing the intermediate default_dashboard_host variable and inlining OsStr::new(DEFAULT_DASHBOARD_HOST) directly into the option chain. This makes the code more concise and idiomatic.
let effective_host = dashboard_host_env
.as_deref()
.or(astrbot_dashboard_host_env.as_deref())
.or(Some(OsStr::new(DEFAULT_DASHBOARD_HOST)));
zouyonghe
changed the title
zouyonghe
changed the title
Member
Author
|
@sourcery-ai review |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
read_desktop_dashboard_configpath currently parses viaserde_json::Valueand manual extraction; consider defining a small typed config struct and derivingDeserializefordashboard.host/dashboard.portso the parsing logic is simpler and schema issues are caught more explicitly. - Since
configure_desktop_dashboard_environmentnow always reads and parses the desktop config file whenroot_diris present, consider whether this should be cached or passed in from the caller if the function can be invoked multiple times during a single run to avoid repeated disk I/O and JSON parsing.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `read_desktop_dashboard_config` path currently parses via `serde_json::Value` and manual extraction; consider defining a small typed config struct and deriving `Deserialize` for `dashboard.host`/`dashboard.port` so the parsing logic is simpler and schema issues are caught more explicitly.
- Since `configure_desktop_dashboard_environment` now always reads and parses the desktop config file when `root_dir` is present, consider whether this should be cached or passed in from the caller if the function can be invoked multiple times during a single run to avoid repeated disk I/O and JSON parsing.
## Individual Comments
### Comment 1
<location path="src-tauri/src/backend/launch.rs" line_range="134-140" />
<code_context>
+ }
+ };
+ let dashboard = parsed.get("dashboard");
+ DesktopDashboardConfig {
+ host: dashboard
+ .and_then(|value| value.get("host"))
+ .and_then(parse_dashboard_host),
+ port: dashboard
+ .and_then(|value| value.get("port"))
+ .and_then(parse_dashboard_port),
+ }
+}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Invalid `host`/`port` values in the config are silently ignored; consider logging when values are present but rejected.
When `dashboard.host` or `dashboard.port` are present but fail validation (e.g. blank host, out-of-range port), `parse_dashboard_host/port` return `None` without logging. This makes it unclear to users why their config isn’t applied. Please consider logging when a `dashboard` section is present but its `host`/`port` are rejected so misconfigurations are visible.
Suggested implementation:
```rust
};
let dashboard = parsed.get("dashboard");
```
```rust
let dashboard = parsed.get("dashboard");
let (host, port) = if let Some(dashboard) = dashboard {
let raw_host = dashboard.get("host");
let parsed_host = raw_host.and_then(parse_dashboard_host);
if raw_host.is_some() && parsed_host.is_none() {
log("desktop config: ignoring invalid dashboard.host value");
}
let raw_port = dashboard.get("port");
let parsed_port = raw_port.and_then(parse_dashboard_port);
if raw_port.is_some() && parsed_port.is_none() {
log("desktop config: ignoring invalid dashboard.port value");
}
(parsed_host, parsed_port)
} else {
(None, None)
};
```
```rust
let dashboard = parsed.get("dashboard");
DesktopDashboardConfig {
host,
port,
}
fn sanitize_packaged_python_environment<F>(command: &mut Command, log: F)
```
</issue_to_address>
### Comment 2
<location path="src-tauri/src/backend/launch.rs" line_range="54" />
<code_context>
}
-fn configure_desktop_dashboard_environment(command: &mut Command) {
+fn configure_desktop_dashboard_environment<F>(
+ command: &mut Command,
+ root_dir: Option<&Path>,
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the new dashboard configuration logic by removing the generic logger, using typed deserialization instead of manual JSON handling, and optionally computing an effective config before applying environment variables.
You can keep the new functionality but simplify a few hot spots to reduce cognitive load.
### 1. Decouple logging from `configure_desktop_dashboard_environment`
`configure_desktop_dashboard_environment` doesn’t really need to be generic over a logger; it only needs config values and a `Command`. You can keep logging customizable inside `read_desktop_dashboard_config` instead.
```rust
fn configure_desktop_dashboard_environment(
command: &mut Command,
root_dir: Option<&Path>,
) {
let dashboard_host_env = env::var_os(DASHBOARD_HOST_ENV);
let astrbot_dashboard_host_env = env::var_os(ASTRBOT_DASHBOARD_HOST_ENV);
let dashboard_port_env = env::var_os(DASHBOARD_PORT_ENV);
let astrbot_dashboard_port_env = env::var_os(ASTRBOT_DASHBOARD_PORT_ENV);
let astrbot_skip_auth_env = env::var_os(ASTRBOT_DASHBOARD_SKIP_DEFAULT_PASSWORD_AUTH_ENV);
let legacy_skip_auth_env = env::var_os(DASHBOARD_SKIP_DEFAULT_PASSWORD_AUTH_ENV);
let desktop_config = read_desktop_dashboard_config(root_dir);
let config_host = desktop_config.host.as_deref().map(OsStr::new);
let effective_host = dashboard_host_env
.as_deref()
.or(astrbot_dashboard_host_env.as_deref())
.or(config_host);
let has_explicit_skip_auth =
astrbot_skip_auth_env.is_some() || legacy_skip_auth_env.is_some();
if dashboard_host_env.is_none() && astrbot_dashboard_host_env.is_none() {
command.env(
DASHBOARD_HOST_ENV,
desktop_config.host.as_deref().unwrap_or(DEFAULT_DASHBOARD_HOST),
);
}
if dashboard_port_env.is_none() && astrbot_dashboard_port_env.is_none() {
command.env(
DASHBOARD_PORT_ENV,
desktop_config.port.as_deref().unwrap_or(DEFAULT_DASHBOARD_PORT),
);
}
if should_skip_default_password_auth(has_explicit_skip_auth, effective_host) {
command.env(ASTRBOT_DASHBOARD_SKIP_DEFAULT_PASSWORD_AUTH_ENV, "true");
}
}
```
Then `start_backend` just calls:
```rust
configure_desktop_dashboard_environment(&mut command, plan.root_dir.as_deref());
```
### 2. Use a typed config instead of `serde_json::Value`
You can keep the same validation behavior but remove most of the manual JSON walking:
```rust
#[derive(Debug, Default, serde::Deserialize)]
struct DesktopDashboardConfigFile {
#[serde(default)]
dashboard: DesktopDashboardConfig,
}
#[derive(Debug, Default, serde::Deserialize)]
struct DesktopDashboardConfig {
#[serde(deserialize_with = "deserialize_host", default)]
host: Option<String>,
#[serde(deserialize_with = "deserialize_port", default)]
port: Option<String>, // keep as String so the env setup stays unchanged
}
```
Deserializer helpers can encapsulate the “trim / range check / string-or-number” logic you already have:
```rust
fn deserialize_host<'de, D>(deserializer: D) -> Result<Option<String>, D::Error>
where
D: serde::Deserializer<'de>,
{
let raw: Option<String> = Option::deserialize(deserializer)?;
Ok(raw
.as_deref()
.map(str::trim)
.filter(|s| !s.is_empty())
.map(ToString::to_string))
}
fn deserialize_port<'de, D>(deserializer: D) -> Result<Option<String>, D::Error>
where
D: serde::Deserializer<'de>,
{
use serde::de::{Error, Unexpected};
#[derive(serde::Deserialize)]
#[serde(untagged)]
enum PortRaw {
Num(u64),
Str(String),
}
let raw: Option<PortRaw> = Option::deserialize(deserializer)?;
let Some(raw) = raw else { return Ok(None); };
let port_u16 = match raw {
PortRaw::Num(n) => {
if n == 0 || n > u16::MAX as u64 {
return Err(D::Error::invalid_value(Unexpected::Unsigned(n), &"1..=65535"));
}
n as u16
}
PortRaw::Str(s) => {
let trimmed = s.trim();
let parsed: u16 = trimmed.parse().map_err(|_| {
D::Error::invalid_value(Unexpected::Str(&s), &"a port number in 1..=65535")
})?;
if parsed == 0 {
return Err(D::Error::invalid_value(Unexpected::Str(&s), &"1..=65535"));
}
parsed
}
};
Ok(Some(port_u16.to_string()))
}
```
With that, `read_desktop_dashboard_config` becomes much simpler and drops the `serde_json::Value` and helper functions:
```rust
fn read_desktop_dashboard_config(root_dir: Option<&Path>) -> DesktopDashboardConfig {
let Some(root_dir) = root_dir else {
return DesktopDashboardConfig::default();
};
let config_path = root_dir.join(DESKTOP_CONFIG_RELATIVE_PATH);
if !config_path.is_file() {
return DesktopDashboardConfig::default();
}
let raw = match fs::read_to_string(&config_path) {
Ok(raw) => raw,
Err(error) => {
append_desktop_log(&format!(
"failed to read desktop config {}: {}",
config_path.display(),
error
));
return DesktopDashboardConfig::default();
}
};
match serde_json::from_str::<DesktopDashboardConfigFile>(&raw) {
Ok(file) => file.dashboard,
Err(error) => {
append_desktop_log(&format!(
"failed to parse desktop config {}: {}",
config_path.display(),
error
));
DesktopDashboardConfig::default()
}
}
}
```
### 3. (Optional) Compute an “effective config” before applying env
If you want to further untangle responsibilities, you can precompute the final values, then apply them and compute skip-auth from that:
```rust
struct EffectiveDashboardConfig<'a> {
host: &'a str,
port: &'a str,
}
fn resolve_effective_dashboard_config(
env_host: Option<&OsStr>,
env_port: Option<&OsStr>,
config: &DesktopDashboardConfig,
) -> EffectiveDashboardConfig<'_> {
let host = env_host
.and_then(|h| h.to_str())
.or(config.host.as_deref())
.unwrap_or(DEFAULT_DASHBOARD_HOST);
let port = env_port
.and_then(|p| p.to_str())
.or(config.port.as_deref())
.unwrap_or(DEFAULT_DASHBOARD_PORT);
EffectiveDashboardConfig { host, port }
}
```
Then in `configure_desktop_dashboard_environment`:
```rust
let effective = resolve_effective_dashboard_config(
dashboard_host_env.as_deref().or(astrbot_dashboard_host_env.as_deref()),
dashboard_port_env.as_deref().or(astrbot_dashboard_port_env.as_deref()),
&desktop_config,
);
command.env(DASHBOARD_HOST_ENV, effective.host);
command.env(DASHBOARD_PORT_ENV, effective.port);
if should_skip_default_password_auth(
has_explicit_skip_auth,
Some(OsStr::new(effective.host)),
) {
command.env(ASTRBOT_DASHBOARD_SKIP_DEFAULT_PASSWORD_AUTH_ENV, "true");
}
```
This keeps all the behavior you added (config file, env precedence, validation, tests) but removes the generic logging parameter, the manual `serde_json::Value` plumbing, and the spread-out config handling.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src-tauri/src/backend/launch.rs" line_range="54" />
<code_context>
}
-fn configure_desktop_dashboard_environment(command: &mut Command) {
+fn configure_desktop_dashboard_environment<F>(
+ command: &mut Command,
+ root_dir: Option<&Path>,
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the new dashboard config handling by removing generic logger types, deserializing into typed structs instead of manual `serde_json::Value` traversal, and centralizing host/port precedence logic in a small helper.
You can keep the new behavior while reducing complexity in three small refactors:
---
### 1. Drop the generic logger parameter
`configure_desktop_dashboard_environment` and `read_desktop_dashboard_config` don’t need to be generic over `F`; using a trait object keeps testability while simplifying signatures and avoiding monomorphization.
```rust
fn configure_desktop_dashboard_environment(
command: &mut Command,
root_dir: Option<&Path>,
log: &mut dyn FnMut(&str),
) {
let dashboard_host_env = env::var_os(DASHBOARD_HOST_ENV);
// ...
let desktop_config = read_desktop_dashboard_config(root_dir, log);
// ...
}
fn read_desktop_dashboard_config(
root_dir: Option<&Path>,
log: &mut dyn FnMut(&str),
) -> DesktopDashboardConfig {
// unchanged body, but call `log(...)` via &mut dyn FnMut
}
```
Call sites stay simple:
```rust
configure_desktop_dashboard_environment(
&mut command,
plan.root_dir.as_deref(),
&mut |msg| append_desktop_log(msg),
);
// tests
configure_desktop_dashboard_environment(&mut command, None, &mut |_| {});
```
This removes the generic noise from public signatures while preserving the ability to capture logs in tests.
---
### 2. Replace `serde_json::Value` plumbing with typed config
You can keep the same validation rules but deserialize into small structs instead of manually walking `serde_json::Value`. That removes several branches and helper functions.
```rust
#[derive(Debug, Default, serde::Deserialize)]
struct DesktopFileConfig {
#[serde(default)]
dashboard: Option<DesktopFileDashboard>,
}
#[derive(Debug, Default, serde::Deserialize)]
struct DesktopFileDashboard {
host: Option<String>,
#[serde(default)]
port: Option<DesktopPortField>,
}
#[derive(Debug, serde::Deserialize)]
#[serde(untagged)]
enum DesktopPortField {
Num(u64),
Str(String),
}
impl DesktopDashboardConfig {
fn from_file_config(cfg: DesktopFileConfig, log: &mut dyn FnMut(&str)) -> Self {
let dashboard = match cfg.dashboard {
Some(d) => d,
None => return DesktopDashboardConfig::default(),
};
let host = dashboard.host.and_then(|h| {
let trimmed = h.trim();
if trimmed.is_empty() {
log("desktop config: ignoring invalid dashboard.host value");
None
} else {
Some(trimmed.to_string())
}
});
let port = dashboard.port.and_then(|p| {
let num = match p {
DesktopPortField::Num(n) => n,
DesktopPortField::Str(s) => match s.trim().parse::<u64>() {
Ok(n) => n,
Err(_) => {
log("desktop config: ignoring invalid dashboard.port value");
return None;
}
},
};
if !(1..=65535).contains(&num) {
log("desktop config: ignoring invalid dashboard.port value");
return None;
}
Some(num.to_string())
});
DesktopDashboardConfig { host, port }
}
}
fn read_desktop_dashboard_config(
root_dir: Option<&Path>,
log: &mut dyn FnMut(&str),
) -> DesktopDashboardConfig {
let Some(root_dir) = root_dir else {
return DesktopDashboardConfig::default();
};
let config_path = root_dir.join(DESKTOP_CONFIG_RELATIVE_PATH);
if !config_path.is_file() {
return DesktopDashboardConfig::default();
}
let raw = match fs::read_to_string(&config_path) {
Ok(raw) => raw,
Err(error) => {
log(&format!(
"failed to read desktop config {}: {}",
config_path.display(),
error
));
return DesktopDashboardConfig::default();
}
};
let parsed: DesktopFileConfig = match serde_json::from_str(&raw) {
Ok(parsed) => parsed,
Err(error) => {
log(&format!(
"failed to parse desktop config {}: {}",
config_path.display(),
error
));
return DesktopDashboardConfig::default();
}
};
DesktopDashboardConfig::from_file_config(parsed, log)
}
```
Behavior is preserved:
- `host`: string, trimmed; empty/whitespace-only → ignored + log.
- `port`: accepts number or string; trims string; must parse, be in `1..=65535`; otherwise ignored + log.
- `DesktopDashboardConfig` still holds `Option<String>` host/port used later for env defaults.
---
### 3. Centralize host/port precedence computation
You currently recompute precedence across env vars, config, and defaults. A tiny helper keeps the rules in one place and makes `configure_desktop_dashboard_environment` easier to scan.
```rust
fn resolve_dashboard_value(
primary_env: Option<OsString>,
legacy_env: Option<OsString>,
config: Option<String>,
default: &str,
) -> (Option<OsString>, OsString) {
// env values are passed through untouched
let effective = primary_env
.clone()
.or(legacy_env.clone())
.or_else(|| config.clone().map(OsString::from))
.unwrap_or_else(|| OsString::from(default));
(primary_env.or(legacy_env), effective)
}
fn configure_desktop_dashboard_environment(
command: &mut Command,
root_dir: Option<&Path>,
log: &mut dyn FnMut(&str),
) {
let dashboard_host_env = env::var_os(DASHBOARD_HOST_ENV);
let astrbot_dashboard_host_env = env::var_os(ASTRBOT_DASHBOARD_HOST_ENV);
let dashboard_port_env = env::var_os(DASHBOARD_PORT_ENV);
let astrbot_dashboard_port_env = env::var_os(ASTRBOT_DASHBOARD_PORT_ENV);
let astrbot_skip_auth_env = env::var_os(ASTRBOT_DASHBOARD_SKIP_DEFAULT_PASSWORD_AUTH_ENV);
let legacy_skip_auth_env = env::var_os(DASHBOARD_SKIP_DEFAULT_PASSWORD_AUTH_ENV);
let desktop_config = read_desktop_dashboard_config(root_dir, log);
let (host_env_any, effective_host) = resolve_dashboard_value(
dashboard_host_env.clone(),
astrbot_dashboard_host_env.clone(),
desktop_config.host.clone(),
DEFAULT_DASHBOARD_HOST,
);
let (port_env_any, effective_port) = resolve_dashboard_value(
dashboard_port_env.clone(),
astrbot_dashboard_port_env.clone(),
desktop_config.port.clone(),
DEFAULT_DASHBOARD_PORT,
);
if host_env_any.is_none() {
command.env(DASHBOARD_HOST_ENV, &effective_host);
}
if port_env_any.is_none() {
command.env(DASHBOARD_PORT_ENV, &effective_port);
}
let has_explicit_skip_auth = astrbot_skip_auth_env.is_some() || legacy_skip_auth_env.is_some();
if should_skip_default_password_auth(has_explicit_skip_auth, effective_host.as_os_str().into())
{
command.env(ASTRBOT_DASHBOARD_SKIP_DEFAULT_PASSWORD_AUTH_ENV, "true");
}
}
```
(The exact `effective_host` → `Option<&OsStr>` conversion for `should_skip_default_password_auth` can be adapted to match your existing checks.)
These three changes keep all existing functionality and tests but reduce cognitive load: no generics in public signatures, no manual `Value` traversal, and one centralized precedence rule for host/port.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
resolve_dashboard_value, you currently clone the resolvedOsString; you could avoid this extra allocation by matching on references and only cloning in the branch where you actually need to construct a default/config value. - The desktop config validation logs (
invalid dashboard.host/invalid dashboard.port) could include the rejected value (or a sanitized form) to make troubleshooting misconfigurations easier while still keeping log output concise.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `resolve_dashboard_value`, you currently clone the resolved `OsString`; you could avoid this extra allocation by matching on references and only cloning in the branch where you actually need to construct a default/config value.
- The desktop config validation logs (`invalid dashboard.host` / `invalid dashboard.port`) could include the rejected value (or a sanitized form) to make troubleshooting misconfigurations easier while still keeping log output concise.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src-tauri/src/backend/launch.rs" line_range="100-109" />
<code_context>
+fn configure_desktop_dashboard_environment(
</code_context>
<issue_to_address>
**issue:** Empty environment variables disable fallback to config/default values.
Since `resolve_dashboard_value` treats any set `primary_env`/`legacy_env` as authoritative, an explicitly empty `DASHBOARD_HOST` or `DASHBOARD_PORT` blocks fallback to `cmd_config` or defaults, leaving host/port unset. It would be safer to treat empty or whitespace-only env values as `None` so that configuration/default values still apply.
</issue_to_address>
### Comment 2
<location path="src-tauri/src/backend/launch.rs" line_range="52" />
<code_context>
+
+#[derive(Debug, Deserialize)]
+#[serde(untagged)]
+enum DesktopPortField {
+ Num(u64),
+ Str(String),
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the cmd_config port handling by deserializing it as a raw JSON value and inlining the parsing logic, eliminating the extra enum and helper functions involved in port parsing.
You can simplify the cmd_config path by inlining the port parsing and dropping `DesktopPortField` + the two parse helpers. This keeps all behavior (number/string support, range check, logging invalid values) but reduces the number of types and functions you have to mentally chase.
Concretely:
1. Change `CmdConfigDashboard` to deserialize `port` as a raw JSON value:
```rust
use serde::Deserialize;
use serde_json::Value;
#[derive(Debug, Deserialize)]
struct CmdConfigDashboard {
host: Option<String>,
port: Option<Value>,
}
```
2. Inline the port parsing logic inside `CmdDashboardConfig::from_file_config` and delete
`DesktopPortField`, `parse_dashboard_port_number`, and `parse_dashboard_port_string`:
```rust
impl CmdDashboardConfig {
fn from_file_config(config: CmdConfigFile, log: &mut dyn FnMut(&str)) -> Self {
let Some(dashboard) = config.dashboard else {
return Self::default();
};
let host = dashboard.host.and_then(|host| {
let trimmed = host.trim();
if trimmed.is_empty() {
log(&format!(
"cmd_config: ignoring invalid dashboard.host value: {host:?}"
));
None
} else {
Some(trimmed.to_string())
}
});
let port = dashboard.port.and_then(|value| {
match value {
Value::Number(n) => {
if let Some(port) = n.as_u64() {
if (1..=65535).contains(&port) {
return Some(port.to_string());
}
log(&format!(
"cmd_config: ignoring invalid dashboard.port value: {port}"
));
None
} else {
log("cmd_config: ignoring non-u64 dashboard.port number");
None
}
}
Value::String(s) => {
let trimmed = s.trim();
match trimmed.parse::<u64>() {
Ok(port) if (1..=65535).contains(&port) => Some(port.to_string()),
Ok(port) => {
log(&format!(
"cmd_config: ignoring invalid dashboard.port value: {port}"
));
None
}
Err(_) => {
log(&format!(
"cmd_config: ignoring invalid dashboard.port value: {s:?}"
));
None
}
}
}
_ => {
log("cmd_config: ignoring non-number/string dashboard.port value");
None
}
}
});
Self { host, port }
}
}
```
This preserves:
- Accepting numeric or string ports.
- Range validation (1–65535).
- Logging of invalid values (including empty/whitespace host).
- Overall behavior of `CmdDashboardConfig`.
But it removes:
- The `DesktopPortField` enum.
- `parse_dashboard_port_number`.
- `parse_dashboard_port_string`.
The net result is fewer types and helpers to understand while reading the dashboard configuration flow, without changing the external behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
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
127.0.0.1:6185data/cmd_config.jsonas the dashboard host/port config sourcedashboard.host/dashboard.portincmd_config.jsonConfig example
~/.astrbot/data/cmd_config.json:{ "dashboard": { "host": "0.0.0.0", "port": 6185 } }Verification
rtk cargo test configure_desktop_dashboard_environment --manifest-path src-tauri/Cargo.tomlrtk cargo fmt --manifest-path src-tauri/Cargo.toml -- --checkrtk cargo check --manifest-path src-tauri/Cargo.tomlrtk cargo test --manifest-path src-tauri/Cargo.tomlFixes #132
Summary by Sourcery
Support configuring AstrBot Desktop dashboard host and port via core cmd_config while keeping local-only defaults and documenting LAN WebUI access.
New Features:
Enhancements:
Documentation:
Tests: