Replace PyInstaller with Tauri-native desktop packaging#44
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMigrates desktop packaging from PyInstaller to a Tauri-based pipeline (including portable Windows single-exe), adds toolchain provisioning and templates, extends Config with packaging fields and path normalization, hardens bundler extraction for symlinks, refactors desktop runtime/standalone wiring, updates CLI/package flow, and expands tests and docs. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Comment |
emmapowers
marked this pull request as ready for review
March 8, 2026 16:24
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (18)
src/trellis/platforms/browser/serve_platform.py (1)
56-60: Minor styling inconsistency with server platform banner.The URL on line 58 is missing the
[underline]markup that's present inserver/platform.py(line 48), and line 60 doesn't bold "Ctrl+C" like the server platform does. Consider aligning for visual consistency across platforms.🔧 Suggested fix for consistency
- _console.print(f" [bold]>[/bold] [cyan]Local:[/cyan] {url}") + _console.print(f" [bold]>[/bold] [cyan]Local:[/cyan] [underline]{url}[/underline]") _console.print() - _console.print(" [dim]Press Ctrl+C to stop[/dim]") + _console.print(" [dim]Press[/dim] [bold]Ctrl+C[/bold] [dim]to stop[/dim]")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/platforms/browser/serve_platform.py` around lines 56 - 60, The banner printing in serve_platform.py uses _console.print to show the URL and the stop hint but is missing the `[underline]` markup on the URL and the bold formatting for "Ctrl+C" to match server/platform.py; update the call that prints the URL (the line using f"... {url}") to wrap the {url} with `[underline]...[/underline]` and update the "Press Ctrl+C to stop" _console.print call to bold "Ctrl+C" (e.g., `[bold]Ctrl+C[/bold]`) so both platform banners display consistent styling.src/trellis/platforms/common/client/src/TrellisClient.ts (1)
12-23: Good fallback implementation for non-secure contexts.The UUID v4 implementation is correct. Minor consistency note: line 14 checks
globalThis.crypto?.randomUUIDbut line 15 callscrypto.randomUUID()directly. Consider usingglobalThis.crypto.randomUUID()on line 15 to match the check pattern.♻️ Optional consistency fix
function generateUUID(): string { if (typeof globalThis.crypto?.randomUUID === "function") { - return crypto.randomUUID(); + return globalThis.crypto.randomUUID(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/platforms/common/client/src/TrellisClient.ts` around lines 12 - 23, The generateUUID function checks globalThis.crypto?.randomUUID but then calls crypto.randomUUID(); update the call to use globalThis.crypto.randomUUID() (or guard with globalThis.crypto) to match the check and avoid potential undefined reference; modify the return in generateUUID to call globalThis.crypto.randomUUID() so the check and usage are consistent.tests/py/unit/test_ports.py (1)
17-24: Close the socket if_bind_port()fails mid-setup.If
bind()orlisten()raises here, cleanup is left to GC. A smalltry/exceptkeeps the helper deterministic and avoids dangling test resources.Suggested cleanup guard
def _bind_port(host: str, port: int) -> socket.socket: """Bind a socket that exclusively holds a port on all platforms.""" sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - if sys.platform == "win32": - sock.setsockopt(socket.SOL_SOCKET, socket.SO_EXCLUSIVEADDRUSE, 1) # type: ignore[attr-defined] - sock.bind((host, port)) - sock.listen(1) - return sock + try: + if sys.platform == "win32": + sock.setsockopt(socket.SOL_SOCKET, socket.SO_EXCLUSIVEADDRUSE, 1) # type: ignore[attr-defined] + sock.bind((host, port)) + sock.listen(1) + return sock + except Exception: + sock.close() + raise🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/py/unit/test_ports.py` around lines 17 - 24, The helper _bind_port creates a socket but doesn't close it if sock.bind(...) or sock.listen(...) raises, leaving dangling sockets; wrap the bind/listen sequence in a try/except (or try/finally) that closes sock on any exception and re-raises, so the socket is always closed if setup fails (reference the local variable sock and the calls to sock.bind and sock.listen in _bind_port).src/trellis/toolchain/python_standalone.py (2)
61-65: Consider adding progress feedback for large downloads.The Python standalone archive can be ~50-80MB. For better UX during
trellis package, consider emitting progress or at minimum a status message before the download starts. This is optional but would improve the developer experience.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/toolchain/python_standalone.py` around lines 61 - 65, Add user feedback when downloading the Python standalone archive by emitting a status message before starting the download or streaming progress while writing to archive_path; locate the download block that creates timeout and uses httpx.stream("GET", url, ..., timeout=timeout) and r.iter_bytes(), and either print/log a simple "Downloading Python standalone..." via the same logger or console prior to the with httpx.stream(...) or, for finer feedback, iterate r.iter_bytes(chunk_size) and update a progress indicator (e.g., bytes downloaded / content-length) while writing chunks to the open file instead of using f.writelines(r.iter_bytes()).
73-77: Guard chmod on Windows to avoid unnecessary calls.Line 76 calls
python_bin.chmod(0o755)unconditionally. On Windows,Path.chmod()is typically a no-op for file permissions since Windows uses ACLs instead of Unix-style permissions. While not a functional issue, the call is unnecessary on Windows. This pattern appears unguarded in multiple places across the toolchain (tauri_cli.py,rustup.py,bun.py). For consistency and clarity, guard the chmod call using theis_windowsvariable already used in this module:🔧 Suggested guard for platform-specific chmod
if not python_bin.exists(): raise RuntimeError(f"Python binary missing after extraction: {python_bin}") - python_bin.chmod(0o755) + if not is_windows: + python_bin.chmod(0o755) return PythonStandalone(python_bin=python_bin, base_dir=base_dir)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/toolchain/python_standalone.py` around lines 73 - 77, The call to python_bin.chmod(0o755) in PythonStandalone creation is unguarded; wrap that chmod call behind the existing is_windows check so it only runs when not is_windows (i.e., if not is_windows: python_bin.chmod(...)) to avoid unnecessary no-op calls on Windows. Apply the same guarded pattern to other similar locations where chmod is unguarded (e.g., in tauri_cli.py, rustup.py, bun.py) by checking the module's is_windows variable before calling Path.chmod; keep the raise RuntimeError and return PythonStandalone(python_bin=python_bin, base_dir=base_dir) behavior unchanged.src/trellis/platforms/desktop/standalone_platform.py (1)
109-115: Consider logging unhandled task exceptions.The
_handler_taskcreated on line 114 has a done callback that handles cleanup, but if the task raises an exception, it will be silently lost. Consider adding exception logging:🔧 Suggested improvement
def _on_handler_done(task: asyncio.Task[None]) -> None: + if task.cancelled(): + pass + elif (exc := task.exception()) is not None: + print(f"[trellis] Handler error: {exc}", flush=True) if self._handler is not None: get_global_registry().unregister(self._handler) _clear_dialog_runtime()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/platforms/desktop/standalone_platform.py` around lines 109 - 115, The done callback _on_handler_done should check for exceptions on _handler_task and log them instead of letting them be lost; update _on_handler_done to inspect task.exception() (or task.cancelled()) for the asyncio.Task passed in, and if an exception exists call the appropriate logger to record the traceback and context (include reference to self._handler and the handler run via self._handler.run()); keep the existing cleanup steps (get_global_registry().unregister(self._handler) and _clear_dialog_runtime()) after logging so unregister/cleanup still occur.src/trellis/platforms/desktop/platform.py (1)
99-108: Add error handling for Windows junction creation.The
_winapi.CreateJunctioncall can fail (e.g., permission issues, existing non-junction file). Consider wrapping in try/except with a helpful error message:🔧 Suggested improvement
if not dist_link.exists(): import _winapi # noqa: PLC0415 - _winapi.CreateJunction(str(dist_dir.resolve()), str(dist_link)) + try: + _winapi.CreateJunction(str(dist_dir.resolve()), str(dist_link)) + except OSError as e: + raise RuntimeError( + f"Failed to create junction at {dist_link}: {e}. " + "Try running as administrator or remove the existing file." + ) from e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/platforms/desktop/platform.py` around lines 99 - 108, Wrap the Windows junction creation around _winapi.CreateJunction in a try/except that catches OSError/Exception, so CreateJunction failures (permissions, existing non-junction file, etc.) are handled gracefully; before creating, check that dist_link does not exist or is already a junction and if an existing non-junction file/dir is present raise or log a clear error referencing config_dir/dist_link and the underlying exception; on failure provide a helpful message (including the caught exception) and either fall back to using the absolute dist_dir path or re-raise a RuntimeError so callers of the platform initialization (the code around CreateJunction, dist_link, dist_path) can handle it.src/trellis/packaging/templates/tauri.conf.json.j2 (1)
22-25: Minor formatting issue in icon array rendering.The template has an extra blank line (line 24) inside the loop which will produce unnecessary blank lines in the generated JSON:
"icon": [ "path/to/icon1.png", "path/to/icon2.icns" ]💅 Proposed fix to remove extra blank lines
"icon": [ -{% for icon in icon_paths %} "{{ icon }}"{% if not loop.last %},{% endif %} - -{% endfor %} ], +{% for icon in icon_paths %} + "{{ icon }}"{% if not loop.last %},{% endif %} +{% endfor %} + ],Or more compact:
"icon": [ {% for icon in icon_paths %} "{{ icon }}"{{ "," if not loop.last else "" }} {% endfor %} ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/templates/tauri.conf.json.j2` around lines 22 - 25, The "icon" array template emits an extra blank line between items because the for-loop block includes a stray newline; update the tauri.conf.json.j2 template so the Jinja2 loop over icon_paths does not produce that blank line (keep the "icon" key and the for icon in icon_paths loop but render each "{{ icon }}" inline with conditional comma logic—e.g., use the loop.last check to append a comma without inserting a blank line) to produce a compact JSON array.src/trellis/app/standalone_entry.py (1)
15-16: Use the publictrellis.Appimport here.This new module reaches into
trellis.app.appeven though the repo standard is to importAppfrom the package root.As per coding guidelines "Import trellis core primitives and App directly: `from trellis import App, component, Stateful`".Suggested fix
-from trellis.app.app import App +from trellis import App🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/app/standalone_entry.py` around lines 15 - 16, Replace the internal module import with the public package export: in standalone_entry.py change the line importing App from "from trellis.app.app import App" to use the package root export "from trellis import App" (leave the DesktopStandalonePlatform import as-is); this aligns the module with the repo guideline to import core primitives from trellis and ensures you reference the same App symbol used elsewhere.src/trellis/packaging/portable.py (2)
172-179: Duplicate regex logic for cargo_name normalization.The cargo name sanitization pattern appears here and again in
build_installer_exe(lines 219-220). Consider extracting to a helper function.♻️ Suggested refactor
+def _sanitize_cargo_name(product_name: str) -> str: + """Convert product name to valid Cargo package name.""" + cargo_name = re.sub(r"[^a-z0-9_-]", "-", product_name.lower()) + return re.sub(r"-+", "-", cargo_name).strip("-") + + def build_portable_exe( ... ) -> Path: ... - cargo_name = re.sub(r"[^a-z0-9_-]", "-", product_name.lower()) - cargo_name = re.sub(r"-+", "-", cargo_name).strip("-") + cargo_name = _sanitize_cargo_name(product_name)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/portable.py` around lines 172 - 179, The cargo name normalization logic is duplicated: the regex-based sanitization that builds cargo_name in this block is repeated in build_installer_exe; extract that logic into a single helper function (e.g., normalize_cargo_name or sanitize_product_name) and replace both occurrences with calls to that function. Ensure the helper accepts product_name, lowercases it, replaces invalid chars with "-", collapses consecutive "-" and strips leading/trailing "-" so cargo_name creation in this module and in build_installer_exe uses the same centralized implementation.
137-139: Hash truncation reduces collision resistance.The SHA-256 hash is truncated to 32 hex characters (128 bits) via
[:32]. While still reasonably collision-resistant for this use case, this is unusual. The constantFOOTER_SIZE = 48accommodates only 32 bytes for the hash. If full SHA-256 (64 hex chars) is desired, adjust the footer format.If 32 characters is intentional for compactness, consider documenting this design choice.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/portable.py` around lines 137 - 139, The code truncates the SHA-256 digest to 32 hex chars in content_hash = hashlib.sha256(archive_bytes).hexdigest()[:32], which reduces collision resistance and mismatches the comment about FOOTER_SIZE = 48; either stop truncating and use the full hexdigest() (64 hex chars) and update FOOTER_SIZE/any footer format logic to allocate the full-length hash, or if truncation is intentional for compactness, add a clear comment next to the content_hash calculation and the FOOTER_SIZE constant explaining the deliberate 128-bit truncation decision and its tradeoffs; update any tests or consumers that read the footer to match the chosen full-length vs truncated format.src/trellis/packaging/tauri.py (2)
82-93: Windows junction creation may fail without elevation.While the comment mentions symlinks require admin privileges, junctions typically don't. However,
CreateJunctionto a non-existent target or across drives may fail. Consider adding error handling.🛡️ Suggested error handling
if sys.platform == "win32": import _winapi # noqa: PLC0415 - Windows-only - _winapi.CreateJunction(str(dist_path.resolve()), str(dist_link)) + try: + _winapi.CreateJunction(str(dist_path.resolve()), str(dist_link)) + except OSError as e: + raise RuntimeError( + f"Failed to create junction {dist_link} -> {dist_path}: {e}" + ) from e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/tauri.py` around lines 82 - 93, The Windows junction creation using _winapi.CreateJunction when creating dist_link from dist_path can fail (non-existent target, cross-drive, permissions); wrap the CreateJunction call (and the non-Windows dist_link.symlink_to) in a try/except, log a clear error including the exception and the values of dist_path and dist_link, and implement a safe fallback (e.g., copy the directory contents from dist_path into scaffold_dir/dist using shutil.copytree or re-raise a descriptive RuntimeError) so packaging doesn't silently fail; update references in this block around dist_link, dist_path, and CreateJunction to use the new error handling and fallback behavior.
550-573: Duplicate cargo_name sanitization logic.Same regex pattern as in
portable.py. Consider extracting to a shared utility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/tauri.py` around lines 550 - 573, The cargo_name sanitization code in src/trellis/packaging/tauri.py duplicates the same regex logic used in portable.py; extract this into a single utility function (e.g., sanitize_cargo_name or make_safe_cargo_name) in a shared module, implement the logic to: lowercase product_name, replace r"[^a-z0-9_-]" with "-", collapse multiple "-" to one, and strip leading/trailing "-", then import and call that helper from tauri.py (replacing the cargo_name assignment) and from portable.py so both use the common function.src/trellis/toolchain/tauri_cli.py (1)
56-61: Consider adding progress indication for large downloads.The Tauri CLI binary can be substantial. For better UX in interactive contexts, consider logging download progress or at least a start/completion message.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/toolchain/tauri_cli.py` around lines 56 - 61, The download currently writes the whole response via r.iter_bytes() with no user feedback; update the block that uses httpx.stream, timeout, url, archive_path and r.iter_bytes() to report progress: log a start message, read the response in chunks (iter_bytes(chunk_size)), write each chunk to archive_path while accumulating bytes_written, and if available use r.headers.get("content-length") to compute and log percentage/completion; if content-length is missing, at minimum log periodic byte counts and a final completion message. Ensure you use the existing logger (or print) consistently and keep the timeout logic intact.tests/py/unit/test_toolchain_rustup.py (1)
177-204: Verify mock coverage for rustup download path.The test verifies that a download was attempted but doesn't verify the rustup-init binary was actually executed. The
mock_runis set up but the assertion only checksmock_streamwas called.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/py/unit/test_toolchain_rustup.py` around lines 177 - 204, The test only asserts that httpx.stream was called but not that the downloaded rustup-init was executed; update test_downloads_rustup_when_nothing_found to also assert subprocess.run was invoked by checking mock_run was called and that its call arguments include the downloaded rustup-init binary name/path (e.g., verify "rustup-init" or the cache_dir / "rust" / "rustup" path appears in mock_run.call_args) so ensure_rustup's execution step is covered.src/trellis/packaging/templates/launcher/main.rs.j2 (2)
248-267: File lock may not release on process crash.The lock file approach using
create_newis simple but has a limitation: if the launcher crashes or is killed, the lock file remains. The 30-second timeout helps, but consider documenting this behavior or adding stale lock detection (e.g., check if lock file is older than expected extraction time).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/templates/launcher/main.rs.j2` around lines 248 - 267, The current acquire_lock function can leave a stale lock file if the process crashes; update acquire_lock to detect and clear stale locks: inside the Err(e) branch where e.kind() == io::ErrorKind::AlreadyExists, stat the existing lock_path (fs::metadata) and check its modified (or created) time against SystemTime::now(), and if it's older than a threshold (e.g., the same 30s timeout or a configurable value) attempt to remove the file (fs::remove_file(lock_path)) and continue the loop, handling errors from metadata/remove by falling back to the existing sleep-and-retry behavior; keep the overall timeout behavior and ensure error messages (returned Err from acquire_lock) reflect failures to remove or read metadata so callers can diagnose stale-lock removal failures.
356-363: Exit code truncation on Windows.
s.code().unwrap_or(1) as u8truncates exit codes larger than 255. On Windows, exit codes can be 32-bit DWORD values. Consider using the full exit code or documenting this limitation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/templates/launcher/main.rs.j2` around lines 356 - 363, The match arm currently maps the child exit code via s.code().unwrap_or(1) as u8 which truncates values >255; instead capture the full i32 exit code from s.code().unwrap_or(1) and call std::process::exit(code) (or use ExitCode::from_raw if available) so Windows 32-bit DWORD exit codes are preserved — update the match that handles Ok(s) (and keep the Err(e) branch using show_error/real_exe) to use the full integer exit value rather than casting to u8.tests/py/unit/test_packaging_tauri.py (1)
514-531: Consider extracting _setup_bundle to a fixture.The
_setup_bundlemethod is used by multiple tests inTestOutputCopying. Per coding guidelines, shared test setup used in 3+ tests could be a fixture inconftest.py.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/py/unit/test_packaging_tauri.py` around lines 514 - 531, Extract the repeated _setup_bundle helper into a pytest fixture (e.g., setup_bundle) and place it in your test fixtures module; move the logic that builds the Config, app_root with .dist, mock_rust and mock_python into that fixture and yield/return the tuple (Config, Path, mock_rust, mock_python), then update each test in the TestOutputCopying test class to accept the fixture as a parameter instead of calling the _setup_bundle method so they reuse the shared setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 17-21: The workflow currently uses the action reference
awalsh128/cache-apt-pkgs-action@latest which is not pinned; update that
reference to a fixed release tag or commit SHA (e.g., replace `@latest` with a
specific tag or SHA) so the step "Install Tauri system dependencies" uses a
reproducible, immutable version; ensure the change targets the action string in
the CI workflow block that defines the Install Tauri system dependencies step.
In `@examples/widget_showcase/trellis_config.py`:
- Line 10: The icon path is currently relative to the CWD
(icon=Path("assets/icon.png")) which breaks when running from repo root; change
the assignment in trellis_config.py to resolve the asset relative to this file
by using the module file location (use Path(__file__).parent joined with
"assets/icon.png") so the icon points to the correct file regardless of working
directory.
In `@pixi.toml`:
- Line 6: pixi.toml currently lacks "osx-64" in the platforms list while
platform.py still maps ("darwin", "x86_64") -> "x86_64-apple-darwin", causing
Intel macOS resolution failures; either add "osx-64" back into the platforms
array in pixi.toml to restore Intel macOS support, or remove the ("darwin",
"x86_64") mapping in platform.py (and update docs) to explicitly drop Intel
macOS — pick one approach, make the corresponding change to the platforms array
or the mapping, and update any comments/docs to reflect the supported macOS
toolchains.
In `@src/trellis/app/standalone_entry.py`:
- Around line 19-29: _find_app currently scans dir(module_name) and returns the
first App instance found which can pick up imported/re-exported apps; instead,
explicitly lookup the exported symbol named "app" on the imported module (use
getattr(mod, "app", None)), verify it's an instance of App, and return it,
otherwise raise the same RuntimeError mentioning module_name and the required
"app = App(...)" contract.
In `@src/trellis/bundler/utils.py`:
- Around line 56-68: The current validation lets later members bypass checks by
following symlinks that will be created earlier; update the loop that handles
member.issym() and member.islnk() to maintain a mapping (e.g., created_links) of
archive-order-created symlink/hardlink paths to their targets and use a custom
resolver (resolve_with_created_links) that walks path components applying that
mapping instead of touching the real FS when validating any member_path, then
validate the final resolved path is inside dest; when encountering
member.issym() or member.islnk(), add the symlink/hardlink to created_links
(store relative target as in archive) rather than allowing instant filesystem
resolution. Add a regression test that creates archive members in the order
"package/link -> subdir/.." then "package/link/../../escape.txt" and assert
validation rejects the second member.
In `@src/trellis/packaging/tauri.py`:
- Around line 391-396: The code hardcodes "python3.13" when building the
site-packages path (in the block that defines site_packages using pyembed_dir),
which will break if PYTHON_STANDALONE_VERSION changes; change the path
construction to use the PYTHON_STANDALONE_VERSION value (e.g., build the
directory name as f"python{PYTHON_STANDALONE_VERSION}" or otherwise format the
version constant) when computing site_packages so the lookup uses the configured
Python standalone version instead of a hardcoded literal; update any references
in the same function (where pyembed_dir and site_packages are used) to rely on
that variable.
In `@src/trellis/packaging/templates/launcher/main_installer.rs.j2`:
- Around line 257-279: acquire_lock returns an open fs::File but release_lock
currently only removes the lock path while the file handle is still live, which
leaves a stale lock on Windows; change the API so the lock file handle is closed
(drop the fs::File) before unlinking: either make release_lock take ownership of
the fs::File (e.g., release_lock(file: fs::File, lock_path: &Path)) and let it
be dropped before calling fs::remove_file(lock_path), or replace the pair with a
LockGuard struct that owns the fs::File and unlinks the path in its Drop
implementation; update all call sites that use acquire_lock and release_lock
(and the other affected region noted) to pass/hold the File or LockGuard
accordingly.
- Around line 41-67: read_footer currently parses Footer (struct Footer,
read_footer) but doesn't validate the parsed hash or archive_size before it's
used to index/allocate; add checks to reject corrupt footers: ensure
archive_size > 0 and archive_size <= file_len - FOOTER_SIZE (so archive_start =
file_len - FOOTER_SIZE - archive_size cannot underflow), validate the hash
string format/length (match the format written by portable.py) and return Err on
invalid values, and in the payload-extraction path compute the archive's digest
and compare it to Footer.hash (fail if mismatch) before allocating vec![0u8;
footer.archive_size as usize] or reading the archive; keep references to
FOOTER_SIZE, MAGIC, Footer, read_footer and archive_size when making these
checks.
- Around line 343-349: The uninstall command fails when the install path
contains spaces because the {dir_s} path isn't preserved inside the nested cmd
/c quotes; update the formatted `script` so the rmdir argument is quoted in a
way that survives into the spawned cmd instance (i.e., ensure the double quotes
around `{dir_s}` are escaped for the Rust string and for cmd parsing), e.g.
change the rmdir fragment that currently contains `"{dir_s}"` to use properly
escaped inner quotes so the final command sees "C:\... path ..." (refer to
`script`, `{dir_s}`, and `lnk_s` in the format call).
In `@src/trellis/packaging/templates/launcher/main.rs.j2`:
- Around line 79-93: The extraction loop uses archive.by_index and entry.name()
to build out_path under dest but does not validate path traversal or absolute
paths; before creating dirs or files (before fs::create_dir_all or
fs::File::create), reject or sanitize entries whose name contains parent
components ("..") or is absolute, and ensure the resolved destination stays
inside dest (e.g., compute a canonicalized dest path and the canonicalized
target path from dest.join(name) and verify target starts_with dest_canonical);
skip or error on entries that fail this check to prevent extraction outside the
intended directory.
In `@src/trellis/platforms/desktop/client/src/index.html.j2`:
- Around line 28-34: The crash handlers (window.onerror and the
unhandledrejection listener) currently set document.body.innerHTML with raw msg,
url and e.reason which creates an XSS and exposes stacks; change both handlers
to build and insert DOM nodes (e.g., createElement/textContent/appendChild)
instead of using innerHTML, escape or use textContent for msg/url/reason, and
gate inclusion of err.stack/e.reason.stack behind a debug flag (NODE_ENV or a
DEBUG global) so stacks are only shown in dev; update references to
window.onerror, window.addEventListener('unhandledrejection') and remove direct
use of document.body.innerHTML.
In `@src/trellis/toolchain/__init__.py`:
- Around line 12-13: Remove the duplicate MINIMUM_RUST_VERSION definition in
rustup.py and import the single source-of-truth constant from
src/trellis/toolchain/__init__.py instead: keep MINIMUM_RUST_VERSION in
__init__.py (alongside TAURI_CLI_VERSION) and update rustup.py to remove its
local MINIMUM_RUST_VERSION and reference the imported MINIMUM_RUST_VERSION
constant where used (ensure an appropriate import statement for
MINIMUM_RUST_VERSION in rustup.py).
In `@src/trellis/toolchain/rustup.py`:
- Around line 186-191: The RustToolchain return constructs cargo_bin and
rustc_bin without the Windows ".exe" suffix; update the code that returns
RustToolchain (the cargo_bin and rustc_bin fields built from cargo_home / "bin"
/ "cargo" and .../"rustc") to append ".exe" when running on Windows (e.g.,
detect Windows via os.name == "nt" or platform.system() == "Windows")—mirror the
same fix used in _download_rustup so the paths point to "cargo.exe" and
"rustc.exe" on Windows and remain unchanged on Unix.
- Around line 130-131: The cargo_bin and rustc_bin paths are built without the
Windows executable suffix; on Windows these should be cargo_home / "bin" /
"cargo.exe" and "rustc.exe". Update the construction of cargo_bin and rustc_bin
(the variables in src/trellis/toolchain/rustup.py) to append ".exe" when running
on Windows (e.g., detect via os.name == "nt" or platform.system() == "Windows"),
or use a small helper that returns the binary name with ".exe" on Windows and
plain name otherwise; ensure the module imports the needed os/platform symbol.
In `@tests/py/unit/test_bundler_utils.py`:
- Around line 398-399: The test method test_allows_internal_symlinks is
currently gated with pytest.mark.skipif for Windows but lacks the platform
marker; update the decorator list to include `@pytest.mark.platform` (and any
other relevant markers per guidelines such as
`@pytest.mark.slow/`@pytest.mark.network if applicable) while keeping the existing
pytest.mark.skipif(sys.platform == "win32", ...) so the test is both skipped on
Windows and properly categorized as a platform-specific test.
In `@tests/py/unit/test_toolchain_tauri_cli.py`:
- Line 11: The test is using the test module's TAURI_CLI_VERSION constant
instead of the patched value used by ensure_tauri_cli(), causing the cached-path
branch to be missed; update the test to read the version from the actual module
under test (trellis.toolchain.tauri_cli.TAURI_CLI_VERSION) or re-import the
tauri_cli module after applying the patch so the cached-path fixture uses the
patched value used by ensure_tauri_cli(). Ensure all references (the cached-path
fixture and the assertion around TAURI_CLI_VERSION at lines ~43-50) use
trellis.toolchain.tauri_cli.TAURI_CLI_VERSION or the re-imported module value so
the test exercises the cached branch.
---
Nitpick comments:
In `@src/trellis/app/standalone_entry.py`:
- Around line 15-16: Replace the internal module import with the public package
export: in standalone_entry.py change the line importing App from "from
trellis.app.app import App" to use the package root export "from trellis import
App" (leave the DesktopStandalonePlatform import as-is); this aligns the module
with the repo guideline to import core primitives from trellis and ensures you
reference the same App symbol used elsewhere.
In `@src/trellis/packaging/portable.py`:
- Around line 172-179: The cargo name normalization logic is duplicated: the
regex-based sanitization that builds cargo_name in this block is repeated in
build_installer_exe; extract that logic into a single helper function (e.g.,
normalize_cargo_name or sanitize_product_name) and replace both occurrences with
calls to that function. Ensure the helper accepts product_name, lowercases it,
replaces invalid chars with "-", collapses consecutive "-" and strips
leading/trailing "-" so cargo_name creation in this module and in
build_installer_exe uses the same centralized implementation.
- Around line 137-139: The code truncates the SHA-256 digest to 32 hex chars in
content_hash = hashlib.sha256(archive_bytes).hexdigest()[:32], which reduces
collision resistance and mismatches the comment about FOOTER_SIZE = 48; either
stop truncating and use the full hexdigest() (64 hex chars) and update
FOOTER_SIZE/any footer format logic to allocate the full-length hash, or if
truncation is intentional for compactness, add a clear comment next to the
content_hash calculation and the FOOTER_SIZE constant explaining the deliberate
128-bit truncation decision and its tradeoffs; update any tests or consumers
that read the footer to match the chosen full-length vs truncated format.
In `@src/trellis/packaging/tauri.py`:
- Around line 82-93: The Windows junction creation using _winapi.CreateJunction
when creating dist_link from dist_path can fail (non-existent target,
cross-drive, permissions); wrap the CreateJunction call (and the non-Windows
dist_link.symlink_to) in a try/except, log a clear error including the exception
and the values of dist_path and dist_link, and implement a safe fallback (e.g.,
copy the directory contents from dist_path into scaffold_dir/dist using
shutil.copytree or re-raise a descriptive RuntimeError) so packaging doesn't
silently fail; update references in this block around dist_link, dist_path, and
CreateJunction to use the new error handling and fallback behavior.
- Around line 550-573: The cargo_name sanitization code in
src/trellis/packaging/tauri.py duplicates the same regex logic used in
portable.py; extract this into a single utility function (e.g.,
sanitize_cargo_name or make_safe_cargo_name) in a shared module, implement the
logic to: lowercase product_name, replace r"[^a-z0-9_-]" with "-", collapse
multiple "-" to one, and strip leading/trailing "-", then import and call that
helper from tauri.py (replacing the cargo_name assignment) and from portable.py
so both use the common function.
In `@src/trellis/packaging/templates/launcher/main.rs.j2`:
- Around line 248-267: The current acquire_lock function can leave a stale lock
file if the process crashes; update acquire_lock to detect and clear stale
locks: inside the Err(e) branch where e.kind() == io::ErrorKind::AlreadyExists,
stat the existing lock_path (fs::metadata) and check its modified (or created)
time against SystemTime::now(), and if it's older than a threshold (e.g., the
same 30s timeout or a configurable value) attempt to remove the file
(fs::remove_file(lock_path)) and continue the loop, handling errors from
metadata/remove by falling back to the existing sleep-and-retry behavior; keep
the overall timeout behavior and ensure error messages (returned Err from
acquire_lock) reflect failures to remove or read metadata so callers can
diagnose stale-lock removal failures.
- Around line 356-363: The match arm currently maps the child exit code via
s.code().unwrap_or(1) as u8 which truncates values >255; instead capture the
full i32 exit code from s.code().unwrap_or(1) and call std::process::exit(code)
(or use ExitCode::from_raw if available) so Windows 32-bit DWORD exit codes are
preserved — update the match that handles Ok(s) (and keep the Err(e) branch
using show_error/real_exe) to use the full integer exit value rather than
casting to u8.
In `@src/trellis/packaging/templates/tauri.conf.json.j2`:
- Around line 22-25: The "icon" array template emits an extra blank line between
items because the for-loop block includes a stray newline; update the
tauri.conf.json.j2 template so the Jinja2 loop over icon_paths does not produce
that blank line (keep the "icon" key and the for icon in icon_paths loop but
render each "{{ icon }}" inline with conditional comma logic—e.g., use the
loop.last check to append a comma without inserting a blank line) to produce a
compact JSON array.
In `@src/trellis/platforms/browser/serve_platform.py`:
- Around line 56-60: The banner printing in serve_platform.py uses
_console.print to show the URL and the stop hint but is missing the
`[underline]` markup on the URL and the bold formatting for "Ctrl+C" to match
server/platform.py; update the call that prints the URL (the line using f"...
{url}") to wrap the {url} with `[underline]...[/underline]` and update the
"Press Ctrl+C to stop" _console.print call to bold "Ctrl+C" (e.g.,
`[bold]Ctrl+C[/bold]`) so both platform banners display consistent styling.
In `@src/trellis/platforms/common/client/src/TrellisClient.ts`:
- Around line 12-23: The generateUUID function checks
globalThis.crypto?.randomUUID but then calls crypto.randomUUID(); update the
call to use globalThis.crypto.randomUUID() (or guard with globalThis.crypto) to
match the check and avoid potential undefined reference; modify the return in
generateUUID to call globalThis.crypto.randomUUID() so the check and usage are
consistent.
In `@src/trellis/platforms/desktop/platform.py`:
- Around line 99-108: Wrap the Windows junction creation around
_winapi.CreateJunction in a try/except that catches OSError/Exception, so
CreateJunction failures (permissions, existing non-junction file, etc.) are
handled gracefully; before creating, check that dist_link does not exist or is
already a junction and if an existing non-junction file/dir is present raise or
log a clear error referencing config_dir/dist_link and the underlying exception;
on failure provide a helpful message (including the caught exception) and either
fall back to using the absolute dist_dir path or re-raise a RuntimeError so
callers of the platform initialization (the code around CreateJunction,
dist_link, dist_path) can handle it.
In `@src/trellis/platforms/desktop/standalone_platform.py`:
- Around line 109-115: The done callback _on_handler_done should check for
exceptions on _handler_task and log them instead of letting them be lost; update
_on_handler_done to inspect task.exception() (or task.cancelled()) for the
asyncio.Task passed in, and if an exception exists call the appropriate logger
to record the traceback and context (include reference to self._handler and the
handler run via self._handler.run()); keep the existing cleanup steps
(get_global_registry().unregister(self._handler) and _clear_dialog_runtime())
after logging so unregister/cleanup still occur.
In `@src/trellis/toolchain/python_standalone.py`:
- Around line 61-65: Add user feedback when downloading the Python standalone
archive by emitting a status message before starting the download or streaming
progress while writing to archive_path; locate the download block that creates
timeout and uses httpx.stream("GET", url, ..., timeout=timeout) and
r.iter_bytes(), and either print/log a simple "Downloading Python standalone..."
via the same logger or console prior to the with httpx.stream(...) or, for finer
feedback, iterate r.iter_bytes(chunk_size) and update a progress indicator
(e.g., bytes downloaded / content-length) while writing chunks to the open file
instead of using f.writelines(r.iter_bytes()).
- Around line 73-77: The call to python_bin.chmod(0o755) in PythonStandalone
creation is unguarded; wrap that chmod call behind the existing is_windows check
so it only runs when not is_windows (i.e., if not is_windows:
python_bin.chmod(...)) to avoid unnecessary no-op calls on Windows. Apply the
same guarded pattern to other similar locations where chmod is unguarded (e.g.,
in tauri_cli.py, rustup.py, bun.py) by checking the module's is_windows variable
before calling Path.chmod; keep the raise RuntimeError and return
PythonStandalone(python_bin=python_bin, base_dir=base_dir) behavior unchanged.
In `@src/trellis/toolchain/tauri_cli.py`:
- Around line 56-61: The download currently writes the whole response via
r.iter_bytes() with no user feedback; update the block that uses httpx.stream,
timeout, url, archive_path and r.iter_bytes() to report progress: log a start
message, read the response in chunks (iter_bytes(chunk_size)), write each chunk
to archive_path while accumulating bytes_written, and if available use
r.headers.get("content-length") to compute and log percentage/completion; if
content-length is missing, at minimum log periodic byte counts and a final
completion message. Ensure you use the existing logger (or print) consistently
and keep the timeout logic intact.
In `@tests/py/unit/test_packaging_tauri.py`:
- Around line 514-531: Extract the repeated _setup_bundle helper into a pytest
fixture (e.g., setup_bundle) and place it in your test fixtures module; move the
logic that builds the Config, app_root with .dist, mock_rust and mock_python
into that fixture and yield/return the tuple (Config, Path, mock_rust,
mock_python), then update each test in the TestOutputCopying test class to
accept the fixture as a parameter instead of calling the _setup_bundle method so
they reuse the shared setup.
In `@tests/py/unit/test_ports.py`:
- Around line 17-24: The helper _bind_port creates a socket but doesn't close it
if sock.bind(...) or sock.listen(...) raises, leaving dangling sockets; wrap the
bind/listen sequence in a try/except (or try/finally) that closes sock on any
exception and re-raises, so the socket is always closed if setup fails
(reference the local variable sock and the calls to sock.bind and sock.listen in
_bind_port).
In `@tests/py/unit/test_toolchain_rustup.py`:
- Around line 177-204: The test only asserts that httpx.stream was called but
not that the downloaded rustup-init was executed; update
test_downloads_rustup_when_nothing_found to also assert subprocess.run was
invoked by checking mock_run was called and that its call arguments include the
downloaded rustup-init binary name/path (e.g., verify "rustup-init" or the
cache_dir / "rust" / "rustup" path appears in mock_run.call_args) so
ensure_rustup's execution step is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5a56645c-1b52-4ecf-9225-44940a1dc1e7
⛔ Files ignored due to path filters (2)
examples/widget_showcase/assets/icon.pngis excluded by!**/*.pngpixi.lockis excluded by!**/*.lock
📒 Files selected for processing (64)
.github/workflows/ci.yml.gitignoreexamples/widget_showcase/trellis_config.pyexamples/widget_showcase/widget_showcase/example.pypixi.tomlpyproject.tomlsrc/trellis/app/apploader.pysrc/trellis/app/config.pysrc/trellis/app/standalone_entry.pysrc/trellis/bundler/manifest.pysrc/trellis/bundler/utils.pysrc/trellis/bundler/wheels.pysrc/trellis/cli/init.pysrc/trellis/cli/package.pysrc/trellis/packaging/__init__.pysrc/trellis/packaging/hooks/hook-pytauri.pysrc/trellis/packaging/hooks/hook-rich.pysrc/trellis/packaging/portable.pysrc/trellis/packaging/pyinstaller.pysrc/trellis/packaging/tauri.pysrc/trellis/packaging/templates/Cargo.toml.j2src/trellis/packaging/templates/build.rs.j2src/trellis/packaging/templates/capabilities_default.json.j2src/trellis/packaging/templates/launcher/Cargo.toml.j2src/trellis/packaging/templates/launcher/build.rs.j2src/trellis/packaging/templates/launcher/main.rs.j2src/trellis/packaging/templates/launcher/main_installer.rs.j2src/trellis/packaging/templates/lib.rs.j2src/trellis/packaging/templates/main.rs.j2src/trellis/packaging/templates/tauri.conf.json.j2src/trellis/platforms/browser/serve_platform.pysrc/trellis/platforms/common/client/src/TrellisClient.tssrc/trellis/platforms/common/ports.pysrc/trellis/platforms/desktop/__init__.pysrc/trellis/platforms/desktop/client/src/index.html.j2src/trellis/platforms/desktop/platform.pysrc/trellis/platforms/desktop/standalone_platform.pysrc/trellis/platforms/server/platform.pysrc/trellis/toolchain/__init__.pysrc/trellis/toolchain/platform.pysrc/trellis/toolchain/python_standalone.pysrc/trellis/toolchain/rustup.pysrc/trellis/toolchain/tauri_cli.pysrc/trellis/widgets/table.pytests/py/integration/test_message_handler_integration.pytests/py/integration/test_widget_actions.pytests/py/integration/test_widget_basic.pytests/py/integration/test_widget_inputs.pytests/py/integration/test_widget_navigation.pytests/py/unit/test_apploader_standalone.pytests/py/unit/test_bun_unit.pytests/py/unit/test_bundler_utils.pytests/py/unit/test_cli_package.pytests/py/unit/test_config.pytests/py/unit/test_config_packaging.pytests/py/unit/test_desktop_standalone_platform.pytests/py/unit/test_mutable_unit.pytests/py/unit/test_packaging_portable.pytests/py/unit/test_packaging_pyinstaller.pytests/py/unit/test_packaging_tauri.pytests/py/unit/test_ports.pytests/py/unit/test_toolchain_python_standalone.pytests/py/unit/test_toolchain_rustup.pytests/py/unit/test_toolchain_tauri_cli.py
💤 Files with no reviewable changes (4)
- src/trellis/packaging/hooks/hook-rich.py
- tests/py/unit/test_packaging_pyinstaller.py
- src/trellis/packaging/hooks/hook-pytauri.py
- src/trellis/packaging/pyinstaller.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
♻️ Duplicate comments (1)
pixi.toml (1)
6-6:⚠️ Potential issue | 🟠 MajorIntel macOS support is still inconsistent here.
src/trellis/toolchain/platform.pystill maps("darwin", "x86_64")tox86_64-apple-darwin, but the Pixi workspace no longer includesosx-64. That means Intel Macs will fail environment resolution even though the toolchain layer still claims support. Either addosx-64back here or remove the x86_64 macOS mapping/docs together.Minimal fix if Intel macOS remains supported
-platforms = ["osx-arm64", "linux-64", "win-64"] +platforms = ["osx-arm64", "osx-64", "linux-64", "win-64"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pixi.toml` at line 6, The Pixi workspace platforms list no longer includes osx-64 while the toolchain mapping in src/trellis/toolchain/platform.py still maps ("darwin", "x86_64") -> "x86_64-apple-darwin", causing Intel macOS resolution to fail; fix by either adding "osx-64" back into the platforms array in pixi.toml to match the mapping or remove/update the ("darwin","x86_64") mapping and any related docs to stop advertising Intel macOS support (ensure changes touch the tuple mapping in platform.py and the platforms array in pixi.toml so both stay consistent).
🧹 Nitpick comments (6)
src/trellis/platforms/common/client/src/TrellisClient.ts (1)
12-23: Good fallback pattern for non-secure contexts.The UUIDv4 generation logic is correct—version and variant bits are properly set, and the formatting matches the standard 8-4-4-4-12 structure.
Minor inconsistency: line 14 checks
globalThis.crypto?.randomUUIDbut line 15 callscrypto.randomUUID(). While functionally equivalent in browsers, using a consistent reference improves clarity.🔧 Suggested consistency fix
function generateUUID(): string { if (typeof globalThis.crypto?.randomUUID === "function") { - return crypto.randomUUID(); + return globalThis.crypto.randomUUID(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/platforms/common/client/src/TrellisClient.ts` around lines 12 - 23, The check uses globalThis.crypto?.randomUUID but the call uses crypto.randomUUID(), so update generateUUID to consistently reference globalThis.crypto (e.g., call globalThis.crypto.randomUUID()) to avoid mixed references; keep the existing fallback logic (Uint8Array/getRandomValues and version/variant bit tweaks) intact and only change the direct randomUUID invocation to globalThis.crypto.randomUUID.src/trellis/bundler/utils.py (1)
89-89: Consider: symlink keying uses unresolved path.The symlink registry keys by
dest / member.name(the literal archive path), but lookups in_resolve_through_symlinkstraverse the resolved path after substituting earlier symlinks. This could miss symlinks nested within other symlinked directories in edge cases (e.g.,link1 -> subdir, thenlink1/link2 -> file, then accessing vialink1/link2/..).For typical archive structures this is unlikely to cause issues, but consider adding a test for nested symlinks if this becomes a concern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/bundler/utils.py` at line 89, The symlink registry currently keys entries using the literal archive path expression dest / member.name which can miss nested symlinks when _resolve_through_symlinks walks already-resolved paths; change the registration to use a normalized/resolved form of the archive path (the same normalization used by _resolve_through_symlinks) when inserting into symlinks and when performing lookups so keys and lookups match for nested symlink cases (ensure you update code around the symlinks dict population where dest / member.name is assigned and any related lookup logic in _resolve_through_symlinks to use the same Path normalization routine).src/trellis/app/apploader.py (1)
203-205: Normalizeconfig.iconto an absolute path here.
self.path / config.iconstill depends on callers having already resolvedAppLoader.path. Resolving it here keeps downstreamIconAssetStepusage independent of the current working directory.Suggested change
- if config.icon is not None and not config.icon.is_absolute(): - config.icon = self.path / config.icon + if config.icon is not None and not config.icon.is_absolute(): + config.icon = (self.path / config.icon).resolve()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/app/apploader.py` around lines 203 - 205, Normalize config.icon to an absolute Path: when config.icon is not None and not config.icon.is_absolute(), resolve it against self.path and call .resolve() (or Path(...).absolute equivalent) so config.icon becomes an absolute path; update the block using config.icon and self.path in AppLoader (the code that currently sets config.icon = self.path / config.icon) to instead set config.icon = (self.path / config.icon).resolve() so downstream consumers like IconAssetStep receive an absolute path.tests/py/unit/test_packaging_tauri.py (1)
685-710: Mockpkg-confighere so this remains a unit test.This case still depends on host tooling and machine state, which makes it environment-sensitive. Patch
_get_linux_system_lib_flags()orsubprocess.run()instead so the assertions stay deterministic. As per coding guidelines, "Python unit tests should be isolated and test a single function, class, or module with dependencies faked or stubbed".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/py/unit/test_packaging_tauri.py` around lines 685 - 710, The test test_generates_config_on_linux currently depends on host pkg-config output; patch the dependency by mocking trellis.packaging.tauri._get_linux_system_lib_flags (or subprocess.run used inside it) to return deterministic linker flags during the test so it no longer queries host tooling. In the test’s with patch(...) block, add a patch for _get_linux_system_lib_flags to return a known list (e.g., ["-L/some/path", "-lfoo"]) or patch subprocess.run to return a CompletedProcess with expected stdout, then call _generate_cargo_config as before and assert against the deterministic content; refer to the functions _generate_cargo_config and _get_linux_system_lib_flags (or the subprocess.run call inside it) to locate where to apply the patch.src/trellis/packaging/tauri.py (1)
447-447: Move_output_filenameto the module imports or explain the cycle.This file already imports from
trellis.packaging.portableat the top, so the local import at Line 447 just adds another hidden dependency edge in the middle of the function. As per coding guidelines, "Place imports at the top of the file unless there is a compelling reason not to; mid-file imports add complexity and should include a comment explaining why".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/tauri.py` at line 447, The local mid-function import of _output_filename from trellis.packaging.portable should be moved to the top-level imports or justified with a comment about an import cycle; locate the local statement "from trellis.packaging.portable import _output_filename" and either (a) hoist it to the module imports alongside other imports (preserve any noqa/flake8 directive if needed), or (b) if a circular import forces the local import, add a brief explanatory comment immediately above it stating why the import must be deferred (mentioning the import cycle with trellis.packaging.portable) and keep the local import as-is to make the reason explicit for future readers.src/trellis/packaging/portable.py (1)
190-237: Consider consolidating withbuild_portable_exeto reduce duplication.Both functions share nearly identical logic, differing only in
mode,build_dirname, and output filename. A single internal helper with mode parameter would reduce code duplication.♻️ Example refactor
+def _build_bundle( + *, + rust: RustToolchain, + scaffold_dir: Path, + product_name: str, + exe_name: str, + version: str, + output_dir: Path, + mode: str, +) -> Path: + """Internal helper to build portable or installer bundle.""" + release_dir = scaffold_dir / "target" / "release" + files = _collect_app_files(release_dir) + if not files: + raise RuntimeError(f"No app files found in {release_dir}") + + build_dir = scaffold_dir / "target" / f"{mode}-build" + build_dir.mkdir(parents=True, exist_ok=True) + archive_path = build_dir / "app.zip" + _create_archive(files, archive_path) + + cargo_name = re.sub(r"[^a-z0-9_-]", "-", product_name.lower()) + cargo_name = re.sub(r"-+", "-", cargo_name).strip("-") + launcher_dir = build_dir / "launcher" + icon_path = scaffold_dir / "icons" / "icon.ico" + _generate_launcher_scaffold( + launcher_dir, product_name, exe_name, cargo_name, icon_path, + mode=mode, version=version, + ) + launcher_exe = _build_launcher(rust, launcher_dir, cargo_name) + + is_installer = mode == "installer" + output_path = output_dir / _output_filename(product_name, version, "exe", installer=is_installer) + _assemble_portable_exe(launcher_exe, archive_path, output_path) + return output_path🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/portable.py` around lines 190 - 237, The build_installer_exe implementation duplicates build_portable_exe; extract the shared logic into a single helper (e.g. _build_exe_bundle or similar) that accepts parameters for mode ("installer" vs "portable"), build_dir name, and installer flag for the output filename; have build_installer_exe and build_portable_exe call that helper and reuse existing helpers (_generate_launcher_scaffold, _build_launcher, _assemble_portable_exe, _output_filename) so only the mode, build_dir/archivename, and installer boolean differ.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/trellis/bundler/wheels.py`:
- Around line 249-252: The code currently builds name_and_version by splitting
str(req) at the first semicolon which breaks on semicolons inside URLs; instead
reconstruct the pip argument from the Requirement object: if req.url is present,
build the requirement as "<name>[extras] @ <req.url>" (include extras only if
req.extras is non-empty), otherwise build it as "<name>[extras]<specifier>"
using req.name, req.extras and str(req.specifier); do not use str(req) or split
by ";" so environment markers are ignored and URLs remain intact (update the
variables version_spec/name_and_version usage accordingly).
In `@src/trellis/packaging/portable.py`:
- Around line 120-129: The returned launcher path in _build_launcher is using a
hyphenated filename but Cargo produces binaries with underscores; update the
return to use an underscored binary name derived from cargo_name (e.g., replace
hyphens with underscores or append "_launcher.exe" to cargo_name after
normalizing) so the function returns the actual built file (adjust the
expression that builds the filename for the Path returned by _build_launcher).
In `@src/trellis/packaging/tauri.py`:
- Around line 155-179: The code currently calls pip using the shared python_bin
and then copies that mutated interpreter to pyembed_dir, which causes installed
app/deps to persist across builds; to fix, avoid mutating the cached interpreter
by copying the standalone interpreter into pyembed_dir first (respecting the
existing src_dir logic that derives src_dir from python_bin), remove pyembed_dir
if it exists, copy src_dir -> pyembed_dir, then derive the python executable
inside the copied tree (mirror the python_bin relative path) and run
subprocess.run([... str(new_python_in_pyembed), "-m", "pip", "install", ...],
check=True) so installs target the fresh copy; update code paths that reference
python_bin/pyembed_dir and ensure this change is applied where
ensure_python_standalone() is used.
- Around line 218-223: The macOS and Windows branches currently append "-L
{lib_dir}" into RUSTFLAGS which breaks on paths with spaces; change them to use
the same .cargo/config.toml approach used for Linux instead of embedding -L in
RUSTFLAGS. Update the code handling _platform == "darwin" (and the Windows
branch around the same pattern) to write the library search path into the
existing .cargo/config.toml generation logic (reuse the function/logic that
already handles product_name="My App") rather than concatenating to flags, and
remove the "-L {lib_dir}" insertion so lib_dir paths with spaces are preserved.
- Around line 67-73: The Windows .exe lookup re-derives cargo_name from
product_name (template_vars["product_name"]) causing mismatches when
config.title differs from config.name; update the Windows executable name
computation to derive and normalize from config.name (the same source used to
build the Rust binary) and reuse the same normalization logic used for
cargo_name (lowercase, replace non alnum/_/- with "-", collapse "-+" and strip
"-" ) instead of using product_name so the exe lookup matches the built binary.
In `@src/trellis/packaging/templates/capabilities_default.json.j2`:
- Line 3: The description value is interpolating raw product_name into JSON,
which breaks when product_name contains quotes/backslashes; update the template
to JSON-escape the full string by using Jinja2's tojson filter. Replace the
current line that uses "Default capabilities for {{ product_name }}" with a
version that constructs the full string and pipes it to tojson (e.g., ("Default
capabilities for " + product_name)|tojson) so the output is a safe JSON string;
change the symbol at the current template line (the description entry) in
src/trellis/packaging/templates/capabilities_default.json.j2 accordingly.
In `@src/trellis/packaging/templates/launcher/main_installer.rs.j2`:
- Around line 95-102: The current zip-extraction loop builds paths from
entry.name() and checks for ".." and leading separators but misses Windows
absolute paths; replace the manual string validation in the for loop that calls
archive.by_index(i) and uses entry.name() and dest.join(&name) with using
entry.enclosed_name() (or return an Err if enclosed_name() is None) and join the
returned Path (e.g., dest.join(enclosed)) so absolute/unsafe paths are rejected
safely and Zip Slip is prevented; ensure any error messages reference the entry
index/name for debugging.
In `@src/trellis/packaging/templates/launcher/main.rs.j2`:
- Around line 51-62: The code currently only validates the footer.hash format
but never verifies the embedded payload bytes against it; compute the SHA-256
(or the same hash algorithm used to produce footer.hash) of the extracted
payload bytes (the buffer read in the payload-read section referenced around
lines 73-78), hex-encode that digest and compare it to the Footer.hash string,
and if they differ return an error (e.g., Err("payload hash mismatch").into())
before proceeding to extract/cache; reference variables/functions Footer { hash,
archive_size }, footer, archive_size, FOOTER_SIZE and the payload byte buffer so
the check is placed immediately after reading the payload and before any
extraction or caching.
In `@src/trellis/packaging/templates/tauri.conf.json.j2`:
- Around line 3-5: The template currently embeds raw template variables (e.g.,
product_name, version, identifier and the icon_paths/list entries) directly into
JSON which breaks for strings with quotes or backslashes; update the Jinja2
template to serialize these values using the tojson filter (e.g., replace "{{
product_name }}" with {{ product_name|tojson }}) and ensure any path lists
(icon_paths) are stringified before serializing so backslashes are escaped
properly; apply the same tojson change to the other templated JSON entries
referenced (lines around 13, 22-25, 34-35) so all dynamic values are emitted as
valid JSON strings/arrays.
In `@src/trellis/platforms/desktop/platform.py`:
- Around line 96-116: In _get_config_override, the Windows junction at
config/dist is created unconditionally if missing and can become stale when
dist_dir changes; update the logic to detect when dist_link exists but points to
a different target and recreate it: resolve the current dist_dir
(dist_dir.resolve()) and compare it to the junction target (use the same
resolution method used when creating the junction), and if they differ, remove
the existing dist_link and call _winapi.CreateJunction to recreate it; keep the
existing branch that sets dist_path="./dist" and ensure any removal/recreation
is only performed on Windows before returning the _build_tauri_config_override
result.
In `@src/trellis/platforms/desktop/standalone_platform.py`:
- Around line 100-115: The done-callback currently closes over mutable
self._handler which can be replaced by subsequent trellis_connect calls; capture
the specific PyTauriMessageHandler instance when creating the callback and use
that captured reference to call get_global_registry().unregister(...) and to
clear/reset the platform state (set self._handler to None and call
_clear_dialog_runtime()) so finished tasks never unregister a different handler
or leave a stale handler behind; update the _on_handler_done callback (used with
self._handler_task.add_done_callback) and any analogous handlers at lines
~121-123 to operate on the captured handler variable rather than self._handler
and ensure self._handler is cleared on exit.
In `@src/trellis/toolchain/__init__.py`:
- Around line 12-14: Remove the duplicated constants PYTHON_STANDALONE_VERSION
and PYTHON_STANDALONE_RELEASE from src/trellis/toolchain/__init__.py and instead
import them from python_standalone.py (same pattern used for
MINIMUM_RUST_VERSION); locate the definitions in python_standalone.py (where
they should remain the single source of truth) and add import lines in
__init__.py to re-export PYTHON_STANDALONE_VERSION and PYTHON_STANDALONE_RELEASE
so other modules use the imported values rather than duplicate constants.
In `@src/trellis/toolchain/python_standalone.py`:
- Around line 35-47: The cached install directory is currently target-agnostic
causing get_rust_target() to pick up the wrong interpreter; update the
install_dir construction in python_standalone.py to include the target triple
(the variable target returned by get_rust_target()) so the path is
target-specific (e.g., incorporate target into the name used for install_dir),
and ensure PythonStandalone is returned using that target-keyed install_dir
(affecting python_bin and base_dir resolution) as well as the code path that
downloads/extracts the archive around the existing download/creation logic so
the on-disk cache is keyed by target triple.
In `@src/trellis/toolchain/rustup.py`:
- Around line 96-101: Don't infer CARGO_HOME/RUSTUP_HOME from binary locations
(the rustc/cargo Path objects); instead read the environment variables
CARGO_HOME and RUSTUP_HOME and only fall back to per-user defaults. Update the
code that sets cargo_home and rustup_home (currently using cargo.parent and
cargo_home.parent / "rustup") to first check os.environ.get("CARGO_HOME") and
os.environ.get("RUSTUP_HOME"), and if those are unset use Path.home() / ".cargo"
and Path.home() / ".rustup" respectively so RustToolchain.env() writes to a
user-writable location.
- Around line 157-184: The code downloads rustup-init to init_path from url and
immediately executes it via subprocess.run; to mitigate RCE risk, fetch and
validate the official checksum or signature for that release before running:
after downloading (before init_path.chmod and subprocess.run) compute a strong
hash (or verify the cryptographic signature) of the file at init_path and
compare it against the expected value obtained from a trusted source (e.g., Rust
release checksum endpoint or GPG signature) and raise an error/abort if
validation fails; update the sequence around httpx.stream, init_path, and
subprocess.run to perform this verification step (using MINIMUM_RUST_VERSION to
determine expected release if needed) and only proceed to chmod and
subprocess.run when the integrity check passes.
- Around line 212-233: The env-var fast path checks only rustc but not cargo, so
a missing or corrupted cargo binary can slip through; in the branch where
env_cargo and env_rustup are set (using cargo_home, rustup_home) validate that
the cargo binary (cargo variable) exists and is executable before returning a
RustToolchain: if cargo is missing or not usable, skip this branch (or
raise/return None) the same way you currently do when rustc is absent/too-old.
Update the logic around rustc/cargo version checks in the function that
constructs RustToolchain to ensure both rustc.exists() and cargo.exists() (and
optionally a quick cargo --version run) are required before returning the
RustToolchain instance.
In `@src/trellis/toolchain/tauri_cli.py`:
- Around line 34-42: The current lookup only checks BIN_DIR /
f"tauri-cli-{TAURI_CLI_VERSION}" and can return a binary built for a different
Rust target; update the resolver in get_rust_target / where binary_path is
computed so the cache is target-scoped (e.g., include the target triple from
get_rust_target in the extract_dir name) and also record and reuse the
cargo-installed fallback from _cargo_install_tauri_cli (store the fallback path
or a small marker file under the target-scoped cache after successful cargo
install so subsequent calls return that path instead of re-installing). Apply
this same target-scoped cache + fallback-recording logic to the other lookup
blocks referenced (lines handling binary lookup around the 84-91 and 94-108
ranges) so all code paths consistently check target-specific
BIN_DIR/tauri-cli-{TAURI_CLI_VERSION}-{target} and then the recorded cargo_home
fallback before hitting GitHub again.
In `@tests/py/unit/test_apploader_standalone.py`:
- Line 35: The test uses isinstance(apploader.platform,
DesktopStandalonePlatform) which will also pass for DesktopPlatform (a subclass)
and won't detect regressions; change the assertion to check exact type equality
instead—e.g., assert type(apploader.platform) is DesktopStandalonePlatform (or
apploader.platform.__class__ is DesktopStandalonePlatform) so the test fails if
AppLoader.platform returns a subclass like DesktopPlatform.
---
Duplicate comments:
In `@pixi.toml`:
- Line 6: The Pixi workspace platforms list no longer includes osx-64 while the
toolchain mapping in src/trellis/toolchain/platform.py still maps ("darwin",
"x86_64") -> "x86_64-apple-darwin", causing Intel macOS resolution to fail; fix
by either adding "osx-64" back into the platforms array in pixi.toml to match
the mapping or remove/update the ("darwin","x86_64") mapping and any related
docs to stop advertising Intel macOS support (ensure changes touch the tuple
mapping in platform.py and the platforms array in pixi.toml so both stay
consistent).
---
Nitpick comments:
In `@src/trellis/app/apploader.py`:
- Around line 203-205: Normalize config.icon to an absolute Path: when
config.icon is not None and not config.icon.is_absolute(), resolve it against
self.path and call .resolve() (or Path(...).absolute equivalent) so config.icon
becomes an absolute path; update the block using config.icon and self.path in
AppLoader (the code that currently sets config.icon = self.path / config.icon)
to instead set config.icon = (self.path / config.icon).resolve() so downstream
consumers like IconAssetStep receive an absolute path.
In `@src/trellis/bundler/utils.py`:
- Line 89: The symlink registry currently keys entries using the literal archive
path expression dest / member.name which can miss nested symlinks when
_resolve_through_symlinks walks already-resolved paths; change the registration
to use a normalized/resolved form of the archive path (the same normalization
used by _resolve_through_symlinks) when inserting into symlinks and when
performing lookups so keys and lookups match for nested symlink cases (ensure
you update code around the symlinks dict population where dest / member.name is
assigned and any related lookup logic in _resolve_through_symlinks to use the
same Path normalization routine).
In `@src/trellis/packaging/portable.py`:
- Around line 190-237: The build_installer_exe implementation duplicates
build_portable_exe; extract the shared logic into a single helper (e.g.
_build_exe_bundle or similar) that accepts parameters for mode ("installer" vs
"portable"), build_dir name, and installer flag for the output filename; have
build_installer_exe and build_portable_exe call that helper and reuse existing
helpers (_generate_launcher_scaffold, _build_launcher, _assemble_portable_exe,
_output_filename) so only the mode, build_dir/archivename, and installer boolean
differ.
In `@src/trellis/packaging/tauri.py`:
- Line 447: The local mid-function import of _output_filename from
trellis.packaging.portable should be moved to the top-level imports or justified
with a comment about an import cycle; locate the local statement "from
trellis.packaging.portable import _output_filename" and either (a) hoist it to
the module imports alongside other imports (preserve any noqa/flake8 directive
if needed), or (b) if a circular import forces the local import, add a brief
explanatory comment immediately above it stating why the import must be deferred
(mentioning the import cycle with trellis.packaging.portable) and keep the local
import as-is to make the reason explicit for future readers.
In `@src/trellis/platforms/common/client/src/TrellisClient.ts`:
- Around line 12-23: The check uses globalThis.crypto?.randomUUID but the call
uses crypto.randomUUID(), so update generateUUID to consistently reference
globalThis.crypto (e.g., call globalThis.crypto.randomUUID()) to avoid mixed
references; keep the existing fallback logic (Uint8Array/getRandomValues and
version/variant bit tweaks) intact and only change the direct randomUUID
invocation to globalThis.crypto.randomUUID.
In `@tests/py/unit/test_packaging_tauri.py`:
- Around line 685-710: The test test_generates_config_on_linux currently depends
on host pkg-config output; patch the dependency by mocking
trellis.packaging.tauri._get_linux_system_lib_flags (or subprocess.run used
inside it) to return deterministic linker flags during the test so it no longer
queries host tooling. In the test’s with patch(...) block, add a patch for
_get_linux_system_lib_flags to return a known list (e.g., ["-L/some/path",
"-lfoo"]) or patch subprocess.run to return a CompletedProcess with expected
stdout, then call _generate_cargo_config as before and assert against the
deterministic content; refer to the functions _generate_cargo_config and
_get_linux_system_lib_flags (or the subprocess.run call inside it) to locate
where to apply the patch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fdae6389-c745-429a-b499-2ea046e29891
⛔ Files ignored due to path filters (2)
examples/widget_showcase/assets/icon.pngis excluded by!**/*.pngpixi.lockis excluded by!**/*.lock
📒 Files selected for processing (67)
.github/workflows/ci.yml.gitignoreexamples/widget_showcase/trellis_config.pyexamples/widget_showcase/widget_showcase/example.pypixi.tomlpyproject.tomlsrc/trellis/app/apploader.pysrc/trellis/app/config.pysrc/trellis/app/standalone_entry.pysrc/trellis/bundler/manifest.pysrc/trellis/bundler/utils.pysrc/trellis/bundler/wheels.pysrc/trellis/cli/init.pysrc/trellis/cli/package.pysrc/trellis/packaging/__init__.pysrc/trellis/packaging/hooks/hook-pytauri.pysrc/trellis/packaging/hooks/hook-rich.pysrc/trellis/packaging/portable.pysrc/trellis/packaging/pyinstaller.pysrc/trellis/packaging/tauri.pysrc/trellis/packaging/templates/Cargo.toml.j2src/trellis/packaging/templates/build.rs.j2src/trellis/packaging/templates/capabilities_default.json.j2src/trellis/packaging/templates/launcher/Cargo.toml.j2src/trellis/packaging/templates/launcher/build.rs.j2src/trellis/packaging/templates/launcher/main.rs.j2src/trellis/packaging/templates/launcher/main_installer.rs.j2src/trellis/packaging/templates/lib.rs.j2src/trellis/packaging/templates/main.rs.j2src/trellis/packaging/templates/tauri.conf.json.j2src/trellis/platforms/browser/serve_platform.pysrc/trellis/platforms/common/client/src/TrellisClient.tssrc/trellis/platforms/common/ports.pysrc/trellis/platforms/desktop/__init__.pysrc/trellis/platforms/desktop/client/src/index.html.j2src/trellis/platforms/desktop/platform.pysrc/trellis/platforms/desktop/standalone_platform.pysrc/trellis/platforms/server/platform.pysrc/trellis/toolchain/__init__.pysrc/trellis/toolchain/platform.pysrc/trellis/toolchain/python_standalone.pysrc/trellis/toolchain/rustup.pysrc/trellis/toolchain/tauri_cli.pysrc/trellis/widgets/table.pytests/conftest.pytests/py/integration/test_message_handler_integration.pytests/py/integration/test_widget_actions.pytests/py/integration/test_widget_basic.pytests/py/integration/test_widget_inputs.pytests/py/integration/test_widget_navigation.pytests/py/unit/test_apploader_standalone.pytests/py/unit/test_build_steps.pytests/py/unit/test_bun_unit.pytests/py/unit/test_bundler_utils.pytests/py/unit/test_cli_package.pytests/py/unit/test_config.pytests/py/unit/test_config_packaging.pytests/py/unit/test_desktop_standalone_platform.pytests/py/unit/test_mutable_unit.pytests/py/unit/test_packages_unit.pytests/py/unit/test_packaging_portable.pytests/py/unit/test_packaging_pyinstaller.pytests/py/unit/test_packaging_tauri.pytests/py/unit/test_ports.pytests/py/unit/test_toolchain_python_standalone.pytests/py/unit/test_toolchain_rustup.pytests/py/unit/test_toolchain_tauri_cli.py
💤 Files with no reviewable changes (4)
- src/trellis/packaging/hooks/hook-pytauri.py
- src/trellis/packaging/hooks/hook-rich.py
- src/trellis/packaging/pyinstaller.py
- tests/py/unit/test_packaging_pyinstaller.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/trellis/packaging/tauri.py (1)
545-548:⚠️ Potential issue | 🟡 MinorUse consistent source for cargo_name derivation.
At line 68, the scaffold generation derives
cargo_namefromconfig.name. At line 547, the Windows exe lookup derives it fromproduct_name(which isconfig.title or config.name). Whenconfig.titlediffers fromconfig.name, the Rust binary will be named afterconfig.namebut the Windows exe build will search for a binary named afterconfig.title, causing a mismatch.Suggested fix
# 6. On Windows, build self-extracting exe (post-processing after Tauri compile) if sys.platform == "win32": - cargo_name = _make_cargo_name(product_name) + cargo_name = _make_cargo_name(config.name) exe_name = cargo_name + ".exe"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/tauri.py` around lines 545 - 548, The Windows exe lookup uses product_name to build cargo_name (cargo_name = _make_cargo_name(product_name)) which can mismatch earlier scaffold naming that used config.name; change the derivation to use the same source as scaffold generation (use config.name or the same variable used at line ~68) by calling _make_cargo_name with config.name (or the previously computed cargo_name variable) instead of product_name so the Rust binary name and exe lookup remain consistent (update references around cargo_name, _make_cargo_name, product_name, config.title accordingly).
🧹 Nitpick comments (2)
src/trellis/toolchain/tauri_cli.py (1)
84-91: Cargo install fallback doesn't populate target-scoped cache.When the prebuilt download fails,
_cargo_install_tauri_cliinstalls torust.cargo_home/bin, but subsequent calls will still miss the target-scoped cache check at line 41 and re-attempt the prebuilt download. Consider copying the cargo-installed binary to the target-scoped cache for consistency.♻️ Suggested improvement
except Exception: logger.warning( "Prebuilt Tauri CLI download failed for %s, falling back to cargo install", target, exc_info=True, ) archive_path.unlink(missing_ok=True) - return _cargo_install_tauri_cli(rust) + installed_path = _cargo_install_tauri_cli(rust) + # Cache the cargo-installed binary in target-scoped dir for future runs + extract_dir.mkdir(parents=True, exist_ok=True) + shutil.copy2(installed_path, binary_path) + binary_path.chmod(0o755) + return binary_pathThis requires adding
import shutilat the top.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/toolchain/tauri_cli.py` around lines 84 - 91, When the prebuilt download fails and you fall back to _cargo_install_tauri_cli(rust), ensure you copy the cargo-installed binary from rust.cargo_home / "bin" (the installed tauri binary) into the same target-scoped cache location that the prebuilt path check uses so subsequent calls find it; after calling _cargo_install_tauri_cli, locate the binary in rust.cargo_home / "bin" and use shutil.copy2 (add import shutil) to copy it into the target-scoped cache path (preserving permissions and making it executable if needed) before returning.tests/py/unit/test_toolchain_tauri_cli.py (1)
88-141: Add regression tests for malicious ZIP members.These tests only cover valid ZIP payloads, but
ensure_tauri_cli()now contains archive-traversal defenses. Please add cases like../cargo-tauri, an absolute path, and a Windows drive-letter path so regressions in that security branch do not slip through unnoticed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/py/unit/test_toolchain_tauri_cli.py` around lines 88 - 141, Add regression tests to tests/py/unit/test_toolchain_tauri_cli.py that create ZIP archives containing malicious members (e.g., "../cargo-tauri", an absolute path like "/tmp/cargo-tauri", and a Windows drive-letter path like "C:\\cargo-tauri.exe") and verify ensure_tauri_cli(rust) still extracts only the safe member (e.g., "cargo-tauri" or "cargo-tauri.exe") and ignores/rejects the malicious entries; reuse the existing test structure and patches (patching BIN_DIR, TAURI_CLI_VERSION, get_rust_target and httpx.stream returning a MagicMock with iter_bytes) from test_downloads_and_extracts_zip_on_macos and test_downloads_zip_on_windows, build the zip_content with the malicious names plus a valid safe entry, call ensure_tauri_cli and assert the returned Path exists and has the expected safe filename, and that no files are written outside BIN_DIR.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/trellis/packaging/portable.py`:
- Around line 44-75: The archive entries are non-deterministic because
_collect_app_files() relies on filesystem iteration order and _create_archive()
preserves that; fix by producing a stable ordering before writing: sort the
files list by the archive-relative path (the second element of the (Path, str)
tuples) — either return a sorted list from _collect_app_files() or sort the
files inside _create_archive() (e.g., files.sort(key=lambda t: t[1])) so the zip
entries are written in a deterministic order.
- Around line 116-123: The code only writes icon.ico and build.rs when icon_path
exists but never removes them when icon is absent, causing stale icons to
persist across builds; update the branch handling icon_path in portable.py
(symbols: icon_path, launcher_dir, file_map, template "build.rs.j2", output
"build.rs", "icon.ico") so that in the else-case you remove any existing
launcher_dir/"icon.ico" and launcher_dir/"build.rs" (or remove the "build.rs.j2"
entry from file_map) before rendering templates, ensuring old artifacts are
deleted when no icon is provided.
- Around line 32-41: _product_filename currently only replaces spaces and can
include path-unsafe characters; update the _output_filename(product_name: str,
version: str, ext: str, *, installer: bool = False) function to sanitize
product_name before using it: strip any drive letters or absolute path prefixes,
remove or replace path separators ('/', '\\'), collapse sequences of '.' that
could form '..', and replace any characters not in a safe whitelist (letters,
digits, '-', '_') with '-' (also trim leading/trailing '-' and ensure the result
is not empty), then use that sanitized name to build the filename; this prevents
escaping output_dir when build_portable_exe() or build_installer_exe() pass the
name into output paths.
In `@src/trellis/platforms/desktop/standalone_platform.py`:
- Around line 106-117: The done-callback _on_handler_done currently always calls
_clear_dialog_runtime(), which can tear down dialog support if a new handler has
replaced self._handler; change _on_handler_done so it only calls
_clear_dialog_runtime() when the finishing handler is still the active one
(i.e., wrap the _clear_dialog_runtime() call inside the existing if
self._handler is handler check). Keep the unregister via
get_global_registry().unregister(handler) as-is so the finished handler is
removed, but only clear dialog runtime when self._handler === handler to avoid
affecting a newer session.
---
Duplicate comments:
In `@src/trellis/packaging/tauri.py`:
- Around line 545-548: The Windows exe lookup uses product_name to build
cargo_name (cargo_name = _make_cargo_name(product_name)) which can mismatch
earlier scaffold naming that used config.name; change the derivation to use the
same source as scaffold generation (use config.name or the same variable used at
line ~68) by calling _make_cargo_name with config.name (or the previously
computed cargo_name variable) instead of product_name so the Rust binary name
and exe lookup remain consistent (update references around cargo_name,
_make_cargo_name, product_name, config.title accordingly).
---
Nitpick comments:
In `@src/trellis/toolchain/tauri_cli.py`:
- Around line 84-91: When the prebuilt download fails and you fall back to
_cargo_install_tauri_cli(rust), ensure you copy the cargo-installed binary from
rust.cargo_home / "bin" (the installed tauri binary) into the same target-scoped
cache location that the prebuilt path check uses so subsequent calls find it;
after calling _cargo_install_tauri_cli, locate the binary in rust.cargo_home /
"bin" and use shutil.copy2 (add import shutil) to copy it into the target-scoped
cache path (preserving permissions and making it executable if needed) before
returning.
In `@tests/py/unit/test_toolchain_tauri_cli.py`:
- Around line 88-141: Add regression tests to
tests/py/unit/test_toolchain_tauri_cli.py that create ZIP archives containing
malicious members (e.g., "../cargo-tauri", an absolute path like
"/tmp/cargo-tauri", and a Windows drive-letter path like "C:\\cargo-tauri.exe")
and verify ensure_tauri_cli(rust) still extracts only the safe member (e.g.,
"cargo-tauri" or "cargo-tauri.exe") and ignores/rejects the malicious entries;
reuse the existing test structure and patches (patching BIN_DIR,
TAURI_CLI_VERSION, get_rust_target and httpx.stream returning a MagicMock with
iter_bytes) from test_downloads_and_extracts_zip_on_macos and
test_downloads_zip_on_windows, build the zip_content with the malicious names
plus a valid safe entry, call ensure_tauri_cli and assert the returned Path
exists and has the expected safe filename, and that no files are written outside
BIN_DIR.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6dd6ca31-a744-41bd-bb50-9b20ba4c8711
📒 Files selected for processing (17)
src/trellis/packaging/portable.pysrc/trellis/packaging/tauri.pysrc/trellis/packaging/templates/capabilities_default.json.j2src/trellis/packaging/templates/launcher/Cargo.toml.j2src/trellis/packaging/templates/launcher/main.rs.j2src/trellis/packaging/templates/launcher/main_installer.rs.j2src/trellis/packaging/templates/tauri.conf.json.j2src/trellis/platforms/desktop/standalone_platform.pysrc/trellis/toolchain/__init__.pysrc/trellis/toolchain/python_standalone.pysrc/trellis/toolchain/rustup.pysrc/trellis/toolchain/tauri_cli.pytests/py/unit/test_apploader_standalone.pytests/py/unit/test_packaging_portable.pytests/py/unit/test_packaging_tauri.pytests/py/unit/test_toolchain_python_standalone.pytests/py/unit/test_toolchain_tauri_cli.py
🚧 Files skipped from review as they are similar to previous changes (6)
- src/trellis/packaging/templates/launcher/Cargo.toml.j2
- tests/py/unit/test_toolchain_python_standalone.py
- src/trellis/toolchain/init.py
- tests/py/unit/test_apploader_standalone.py
- src/trellis/toolchain/python_standalone.py
- src/trellis/packaging/templates/capabilities_default.json.j2
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/trellis/platforms/desktop/standalone_platform.py (1)
110-117: Consider logging task exceptions in the done callback.If
handler.run()raises an unhandled exception, the done callback proceeds but the exception is silently discarded. While asyncio will eventually log uncaught exceptions, explicitly checking can provide more immediate debugging context.♻️ Optional: Add exception logging
def _on_handler_done(task: asyncio.Task[None]) -> None: + if exc := task.exception(): + print(f"[trellis] Handler task failed: {exc}", flush=True) get_global_registry().unregister(handler) if self._handler is handler: self._handler = None _clear_dialog_runtime()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/platforms/desktop/standalone_platform.py` around lines 110 - 117, The done callback _on_handler_done should check the completed _handler_task for exceptions and log them; update _on_handler_done (which takes task: asyncio.Task[None]) to call task.exception() (or inspect task.cancelled()) and, if an exception exists, log the error with traceback/context (use the module logger or processLogger) before calling get_global_registry().unregister(handler) and clearing self._handler/_clear_dialog_runtime() so exceptions from handler.run() are not silently discarded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/trellis/packaging/portable.py`:
- Around line 32-45: The _output_filename function currently sanitizes
product_name but uses version verbatim; modify _output_filename to sanitize the
version string before interpolation by removing path separators and control
characters (e.g. replace '/' '\' and ASCII control chars with '-'), collapse
repeated separators/whitespace, strip leading/trailing dots, spaces and hyphens,
and explicitly forbid relative-segment values like ".." (raise ValueError if the
sanitized version is empty or equals ".."); update the function so the final
return uses the sanitized version variable while keeping the existing installer
suffix logic.
- Around line 26-29: The normalization in _make_cargo_name can produce an empty
string for non-ASCII or punctuation-only inputs; after computing result =
re.sub(...).strip("-"), detect if it's empty and return a safe fallback name
(e.g., "package" or "package-<unique-suffix>") instead of returning "" so the
generated Cargo.toml remains valid; update _make_cargo_name to check the final
name and replace empties with the chosen fallback.
- Around line 127-130: The loop that renders templates (using env.get_template
and template.render over file_map) writes files with Path.write_text() without
an encoding, which can use a non-UTF-8 default on Windows; change the write to
explicitly write UTF-8 (e.g., (launcher_dir / output_name).write_text(content,
encoding="utf-8")) so generated Cargo.toml and main.rs handle non-ASCII
product_name/version correctly.
- Around line 135-143: The build currently ignores any external CARGO_TARGET_DIR
and always returns launcher_dir / "target" / "release" / binary_name; fix this
by pinning Cargo's target dir in the environment before invoking cargo: update
the env dict created from os.environ and rust.env() (the variable named env used
with subprocess.run and cargo_bin) to set CARGO_TARGET_DIR to launcher_dir /
"target" (or a str of that path), then run subprocess.run as before and
construct the returned path using that same pinned target dir and binary_name
(referencing cargo_name, launcher_dir, and binary_name) so the returned path
matches where cargo actually writes the binary.
---
Nitpick comments:
In `@src/trellis/platforms/desktop/standalone_platform.py`:
- Around line 110-117: The done callback _on_handler_done should check the
completed _handler_task for exceptions and log them; update _on_handler_done
(which takes task: asyncio.Task[None]) to call task.exception() (or inspect
task.cancelled()) and, if an exception exists, log the error with
traceback/context (use the module logger or processLogger) before calling
get_global_registry().unregister(handler) and clearing
self._handler/_clear_dialog_runtime() so exceptions from handler.run() are not
silently discarded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c17bb3ce-d463-43a9-a333-11ebfdb02bab
📒 Files selected for processing (2)
src/trellis/packaging/portable.pysrc/trellis/platforms/desktop/standalone_platform.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/trellis/packaging/portable.py`:
- Around line 56-80: The root-level executable collection in _collect_app_files
is currently indiscriminately including any .exe in release_dir which causes
rebundling of previously generated packages; update _collect_app_files to only
include the main app exe by comparing filenames against the expected exe_name
(or alternatively validate and reject when output_dir/dest is a descendant of
release_dir) and apply the same filter/validation to the other collectors
referenced (the functions handling the other ranges around lines 173-181,
206-209, 212-220, 255-258) so they ignore or error on bundle artifacts placed
under release_dir rather than rebundling them.
- Around line 64-69: The loop that filters Windows artifacts uses Path.suffix
directly, which preserves casing and can miss matches; update the iteration in
portable.py to compute a single lowercase suffix variable (e.g., suffix =
path.suffix.lower()) before the two membership checks, then use that suffix when
testing against _WINDOWS_SKIP_EXTENSIONS and _WINDOWS_APP_EXTENSIONS so
casing-insensitive matches correctly include/exclude files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8de513d4-4b31-4c8a-b5dd-23d9d51f8553
📒 Files selected for processing (1)
src/trellis/packaging/portable.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/trellis/packaging/portable.py (1)
83-88:⚠️ Potential issue | 🟠 MajorStill reject output destinations inside the release tree.
Filtering root-level
.exefiles fixed one rebundling path, butpyembed/is still archived recursively. Ifoutput_dirpoints anywhere undertarget/release/pyembed, the next run will bundle the previous portable/installer output intoapp.zip. Please fail fast whenoutput_diris insiderelease_dir.Also applies to: 199-219, 240-267
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/portable.py` around lines 83 - 88, The code currently archives release_dir/pyembed recursively (pyembed_dir, release_dir, files), which allows output_dir located under release_dir to be re-bundled; change the logic to detect and reject any output_dir that is a subpath of release_dir early (fail fast) before collecting files: add a check that resolves output_dir and release_dir to absolute/real paths and if output_dir.is_relative_to(release_dir) (or equivalent Path comparison) raise an error; apply the same guard to the other file-collection blocks that recurse the release tree (the blocks handling pyembed and the similar routines referenced around the other collection sections) so no recursive archive includes previous outputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/trellis/packaging/portable.py`:
- Around line 69-90: The collection loop currently still accumulates DLLs and
pyembed files even when a required exe (exe_name) was never found; after
building the files list (including the pyembed_dir recursion) check whether
exe_name is set and whether an entry matching exe_name was added to files (or
track a boolean like exe_found during the initial iteration), and if not raise a
clear exception (e.g., RuntimeError or ValueError) indicating the expected
executable was not collected so the build fails fast; reference the variables
exe_name, files, release_dir and pyembed_dir (or an added exe_found flag) and
update callers to expect the raised error.
- Around line 95-97: The ZIP creation uses zipfile.ZipFile.write which preserves
file mtimes, causing non-deterministic archives; replace write() with creating a
zipfile.ZipInfo for each arc_name, set its date_time to a fixed timestamp (e.g.
(1980,1,1,0,0,0)), copy needed external attributes/mode if required, and use
zf.writestr(info, file_bytes) (reading abs_path contents) so the resulting
archive (created in the block using zipfile.ZipFile, archive_path, and iterating
files) is deterministic and yields stable bundle hashes.
---
Duplicate comments:
In `@src/trellis/packaging/portable.py`:
- Around line 83-88: The code currently archives release_dir/pyembed recursively
(pyembed_dir, release_dir, files), which allows output_dir located under
release_dir to be re-bundled; change the logic to detect and reject any
output_dir that is a subpath of release_dir early (fail fast) before collecting
files: add a check that resolves output_dir and release_dir to absolute/real
paths and if output_dir.is_relative_to(release_dir) (or equivalent Path
comparison) raise an error; apply the same guard to the other file-collection
blocks that recurse the release tree (the blocks handling pyembed and the
similar routines referenced around the other collection sections) so no
recursive archive includes previous outputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9f6c07ac-1094-4e78-a405-3847d7cc7418
📒 Files selected for processing (1)
src/trellis/packaging/portable.py
…hon_standalone New src/trellis/toolchain/ package with platform detection and three ensure_*() functions that download and cache Rust, Tauri CLI, and portable Python binaries. Follows existing ensure_bun() patterns.
…onfig fields Extract DesktopStandalonePlatform as the runtime base class for packaged apps. DesktopPlatform inherits from it and adds dev features (build config, hot reload, banner). Add identifier, version, update_url, and update_pubkey config fields. Remove collect_bundle_extras. Update apploader to select standalone platform when sys._pytauri_standalone is set.
Jinja2 templates for Cargo.toml, tauri.conf.json, Rust sources, and capabilities. build_desktop_app_bundle() orchestrates the full pipeline: ensure toolchains, generate scaffold, install app into portable Python, run tauri build.
Wire build_desktop_app_bundle from trellis.packaging.tauri into the package command. Delete pyinstaller.py, PyInstaller hooks, and related tests. Remove pyinstaller from desktop optional dependencies.
- Patch libpython install_name on macOS to use @rpath so dyld finds it - Set RUSTFLAGS with -L and -rpath for linking against embedded Python - Set PYO3_PYTHON to point at the pyembed Python binary - Add standalone_entry.py bootstrap that discovers App and starts runtime - Install pytauri-wheel into embedded Python for pytauri Python modules - Enable pytauri plugin features (dialog, window-state, opener) in Cargo - Build with --bundles app to skip DMG (avoids bundle_dmg.sh issue) - Allow internal symlinks in safe_extract for python-build-standalone - Update tests to match new API signatures and path layout
- Add bundles parameter to run_tauri_build and build_desktop_app_bundle - Copy .app/.dmg/.exe/.deb from Tauri bundle dir to output_dir (default: app_root/dist) - Add --dmg CLI flag to also create DMG disk images - Copy bundler-generated icons (icns/ico/png) into Tauri scaffold - Add bundle.icon array to tauri.conf.json so Tauri embeds icons into the .app - Generate 512x512 icon.png from config.icon source when available
iconutil is macOS-only, so skip .icns generation on other platforms.
crypto.randomUUID() is only available in secure contexts (HTTPS or localhost). Fall back to crypto.getRandomValues() for plain HTTP.
- Fix AppImage builds by adding DEPLOY_GTK_VERSION=3, librsvg2-dev to system deps check, and LD_LIBRARY_PATH for linuxdeploy - Fix runtime pyembed path on Linux (binary at usr/bin/, resources at usr/lib/<product_name>/pyembed/) - Use .cargo/config.toml for linker flags to handle spaces in product name - Add --bundles CLI option for choosing bundle formats (deb, appimage, rpm) - Default Linux bundle to deb; bump minimum Rust to 1.93.1 - Add system dep check for librsvg2-dev (required by linuxdeploy GTK plugin) - Dynamic icon list in tauri.conf.json (only list icons that exist)
- Use junctions instead of symlinks (no admin required on Windows) - Fix Python standalone layout detection (python/python.exe vs bin/python3) - Fix lib path (libs/ vs lib/) and site-packages path (Lib/ vs lib/) - Stage python313.dll next to exe (load-time dependency) - Bundle Python DLLs as Tauri resources on Windows - Hide console window with windows_subsystem = "windows" - Fix frontendDist path for dev mode (relative path via junction) - Replace non-ASCII chars in startup banners (cp1252 compat) - Add visible JS error handler in desktop index.html Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace NSIS installer as the default Windows bundle type with a portable self-extracting exe. A small Rust launcher stub reads a zip archive appended to itself, extracts to %LOCALAPPDATA%/<AppName>/<content_hash>/ on first run, and launches the real exe directly on subsequent runs. Old cached versions are cleaned up after 90 days. The launcher embeds the app icon and shows a Win32 progress bar during extraction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
New `--bundles=installer` option produces a self-extracting setup exe that installs to %LOCALAPPDATA%/<AppName>/, creates a Start Menu shortcut via PowerShell, and registers an uninstaller in Add/Remove Programs via the Win32 registry API. Uses the app version to detect and replace previous installs. Uninstall.bat handles cleanup including deferred self-deletion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…anifest Switch installer from version-based to hash-based install detection so rebuilds trigger re-extraction without bumping version. Show a success message box after install completes. Enable comctl32 v6 visual styles and DPI awareness via embedded manifest for modern-looking progress bar. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace --bundles and --dmg with a single --installer flag. Default builds portable format per platform (.app, AppImage, portable .exe). With --installer, builds installer format (.dmg, .deb, installer .exe). Consistent output naming: Widget-Showcase-0.1.0.exe across all platforms. Windows installer gets -installer suffix to disambiguate from portable. - Refactor run_tauri_build to take installer: bool instead of bundles list - Extract _tauri_bundles() for platform/mode -> bundle type mapping - Add _output_filename() helper for consistent naming - Add version parameter to build_portable_exe - Remove dead --dmg flag and bundles list plumbing - --bundles preserved as hidden flag that errors with migration message Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…test portability Use POSIX paths in JSON serialization (config, manifest) and Jinja2 template loading. Fix port tests using SO_REUSEADDR on Windows. Skip platform-specific tests (symlinks, Unix permissions, pkg-config) on Windows. Fix unnecessary lambda lint errors across test files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
SO_REUSEADDR has different semantics on Windows vs Unix: on Windows it allows binding to ports already in active use, making availability checks ineffective. Use SO_EXCLUSIVEADDRUSE on Windows instead, which correctly rejects active ports while still allowing TIME_WAIT reuse. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- _collect_app_files raises RuntimeError when exe_name is set but the expected executable is not found in release_dir - _create_archive uses ZipInfo with fixed timestamps so archive hashes depend only on file contents, not filesystem mtimes
emmapowers
force-pushed
the
tauri-packaging
branch
from
March 14, 2026 14:36
ccd25df to
f22e125
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/trellis/bundler/utils.py (1)
83-89:⚠️ Potential issue | 🟠 MajorResolve symlink targets through earlier archive symlinks too.
Line 86 still resolves
member.linknamelexically. An archive likepackage/subdir -> ..followed bypackage/link -> subdir/../..passes this preflight even though the second symlink escapes once the first one is materialized, sosafe_extract()can start extracting before the bad member is rejected. Reuse_resolve_through_symlinks()for symlink targets as well, and add a regression test for that chain.🛡️ Suggested fix
- target_path = (link_dir / member.linkname).resolve() + target_path = _resolve_through_symlinks(link_dir / member.linkname, dest, symlinks)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/bundler/utils.py` around lines 83 - 89, The symlink target check currently resolves member.linkname lexically and misses cases where earlier archive symlinks affect the final target; update the logic in the symlink handling block (where member.issym() and member_path are used) to pass the candidate target through _resolve_through_symlinks() (reusing the same symlinks mapping and dest context) instead of a plain Path.resolve(), then validate the returned resolved path with is_relative_to(dest) and store the original link (or appropriate normalized form) in the symlinks dict; ensure this change integrates with safe_extract()’s symlink resolution flow and add the regression test for chained symlinks that escape when earlier archive symlinks are materialized.src/trellis/packaging/tauri.py (1)
544-548:⚠️ Potential issue | 🟠 MajorPotential mismatch between Rust binary name and Windows exe lookup.
The scaffold generation at line 68 derives
cargo_namefromconfig.name, which becomes the Cargo package name and thus the compiled binary name. However, line 546 derivescargo_namefromproduct_name(which isconfig.title or config.name).When
config.name="myapp"andconfig.title="My App":
- Tauri builds
myapp.exe(from line 68)- Windows post-processing looks for
my-app.exe(from line 546)This causes the portable/installer exe lookup to fail.
🐛 Suggested fix to use config.name consistently
# 6. On Windows, build self-extracting exe (post-processing after Tauri compile) if sys.platform == "win32": - cargo_name = _make_cargo_name(product_name) + cargo_name = _make_cargo_name(config.name) exe_name = cargo_name + ".exe"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/tauri.py` around lines 544 - 548, The Windows post-processing uses _make_cargo_name(product_name) which can mismatch the actual Rust binary name created earlier from config.name; update the lookup to derive the cargo binary name from the same source used during scaffold/compile (use _make_cargo_name(config.name or the variable that held package name earlier) instead of product_name) so _make_cargo_name is called with the canonical package name used for Cargo (the original config.name) ensuring exe_name matches the compiled binary; adjust any local variable names accordingly where cargo_name and exe_name are computed.
🧹 Nitpick comments (2)
tests/py/unit/test_project_metadata.py (1)
18-20: Keep this assertion scoped to the desktop change.Line 20 will fail as soon as the project adds any unrelated extra, even if the desktop packaging rules from this PR are still correct. Checking that the
desktopextra is absent preserves the intent here without freezing the entire optional-dependency surface.♻️ Suggested tweak
- assert "optional-dependencies" not in project + optional_dependencies = project.get("optional-dependencies", {}) + assert "desktop" not in optional_dependencies🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/py/unit/test_project_metadata.py` around lines 18 - 20, The test currently asserts that "optional-dependencies" is not in project which is too broad; instead restrict the check to the desktop extra only. Update the assertion in tests/py/unit/test_project_metadata.py to verify that the "desktop" extra is absent (e.g., assert "desktop" not in project.get("optional-dependencies", {})) while leaving the other dependency assertions (like the dependencies variable checks) intact; target the assertion referring to the project variable so it no longer fails when unrelated extras are later added.src/trellis/toolchain/tauri_cli.py (1)
94-108: Cargo fallback doesn't populate the target-scoped cache.When
_cargo_install_tauri_cli()succeeds, it returns the path fromrust.cargo_home / "bin"rather than the target-scopedextract_dir. On subsequent calls,binary_path.exists()at line 41 will returnFalse, triggering another download attempt (which fails) and re-runningcargo install.Consider copying the cargo-installed binary into the target-scoped cache so the early-return path works on subsequent invocations.
♻️ Suggested fix
def _cargo_install_tauri_cli(rust: RustToolchain) -> Path: """Install Tauri CLI via cargo as a fallback.""" + target = get_rust_target() + is_windows = target.endswith("-msvc") + binary_name = "cargo-tauri.exe" if is_windows else "cargo-tauri" + + # Check if we already have a cached copy from a previous cargo install + extract_dir = BIN_DIR / f"tauri-cli-{TAURI_CLI_VERSION}-{target}" + cached_path = extract_dir / binary_name + if cached_path.exists(): + return cached_path + subprocess.run( [str(rust.cargo_bin), "install", "tauri-cli", "--version", TAURI_CLI_VERSION], check=True, env=rust.env(), ) - binary_name = "cargo-tauri.exe" if str(rust.cargo_bin).endswith(".exe") else "cargo-tauri" installed_path = rust.cargo_home / "bin" / binary_name if not installed_path.exists(): raise RuntimeError(f"Tauri CLI not found after cargo install: {installed_path}") - return installed_path + # Copy to target-scoped cache for future lookups + extract_dir.mkdir(parents=True, exist_ok=True) + cached_path.write_bytes(installed_path.read_bytes()) + cached_path.chmod(0o755) + return cached_path🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/toolchain/tauri_cli.py` around lines 94 - 108, _cargo_install_tauri_cli currently installs into rust.cargo_home/bin and returns that path, but the rest of the code expects the binary in the target-scoped cache (extract_dir) so later checks (binary_path.exists()) fail; modify _cargo_install_tauri_cli to, after successful cargo install and verifying installed_path exists, ensure the target-scoped cache directory (extract_dir) exists, copy the installed binary (respecting binary_name and Windows ".exe" handling) into extract_dir (preserving executable bits, e.g., via shutil.copy2 and os.chmod as needed), and then return the cached target-scoped path (extract_dir / binary_name) instead of rust.cargo_home/bin so subsequent calls find the binary in the expected location.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/trellis/packaging/templates/launcher/Cargo.toml.j2`:
- Around line 10-11: The Cargo.toml template currently unconditionally lists
winres = "0.1" under [build-dependencies], which breaks non-Windows builds;
update the template to gate this dependency for Windows only by moving the entry
under the [target.'cfg(windows)'.build-dependencies] table so the winres crate
(winres = "0.1") is only applied when cfg(windows) is true, keeping the rest of
the file unchanged and ensuring cross-platform builds succeed.
In `@src/trellis/toolchain/python_standalone.py`:
- Around line 46-77: The code treats python_bin as a cache hit but downloads and
extracts directly into the shared BIN_DIR/install_dir, allowing race conditions;
change the download+extraction path to perform work in a per-run temporary
directory under BIN_DIR (e.g., create a temp_install_dir alongside install_dir),
acquire a lock/marker for the target (use a file lock or atomic directory rename
keyed by target), download archive_path into the temp dir, call safe_extract
into the temp_install_dir, verify python_bin exists inside temp_install_dir,
then atomically rename/move the temp_install_dir into the final install_dir and
cleanup the temp files; ensure python_bin existence checks and chmod are done on
the promoted install_dir and release the lock/marker only after successful
promotion so concurrent runs see either old complete install or the new
fully-populated install.
---
Duplicate comments:
In `@src/trellis/bundler/utils.py`:
- Around line 83-89: The symlink target check currently resolves member.linkname
lexically and misses cases where earlier archive symlinks affect the final
target; update the logic in the symlink handling block (where member.issym() and
member_path are used) to pass the candidate target through
_resolve_through_symlinks() (reusing the same symlinks mapping and dest context)
instead of a plain Path.resolve(), then validate the returned resolved path with
is_relative_to(dest) and store the original link (or appropriate normalized
form) in the symlinks dict; ensure this change integrates with safe_extract()’s
symlink resolution flow and add the regression test for chained symlinks that
escape when earlier archive symlinks are materialized.
In `@src/trellis/packaging/tauri.py`:
- Around line 544-548: The Windows post-processing uses
_make_cargo_name(product_name) which can mismatch the actual Rust binary name
created earlier from config.name; update the lookup to derive the cargo binary
name from the same source used during scaffold/compile (use
_make_cargo_name(config.name or the variable that held package name earlier)
instead of product_name) so _make_cargo_name is called with the canonical
package name used for Cargo (the original config.name) ensuring exe_name matches
the compiled binary; adjust any local variable names accordingly where
cargo_name and exe_name are computed.
---
Nitpick comments:
In `@src/trellis/toolchain/tauri_cli.py`:
- Around line 94-108: _cargo_install_tauri_cli currently installs into
rust.cargo_home/bin and returns that path, but the rest of the code expects the
binary in the target-scoped cache (extract_dir) so later checks
(binary_path.exists()) fail; modify _cargo_install_tauri_cli to, after
successful cargo install and verifying installed_path exists, ensure the
target-scoped cache directory (extract_dir) exists, copy the installed binary
(respecting binary_name and Windows ".exe" handling) into extract_dir
(preserving executable bits, e.g., via shutil.copy2 and os.chmod as needed), and
then return the cached target-scoped path (extract_dir / binary_name) instead of
rust.cargo_home/bin so subsequent calls find the binary in the expected
location.
In `@tests/py/unit/test_project_metadata.py`:
- Around line 18-20: The test currently asserts that "optional-dependencies" is
not in project which is too broad; instead restrict the check to the desktop
extra only. Update the assertion in tests/py/unit/test_project_metadata.py to
verify that the "desktop" extra is absent (e.g., assert "desktop" not in
project.get("optional-dependencies", {})) while leaving the other dependency
assertions (like the dependencies variable checks) intact; target the assertion
referring to the project variable so it no longer fails when unrelated extras
are later added.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e908f654-c11d-4602-a831-864d6cf5e882
⛔ Files ignored due to path filters (2)
examples/widget_showcase/assets/icon.pngis excluded by!**/*.pnguv.lockis excluded by!**/*.lock
📒 Files selected for processing (66)
.gitignoreREADME.mddocs/docs/quickstart.mdexamples/widget_showcase/trellis_config.pypyproject.tomlsrc/trellis/app/apploader.pysrc/trellis/app/config.pysrc/trellis/app/standalone_entry.pysrc/trellis/bundler/manifest.pysrc/trellis/bundler/utils.pysrc/trellis/cli/init.pysrc/trellis/cli/package.pysrc/trellis/packaging/__init__.pysrc/trellis/packaging/hooks/hook-pytauri.pysrc/trellis/packaging/hooks/hook-rich.pysrc/trellis/packaging/portable.pysrc/trellis/packaging/pyinstaller.pysrc/trellis/packaging/tauri.pysrc/trellis/packaging/templates/Cargo.toml.j2src/trellis/packaging/templates/build.rs.j2src/trellis/packaging/templates/capabilities_default.json.j2src/trellis/packaging/templates/launcher/Cargo.toml.j2src/trellis/packaging/templates/launcher/build.rs.j2src/trellis/packaging/templates/launcher/main.rs.j2src/trellis/packaging/templates/launcher/main_installer.rs.j2src/trellis/packaging/templates/lib.rs.j2src/trellis/packaging/templates/main.rs.j2src/trellis/packaging/templates/tauri.conf.json.j2src/trellis/platforms/browser/serve_platform.pysrc/trellis/platforms/common/client/src/TrellisClient.tssrc/trellis/platforms/common/ports.pysrc/trellis/platforms/desktop/__init__.pysrc/trellis/platforms/desktop/client/src/index.html.j2src/trellis/platforms/desktop/platform.pysrc/trellis/platforms/desktop/standalone_platform.pysrc/trellis/platforms/server/platform.pysrc/trellis/toolchain/__init__.pysrc/trellis/toolchain/platform.pysrc/trellis/toolchain/python_standalone.pysrc/trellis/toolchain/rustup.pysrc/trellis/toolchain/tauri_cli.pytests/CLAUDE.mdtests/conftest.pytests/py/integration/test_message_handler_integration.pytests/py/integration/test_widget_actions.pytests/py/integration/test_widget_basic.pytests/py/integration/test_widget_inputs.pytests/py/integration/test_widget_navigation.pytests/py/unit/test_apploader_standalone.pytests/py/unit/test_build_steps.pytests/py/unit/test_bun_unit.pytests/py/unit/test_bundler_utils.pytests/py/unit/test_cli_package.pytests/py/unit/test_config.pytests/py/unit/test_config_packaging.pytests/py/unit/test_desktop_standalone_platform.pytests/py/unit/test_mutable_unit.pytests/py/unit/test_packages_unit.pytests/py/unit/test_packaging_portable.pytests/py/unit/test_packaging_pyinstaller.pytests/py/unit/test_packaging_tauri.pytests/py/unit/test_ports.pytests/py/unit/test_project_metadata.pytests/py/unit/test_toolchain_python_standalone.pytests/py/unit/test_toolchain_rustup.pytests/py/unit/test_toolchain_tauri_cli.py
💤 Files with no reviewable changes (5)
- README.md
- src/trellis/packaging/pyinstaller.py
- src/trellis/packaging/hooks/hook-rich.py
- tests/py/unit/test_packaging_pyinstaller.py
- src/trellis/packaging/hooks/hook-pytauri.py
✅ Files skipped from review due to trivial changes (1)
- tests/CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (29)
- .gitignore
- src/trellis/packaging/templates/launcher/build.rs.j2
- src/trellis/packaging/templates/main.rs.j2
- src/trellis/bundler/manifest.py
- tests/py/integration/test_widget_navigation.py
- src/trellis/platforms/browser/serve_platform.py
- src/trellis/cli/init.py
- src/trellis/platforms/common/client/src/TrellisClient.ts
- tests/py/unit/test_packages_unit.py
- src/trellis/toolchain/init.py
- tests/py/unit/test_cli_package.py
- src/trellis/packaging/templates/capabilities_default.json.j2
- src/trellis/packaging/templates/lib.rs.j2
- tests/py/unit/test_mutable_unit.py
- tests/py/unit/test_bun_unit.py
- tests/py/unit/test_apploader_standalone.py
- pyproject.toml
- src/trellis/platforms/server/platform.py
- tests/py/unit/test_config.py
- tests/py/integration/test_widget_actions.py
- tests/py/unit/test_toolchain_python_standalone.py
- tests/py/unit/test_toolchain_rustup.py
- src/trellis/packaging/templates/tauri.conf.json.j2
- src/trellis/packaging/init.py
- tests/py/unit/test_toolchain_tauri_cli.py
- tests/py/unit/test_ports.py
- tests/py/unit/test_packaging_tauri.py
- src/trellis/platforms/desktop/client/src/index.html.j2
- src/trellis/packaging/templates/build.rs.j2
emmapowers
commented
Mar 14, 2026
emmapowers
commented
Mar 14, 2026
… toolchain - Eliminate bespoke app discovery in standalone_entry.py: use AppLoader with TRELLIS_APP_ROOT so packaged apps get full trellis_config.py settings (batch_delay, window_size, etc.) - Copy trellis_config.py into pyembed during packaging - Restore --bundles flag in CLI for specific bundle formats, with mutual exclusion against --installer - Merge build_portable_exe/build_installer_exe into build_windows_exe - Handle maximized window size: set maximized=true in Tauri config instead of baking in default dimensions - Add macOS Xcode CLI tools preflight check with install instructions - Switch Windows launcher templates to windows-sys crate - Factor shared launcher logic (footer, extraction, progress UI, locks) into common.rs.j2 module - Replace Windows junction with os.path.relpath for dev-mode frontendDist - Move trellis.toolchain to trellis.packaging.toolchain
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/trellis/packaging/tauri.py (1)
584-598:⚠️ Potential issue | 🟠 MajorWindows exe name mismatch:
cargo_namederived fromproduct_nameinstead ofconfig.name.At line 68, the Rust binary name is derived from
config.namevia_make_cargo_name(config.name). However, at line 586, the Windows exe lookup derivescargo_namefromproduct_name(which isconfig.title or config.name). Whenconfig.titlediffers fromconfig.name, the exe lookup will search for the wrong binary name.🐛 Suggested fix
# 6. On Windows, build self-extracting exe (post-processing after Tauri compile) if sys.platform == "win32": - cargo_name = _make_cargo_name(product_name) + cargo_name = _make_cargo_name(config.name) exe_name = cargo_name + ".exe"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/tauri.py` around lines 584 - 598, The Windows exe lookup derives cargo_name from product_name but earlier the Rust binary was made from config.name via _make_cargo_name(config.name); update the code that sets cargo_name before calling build_windows_exe to use _make_cargo_name(config.name) (or the same variable used where the Rust binary was created) so exe_name matches the actual built binary; ensure build_windows_exe is passed the corrected exe_name.
🧹 Nitpick comments (8)
src/trellis/packaging/toolchain/pytauri_wheel.py (1)
43-47: Multiple cached wheels: consider cleanup or stricter matching.When multiple wheels exist in the version directory, the code picks the most recently modified one. This could lead to stale wheels accumulating. Consider either:
- Cleaning up older wheels when a new one is downloaded
- Using exact filename matching instead of glob + mtime
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/toolchain/pytauri_wheel.py` around lines 43 - 47, Multiple cached wheels in version_dir are left to accumulate and the code currently just returns the newest wheel; change the >1 branch to pick the newest wheel (keep using max(existing_wheels, key=lambda wheel: wheel.stat().st_mtime)) but then delete the older files to avoid stale accumulation: compute newest_wheel, iterate over existing_wheels excluding newest_wheel and unlink() each older wheel (handle/remove errors as appropriate), then return newest_wheel; reference the existing_wheels list and version_dir in the updated logic (or alternatively implement an exact filename match if a canonical wheel name is available instead of glob).src/trellis/packaging/toolchain/rustup.py (1)
186-192: Consider adding a read timeout for the download stream.The
httpx.Timeout(60.0, connect=10.0)sets connect timeout but uses 60s for read. For large downloads over slow connections, individual chunk reads could stall. This is likely acceptable for rustup-init (~10MB), but worth noting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/toolchain/rustup.py` around lines 186 - 192, The download stream currently builds httpx.Timeout(60.0, connect=10.0) which sets a long default read timeout; update the Timeout creation used in the block where httpx.stream("GET", url, ...) is called (the variable timeout in src/trellis/packaging/toolchain/rustup.py) to explicitly set a read timeout (for example read=30.0 or an appropriate value) in addition to connect, so per-chunk reads will time out instead of stalling — adjust the timeout instantiation passed to httpx.stream accordingly and keep the rest of the streaming/writing logic (open init_path, r.iter_bytes(), init_path.chmod) unchanged.src/trellis/app/standalone_entry.py (1)
33-36: Consider validatingwindow_sizeformat before parsing.If
config.window_sizeis malformed (e.g., missing thexseparator or containing non-numeric values), this will raise an unhandledValueErrororIndexError. Since this is the packaged app entry point, a clearer error message would improve debuggability.💡 Possible defensive parsing
if config.window_size != "maximized": - parts = config.window_size.split("x") - run_kwargs["window_width"] = int(parts[0]) - run_kwargs["window_height"] = int(parts[1]) + try: + width, height = config.window_size.split("x") + run_kwargs["window_width"] = int(width) + run_kwargs["window_height"] = int(height) + except (ValueError, IndexError) as e: + raise RuntimeError( + f"Invalid window_size format '{config.window_size}'. " + "Expected 'WIDTHxHEIGHT' or 'maximized'." + ) from e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/app/standalone_entry.py` around lines 33 - 36, The code that parses config.window_size (in standalone_entry.py) assumes a "<width>x<height>" format and will raise IndexError/ValueError if malformed; update the logic around the if config.window_size != "maximized" block to validate the format before splitting (e.g., check for a single "x" separator and that both sides are digits or use a regex), then convert to ints inside a guarded try/except and raise or log a clear, user-friendly error referencing config.window_size; ensure you still set run_kwargs["window_width"] and run_kwargs["window_height"] only after successful validation/parsing.src/trellis/packaging/toolchain/tauri_cli.py (1)
84-91: Cargo-installed binary not cached to BIN_DIR, causing repeated installs.When the prebuilt download fails,
_cargo_install_tauri_cliinstalls torust.cargo_home/bin, but this path isn't cached inBIN_DIR. Subsequent calls will still miss the cache check at line 41-42 and attempt to re-download/re-install.Consider caching the cargo-installed binary to the target-scoped
extract_dirso future calls find it:♻️ Suggested fix
except Exception: logger.warning( "Prebuilt Tauri CLI download failed for %s, falling back to cargo install", target, exc_info=True, ) archive_path.unlink(missing_ok=True) - return _cargo_install_tauri_cli(rust) + installed_path = _cargo_install_tauri_cli(rust) + # Cache the cargo-installed binary so future calls find it + extract_dir.mkdir(parents=True, exist_ok=True) + shutil.copy2(installed_path, binary_path) + binary_path.chmod(0o755) + return binary_pathYou'll need to add
import shutilat the top.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/toolchain/tauri_cli.py` around lines 84 - 91, When the prebuilt download fails and _cargo_install_tauri_cli(rust) is used, the installed binary ends up in rust.cargo_home/bin and isn't copied into the target-scoped extract_dir/BIN_DIR so later cache checks still miss it; modify the except block to, after calling _cargo_install_tauri_cli, copy the installed tauri binary from rust.cargo_home/bin into the extract_dir (or BIN_DIR) for the target so future runs hit the cache, and add import shutil at the top to perform the copy; reference extract_dir, archive_path, BIN_DIR and _cargo_install_tauri_cli when making the change.src/trellis/packaging/toolchain/python_standalone.py (1)
58-77: Concurrent cache population may cause corruption.The download and extraction happens directly into the final
install_dirwithout a lock or atomic promotion. If two processes runensure_python_standalone()simultaneously, they can interleave extraction and leave a corrupt cache that passes thepython_bin.exists()check.Consider extracting to a temp directory and atomically renaming:
♻️ Suggested atomic extraction pattern
+ import tempfile + BIN_DIR.mkdir(parents=True, exist_ok=True) archive_path = BIN_DIR / f"python-standalone-{PYTHON_STANDALONE_VERSION}-{target}.tar.gz" + temp_dir = Path(tempfile.mkdtemp(dir=BIN_DIR, prefix=f"python-{target}-")) timeout = httpx.Timeout(60.0, connect=10.0) with httpx.stream("GET", url, follow_redirects=True, timeout=timeout) as r: r.raise_for_status() with open(archive_path, "wb") as f: f.writelines(r.iter_bytes()) - install_dir.mkdir(parents=True, exist_ok=True) with tarfile.open(archive_path, "r:gz") as tf: - safe_extract(tf, install_dir) + safe_extract(tf, temp_dir) archive_path.unlink() - if not python_bin.exists(): - raise RuntimeError(f"Python binary missing after extraction: {python_bin}") + temp_python = temp_dir / "python" / ("python.exe" if is_windows else "bin/python3") + if not temp_python.exists(): + shutil.rmtree(temp_dir) + raise RuntimeError(f"Python binary missing after extraction") + + # Atomic rename (on same filesystem) + try: + temp_dir.rename(install_dir) + except OSError: + # install_dir may already exist from concurrent run + shutil.rmtree(temp_dir) + if not python_bin.exists(): + raise RuntimeError(f"Python binary missing: {python_bin}") python_bin.chmod(0o755) return PythonStandalone(python_bin=python_bin, base_dir=base_dir)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/toolchain/python_standalone.py` around lines 58 - 77, The extraction writes directly into install_dir allowing concurrent runs to corrupt the cache; change ensure_python_standalone() to extract into a temporary directory (e.g., tempfile.TemporaryDirectory or install_dir.with_suffix(".tmp-<pid|uuid>")), run safe_extract into that temp_dir, verify python_bin inside temp_dir, then atomically rename/move the temp_dir to install_dir using os.replace or shutil.move (or pathlib.Path.replace) so only one process wins; ensure BIN_DIR and archive_path handling remains the same and clean up the temp_dir and archive_path on failure so partial extracts are removed.tests/py/unit/test_toolchain_tauri_cli.py (1)
15-30: Consider extracting shared helper to conftest.py.
_make_rust_toolchainis duplicated here and intests/py/unit/test_packaging_tauri.py(lines 30-45). Per coding guidelines, common test code appearing in 3+ tests should be extracted to a fixture.♻️ Suggested refactor
Add to
tests/py/unit/conftest.py:`@pytest.fixture` def make_rust_toolchain(tmp_path: Path) -> RustToolchain: """Create a minimal RustToolchain for testing.""" cargo_home = tmp_path / "cargo" rustup_home = tmp_path / "rustup" cargo_bin = cargo_home / "bin" cargo_bin.mkdir(parents=True) rustc = cargo_bin / "rustc" rustc.write_text("fake") cargo = cargo_bin / "cargo" cargo.write_text("fake") return RustToolchain( cargo_home=cargo_home, rustup_home=rustup_home, cargo_bin=cargo, rustc_bin=rustc, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/py/unit/test_toolchain_tauri_cli.py` around lines 15 - 30, Extract the duplicated helper _make_rust_toolchain into a pytest fixture named make_rust_toolchain placed in conftest.py: convert the function into `@pytest.fixture` def make_rust_toolchain(tmp_path: Path) -> RustToolchain with the same body and return value, then update tests that call _make_rust_toolchain to accept the fixture (add make_rust_toolchain param) and use it instead of calling the helper; ensure the fixture returns a RustToolchain and keep the same cargo/rustc file setup as in _make_rust_toolchain.src/trellis/packaging/templates/launcher/common.rs.j2 (2)
214-231: Lock timeout of 30 seconds may be insufficient for slow extractions.The lock acquisition retries 300 times with 100ms sleep (30 seconds total). If another instance is performing a slow extraction (large archive on slow disk), this could timeout prematurely.
Consider making the timeout configurable or increasing it, or providing a more informative error message.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/templates/launcher/common.rs.j2` around lines 214 - 231, The acquire_lock function currently retries 300 times with 100ms sleeps (30s) which can time out on slow extractions; update acquire_lock to accept a configurable timeout or retry/backoff parameters (e.g., max_retries or total_timeout and sleep_duration) instead of hard-coded values, use those values in the loop instead of 300/100ms, and when returning a timeout error include contextual details (the lock_path and configured timeout) in the error string; ensure callers that invoke acquire_lock are updated to pass a sensible default (or read from config/env) if they don’t supply explicit values.
101-103: Path traversal check may miss normalized paths containing...The check
name.contains("..")catches obvious cases like../foo, but after normalization a path likefoo/bar/../../../etcwould still contain... Thezipcrate'senclosed_name()method provides a more robust check.♻️ Suggested improvement
let name = entry.name().to_string(); - if name.contains("..") || name.starts_with('/') || name.starts_with('\\') || name.contains(':') { - return Err(format!("zip entry contains path traversal: {name}")); + let safe_path = match entry.enclosed_name() { + Some(p) => p.to_path_buf(), + None => return Err(format!("zip entry contains path traversal: {name}")), + }; - let out_path = dest.join(&name); + let out_path = dest.join(&safe_path);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trellis/packaging/templates/launcher/common.rs.j2` around lines 101 - 103, Replace the brittle string checks on the zip entry name with the zip crate's robust path normalization: call entry.enclosed_name() (or the equivalent API used to get the Zip entry) instead of checking name.contains("..")/starts_with, and if enclosed_name() returns None return Err(format!("zip entry contains path traversal or absolute path: {name}")) so normalized paths like "foo/bar/../../../etc" are caught; keep referencing the original name variable in the error message for diagnostics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Around line 45-47: Update the pyproject.toml dependency for "pydantic" to
include the same platform marker used for "pytauri" so pydantic is only
installed for non-emscripten platforms; specifically, change the pydantic entry
next to "pytauri>=0.8.0; sys_platform != 'emscripten'" to also include ";
sys_platform != 'emscripten'". This ensures imports in
src/trellis/platforms/desktop/platform.py (where pydantic is used) remain gated
and prevents pulling pydantic into Pyodide/emscripten/browser builds.
In `@src/trellis/packaging/templates/launcher/common.rs.j2`:
- Around line 18-19: The footer currently stores a 32-character hex hash
(FOOTER_SIZE = 48) but computes a full SHA-256 (64 hex chars) and then truncates
it with &computed[..32], weakening integrity; stop truncating and use the full
64-hex-character hash everywhere: update FOOTER_SIZE to account for a 64-char
hex hash (64 + 8 + 8 = 80), change any writer/reader code that writes or parses
the hash to use 64 hex chars, and replace comparisons of &computed[..32] with
the full computed string (e.g., computed or computed.as_str()) so the stored and
computed hashes are compared in full; keep MAGIC unchanged.
In `@src/trellis/packaging/templates/launcher/main.rs.j2`:
- Around line 101-104: The match arm currently truncates the child exit code by
casting s.code().unwrap_or(1) to u8 and constructing an ExitCode, losing values
>255; instead preserve the full i32 exit code by using std::process::exit when a
code is present (e.g. call std::process::exit(code) with the i32 from s.code()),
and only fall back to returning an ExitCode (or exiting with 1) when s.code() is
None; update the Ok(s) arm that currently uses ExitCode::from(...) and adjust
control flow so show_error(&format!(...)) remains in the Err(e) arm.
In `@tests/py/unit/test_toolchain_python_standalone.py`:
- Around line 71-103: The test patches PYTHON_STANDALONE_VERSION but not
PYTHON_STANDALONE_RELEASE while ensure_python_standalone builds the download URL
from both constants; update the test to also patch PYTHON_STANDALONE_RELEASE to
a known value so the constructed URL is deterministic and matches assertions
(i.e., add a patch for
"trellis.packaging.toolchain.python_standalone.PYTHON_STANDALONE_RELEASE"
alongside the existing patches for BIN_DIR, PYTHON_STANDALONE_VERSION,
get_rust_target, and httpx.stream used around the ensure_python_standalone
call).
---
Duplicate comments:
In `@src/trellis/packaging/tauri.py`:
- Around line 584-598: The Windows exe lookup derives cargo_name from
product_name but earlier the Rust binary was made from config.name via
_make_cargo_name(config.name); update the code that sets cargo_name before
calling build_windows_exe to use _make_cargo_name(config.name) (or the same
variable used where the Rust binary was created) so exe_name matches the actual
built binary; ensure build_windows_exe is passed the corrected exe_name.
---
Nitpick comments:
In `@src/trellis/app/standalone_entry.py`:
- Around line 33-36: The code that parses config.window_size (in
standalone_entry.py) assumes a "<width>x<height>" format and will raise
IndexError/ValueError if malformed; update the logic around the if
config.window_size != "maximized" block to validate the format before splitting
(e.g., check for a single "x" separator and that both sides are digits or use a
regex), then convert to ints inside a guarded try/except and raise or log a
clear, user-friendly error referencing config.window_size; ensure you still set
run_kwargs["window_width"] and run_kwargs["window_height"] only after successful
validation/parsing.
In `@src/trellis/packaging/templates/launcher/common.rs.j2`:
- Around line 214-231: The acquire_lock function currently retries 300 times
with 100ms sleeps (30s) which can time out on slow extractions; update
acquire_lock to accept a configurable timeout or retry/backoff parameters (e.g.,
max_retries or total_timeout and sleep_duration) instead of hard-coded values,
use those values in the loop instead of 300/100ms, and when returning a timeout
error include contextual details (the lock_path and configured timeout) in the
error string; ensure callers that invoke acquire_lock are updated to pass a
sensible default (or read from config/env) if they don’t supply explicit values.
- Around line 101-103: Replace the brittle string checks on the zip entry name
with the zip crate's robust path normalization: call entry.enclosed_name() (or
the equivalent API used to get the Zip entry) instead of checking
name.contains("..")/starts_with, and if enclosed_name() returns None return
Err(format!("zip entry contains path traversal or absolute path: {name}")) so
normalized paths like "foo/bar/../../../etc" are caught; keep referencing the
original name variable in the error message for diagnostics.
In `@src/trellis/packaging/toolchain/pytauri_wheel.py`:
- Around line 43-47: Multiple cached wheels in version_dir are left to
accumulate and the code currently just returns the newest wheel; change the >1
branch to pick the newest wheel (keep using max(existing_wheels, key=lambda
wheel: wheel.stat().st_mtime)) but then delete the older files to avoid stale
accumulation: compute newest_wheel, iterate over existing_wheels excluding
newest_wheel and unlink() each older wheel (handle/remove errors as
appropriate), then return newest_wheel; reference the existing_wheels list and
version_dir in the updated logic (or alternatively implement an exact filename
match if a canonical wheel name is available instead of glob).
In `@src/trellis/packaging/toolchain/python_standalone.py`:
- Around line 58-77: The extraction writes directly into install_dir allowing
concurrent runs to corrupt the cache; change ensure_python_standalone() to
extract into a temporary directory (e.g., tempfile.TemporaryDirectory or
install_dir.with_suffix(".tmp-<pid|uuid>")), run safe_extract into that
temp_dir, verify python_bin inside temp_dir, then atomically rename/move the
temp_dir to install_dir using os.replace or shutil.move (or
pathlib.Path.replace) so only one process wins; ensure BIN_DIR and archive_path
handling remains the same and clean up the temp_dir and archive_path on failure
so partial extracts are removed.
In `@src/trellis/packaging/toolchain/rustup.py`:
- Around line 186-192: The download stream currently builds httpx.Timeout(60.0,
connect=10.0) which sets a long default read timeout; update the Timeout
creation used in the block where httpx.stream("GET", url, ...) is called (the
variable timeout in src/trellis/packaging/toolchain/rustup.py) to explicitly set
a read timeout (for example read=30.0 or an appropriate value) in addition to
connect, so per-chunk reads will time out instead of stalling — adjust the
timeout instantiation passed to httpx.stream accordingly and keep the rest of
the streaming/writing logic (open init_path, r.iter_bytes(), init_path.chmod)
unchanged.
In `@src/trellis/packaging/toolchain/tauri_cli.py`:
- Around line 84-91: When the prebuilt download fails and
_cargo_install_tauri_cli(rust) is used, the installed binary ends up in
rust.cargo_home/bin and isn't copied into the target-scoped extract_dir/BIN_DIR
so later cache checks still miss it; modify the except block to, after calling
_cargo_install_tauri_cli, copy the installed tauri binary from
rust.cargo_home/bin into the extract_dir (or BIN_DIR) for the target so future
runs hit the cache, and add import shutil at the top to perform the copy;
reference extract_dir, archive_path, BIN_DIR and _cargo_install_tauri_cli when
making the change.
In `@tests/py/unit/test_toolchain_tauri_cli.py`:
- Around line 15-30: Extract the duplicated helper _make_rust_toolchain into a
pytest fixture named make_rust_toolchain placed in conftest.py: convert the
function into `@pytest.fixture` def make_rust_toolchain(tmp_path: Path) ->
RustToolchain with the same body and return value, then update tests that call
_make_rust_toolchain to accept the fixture (add make_rust_toolchain param) and
use it instead of calling the helper; ensure the fixture returns a RustToolchain
and keep the same cargo/rustc file setup as in _make_rust_toolchain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fe68485a-d072-41d1-8da4-626b1d75a39c
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (40)
.python-versionREADME.mddocs/docs/quickstart.mdexamples/widget_showcase/trellis_config.pypyproject.tomlsrc/trellis/app/apploader.pysrc/trellis/app/config.pysrc/trellis/app/standalone_entry.pysrc/trellis/cli/package.pysrc/trellis/cli/project_template/trellis_config.py.j2src/trellis/packaging/portable.pysrc/trellis/packaging/tauri.pysrc/trellis/packaging/templates/launcher/Cargo.toml.j2src/trellis/packaging/templates/launcher/common.rs.j2src/trellis/packaging/templates/launcher/main.rs.j2src/trellis/packaging/templates/launcher/main_installer.rs.j2src/trellis/packaging/templates/main.rs.j2src/trellis/packaging/templates/tauri.conf.json.j2src/trellis/packaging/toolchain/__init__.pysrc/trellis/packaging/toolchain/platform.pysrc/trellis/packaging/toolchain/pytauri_wheel.pysrc/trellis/packaging/toolchain/python_standalone.pysrc/trellis/packaging/toolchain/rustup.pysrc/trellis/packaging/toolchain/tauri_cli.pysrc/trellis/platforms/desktop/platform.pytests/helpers.pytests/py/unit/test_apploader.pytests/py/unit/test_apploader_standalone.pytests/py/unit/test_build_config.pytests/py/unit/test_cli_package.pytests/py/unit/test_config.pytests/py/unit/test_desktop_standalone_platform.pytests/py/unit/test_packaging_portable.pytests/py/unit/test_packaging_tauri.pytests/py/unit/test_project_metadata.pytests/py/unit/test_toolchain_pytauri_wheel.pytests/py/unit/test_toolchain_python_standalone.pytests/py/unit/test_toolchain_rustup.pytests/py/unit/test_toolchain_tauri_cli.pytests/py/unit/test_widget_showcase_examples.py
💤 Files with no reviewable changes (1)
- tests/py/unit/test_build_config.py
✅ Files skipped from review due to trivial changes (1)
- .python-version
🚧 Files skipped from review as they are similar to previous changes (8)
- src/trellis/packaging/templates/tauri.conf.json.j2
- tests/py/unit/test_apploader_standalone.py
- src/trellis/packaging/templates/launcher/Cargo.toml.j2
- docs/docs/quickstart.md
- tests/py/unit/test_packaging_portable.py
- tests/py/unit/test_toolchain_rustup.py
- tests/py/unit/test_project_metadata.py
- README.md
Zip entries are now stored uncompressed (ZIP_STORED) and the entire archive is compressed with zstd at level 9. This gives better compression from cross-file redundancy and ~4x faster decompression on first launch. The Rust launcher decompresses the outer zstd layer before extracting the inner zip.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pyproject.toml (1)
48-49:⚠️ Potential issue | 🟠 MajorPlatform-gate
pydanticconsistently with desktop runtime deps.Line 49 is currently unguarded while the adjacent desktop runtime dependency on Line 48 is explicitly excluded on
emscripten. This can pullpydanticinto browser/Pyodide dependency resolution unexpectedly.Suggested patch
- "pydantic>=2.12", + "pydantic>=2.12; sys_platform != 'emscripten'",#!/bin/bash set -euo pipefail # Verify current dependency marker mismatch in pyproject.toml python - <<'PY' from pathlib import Path import tomllib data = tomllib.loads(Path("pyproject.toml").read_text(encoding="utf-8")) deps = data["project"]["dependencies"] for dep in deps: if dep.startswith(("pytauri", "pydantic", "zstandard")): print(dep) PY # Verify where pydantic is imported in source (excluding tests) rg -n --type=py '\bpydantic\b' src -C2Does pydantic>=2.12 (including pydantic-core) officially support installation on Emscripten/Pyodide targets?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` around lines 48 - 49, The pyproject.toml dependency "pydantic>=2.12" is unguarded while the adjacent "pytauri>=0.8.0; sys_platform != 'emscripten'" is explicitly excluded for Emscripten, which can accidentally pull pydantic into browser/Pyodide installs; update the dependency entry for pydantic to include the same platform marker (e.g., "pydantic>=2.12; sys_platform != 'emscripten'") or otherwise gate it consistently with pytauri, and then run your dependency verification (search for pydantic imports in source) to ensure no unintended browser installs occur.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/trellis/packaging/templates/launcher/common.rs.j2`:
- Around line 99-123: The progress window created by create_progress_window(...)
can leak because multiple early returns inside the extraction loop (e.g., calls
to archive.by_index, fs::create_dir_all, fs::File::create, io::copy) return Err
before destroy_progress_window(hwnd) is called; wrap the entire for loop body in
a fallible scope/closure that returns Result<(), String> (for example: let
result = (|| -> Result<(), String> { for ... { ... } Ok(()) })();), then call
destroy_progress_window(hwnd) unconditionally, and finally return the closure
result (propagating the error if any). Ensure the closure contains uses of
entry.name(), entry.is_dir(), update_progress(), and all existing map_err calls
so all early errors go through the closure instead of returning directly.
---
Duplicate comments:
In `@pyproject.toml`:
- Around line 48-49: The pyproject.toml dependency "pydantic>=2.12" is unguarded
while the adjacent "pytauri>=0.8.0; sys_platform != 'emscripten'" is explicitly
excluded for Emscripten, which can accidentally pull pydantic into
browser/Pyodide installs; update the dependency entry for pydantic to include
the same platform marker (e.g., "pydantic>=2.12; sys_platform != 'emscripten'")
or otherwise gate it consistently with pytauri, and then run your dependency
verification (search for pydantic imports in source) to ensure no unintended
browser installs occur.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ec3dcab1-607a-40ce-aa91-3fa452d0d855
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
pyproject.tomlsrc/trellis/packaging/portable.pysrc/trellis/packaging/templates/launcher/Cargo.toml.j2src/trellis/packaging/templates/launcher/common.rs.j2tests/py/unit/test_packaging_portable.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/py/unit/test_packaging_portable.py
The macOS dev tools check crashed on Linux CI because TestOutputCopying mocked sys.platform to "darwin" without patching the xcode-select call. Extract the preflight check patches into a _skip_preflight_checks fixture used by all build_desktop_app_bundle test classes.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/py/unit/test_packaging_tauri.py (1)
189-193: Remove duplicated icon assertionLine 189 and Line 192 assert the same condition; keep one to reduce noise.
✂️ Optional cleanup
icon_list = tauri_conf["bundle"]["icon"] # Only icons that actually exist should be listed - assert "icons/icon.png" in icon_list for icon in icon_list: assert (scaffold_dir / icon).exists() assert "icons/icon.png" in icon_list🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/py/unit/test_packaging_tauri.py` around lines 189 - 193, Remove the duplicated assertion checking for "icons/icon.png" in icon_list: keep a single assert "icons/icon.png" in icon_list and delete the second identical line; locate the duplicate in the test that iterates over icon_list (references: icon_list, scaffold_dir, icon) and ensure only one presence check remains to avoid redundant assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/py/unit/test_packaging_tauri.py`:
- Around line 800-817: The test mocks sys.platform but still allows external
pkg-config probing; make it deterministic by also stubbing pkg-config discovery
and invocation: in test_generates_config_on_linux patch shutil.which to return a
non-None path (e.g. "/usr/bin/pkg-config") and patch subprocess.check_output (or
the function used to call pkg-config) to return a deterministic byte string of
linker flags, then call _generate_cargo_config(scaffold_dir=...,
pyembed_dir=..., product_name="My App") as before so the test no longer depends
on host pkg-config behavior.
---
Nitpick comments:
In `@tests/py/unit/test_packaging_tauri.py`:
- Around line 189-193: Remove the duplicated assertion checking for
"icons/icon.png" in icon_list: keep a single assert "icons/icon.png" in
icon_list and delete the second identical line; locate the duplicate in the test
that iterates over icon_list (references: icon_list, scaffold_dir, icon) and
ensure only one presence check remains to avoid redundant assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b56e7497-42ee-4fdb-9a9d-0fcd4f719ac8
📒 Files selected for processing (1)
tests/py/unit/test_packaging_tauri.py
Replace repeated inline patches across TestBuildDesktopAppBundle and TestOutputCopying with a shared mock_build_pipeline fixture that patches the entire pipeline (preflight checks, toolchain, scaffold, build).
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/py/unit/test_packaging_tauri.py (1)
752-769:⚠️ Potential issue | 🟠 MajorMake the Linux cargo-config unit test deterministic
Line 752 uses host-dependent skip logic, and Line 763 still allows environment-dependent Linux flag probing during a unit test. Please stub Linux lib-flag discovery so this test is deterministic everywhere.
As per coding guidelines `tests/py/unit/**/*.py`: Python unit tests should be isolated and test a single function, class, or module with dependencies faked or stubbed.Proposed fix
- `@pytest.mark.skipif`( - sys.platform == "win32" or shutil.which("pkg-config") is None, - reason="Requires pkg-config (Linux/macOS)", - ) def test_generates_config_on_linux(self, tmp_path: Path) -> None: @@ - with patch("trellis.packaging.tauri.sys") as mock_sys: + with ( + patch("trellis.packaging.tauri.sys") as mock_sys, + patch("trellis.packaging.tauri._get_linux_system_lib_flags", return_value=[]), + ): mock_sys.platform = "linux" _generate_cargo_config( scaffold_dir=scaffold_dir, pyembed_dir=pyembed_dir, product_name="My App", )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/py/unit/test_packaging_tauri.py` around lines 752 - 769, The test test_generates_config_on_linux relies on host-dependent pkg-config/shutil.which and real Linux lib-flag probing; make it deterministic by stubbing the discovery: in the test, patch trellis.packaging.tauri._generate_cargo_config caller dependencies so they always think platform is "linux" and stub the module's lib-flag discovery (e.g., patch trellis.packaging.tauri._discover_lib_flags or the function that queries pkg-config/shutil.which) to return a fixed set of flags; remove/adjust the skip-if that depends on shutil.which so the test runs everywhere while the patched discovery supplies deterministic flags to _generate_cargo_config.
🧹 Nitpick comments (2)
tests/py/unit/test_packaging_tauri.py (2)
225-230: Remove duplicate assertion in icon list checkLine 229 repeats the same assertion as Line 226 (
"icons/icon.png" in icon_list). Keep one assertion to reduce noise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/py/unit/test_packaging_tauri.py` around lines 225 - 230, The test contains a duplicate assertion checking that "icons/icon.png" is in icon_list; remove the repeated assertion so only one check for "icons/icon.png" remains (keep the first occurrence near the existing assert "icons/icon.png" in icon_list), leaving the loop that verifies each path exists using scaffold_dir / icon and the single membership assertion on icon_list.
593-606: Prefer fixture return values over mutatingselfin pytest setupThe autouse fixture currently stores shared test state on
self. Returning a context fixture (and passing it to tests) keeps tests more explicit and aligns better with pytest style.As per coding guidelines
**/test_*.py: Use pytest style instead of xunit for writing tests; prefer fixtures for shared code instead of class members and inheritance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/py/unit/test_packaging_tauri.py` around lines 593 - 606, The autouse fixture _setup mutates self (self.config, self.app_root, self.pipeline); change it into a regular fixture that constructs and returns a context object (e.g., a simple namespace or dataclass with config, app_root, pipeline) instead of assigning to self, update tests to accept that fixture parameter rather than relying on self, and keep the tmp_path and mock_build_pipeline dependencies; ensure all references to self.config/self.app_root/self.pipeline in tests are replaced with context.config/context.app_root/context.pipeline (or whatever name you choose).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tests/py/unit/test_packaging_tauri.py`:
- Around line 752-769: The test test_generates_config_on_linux relies on
host-dependent pkg-config/shutil.which and real Linux lib-flag probing; make it
deterministic by stubbing the discovery: in the test, patch
trellis.packaging.tauri._generate_cargo_config caller dependencies so they
always think platform is "linux" and stub the module's lib-flag discovery (e.g.,
patch trellis.packaging.tauri._discover_lib_flags or the function that queries
pkg-config/shutil.which) to return a fixed set of flags; remove/adjust the
skip-if that depends on shutil.which so the test runs everywhere while the
patched discovery supplies deterministic flags to _generate_cargo_config.
---
Nitpick comments:
In `@tests/py/unit/test_packaging_tauri.py`:
- Around line 225-230: The test contains a duplicate assertion checking that
"icons/icon.png" is in icon_list; remove the repeated assertion so only one
check for "icons/icon.png" remains (keep the first occurrence near the existing
assert "icons/icon.png" in icon_list), leaving the loop that verifies each path
exists using scaffold_dir / icon and the single membership assertion on
icon_list.
- Around line 593-606: The autouse fixture _setup mutates self (self.config,
self.app_root, self.pipeline); change it into a regular fixture that constructs
and returns a context object (e.g., a simple namespace or dataclass with config,
app_root, pipeline) instead of assigning to self, update tests to accept that
fixture parameter rather than relying on self, and keep the tmp_path and
mock_build_pipeline dependencies; ensure all references to
self.config/self.app_root/self.pipeline in tests are replaced with
context.config/context.app_root/context.pipeline (or whatever name you choose).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a90405a3-7e3d-4953-a180-24220c71ce44
📒 Files selected for processing (1)
tests/py/unit/test_packaging_tauri.py
- Atomic cache for python-standalone: extract into temp dir and rename into place so interrupted or concurrent extractions can't leave partial installs - Use full 64-char SHA-256 in portable/installer footer instead of truncated 32-char hash - Platform-gate pydantic dependency (desktop-only, not needed in emscripten/Pyodide) - Wrap extraction loop in closure so progress window is always destroyed on error paths - Preserve full i32 exit code via std::process::exit instead of truncating to u8 - Patch PYTHON_STANDALONE_RELEASE in tests for deterministic URLs - Stub _get_linux_system_lib_flags in Linux cargo config test to remove host pkg-config dependency
The packaged binary is always a desktop app, but trellis_config.py might specify a different platform (e.g. when the user packaged with --platform desktop while the config defaults to server). Override the platform to DESKTOP after loading config.
The package command now always sets the platform to DESKTOP regardless of what trellis_config.py specifies, matching the standalone entry behavior. This allows packaging any app without requiring --platform desktop on the command line.
Tauri generates deb filenames with the product name (spaces, mixed case). Normalize to lowercase with hyphens to follow Debian packaging standards.
Show app name, output directory, and list of built artifacts in a formatted banner matching the style of other Trellis startup banners.
build_desktop_app_bundle now returns (output_dir, artifacts) where artifacts is the list of filenames actually written. The package command banner displays these instead of scanning the directory.
_copy_build_output now takes the resolved bundle type list instead of scanning all subdirectories. This prevents stale artifacts from previous builds (e.g. a .dmg from an earlier --installer run) from being copied and listed in the summary.
… name - Use forward slashes in .cargo/config.toml to avoid TOML unicode escape interpretation of backslashes (e.g. \U in C:\Users) - Add Win32_Graphics_Gdi feature to launcher Cargo.toml and move UpdateWindow import to correct module; use null_mut() for HWND/HICON - Fix launcher binary name: Cargo preserves hyphens in binary names, not underscores - Fix test_toolchain_rustup tests on Windows (preserve USERPROFILE for Path.home(), use .exe suffix, mock get_rust_target) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- PBM_SETPOS, PBM_SETRANGE32, MB_ICONINFORMATION, COLOR_WINDOW - Fix HKEY null pointer in installer registry code (0 → null_mut) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
RegCreateKeyExW requires Win32_Security feature gate. With that feature enabled, HKEY is defined in Win32::System::Registry, not Win32::Foundation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
7 tasks
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
trellis package)dist/with--dmgand--destflagsbundle.iconin Tauri configTest plan
trellis package --platform=desktopproduces.appindist/trellis package --platform=desktop --dmgproduces.appand.dmgindist/trellis package --platform=desktop --dest /tmp/outoutputs to/tmp/out/config.iconpixi run cipassesSummary by CodeRabbit
New Features
Improvements
Tests
Documentation
Chores