Skip to content

docs: add Windows build and run guide / and fix(postgres): resolve pg_dump/pg_restore path cross-platform#63

Merged
RambokDev merged 9 commits into
Portabase:devfrom
tobfel:tf-main
Jun 20, 2026
Merged

docs: add Windows build and run guide / and fix(postgres): resolve pg_dump/pg_restore path cross-platform#63
RambokDev merged 9 commits into
Portabase:devfrom
tobfel:tf-main

Conversation

@tobfel

@tobfel tobfel commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Commit 1

Commit message

fix(postgres): resolve pg_dump/pg_restore path cross-platform

select_pg_path was hardcoded to the Debian/Ubuntu package layout
(/usr/lib/postgresql/<major>/bin), which doesn't exist on Windows or
macOS. This broke the agent on Windows entirely, since the PostgreSQL
binaries (pg_dump, pg_restore, ...) could never be located.

This adds:

  • An optional PG_BIN_DIR environment variable to override detection
    entirely (useful for portable/non-standard installs and CI).
  • Platform-specific default search paths (Windows EDB installer,
    Homebrew/Postgres.app on macOS, Debian/Ubuntu and RPM-based distros
    on Linux).
  • A PATH lookup for pg_dump as a further fallback.
  • Unit tests covering the override and fallback behavior.

The function still always returns a PathBuf (never panics), matching
prior behavior for callers that don't yet handle a "not found" case.


Commit 2

Commit message

docs: add Windows build and run guide

Adds docs/WINDOWS_BUILD.md, covering:

  • working around the openssl-sys vendored build failure on Windows
    by using a prebuilt OpenSSL via vcpkg,
  • installing/running Redis on Windows (no official build exists;
    using the community redis-windows distribution),
  • setting up the required environment variables (via batch script or
    an optional .env file),
  • a note on renaming the build output to portabase-agent.exe
    instead of the current generic app.exe.

This documents the steps that were actually needed to get a working
local Windows build/run, complementing the select_pg_path fix in
this PR.


Pull Request

Title

fix(postgres): make select_pg_path cross-platform + Windows build docs (fixes Windows builds)

Description

Problem

select_pg_path in src/domain/postgres/connection.rs is hardcoded to:

format!("/usr/lib/postgresql/{}/bin", major).into()

This is the Debian/Ubuntu package path and does not exist on Windows or
macOS. As a result, the agent cannot locate pg_dump/pg_restore on
those platforms — backup/restore operations fail even though the
project itself compiles fine on Windows (after working around the
unrelated openssl-sys vendored build requirement).

Fix

select_pg_path now resolves the PostgreSQL bin directory using the
following order:

  1. PG_BIN_DIR environment variable, if set — explicit override
    for non-standard or portable installs.
  2. Platform-specific default install locations:
    • Windows: C:\Program Files\PostgreSQL\<major>\bin and the
      (x86) variant (default EDB installer paths).
    • macOS: Homebrew (/opt/homebrew/... and /usr/local/opt/...)
      and Postgres.app.
    • Linux: the existing Debian/Ubuntu path, plus the common
      /usr/pgsql-<major>/bin RPM-based layout.
  3. PATH lookup for pg_dump (pg_dump.exe on Windows) as a
    further fallback.
  4. Original Debian/Ubuntu path as a last-resort fallback, so the
    function's signature and panic-free behavior are unchanged for
    existing callers.

Each candidate is verified to actually contain a pg_dump binary
before being returned, rather than blindly constructing a path string.

Testing

Added unit tests for:

  • the PG_BIN_DIR override (including that it takes precedence
    regardless of the requested version),
  • the platform-specific pg_dump binary name,
  • pg_dump_exists_in returning false for a non-existent directory.

These tests deliberately avoid asserting on whether a real PostgreSQL
install is found via the default search paths or PATH, since that
depends on what's installed on the machine running the tests (CI
runners and developer machines differ, and may have any PostgreSQL
major version installed, or none) — an earlier draft of these tests
asserted on that and was flaky for exactly this reason on a Windows
machine with PostgreSQL 16 installed. PG_BIN_DIR is used to make
test behavior deterministic instead.

Also note: std::env::set_var/remove_var are unsafe as of the 2024
edition (process-global mutation, UB if raced across threads). Since
cargo test runs tests in parallel by default, the env-mutating tests
are serialized via a Mutex to avoid both real races and a poisoned
mutex cascading into unrelated test failures.

Manually verified end-to-end on Windows 11 with a standard EDB
PostgreSQL 16 install (no PG_BIN_DIR needed — picked up automatically
via the default path) and via PG_BIN_DIR override pointing at a
portable PostgreSQL distribution. cargo test passes for this module;
remaining failures in the full test suite on Windows are pre-existing
and unrelated — they come from Testcontainers/bollard failing to
reach a Docker daemon (select_pg_path tests don't touch Docker at
all).

Windows build/run documentation

Also added docs/WINDOWS_BUILD.md, written up from a successful local
Windows build/run, covering:

  • The openssl-sys vendored feature failing to build on Windows
    (nmake-related issue) and using a prebuilt OpenSSL via vcpkg
    instead, which worked reliably.
  • Installing and running Redis on Windows via the community
    redis-windows
    distribution (no official Windows Redis build exists).
  • The environment variables required to run the agent, and an example
    startup script.
  • A suggestion to rename the build output from the current generic
    app.exe to portabase-agent.exe (via an explicit [[bin]] name =
    in Cargo.toml) for clarity — left as a recommendation rather than
    applied directly in this PR, since I don't have visibility into the
    full Cargo.toml/main.rs and didn't want to guess at a rename
    that might conflict with existing tooling/CI.
  • A suggestion (with example code) to support loading configuration
    from a .env file via the dotenvy crate, for easier local
    development on Windows — also left as a recommendation pending
    maintainer input, since src/services/config.rs likely already
    owns configuration loading and might be a better place for this
    than a raw dotenvy::dotenv() call in main().

Notes / open questions for maintainers

  • I kept the "always return a PathBuf, never an error" behavior to
    avoid changing the function signature and downstream call sites in
    this PR. Happy to follow up with a Result/Option-based API if
    preferred — let me know and I'll adjust.
  • Not 100% sure about the exact install layout for every Linux distro
    (e.g. Arch, NixOS); included the most common ones. Open to adding
    more candidates if maintainers know of others worth covering.

Summary by CodeRabbit

New Features

  • Added cross-platform PostgreSQL binary path detection for Windows, macOS, and Linux with automatic fallback to PATH lookup.
  • Added PG_BIN_DIR environment variable configuration option to override the default PostgreSQL binary directory.

Chores

  • Enabled automated Windows release builds via GitHub Actions.
  • Updated Docker environment configuration.

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 65291e38-efaf-4fc5-93d3-93181c2b995b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Rewrites select_pg_path in connection.rs into a cross-platform resolver that checks a new PG_BIN_DIR config override, OS-specific candidate directories, PATH scanning, and a Debian/Ubuntu fallback. Adds a pg_bin_dir field to Settings, exposes the connection module as pub(crate), and adds unit tests. Introduces a GitHub Actions workflow for automated Windows release builds using vcpkg-managed OpenSSL.

Changes

Cross-platform pg_dump path resolution

Layer / File(s) Summary
PG_BIN_DIR config field and module visibility
src/settings.rs, src/domain/postgres/mod.rs
Adds pg_bin_dir: String to Settings, populates it from PG_BIN_DIR (defaulting to empty string) in from_env(), and changes the connection submodule to pub(crate) visibility.
Cross-platform select_pg_path implementation and tests
src/domain/postgres/connection.rs, src/tests/domain/postgres.rs
Replaces the hardcoded Debian path with a multi-step resolver: PG_BIN_DIR override → OS-specific candidate directories → PATH scan → Debian/Ubuntu fallback. Adds helpers pg_dump_binary_name, pg_dump_exists_in, find_pg_dump_in_path, and unit tests covering all resolution cases.

GitHub Actions Windows release workflow

Layer / File(s) Summary
Workflow triggers, toolchain, and OpenSSL setup
.github/workflows/windows-release.yml (lines 1–48)
Defines workflow_dispatch and version-tag/branch push triggers, installs stable-x86_64-pc-windows-msvc toolchain, bootstraps vcpkg to install openssl:x64-windows, exports VCPKG_ROOT/OPENSSL_DIR, and runs cargo build --release.
Zip packaging and GitHub Release publishing
.github/workflows/windows-release.yml (lines 49–99)
Locates the release exe, packages it into windows-release-$tag.zip, uploads the zip as a workflow artifact, and on tag pushes creates a GitHub Release with the zip as an asset.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 Hippity-hop across the OS divide,
No more Debian paths where Windows must hide!
pg_dump found whether snow falls or sun,
A ZIP flies to GitHub when tagging is done.
From candidate dirs to PATH's winding trail,
This bunny ensures your backups won't fail! 🗂️

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title describes two distinct changes: Windows build documentation and cross-platform PostgreSQL path resolution, but the latter is the more significant fix. The title is related to the changeset but combines two separate concerns.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tobfel tobfel marked this pull request as draft June 19, 2026 13:33
@tobfel

tobfel commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

@RambokDev
you can look at the changes and the doc.
if i find time i can also try to build a git action/workflow for the windows build.
But i'm still learning all the git stuff. So sorry if something is wrong with this PR - process.
FYI: I also know testet a restore. with the windows agent. also worked.

@tobfel tobfel marked this pull request as ready for review June 19, 2026 13:36

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/domain/postgres/connection.rs`:
- Around line 158-176: The test function respects_pg_bin_dir_override uses
unsafe environment variable manipulation that is not panic-safe: if a panic
occurs after the set_var call on line 168 and before the remove_var cleanup on
line 175, the PG_BIN_DIR environment variable will leak into subsequent tests.
To fix this, create a guard struct that implements the Drop trait to manage the
environment variable lifecycle. The guard should set PG_BIN_DIR in its
constructor and remove it in the Drop implementation, ensuring cleanup happens
automatically regardless of panics. Apply the same panic-safe pattern to the
other test mentioned (around lines 178-198) that also has this issue.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 58a010b2-9792-4c56-8fe8-4d58958348c1

📥 Commits

Reviewing files that changed from the base of the PR and between 23678bf and 2052ab0.

📒 Files selected for processing (3)
  • .gitignore
  • docs/WINDOWS_BUILD.md
  • src/domain/postgres/connection.rs
💤 Files with no reviewable changes (1)
  • .gitignore

Comment thread src/domain/postgres/connection.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/windows-release.yml:
- Around line 3-14: The windows-release.yml workflow lacks explicit permission
restrictions and relies on default token permissions which are overly broad. Add
a `permissions:` block at the workflow level (after the `on:` section and before
the `jobs:` section) to explicitly define the minimum required permissions for
the release build. For the `build-windows` job, grant only the necessary
permissions such as `contents: write` for creating releases, and use `read`
permissions for any other operations that need minimal access. This ensures the
workflow adheres to least-privilege security principles.
- Around line 17-18: In the windows-release.yml workflow file, locate the
"Checkout repository" step that uses actions/checkout@v4 and add the
persist-credentials input parameter set to false. This configuration parameter
should be added to the uses directive to prevent the GitHub token from being
persisted in the Git configuration, reducing the security surface area for token
exposure in subsequent workflow steps.
- Line 46: The cargo build command on line 46 does not specify an explicit
binary target, and the subsequent artifact selection logic on lines 57-62
attempts to find portabase-agent.exe but falls back to the first .exe file
found, which can be non-deterministic if multiple binaries exist. Modify the
cargo build --release command to explicitly specify the bin target that is
declared in Cargo.toml using the --bin flag, and update the artifact selection
logic on lines 57-62 to match the explicitly built binary name instead of
relying on heuristic fallback matching. This ensures the correct executable is
always packaged regardless of how many binaries exist in the target directory.
- Line 21: Replace the archived `actions-rs/toolchain@v1` action in the
windows-release.yml workflow with the maintained alternative
`dtolnay/rust-toolchain`. Additionally, scan the entire workflow file for other
unmaintained actions mentioned in the review: replace
`actions/upload-release-asset@v1` with `softprops/action-gh-release@v3` (or use
GitHub CLI), and upgrade `softprops/action-gh-release@v1` to
`softprops/action-gh-release@v3` to ensure all GitHub Actions used in the
workflow are current and actively maintained.
- Line 18: Replace the version tag references in the `uses:` statements
throughout the workflow file with full 40-character commit SHAs instead.
Specifically, update the actions/checkout@v4 reference on line 18, and similarly
update all other version tag references on lines 21, 77, 85, and 93 by replacing
the `@v` format with the complete commit SHA. You can optionally append the
version tag as a comment after the SHA for readability (e.g., uses:
owner/repo@<40-char-sha> # v4). This ensures immutability and prevents tag
retargeting attacks in the release pipeline.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 992922ae-1d5b-4480-8bd3-0c8c5f49b9de

📥 Commits

Reviewing files that changed from the base of the PR and between 2052ab0 and 0f6c93e.

📒 Files selected for processing (2)
  • .github/workflows/windows-release.yml
  • src/domain/postgres/connection.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/domain/postgres/connection.rs

Comment on lines +3 to +14
on:
workflow_dispatch:
push:
tags:
- 'v*'
branches:
- main
- master

jobs:
build-windows:
runs-on: windows-latest

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Set explicit least-privilege workflow permissions.

The workflow currently relies on default token permissions, which are broader than needed for a release build job. Add a permissions: block at workflow/job scope and grant only what each step needs (e.g., contents: write for release, minimal read elsewhere).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/windows-release.yml around lines 3 - 14, The
windows-release.yml workflow lacks explicit permission restrictions and relies
on default token permissions which are overly broad. Add a `permissions:` block
at the workflow level (after the `on:` section and before the `jobs:` section)
to explicitly define the minimum required permissions for the release build. For
the `build-windows` job, grant only the necessary permissions such as `contents:
write` for creating releases, and use `read` permissions for any other
operations that need minimal access. This ensures the workflow adheres to
least-privilege security principles.

Source: Linters/SAST tools

Comment on lines +17 to +18
- name: Checkout repository
uses: actions/checkout@v4

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Disable credential persistence in checkout.

actions/checkout should set persist-credentials: false in this release workflow to reduce token exposure surface in subsequent steps/artifacts.

🧰 Tools
🪛 zizmor (1.25.2)

[warning] 17-18: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[error] 18-18: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/windows-release.yml around lines 17 - 18, In the
windows-release.yml workflow file, locate the "Checkout repository" step that
uses actions/checkout@v4 and add the persist-credentials input parameter set to
false. This configuration parameter should be added to the uses directive to
prevent the GitHub token from being persisted in the Git configuration, reducing
the security surface area for token exposure in subsequent workflow steps.

Source: Linters/SAST tools


steps:
- name: Checkout repository
uses: actions/checkout@v4

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "windows-release.yml" -type f

Repository: Portabase/agent

Length of output: 99


🏁 Script executed:

cat -n ./.github/workflows/windows-release.yml

Repository: Portabase/agent

Length of output: 4256


🌐 Web query:

GitHub Actions security best practices pinning actions to commit SHA

💡 Result:

Pinning GitHub Actions to a full-length commit SHA is a critical security best practice designed to protect against supply chain attacks [1][2][3]. When you reference an action using a branch name or version tag (e.g., @v4 or @main), you are using a mutable reference that can be modified or "retargeted" by the action owner or an attacker who gains control of the repository [1][4][5]. Because tags can be moved to point to new, potentially malicious code, workflows using them are vulnerable to unexpected changes [2][6]. Key security benefits and practices include: Immutability: A full-length commit SHA uniquely identifies a specific state of an action's codebase [3]. Because it is content-addressed, the code cannot be altered once pinned, providing a guarantee that your workflow will execute the exact same logic every time [4][6][3]. Mitigating Supply Chain Attacks: Pinning prevents "tag retargeting" attacks, where malicious code is pushed to an existing tag [2][7]. By using a SHA, you ensure that you are not affected by unauthorized changes to an action's release tags [6][5]. Operational Policies: GitHub supports organization-level policies that can enforce SHA pinning [1][8]. When enabled, workflows attempting to use non-pinned actions will fail, forcing compliance across all repositories in an organization [8][4][2]. Maintenance with Automation: Pinning creates a maintenance burden, as you must manually update SHAs to receive security patches or features [2][5]. To manage this, use automated dependency management tools like Dependabot or Renovate [2][7][6]. These tools can monitor for updates and open pull requests to update your pinned SHAs [2][7][5]. Best Practice for Implementation: - Always use the full 40-character commit SHA [1][5]. - Include the version tag as a comment in your workflow file (e.g., uses: owner/repo@sha # v4.0.0) [2][7]. This makes the dependency human-readable and helps tools like Renovate identify which version to update to [2][5]. - If you maintain shared actions, pin any internal sub-actions to full commit SHAs to prevent breaking the workflows of users who have enforced SHA-pinning policies [4]. - Treat only GitHub-owned actions (under the actions/ namespace) as potentially safe to use with tags, though SHA pinning remains the most secure standard for all dependencies [5].

Citations:


Pin all uses: actions to commit SHAs instead of version tags.

Lines 18, 21, 77, 85, and 93 reference actions by version tag only (e.g., @v4, @v1), which are mutable references vulnerable to tag retargeting attacks. For a release pipeline, use full 40-character commit SHAs to ensure immutability and prevent supply-chain drift. Optionally include the version tag as a comment (e.g., uses: owner/repo@<sha> # v4.0.0) for readability, and use Dependabot or Renovate to automate SHA updates.

🧰 Tools
🪛 zizmor (1.25.2)

[warning] 17-18: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[error] 18-18: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/windows-release.yml at line 18, Replace the version tag
references in the `uses:` statements throughout the workflow file with full
40-character commit SHAs instead. Specifically, update the actions/checkout@v4
reference on line 18, and similarly update all other version tag references on
lines 21, 77, 85, and 93 by replacing the `@v` format with the complete commit
SHA. You can optionally append the version tag as a comment after the SHA for
readability (e.g., uses: owner/repo@<40-char-sha> # v4). This ensures
immutability and prevents tag retargeting attacks in the release pipeline.

Source: Linters/SAST tools

Comment thread .github/workflows/windows-release.yml
Comment thread .github/workflows/windows-release.yml Outdated
run: |
# Ensure the environment variable is present for this step
if (-Not $env:OPENSSL_DIR) { Write-Host "OPENSSL_DIR not set, printing env for debugging"; Get-ChildItem Env: | ForEach-Object { Write-Host $_ } }
cargo build --release

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make artifact selection deterministic against the declared Rust bin target.

Line 57 prefers portabase-agent.exe, but Cargo.toml declares [[bin]] name = "app". The fallback to “first .exe” can package the wrong executable if multiple binaries exist. Build and package a specific bin explicitly.

Suggested fix
-      - name: Build (cargo release)
+      - name: Build (cargo release)
         shell: pwsh
         env:
           # Cargo / openssl-sys will pick up OPENSSL_DIR from the environment
           OPENSSL_DIR: ${{ env.OPENSSL_DIR }}
         run: |
           # Ensure the environment variable is present for this step
           if (-Not $env:OPENSSL_DIR) { Write-Host "OPENSSL_DIR not set, printing env for debugging"; Get-ChildItem Env: | ForEach-Object { Write-Host $_ } }
-          cargo build --release
+          cargo build --release --bin app
@@
-          # Find the built executable. Prefer an explicit portabase-agent.exe, fall back to first exe in target/release
-          $exe = "target\release\portabase-agent.exe"
-          if (-not (Test-Path $exe)) {
-            $first = Get-ChildItem -Path target\release -Filter *.exe | Select-Object -First 1
-            if ($first) { $exe = $first.FullName }
-          }
+          # Use the explicit release binary declared in Cargo.toml
+          $exe = "target\release\app.exe"

Also applies to: 57-62

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/windows-release.yml at line 46, The cargo build command on
line 46 does not specify an explicit binary target, and the subsequent artifact
selection logic on lines 57-62 attempts to find portabase-agent.exe but falls
back to the first .exe file found, which can be non-deterministic if multiple
binaries exist. Modify the cargo build --release command to explicitly specify
the bin target that is declared in Cargo.toml using the --bin flag, and update
the artifact selection logic on lines 57-62 to match the explicitly built binary
name instead of relying on heuristic fallback matching. This ensures the correct
executable is always packaged regardless of how many binaries exist in the
target directory.

@tobfel

tobfel commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

@RambokDev
windows build via github action also worked.
https://github.com/tobfel/portabase_agent/actions/runs/27831791825 (need some time: 22m 20s)
I have tried the github compiled executable - worked. 💪
Also packed it for "production" in winsw-service-wrapper so that redis and the agent is startet and stopped on system startup. I have tried it on a staging test server. Working 💪

@RambokDev

Copy link
Copy Markdown
Contributor

Hi @tobfel,
Thanks for all of that will start reviewing it.

Regards 😉

@RambokDev RambokDev changed the base branch from main to dev June 20, 2026 10:34

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/domain/postgres/connection.rs (1)

93-99: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate candidate directories against both client binaries.

The resolver currently returns a directory when pg_dump exists, but restore also depends on pg_restore from the same directory. This can pass resolution and still fail at runtime during restore.

Suggested fix
-    if let Some(found) = candidates.into_iter().find(|p| pg_dump_exists_in(p)) {
+    if let Some(found) = candidates.into_iter().find(|p| pg_clients_exist_in(p)) {
         return found;
     }

-    if let Some(dir) = find_pg_dump_in_path() {
+    if let Some(dir) = find_pg_dump_in_path().filter(|d| pg_clients_exist_in(d)) {
         return dir;
     }
@@
 pub(crate) fn pg_dump_exists_in(dir: &std::path::Path) -> bool {
     dir.join(pg_dump_binary_name()).is_file()
 }
+
+fn pg_restore_binary_name() -> &'static str {
+    if cfg!(target_os = "windows") {
+        "pg_restore.exe"
+    } else {
+        "pg_restore"
+    }
+}
+
+fn pg_restore_exists_in(dir: &std::path::Path) -> bool {
+    dir.join(pg_restore_binary_name()).is_file()
+}
+
+fn pg_clients_exist_in(dir: &std::path::Path) -> bool {
+    pg_dump_exists_in(dir) && pg_restore_exists_in(dir)
+}

Also applies to: 112-114

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/domain/postgres/connection.rs` around lines 93 - 99, The validation logic
in the find operation is incomplete because it only checks for the existence of
pg_dump but not pg_restore, even though both binaries are required from the same
directory for restore operations. Update the candidate selection in the find
call (where pg_dump_exists_in is checked) to also validate that pg_restore
exists in the same directory. Apply the same dual validation to the result
returned from find_pg_dump_in_path() at the location marked "Also applies to".
Either enhance pg_dump_exists_in to check for both binaries or create a separate
validation function that confirms both pg_dump and pg_restore are present in the
candidate directory before returning it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/windows-release.yml:
- Around line 7-8: The tag glob pattern `[0-9]+.[0-9]+.[0-9]+` in the
windows-release.yml workflow file uses regex syntax instead of GitHub Actions
glob syntax, which will not match semantic version tags like 1.2.3. Replace this
pattern with a valid glob pattern such as `v[0-9]*.[0-9]*.[0-9]*` (if your tags
are prefixed with `v`) or adjust it to match your actual tag naming convention,
keeping in mind that `*` matches zero or more characters in glob patterns, not
`+` which is treated as a literal character.

In `@docker-compose.yml`:
- Line 22: The EDGE_KEY environment variable in the docker-compose.yml file
contains a hardcoded base64-encoded credential payload that includes a
masterKeyB64, which represents a security risk when committed to source control.
Replace the hardcoded EDGE_KEY value with environment variable interpolation
(using the standard docker-compose syntax for environment variables) so that the
actual credential is provided at runtime rather than being versioned.
Additionally, ensure the exposed credential is rotated to prevent unauthorized
access.

---

Outside diff comments:
In `@src/domain/postgres/connection.rs`:
- Around line 93-99: The validation logic in the find operation is incomplete
because it only checks for the existence of pg_dump but not pg_restore, even
though both binaries are required from the same directory for restore
operations. Update the candidate selection in the find call (where
pg_dump_exists_in is checked) to also validate that pg_restore exists in the
same directory. Apply the same dual validation to the result returned from
find_pg_dump_in_path() at the location marked "Also applies to". Either enhance
pg_dump_exists_in to check for both binaries or create a separate validation
function that confirms both pg_dump and pg_restore are present in the candidate
directory before returning it.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: de68ab9a-ebb9-44cd-8e69-dd92a786b73c

📥 Commits

Reviewing files that changed from the base of the PR and between 0f6c93e and 1615232.

📒 Files selected for processing (7)
  • .github/workflows/windows-release.yml
  • .gitignore
  • docker-compose.yml
  • src/domain/postgres/connection.rs
  • src/domain/postgres/mod.rs
  • src/settings.rs
  • src/tests/domain/postgres.rs
✅ Files skipped from review due to trivial changes (1)
  • .gitignore

Comment thread .github/workflows/windows-release.yml
Comment thread docker-compose.yml
@RambokDev RambokDev merged commit 628c017 into Portabase:dev Jun 20, 2026
1 check passed
@RambokDev

Copy link
Copy Markdown
Contributor

Hi @tobfel,

That is marged, and will be added to main ASAP. Thanks for you contribution to Portabase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants