fix: portable runtime layout#111
Merged
zouyonghe merged 6 commits intoAstrBotDevs:mainfrom Apr 5, 2026
Merged
Conversation
zouyonghe
changed the title
zouyonghe
force-pushed
the
codex/fix-portable-runtime-layout
branch
from
053f266 to
71b2dba
Compare
![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 1 issue, and left some high level feedback:
- Consider centralizing the portable layout paths by reusing
PORTABLE_BACKEND_LAYOUT_RELATIVE_PATHandPORTABLE_WEBUI_LAYOUT_RELATIVE_PATH(or a shared helper) in the test module as well, so future layout changes only require updating a single definition. - It may be clearer to distinguish between 'packaged resource location' and 'portable layout location' in the constant names or via a brief comment near the new
PORTABLE_*_LAYOUT_RELATIVE_PATHdefinitions to reduce confusion if the resource layout changes again.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider centralizing the portable layout paths by reusing `PORTABLE_BACKEND_LAYOUT_RELATIVE_PATH` and `PORTABLE_WEBUI_LAYOUT_RELATIVE_PATH` (or a shared helper) in the test module as well, so future layout changes only require updating a single definition.
- It may be clearer to distinguish between 'packaged resource location' and 'portable layout location' in the constant names or via a brief comment near the new `PORTABLE_*_LAYOUT_RELATIVE_PATH` definitions to reduce confusion if the resource layout changes again.
## Individual Comments
### Comment 1
<location path="scripts/ci/test_package_windows_portable.py" line_range="423-431" />
<code_context>
self.assertFalse((destination_root / "astrbot-desktop-tauri.exe").exists())
self.assertTrue((destination_root / "WebView2Loader.dll").is_file())
self.assertTrue(
- (
- destination_root / "resources" / "backend" / "runtime-manifest.json"
- ).is_file()
- )
- self.assertTrue(
- (destination_root / "resources" / "webui" / "index.html").is_file()
+ (destination_root / "backend" / "runtime-manifest.json").is_file()
)
+ self.assertTrue((destination_root / "webui" / "index.html").is_file())
</code_context>
<issue_to_address>
**suggestion (testing):** Assert that the legacy resources/backend and resources/webui paths are not created anymore
Please also have this test assert that the old `resources/backend` and `resources/webui` directories do not exist (e.g. `self.assertFalse((destination_root / "resources" / "backend").exists())` and similarly for `webui`). This will catch any future change that mistakenly writes files to both the old and new layouts.
```suggestion
self.assertFalse((destination_root / "astrbot-desktop-tauri.exe").exists())
self.assertTrue((destination_root / "WebView2Loader.dll").is_file())
self.assertTrue(
(destination_root / "backend" / "runtime-manifest.json").is_file()
)
self.assertTrue((destination_root / "webui" / "index.html").is_file())
# Legacy layout paths must no longer be created
self.assertFalse((destination_root / "resources" / "backend").exists())
self.assertFalse((destination_root / "resources" / "webui").exists())
self.assertTrue((destination_root / "kill-backend-processes.ps1").is_file())
self.assertTrue((destination_root / "portable.flag").is_file())
self.assertTrue((destination_root / MODULE.PORTABLE_README_NAME).is_file())
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
423
to
431
| self.assertFalse((destination_root / "astrbot-desktop-tauri.exe").exists()) | ||
| self.assertTrue((destination_root / "WebView2Loader.dll").is_file()) | ||
| self.assertTrue( | ||
| ( | ||
| destination_root / "resources" / "backend" / "runtime-manifest.json" | ||
| ).is_file() | ||
| ) | ||
| self.assertTrue( | ||
| (destination_root / "resources" / "webui" / "index.html").is_file() | ||
| (destination_root / "backend" / "runtime-manifest.json").is_file() | ||
| ) | ||
| self.assertTrue((destination_root / "webui" / "index.html").is_file()) | ||
| self.assertTrue((destination_root / "kill-backend-processes.ps1").is_file()) | ||
| self.assertTrue((destination_root / "portable.flag").is_file()) | ||
| self.assertTrue((destination_root / MODULE.PORTABLE_README_NAME).is_file()) |
There was a problem hiding this comment.
suggestion (testing): Assert that the legacy resources/backend and resources/webui paths are not created anymore
Please also have this test assert that the old resources/backend and resources/webui directories do not exist (e.g. self.assertFalse((destination_root / "resources" / "backend").exists()) and similarly for webui). This will catch any future change that mistakenly writes files to both the old and new layouts.
Suggested change
| self.assertFalse((destination_root / "astrbot-desktop-tauri.exe").exists()) | |
| self.assertTrue((destination_root / "WebView2Loader.dll").is_file()) | |
| self.assertTrue( | |
| ( | |
| destination_root / "resources" / "backend" / "runtime-manifest.json" | |
| ).is_file() | |
| ) | |
| self.assertTrue( | |
| (destination_root / "resources" / "webui" / "index.html").is_file() | |
| (destination_root / "backend" / "runtime-manifest.json").is_file() | |
| ) | |
| self.assertTrue((destination_root / "webui" / "index.html").is_file()) | |
| self.assertTrue((destination_root / "kill-backend-processes.ps1").is_file()) | |
| self.assertTrue((destination_root / "portable.flag").is_file()) | |
| self.assertTrue((destination_root / MODULE.PORTABLE_README_NAME).is_file()) | |
| self.assertFalse((destination_root / "astrbot-desktop-tauri.exe").exists()) | |
| self.assertTrue((destination_root / "WebView2Loader.dll").is_file()) | |
| self.assertTrue( | |
| (destination_root / "backend" / "runtime-manifest.json").is_file() | |
| ) | |
| self.assertTrue((destination_root / "webui" / "index.html").is_file()) | |
| # Legacy layout paths must no longer be created | |
| self.assertFalse((destination_root / "resources" / "backend").exists()) | |
| self.assertFalse((destination_root / "resources" / "webui").exists()) | |
| self.assertTrue((destination_root / "kill-backend-processes.ps1").is_file()) | |
| self.assertTrue((destination_root / "portable.flag").is_file()) | |
| self.assertTrue((destination_root / MODULE.PORTABLE_README_NAME).is_file()) |
![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 refactors the Windows portable package layout by moving backend and webui resources from a nested "resources" directory to the root level. The changes include updating path constants, the file copying logic, and validation checks, along with corresponding updates to the test suite. A recommendation was made to exclude Python development artifacts like "pycache" during the packaging process to reduce bloat and prevent potential version conflicts.
| if not backend_src.is_dir(): | ||
| raise FileNotFoundError(f"Required directory not found: {backend_src}") | ||
| shutil.copytree(backend_src, resources_root / "backend") | ||
| shutil.copytree(backend_src, destination_root / PORTABLE_BACKEND_LAYOUT_RELATIVE_PATH) |
There was a problem hiding this comment.
When packaging the backend resources, it is a good practice to exclude development artifacts like __pycache__ and compiled Python files (.pyc, .pyo). These files are not necessary for the portable runtime and can bloat the package or cause issues if they were generated with a different Python version.
Suggested change
| shutil.copytree(backend_src, destination_root / PORTABLE_BACKEND_LAYOUT_RELATIVE_PATH) | |
| shutil.copytree( | |
| backend_src, | |
| destination_root / PORTABLE_BACKEND_LAYOUT_RELATIVE_PATH, | |
| ignore=shutil.ignore_patterns("__pycache__", "*.pyc", "*.pyo") | |
| ) |
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- To keep the tests aligned with the runtime layout contract, consider using the new
PORTABLE_BACKEND_LAYOUT_RELATIVE_PATHandPORTABLE_WEBUI_LAYOUT_RELATIVE_PATHconstants (or derived values) intest_package_windows_portable.pyinstead of hardcodingbackend/webuipaths, so future layout changes don’t require touching multiple places.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- To keep the tests aligned with the runtime layout contract, consider using the new `PORTABLE_BACKEND_LAYOUT_RELATIVE_PATH` and `PORTABLE_WEBUI_LAYOUT_RELATIVE_PATH` constants (or derived values) in `test_package_windows_portable.py` instead of hardcoding `backend`/`webui` paths, so future layout changes don’t require touching multiple places.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:
- Since the portable layout is meant to stay aligned with the Tauri resource aliases, consider deriving
PORTABLE_BACKEND_LAYOUT_RELATIVE_PATHandPORTABLE_WEBUI_LAYOUT_RELATIVE_PATHfromtauri.conf.json(or a single shared config module) instead of hardcoding them here and in the desktop app to avoid future drift.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since the portable layout is meant to stay aligned with the Tauri resource aliases, consider deriving `PORTABLE_BACKEND_LAYOUT_RELATIVE_PATH` and `PORTABLE_WEBUI_LAYOUT_RELATIVE_PATH` from `tauri.conf.json` (or a single shared config module) instead of hardcoding them here and in the desktop app to avoid future drift.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, and left some high level feedback:
- The alias validation logic for bundle.resources (relative, no traversal, etc.) is now duplicated in both Python (resolve_bundle_resource_alias_from_tauri_config) and Rust (load_bundle_resource_alias); consider centralizing or at least aligning the validation messages and rules via shared documentation or a single source of truth to keep them from drifting.
- In resolve_bundle_resource_alias_from_tauri_config, the error messages refer to a "bundle.resources table" while the config is JSON; consider adjusting the wording to "object" or "map" to better reflect the actual structure and make diagnostics clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The alias validation logic for bundle.resources (relative, no traversal, etc.) is now duplicated in both Python (resolve_bundle_resource_alias_from_tauri_config) and Rust (load_bundle_resource_alias); consider centralizing or at least aligning the validation messages and rules via shared documentation or a single source of truth to keep them from drifting.
- In resolve_bundle_resource_alias_from_tauri_config, the error messages refer to a "bundle.resources table" while the config is JSON; consider adjusting the wording to "object" or "map" to better reflect the actual structure and make diagnostics clearer.
## Individual Comments
### Comment 1
<location path="scripts/ci/package_windows_portable.py" line_range="105-114" />
<code_context>
return marker_name
+def resolve_bundle_resource_alias_from_tauri_config(
+ project_root: pathlib.Path,
+ tauri_config: dict,
+ source_relative_path: pathlib.Path,
+) -> pathlib.Path:
+ bundle_table = tauri_config.get("bundle")
+ if not isinstance(bundle_table, dict):
+ raise ValueError(f"Missing bundle table in {TAURI_CONFIG_RELATIVE_PATH}")
+
+ resources_table = bundle_table.get("resources")
+ if not isinstance(resources_table, dict):
+ raise ValueError(
+ f"Missing bundle.resources table in {TAURI_CONFIG_RELATIVE_PATH}"
+ )
+
+ tauri_config_dir = (project_root / TAURI_CONFIG_RELATIVE_PATH).parent.resolve()
+ expected_source_path = (project_root / source_relative_path).resolve()
+ for source_path_text, alias_text in resources_table.items():
+ candidate_source_path = (tauri_config_dir / str(source_path_text)).resolve()
+ if candidate_source_path != expected_source_path:
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider aligning lookup strategy with build.rs by using the config key string instead of walking and resolving all entries.
This loop resolves and compares every `bundle.resources` entry against `expected_source_path`, which is looser than the `build.rs` behavior (`resources.get(source_relative_path)` requiring an exact key like "../resources/backend"). That mismatch can allow configs that pass packaging but fail at compile time. To mirror the Rust behavior and avoid these subtle inconsistencies, derive the expected config key string (e.g., path from the config directory to `BACKEND_RESOURCE_RELATIVE_PATH`) and look it up directly in `resources_table` instead of scanning and resolving all entries.
</issue_to_address>
### Comment 2
<location path="scripts/ci/test_package_windows_portable.py" line_range="525-534" />
<code_context>
+ def test_validate_portable_root_accepts_expected_layout(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Add a validation test where backend/webui aliases are nested (e.g. `runtime/backend`)
The current `test_validate_portable_root_*` cases still only cover the default `backend` / `webui` layout. Since `load_project_config_from` now reads aliases from `bundle.resources`, please add an end-to-end validation test using non-trivial aliases (e.g. `runtime/backend`, `runtime/webui`). For example, call `make_project_layout` with `tauri_resources` mapping `"../resources/backend" → "runtime/backend"` and `"../resources/webui" → "runtime/webui"`, build a portable root using those nested paths, and assert that `MODULE.validate_portable_root(root, project_config)` passes without requiring `resources/backend` or `resources/webui` directories. This will verify that `validate_portable_root` respects non-root aliases and protect against regressions that assume fixed `backend/` / `webui/` locations.
Suggested implementation:
```python
MODULE.add_portable_runtime_files(root, project_config)
def test_validate_portable_root_accepts_expected_layout(self):
layout = self.make_project_layout()
project_config = MODULE.load_project_config_from(layout["script_path"])
with tempfile.TemporaryDirectory() as tmpdir:
root = Path(tmpdir)
# default layout: backend/ and webui/ at the portable root
(root / "AstrBot.exe").write_text("binary")
(root / "portable.flag").write_text("marker")
(root / "backend").mkdir()
(root / "webui").mkdir()
MODULE.validate_portable_root(root, project_config)
def test_validate_portable_root_accepts_nested_alias_layout(self):
layout = self.make_project_layout(
tauri_resources={
"../resources/backend": "runtime/backend",
"../resources/webui": "runtime/webui",
}
)
project_config = MODULE.load_project_config_from(layout["script_path"])
with tempfile.TemporaryDirectory() as tmpdir:
root = Path(tmpdir)
# binary + portable marker at root
(root / "AstrBot.exe").write_text("binary")
(root / "portable.flag").write_text("marker")
# resources are placed under their aliased nested locations
(root / "runtime" / "backend").mkdir(parents=True)
(root / "runtime" / "webui").mkdir(parents=True)
# Intentionally do NOT create root-level backend/ or webui/ directories.
# validate_portable_root should honor the aliased paths instead.
MODULE.validate_portable_root(root, project_config)
```
If `make_project_layout` uses a different keyword than `tauri_resources` for the resources mapping, update the argument name in `test_validate_portable_root_accepts_nested_alias_layout` accordingly.
If `validate_portable_root` requires additional files or directories beyond those shown (for example, specific backend/webui content rather than just directories), mirror whatever the other `test_validate_portable_root_*` tests create under the aliased `runtime/backend` and `runtime/webui` paths so this test exercises the same invariants with nested aliases instead of the default layout.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
525
to
+534
| def test_validate_portable_root_accepts_expected_layout(self): | ||
| layout = self.make_project_layout() | ||
| project_config = MODULE.load_project_config_from(layout["script_path"]) | ||
|
|
||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| root = Path(tmpdir) | ||
| (root / "AstrBot.exe").write_text("binary") | ||
| (root / "resources" / "backend").mkdir(parents=True) | ||
| (root / "resources" / "webui").mkdir(parents=True) | ||
| (root / "resources" / "backend" / "runtime-manifest.json").write_text("{}") | ||
| (root / "resources" / "webui" / "index.html").write_text("<html></html>") | ||
| (root / project_config.backend_layout_relative_path).mkdir(parents=True) | ||
| (root / project_config.webui_layout_relative_path).mkdir(parents=True) | ||
| ( |
There was a problem hiding this comment.
suggestion (testing): Add a validation test where backend/webui aliases are nested (e.g. runtime/backend)
The current test_validate_portable_root_* cases still only cover the default backend / webui layout. Since load_project_config_from now reads aliases from bundle.resources, please add an end-to-end validation test using non-trivial aliases (e.g. runtime/backend, runtime/webui). For example, call make_project_layout with tauri_resources mapping "../resources/backend" → "runtime/backend" and "../resources/webui" → "runtime/webui", build a portable root using those nested paths, and assert that MODULE.validate_portable_root(root, project_config) passes without requiring resources/backend or resources/webui directories. This will verify that validate_portable_root respects non-root aliases and protect against regressions that assume fixed backend/ / webui/ locations.
Suggested implementation:
MODULE.add_portable_runtime_files(root, project_config)
def test_validate_portable_root_accepts_expected_layout(self):
layout = self.make_project_layout()
project_config = MODULE.load_project_config_from(layout["script_path"])
with tempfile.TemporaryDirectory() as tmpdir:
root = Path(tmpdir)
# default layout: backend/ and webui/ at the portable root
(root / "AstrBot.exe").write_text("binary")
(root / "portable.flag").write_text("marker")
(root / "backend").mkdir()
(root / "webui").mkdir()
MODULE.validate_portable_root(root, project_config)
def test_validate_portable_root_accepts_nested_alias_layout(self):
layout = self.make_project_layout(
tauri_resources={
"../resources/backend": "runtime/backend",
"../resources/webui": "runtime/webui",
}
)
project_config = MODULE.load_project_config_from(layout["script_path"])
with tempfile.TemporaryDirectory() as tmpdir:
root = Path(tmpdir)
# binary + portable marker at root
(root / "AstrBot.exe").write_text("binary")
(root / "portable.flag").write_text("marker")
# resources are placed under their aliased nested locations
(root / "runtime" / "backend").mkdir(parents=True)
(root / "runtime" / "webui").mkdir(parents=True)
# Intentionally do NOT create root-level backend/ or webui/ directories.
# validate_portable_root should honor the aliased paths instead.
MODULE.validate_portable_root(root, project_config)If make_project_layout uses a different keyword than tauri_resources for the resources mapping, update the argument name in test_validate_portable_root_accepts_nested_alias_layout accordingly.
If validate_portable_root requires additional files or directories beyond those shown (for example, specific backend/webui content rather than just directories), mirror whatever the other test_validate_portable_root_* tests create under the aliased runtime/backend and runtime/webui paths so this test exercises the same invariants with nested aliases instead of the default layout.
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="scripts/ci/package_windows_portable.py" line_range="122-124" />
<code_context>
+ f"Missing bundle.resources object in {TAURI_CONFIG_RELATIVE_PATH}"
+ )
+
+ tauri_config_dir = (project_root / TAURI_CONFIG_RELATIVE_PATH).parent.resolve()
+ expected_source_path = (project_root / source_relative_path).resolve()
+ expected_source_key = pathlib.PurePosixPath(
+ os.path.relpath(expected_source_path, tauri_config_dir)
+ ).as_posix()
</code_context>
<issue_to_address>
**issue (bug_risk):** Path key computation for bundle.resources is likely wrong on Windows due to backslash handling.
On Windows, `os.path.relpath` returns backslash-separated paths (e.g. `"..\resources\backend"`), and `PurePosixPath(...).as_posix()` does not convert those backslashes to `/`. That means the computed `expected_source_key` won’t match the POSIX-style keys in `tauri.conf.json` (e.g. `"../resources/backend"`). To make this cross-platform and match the `BACKEND_RESOURCE_SOURCE` / `WEBUI_RESOURCE_SOURCE` keys, derive the relative path using `Path` and then call `as_posix()`:
```python
rel = os.path.relpath(expected_source_path, tauri_config_dir)
expected_source_key = pathlib.Path(rel).as_posix()
```
or, avoiding `os.path`:
```python
expected_source_key = (
expected_source_path.relative_to(tauri_config_dir)
.as_posix()
)
```
</issue_to_address>
### Comment 2
<location path="src-tauri/src/launch_plan.rs" line_range="13" />
<code_context>
+const BACKEND_RESOURCE_ALIAS: &str = env!("ASTRBOT_BACKEND_RESOURCE_ALIAS");
+const WEBUI_RESOURCE_ALIAS: &str = env!("ASTRBOT_WEBUI_RESOURCE_ALIAS");
+
+fn build_packaged_resource_relative_path(resource_alias: &str, leaf_name: &str) -> PathBuf {
+ PathBuf::from(resource_alias).join(leaf_name)
+}
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the helper function and PathBuf-based path construction with compile-time `const` string paths that use the env aliases directly.
You can keep the alias-based flexibility while simplifying the indirection and dropping the `PathBuf` dance entirely by using `const` string composition instead of a helper + lossy conversions.
For example, replace the helper and its test with `const` relative paths built at compile time:
```rust
const BACKEND_RESOURCE_ALIAS: &str = env!("ASTRBOT_BACKEND_RESOURCE_ALIAS");
const WEBUI_RESOURCE_ALIAS: &str = env!("ASTRBOT_WEBUI_RESOURCE_ALIAS");
const BACKEND_MANIFEST_RELATIVE_PATH: &str =
concat!(env!("ASTRBOT_BACKEND_RESOURCE_ALIAS"), "/runtime-manifest.json");
const WEBUI_INDEX_RELATIVE_PATH: &str =
concat!(env!("ASTRBOT_WEBUI_RESOURCE_ALIAS"), "/index.html");
```
Then simplify the call sites to use these constants directly:
```rust
pub fn resolve_packaged_launch<F>(
app: &AppHandle,
default_shell_locale: &'static str,
log: F,
) -> Result<Option<LaunchPlan>, String>
where
F: Fn(&str) + Copy,
{
let manifest_path = match runtime_paths::resolve_resource_path(
app,
BACKEND_MANIFEST_RELATIVE_PATH,
log,
) {
Some(path) if path.is_file() => path,
_ => return Ok(None),
};
// ...
}
```
```rust
let embedded_webui_dir = env::var("ASTRBOT_WEBUI_DIR")
.ok()
.map(PathBuf::from)
.or_else(|| {
runtime_paths::resolve_resource_path(app, WEBUI_INDEX_RELATIVE_PATH, log)
.and_then(|index_path| index_path.parent().map(Path::to_path_buf))
});
```
This:
- Removes `build_packaged_resource_relative_path` and its test (no longer needed).
- Eliminates `PathBuf` creation and `to_string_lossy` just to get a relative string.
- Keeps the env-based aliasing behavior intact, with straightforward `&'static str` usage at call sites.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const BACKEND_RESOURCE_ALIAS: &str = env!("ASTRBOT_BACKEND_RESOURCE_ALIAS"); | ||
| const WEBUI_RESOURCE_ALIAS: &str = env!("ASTRBOT_WEBUI_RESOURCE_ALIAS"); | ||
|
|
||
| fn build_packaged_resource_relative_path(resource_alias: &str, leaf_name: &str) -> PathBuf { |
There was a problem hiding this comment.
issue (complexity): Consider replacing the helper function and PathBuf-based path construction with compile-time const string paths that use the env aliases directly.
You can keep the alias-based flexibility while simplifying the indirection and dropping the PathBuf dance entirely by using const string composition instead of a helper + lossy conversions.
For example, replace the helper and its test with const relative paths built at compile time:
const BACKEND_RESOURCE_ALIAS: &str = env!("ASTRBOT_BACKEND_RESOURCE_ALIAS");
const WEBUI_RESOURCE_ALIAS: &str = env!("ASTRBOT_WEBUI_RESOURCE_ALIAS");
const BACKEND_MANIFEST_RELATIVE_PATH: &str =
concat!(env!("ASTRBOT_BACKEND_RESOURCE_ALIAS"), "/runtime-manifest.json");
const WEBUI_INDEX_RELATIVE_PATH: &str =
concat!(env!("ASTRBOT_WEBUI_RESOURCE_ALIAS"), "/index.html");Then simplify the call sites to use these constants directly:
pub fn resolve_packaged_launch<F>(
app: &AppHandle,
default_shell_locale: &'static str,
log: F,
) -> Result<Option<LaunchPlan>, String>
where
F: Fn(&str) + Copy,
{
let manifest_path = match runtime_paths::resolve_resource_path(
app,
BACKEND_MANIFEST_RELATIVE_PATH,
log,
) {
Some(path) if path.is_file() => path,
_ => return Ok(None),
};
// ...
}let embedded_webui_dir = env::var("ASTRBOT_WEBUI_DIR")
.ok()
.map(PathBuf::from)
.or_else(|| {
runtime_paths::resolve_resource_path(app, WEBUI_INDEX_RELATIVE_PATH, log)
.and_then(|index_path| index_path.parent().map(Path::to_path_buf))
});This:
- Removes
build_packaged_resource_relative_pathand its test (no longer needed). - Eliminates
PathBufcreation andto_string_lossyjust to get a relative string. - Keeps the env-based aliasing behavior intact, with straightforward
&'static strusage at call sites.
SourceryAI
reviewed
Apr 5, 2026
SourceryAI
left a comment
SourceryAI
left a comment
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The alias validation logic is now duplicated between
resolve_bundle_resource_alias_from_tauri_configin Python andload_bundle_resource_aliasinbuild.rs; consider centralizing the rules or adding a shared test fixture (e.g., sampletauri.conf.json) that both paths consume so they can’t silently diverge over time. - In
validate_portable_root, the optionalproject_configparameter falls back toload_project_config()if not provided; if this function is called from other scripts or tooling, you may want to makeproject_configrequired to avoid surprising behavior tied to the current working directory or script location.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The alias validation logic is now duplicated between `resolve_bundle_resource_alias_from_tauri_config` in Python and `load_bundle_resource_alias` in `build.rs`; consider centralizing the rules or adding a shared test fixture (e.g., sample `tauri.conf.json`) that both paths consume so they can’t silently diverge over time.
- In `validate_portable_root`, the optional `project_config` parameter falls back to `load_project_config()` if not provided; if this function is called from other scripts or tooling, you may want to make `project_config` required to avoid surprising behavior tied to the current working directory or script location.
## Individual Comments
### Comment 1
<location path="src-tauri/src/launch_plan.rs" line_range="13-19" />
<code_context>
+const BACKEND_RESOURCE_ALIAS: &str = env!("ASTRBOT_BACKEND_RESOURCE_ALIAS");
+const WEBUI_RESOURCE_ALIAS: &str = env!("ASTRBOT_WEBUI_RESOURCE_ALIAS");
+
+fn build_packaged_resource_relative_path(resource_alias: &str, leaf_name: &str) -> PathBuf {
+ PathBuf::from(resource_alias).join(leaf_name)
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** Building the runtime resource path via `PathBuf` and `to_string_lossy` may introduce platform-specific separators that don’t match tauri’s resource path expectations.
The previous implementation used static POSIX-style strings (e.g. "backend/runtime-manifest.json"), but `PathBuf::from(alias).join(leaf_name).to_string_lossy()` will emit OS-specific separators (like `\` on Windows). If `resolve_resource_path`/tauri expects forward-slash logical paths, this can cause resource lookup failures. Consider constructing the lookup string without `PathBuf`, e.g. `format!("{}/{}", resource_alias.trim_end_matches('/'), leaf_name)` so it’s consistently `/`-separated across platforms.
</issue_to_address>Hi @zouyonghe! 👋
Thanks for trying out Sourcery by commenting with @sourcery-ai review! 🚀
Install the sourcery-ai bot to get automatic code reviews on every pull request ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+13
to
19
| fn build_packaged_resource_relative_path(resource_alias: &str, leaf_name: &str) -> PathBuf { | ||
| PathBuf::from(resource_alias).join(leaf_name) | ||
| } | ||
|
|
||
| pub fn resolve_custom_launch(custom_cmd: String) -> Result<LaunchPlan, String> { | ||
| let mut pieces = shlex::split(&custom_cmd) | ||
| .ok_or_else(|| format!("Invalid ASTRBOT_BACKEND_CMD: {custom_cmd}"))?; |
There was a problem hiding this comment.
issue (bug_risk): Building the runtime resource path via PathBuf and to_string_lossy may introduce platform-specific separators that don’t match tauri’s resource path expectations.
The previous implementation used static POSIX-style strings (e.g. "backend/runtime-manifest.json"), but PathBuf::from(alias).join(leaf_name).to_string_lossy() will emit OS-specific separators (like \ on Windows). If resolve_resource_path/tauri expects forward-slash logical paths, this can cause resource lookup failures. Consider constructing the lookup string without PathBuf, e.g. format!("{}/{}", resource_alias.trim_end_matches('/'), leaf_name) so it’s consistently /-separated across platforms.
Member
Author
|
@sourcery-ai review |
zouyonghe
changed the title
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
2 participants
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.
This change fixes a Windows portable startup failure caused by a mismatch between the packaged resource layout and the runtime lookup path.
In the portable zip,
scripts/ci/package_windows_portable.pywas copying the bundled backend and web UI intoresources/backendandresources/webui. At runtime, the desktop app resolves packaged resources using the Tauri resource aliases declared insrc-tauri/tauri.conf.json, which place these assets at the resource root asbackendandwebui.For portable users, that mismatch meant the app could not find
backend/runtime-manifest.jsonduring startup, so packaged mode was skipped and the process fell back to development-mode source discovery. On machines without an AstrBot source checkout, startup then failed withCannot locate AstrBot source directory, which makes the portable build unusable.This PR aligns the portable packaging script with the runtime contract by copying the resource directories to top-level
backend/andwebui/inside the portable archive. It also updates the portable layout validation checks and the regression tests so CI will catch this exact mistake if it reappears.Validation:
python3 -m unittest scripts.ci.test_package_windows_portableSummary by Sourcery
Align the Windows portable runtime layout with the Tauri resource configuration so packaged backend and web UI assets are discovered correctly at startup.
Bug Fixes:
Enhancements:
Tests: