Skip to content

fix(desktop): assert Linux AppImage embeds Postgres packaging#2131

Merged
gsxdsm merged 1 commit into
mainfrom
feature/linux-appimage-verify
Jul 15, 2026
Merged

fix(desktop): assert Linux AppImage embeds Postgres packaging#2131
gsxdsm merged 1 commit into
mainfrom
feature/linux-appimage-verify

Conversation

@gsxdsm

@gsxdsm gsxdsm commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Verification of the Fusion Linux AppImage + embedded Postgres packaging surface (follow-on to #2106 Mac packaging and #2117 Windows PG work).

What we found

  1. Published v0.60.0 AppImages are broken for Local/embedded Postgres (pre-fix(desktop): boot embedded Postgres in packaged app and ship omp dist #2106):

    • No embedded-postgres / @embedded-postgres/* in app.asar or app.asar.unpacked
    • package.json main is still dist/main.js (no main-bootstrap.cjs)
    • No omp-runtime packaged
    • asar.unpacked only has incidental natives (pi-tui, esbuild, node-pty)
  2. Current main (post-fix(desktop): boot embedded Postgres in packaged app and ship omp dist #2106) packaging config is correct (verified via mac --dir pack on this host):

    • maindist/main-bootstrap.cjs
    • Full asarUnpack of embedded-postgres + @embedded-postgres/**
    • Native bins present under app.asar.unpacked
    • omp-runtime dist present in asar
  3. Linux arm64 native PG binary smoke (@embedded-postgres/linux-arm64 15.18) in Docker: initdb → start → create DB → persist across restart → OK (requires postinstall soname symlinks from hydrate-symlinks.js / pg-symlinks.json).

  4. Host blocker: this machine is macOS arm64 — cannot produce or execute a Linux AppImage end-to-end. Linux packaging must run on ubuntu-latest CI.

Fix in this PR

Release jobs only checked that *.AppImage files existed — which is how v0.60.0 shipped empty of Postgres. Add:

  • scripts/verify-desktop-linux-pg-packaging.mjs — inspects linux-*-unpacked trees for:
    • app.asar.unpacked embedded-postgres + @embedded-postgres/linux-* bins
    • dist/main-bootstrap.cjs + package.json main
    • omp-runtime presence
  • Wire into release.yml + test-release.yml after AppImage artifact checks
  • Unit test lock in release-workflow.test.ts

Test plan

  • pnpm --filter @fusion/core test:embedded-postgres (33/33 with 60s timeout; default 15s flaked under load)
  • Desktop packaging unit tests (electron-builder-config, build-bundling, release-workflow)
  • Inspected published Fusion-0.60.0-linux-arm64.AppImage (checksum OK; PG packaging absent)
  • Post-fix(desktop): boot embedded Postgres in packaged app and ship omp dist #2106 electron-builder --mac --dir: asar.unpacked has PG + bootstrap + omp
  • Docker linux-arm64 native binary lifecycle smoke
  • CI build-desktop-linux on this PR (runs the new verifier against real linux-unpacked)

Gaps remaining (not fixed here)

Gap Notes
Full AppImage launch + /api/health on Linux Needs Linux host/CI with display or headless Electron
No post-#2106 published AppImage yet Next release will include packaging fixes; this PR stops empty AppImages
README still says linux-x64.AppImage Actual name is linux-x86_64.AppImage (workflow already correct)
Windows packaged Local Tracked by #2117 / verify-desktop

Summary by CodeRabbit

  • Bug Fixes

    • Added verification for Linux AppImage packaging to ensure embedded Postgres binaries, platform files, symlinks, and runtime assets are included correctly.
    • Confirmed packaged application metadata points to the expected startup entry point.
    • Improved detection of invalid, missing, or incorrectly formatted packaging artifacts.
  • Tests

    • Added coverage for architecture-specific binaries, exact ASAR paths, executable files, and symlink metadata.
    • Verified packaging checks run after Linux desktop artifacts are created.

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a Node-based verifier for Linux AppImage embedded Postgres packaging, tests its file and metadata checks, and verifies release workflows run it after packaging Linux desktop artifacts.

Changes

Linux AppImage packaging verification

Layer / File(s) Summary
Verifier discovery and setup
scripts/verify-desktop-linux-pg-packaging.mjs
Parses an optional distribution path, locates asar, discovers Linux unpacked outputs, and resolves their resources directories.
Packaging content assertions
scripts/verify-desktop-linux-pg-packaging.mjs
Validates embedded Postgres binaries, executable files, soname links, ASAR runtime entries, and the extracted package.json.main value.
Verifier tests and workflow ordering
packages/desktop/src/__tests__/linux-pg-packaging-verifier.test.ts, packages/desktop/src/__tests__/release-workflow.test.ts
Tests ASAR matching, platform and ELF checks, soname metadata handling, and verifier execution order after Linux artifact packaging.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ReleaseWorkflow
  participant PackagingVerifier
  participant LinuxAppImage
  participant Asar
  ReleaseWorkflow->>PackagingVerifier: Run verification after packaging
  PackagingVerifier->>LinuxAppImage: Discover unpacked resources
  PackagingVerifier->>LinuxAppImage: Validate Postgres binaries and symlinks
  PackagingVerifier->>Asar: List and extract app.asar
  Asar-->>PackagingVerifier: Return runtime entries and package metadata
  PackagingVerifier-->>ReleaseWorkflow: Report verification status
Loading

Possibly related PRs

🚥 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 clearly matches the main change: adding verification that Linux AppImages embed Postgres packaging.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/linux-appimage-verify

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.

@greptile-apps

greptile-apps Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds release checks for embedded Postgres in Linux AppImages. The main changes are:

  • Verify required Postgres binaries, SONAME links, ELF architecture, bootstrap entrypoint, and OMP runtime files.
  • Match required ASAR entries by exact normalized paths.
  • Add focused verifier tests and release-workflow ordering checks.

Confidence Score: 5/5

This looks safe to merge.

  • The required SONAME manifest entries now fail closed when malformed or missing.
  • Exact ASAR matching rejects source-map files that only contain the required path as a prefix.
  • No blocking issues remain in the changed code.

Important Files Changed

Filename Overview
scripts/verify-desktop-linux-pg-packaging.mjs Adds strict checks for runtime entrypoints, Postgres links, executable files, and target ELF architecture.
packages/desktop/src/tests/linux-pg-packaging-verifier.test.ts Covers exact ASAR matching, platform selection, executable checks, ELF architecture, and malformed link manifests.
packages/desktop/src/tests/release-workflow.test.ts Adds checks that Linux packaging verification is wired after packaging.

Reviews (5): Last reviewed commit: "fix(desktop): harden AppImage packaging ..." | Re-trigger Greptile

Comment thread scripts/verify-desktop-linux-pg-packaging.mjs Outdated
Comment thread scripts/verify-desktop-linux-pg-packaging.mjs Outdated
gsxdsm added a commit that referenced this pull request Jul 15, 2026
Address Greptile P1s: require hydrate soname targets from pg-symlinks.json
(not just initdb/pg_ctl/postgres) and require omp-runtime dist/index.js +
dist/probe.js entrypoints instead of a bare package-name substring.
Comment on lines +172 to +176
for (const entry of entries) {
if (!entry || typeof entry !== "object") continue;
const sourceRel = typeof entry.source === "string" ? entry.source : "";
const targetRel = typeof entry.target === "string" ? entry.target : "";
if (!sourceRel || !targetRel) continue;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Reject invalid link entries. This loop skips non-object entries and objects without string source and target fields. A non-empty manifest such as [{}] therefore leaves missing at zero and reports success without checking any required ABI links. The release can pass while embedded Postgres still fails to load its shared libraries. Treat each malformed entry as a verification failure.

Suggested change
for (const entry of entries) {
if (!entry || typeof entry !== "object") continue;
const sourceRel = typeof entry.source === "string" ? entry.source : "";
const targetRel = typeof entry.target === "string" ? entry.target : "";
if (!sourceRel || !targetRel) continue;
for (const entry of entries) {
if (!entry || typeof entry !== "object") {
fail(`${unpackedDir}: ${platform} native/pg-symlinks.json contains an invalid link entry`);
missing += 1;
continue;
}
const sourceRel = typeof entry.source === "string" ? entry.source : "";
const targetRel = typeof entry.target === "string" ? entry.target : "";
if (!sourceRel || !targetRel) {
fail(`${unpackedDir}: ${platform} native/pg-symlinks.json contains an entry without source and target paths`);
missing += 1;
continue;
}

Comment thread scripts/verify-desktop-linux-pg-packaging.mjs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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 `@packages/desktop/src/__tests__/release-workflow.test.ts`:
- Around line 66-92: Replace the substring-only assertions in the test covering
release and test-release workflows with fixture-based behavioral tests for valid
and broken Linux unpacked trees, asserting the verifier succeeds and rejects the
missing packaging cases. Import or invoke the verifier’s executable entrypoint
through the supported command path, and verify in both workflow contents that
“node scripts/verify-desktop-linux-pg-packaging.mjs” occurs after the
electron-builder packaging command rather than merely being present.

In `@scripts/verify-desktop-linux-pg-packaging.mjs`:
- Around line 216-219: Update listIncludesAsarPath to parse the
newline-delimited ASAR listing into a normalized Set and require exact path
matches, removing substring-based list.includes checks. Use the resulting
asarPaths.has checks for the bootstrap and OMP entrypoint validations so
similarly named files cannot satisfy them.
- Around line 242-255: Strengthen the verification loop around platforms and
binPath so each unpacked directory derives its expected architecture and
validates only the matching linux-* package. Require initdb, pg_ctl, and
postgres to be executable regular files, and inspect their ELF machine type when
possible to reject mismatched x64/arm64 payloads or placeholders; preserve the
existing failure reporting and SONAME-link checks.
- Around line 172-176: Update the pg-symlinks.json entry validation in the loop
over entries so malformed objects or entries with non-string or empty
source/target values increment the existing missing failure count instead of
being skipped. Preserve processing of valid entries and ensure any malformed
metadata causes verification to fail.
- Around line 28-30: Update the imports in the module containing __dirname to
explicitly import URL from the appropriate Node module, while preserving the
existing fileURLToPath and __dirname behavior.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ac0575cd-6818-42be-a2eb-fd7f3f0c7c65

📥 Commits

Reviewing files that changed from the base of the PR and between 6c00841 and 80d394f.

📒 Files selected for processing (4)
  • .github/workflows/release.yml
  • .github/workflows/test-release.yml
  • packages/desktop/src/__tests__/release-workflow.test.ts
  • scripts/verify-desktop-linux-pg-packaging.mjs

Comment thread packages/desktop/src/__tests__/release-workflow.test.ts
Comment on lines +28 to +30
import { fileURLToPath } from "node:url";

const __dirname = fileURLToPath(new URL(".", import.meta.url));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Import URL to fix the lint failure.

The repository lint configuration does not recognize URL as a global.

Proposed fix
-import { fileURLToPath } from "node:url";
+import { URL, fileURLToPath } from "node:url";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { fileURLToPath } from "node:url";
const __dirname = fileURLToPath(new URL(".", import.meta.url));
import { URL, fileURLToPath } from "node:url";
const __dirname = fileURLToPath(new URL(".", import.meta.url));
🧰 Tools
🪛 GitHub Check: Lint

[failure] 30-30:
'URL' is not defined

🤖 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 `@scripts/verify-desktop-linux-pg-packaging.mjs` around lines 28 - 30, Update
the imports in the module containing __dirname to explicitly import URL from the
appropriate Node module, while preserving the existing fileURLToPath and
__dirname behavior.

Source: Linters/SAST tools

Comment on lines +172 to +176
for (const entry of entries) {
if (!entry || typeof entry !== "object") continue;
const sourceRel = typeof entry.source === "string" ? entry.source : "";
const targetRel = typeof entry.target === "string" ? entry.target : "";
if (!sourceRel || !targetRel) continue;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reject malformed pg-symlinks.json entries instead of skipping them.

An array containing objects without valid source/target values currently reports success because those entries never increment missing. Fail each malformed entry so corrupted metadata cannot approve a broken package.

🤖 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 `@scripts/verify-desktop-linux-pg-packaging.mjs` around lines 172 - 176, Update
the pg-symlinks.json entry validation in the loop over entries so malformed
objects or entries with non-string or empty source/target values increment the
existing missing failure count instead of being skipped. Preserve processing of
valid entries and ensure any malformed metadata causes verification to fail.

Comment on lines +216 to +219
function listIncludesAsarPath(list, entry) {
// asar list prints paths with a leading slash.
return list.includes(entry) || list.includes(entry.replace(/^\//, ""));
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Match complete ASAR paths, not substrings.

includes() lets entries such as dist/main-bootstrap.cjs.map or dist/index.js.backup satisfy the checks. Parse the newline-delimited ASAR listing into a normalized Set and require exact entries.

Proposed fix
-function listIncludesAsarPath(list, entry) {
-  // asar list prints paths with a leading slash.
-  return list.includes(entry) || list.includes(entry.replace(/^\//, ""));
+function parseAsarPaths(list) {
+  return new Set(
+    list.split(/\r?\n/).filter(Boolean).map((path) =>
+      path.startsWith("/") ? path : `/${path}`,
+    ),
+  );
 }

Then use exact asarPaths.has(...) checks for the bootstrap and OMP entrypoints.

Also applies to: 273-280

🤖 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 `@scripts/verify-desktop-linux-pg-packaging.mjs` around lines 216 - 219, Update
listIncludesAsarPath to parse the newline-delimited ASAR listing into a
normalized Set and require exact path matches, removing substring-based
list.includes checks. Use the resulting asarPaths.has checks for the bootstrap
and OMP entrypoint validations so similarly named files cannot satisfy them.

Comment on lines +242 to +255
const platforms = readdirSync(platformRoot).filter((n) => n.startsWith("linux-"));
if (platforms.length === 0) {
fail(`${unpackedDir}: no @embedded-postgres/linux-* packages in asar.unpacked (got: ${readdirSync(platformRoot).join(", ") || "none"})`);
} else {
ok(`${unpackedDir}: platform packages: ${platforms.join(", ")}`);
for (const platform of platforms) {
for (const bin of ["initdb", "pg_ctl", "postgres"]) {
const binPath = join(platformRoot, platform, "native", "bin", bin);
if (!existsSync(binPath)) {
fail(`${unpackedDir}: missing ${platform}/native/bin/${bin}`);
}
}
assertPlatformSonameLinks(unpackedDir, platformRoot, platform);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Verify the package architecture and executable binaries for each unpacked tree.

The verifier passes when any linux-* package contains files with the expected names. Thus an arm64 tree containing only x64 payloads—or non-executable placeholders—can pass. Derive the expected architecture from each unpacked directory and require executable regular files from its matching platform package; checking the ELF machine type would provide the strongest guarantee.

🤖 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 `@scripts/verify-desktop-linux-pg-packaging.mjs` around lines 242 - 255,
Strengthen the verification loop around platforms and binPath so each unpacked
directory derives its expected architecture and validates only the matching
linux-* package. Require initdb, pg_ctl, and postgres to be executable regular
files, and inspect their ELF machine type when possible to reject mismatched
x64/arm64 payloads or placeholders; preserve the existing failure reporting and
SONAME-link checks.

@gsxdsm

gsxdsm commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded for the AppImage packaging verifier + lint fix by #2138 (also ports the Windows non-admin smoke CI approach). Prefer landing #2138; this PR can close after that merges if the verifier content matches.

gsxdsm added a commit that referenced this pull request Jul 15, 2026
Address Greptile P1s: require hydrate soname targets from pg-symlinks.json
(not just initdb/pg_ctl/postgres) and require omp-runtime dist/index.js +
dist/probe.js entrypoints instead of a bare package-name substring.
@gsxdsm gsxdsm force-pushed the feature/linux-appimage-verify branch from 80d394f to 4bdedf5 Compare July 15, 2026 19:41
gsxdsm added a commit that referenced this pull request Jul 15, 2026
Address Greptile P1s: require hydrate soname targets from pg-symlinks.json
(not just initdb/pg_ctl/postgres) and require omp-runtime dist/index.js +
dist/probe.js entrypoints instead of a bare package-name substring.
@gsxdsm gsxdsm force-pushed the feature/linux-appimage-verify branch from 4bdedf5 to cf9fa56 Compare July 15, 2026 19:41
gsxdsm added a commit that referenced this pull request Jul 15, 2026
#2138)

## Summary
- **Windows CI:** run the embedded Postgres smoke as non-admin
`fusion-pg` (with profile prewarm) so elevated `windows-latest` runners
stop failing with PostgreSQL’s admin-token refusal. Packaging still runs
as the job user.
- **Linux AppImage:** add a packaging content verifier for
`main-bootstrap`, `@embedded-postgres` natives, and `omp-runtime` dist
entrypoints; wire it into `release.yml`, `test-release.yml`, and the
advisory **Desktop packaging** PR lane (after `electron-builder --dir`).
- Fix eslint `no-undef` on bare `URL` in the verifier script (was red on
#2131).

## Context
Desktop packaging on Ubuntu was mostly green; Windows desktop builds and
the AppImage packaging PR (#2131 lint) were the remaining red paths. The
win-pg-diag pivot (run smoke as non-admin) proved green on CI; this
ports that approach without removing main’s elevated-token product path
for end-user “Run as administrator” cases (smoke simply does not take
that path when the process is non-admin).

## Test plan
- [x] `pnpm --filter @fusion/desktop exec vitest run
src/__tests__/release-workflow.test.ts`
- [x] `pnpm exec eslint scripts/verify-desktop-linux-pg-packaging.mjs`
- [ ] Desktop packaging workflow on this PR
- [ ] Desktop Windows Build (workflow_dispatch)
- [ ] Confirm #2131 supersession if this lands the same AppImage checks

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Strengthened Linux desktop AppImage validation to confirm embedded
PostgreSQL artifacts, required binaries, symlink hydration, and the
expected app entrypoints are present after packaging.
* Improved Windows embedded PostgreSQL smoke testing by running under a
non-administrator helper user with a prewarmed profile environment.

* **Tests**
* Added automated packaging/release workflow verification steps (Linux
and Windows) to catch embedded PostgreSQL content regressions earlier,
including during artifact build and release verification.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@gsxdsm gsxdsm force-pushed the feature/linux-appimage-verify branch from fb61a5f to d91a055 Compare July 15, 2026 20:58

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@packages/desktop/src/__tests__/linux-pg-packaging-verifier.test.ts`:
- Around line 74-82: Update the test case around
verifier.assertPlatformSonameLinks to wrap both verification scenarios and their
assertions in a try...finally block, resetting process.exitCode to undefined in
finally so cleanup occurs even when an expectation fails.

In `@packages/desktop/src/__tests__/release-workflow.test.ts`:
- Around line 115-117: Update the ordering assertion in the release workflow
test to first verify that the "Package Linux desktop artifacts" marker exists in
workflow. Then compare the verifier step’s index against that validated
packaging-step index, preserving the intended ordering check and preventing a
missing marker from passing.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0ee2d11a-53da-429f-b0f2-5f43516f0adc

📥 Commits

Reviewing files that changed from the base of the PR and between 80d394f and d91a055.

📒 Files selected for processing (2)
  • packages/desktop/src/__tests__/linux-pg-packaging-verifier.test.ts
  • packages/desktop/src/__tests__/release-workflow.test.ts

Comment on lines +74 to +82
process.exitCode = undefined;
verifier.assertPlatformSonameLinks("fixture", tempDir, platform);
expect(process.exitCode).toBeUndefined();

await writeFile(path.join(nativeRoot, "pg-symlinks.json"), JSON.stringify([{}]));
verifier.assertPlatformSonameLinks("fixture", tempDir, platform);
expect(process.exitCode).toBe(1);
process.exitCode = undefined;
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Prevent global state leakage if test assertions fail.

If an assertion (e.g., expect(process.exitCode).toBeUndefined()) throws an error, process.exitCode will not be reset. This leaks a non-zero exit code to the test worker, potentially causing it to exit prematurely and fail the CI run unpredictably. Wrap the verification logic in a try...finally block to guarantee restoration.

🛠️ Proposed fix
-    process.exitCode = undefined;
-    verifier.assertPlatformSonameLinks("fixture", tempDir, platform);
-    expect(process.exitCode).toBeUndefined();
-
-    await writeFile(path.join(nativeRoot, "pg-symlinks.json"), JSON.stringify([{}]));
-    verifier.assertPlatformSonameLinks("fixture", tempDir, platform);
-    expect(process.exitCode).toBe(1);
-    process.exitCode = undefined;
+    try {
+      process.exitCode = undefined;
+      verifier.assertPlatformSonameLinks("fixture", tempDir, platform);
+      expect(process.exitCode).toBeUndefined();
+
+      await writeFile(path.join(nativeRoot, "pg-symlinks.json"), JSON.stringify([{}]));
+      verifier.assertPlatformSonameLinks("fixture", tempDir, platform);
+      expect(process.exitCode).toBe(1);
+    } finally {
+      process.exitCode = undefined;
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
process.exitCode = undefined;
verifier.assertPlatformSonameLinks("fixture", tempDir, platform);
expect(process.exitCode).toBeUndefined();
await writeFile(path.join(nativeRoot, "pg-symlinks.json"), JSON.stringify([{}]));
verifier.assertPlatformSonameLinks("fixture", tempDir, platform);
expect(process.exitCode).toBe(1);
process.exitCode = undefined;
});
try {
process.exitCode = undefined;
verifier.assertPlatformSonameLinks("fixture", tempDir, platform);
expect(process.exitCode).toBeUndefined();
await writeFile(path.join(nativeRoot, "pg-symlinks.json"), JSON.stringify([{}]));
verifier.assertPlatformSonameLinks("fixture", tempDir, platform);
expect(process.exitCode).toBe(1);
} finally {
process.exitCode = undefined;
}
🤖 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 `@packages/desktop/src/__tests__/linux-pg-packaging-verifier.test.ts` around
lines 74 - 82, Update the test case around verifier.assertPlatformSonameLinks to
wrap both verification scenarios and their assertions in a try...finally block,
resetting process.exitCode to undefined in finally so cleanup occurs even when
an expectation fails.

Comment on lines +115 to +117
expect(workflow.indexOf("node scripts/verify-desktop-linux-pg-packaging.mjs")).toBeGreaterThan(
workflow.indexOf("Package Linux desktop artifacts"),
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Check for the existence of the packaging step before comparing indices.

If the string "Package Linux desktop artifacts" is modified or removed from the workflow, indexOf will return -1. Since the verifier step is present (index > 0), expect(>0).toBeGreaterThan(-1) will incorrectly pass, masking the missing step. Assert that the packaging step exists in the workflow to ensure the ordering check is meaningful.

🛡️ Proposed fix
-      expect(workflow.indexOf("node scripts/verify-desktop-linux-pg-packaging.mjs")).toBeGreaterThan(
-        workflow.indexOf("Package Linux desktop artifacts"),
-      );
+      const packageIndex = workflow.indexOf("Package Linux desktop artifacts");
+      expect(packageIndex).toBeGreaterThan(-1);
+      expect(workflow.indexOf("node scripts/verify-desktop-linux-pg-packaging.mjs")).toBeGreaterThan(packageIndex);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
expect(workflow.indexOf("node scripts/verify-desktop-linux-pg-packaging.mjs")).toBeGreaterThan(
workflow.indexOf("Package Linux desktop artifacts"),
);
const packageIndex = workflow.indexOf("Package Linux desktop artifacts");
expect(packageIndex).toBeGreaterThan(-1);
expect(workflow.indexOf("node scripts/verify-desktop-linux-pg-packaging.mjs")).toBeGreaterThan(packageIndex);
🤖 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 `@packages/desktop/src/__tests__/release-workflow.test.ts` around lines 115 -
117, Update the ordering assertion in the release workflow test to first verify
that the "Package Linux desktop artifacts" marker exists in workflow. Then
compare the verifier step’s index against that validated packaging-step index,
preserving the intended ordering check and preventing a missing marker from
passing.

@gsxdsm gsxdsm merged commit edac617 into main Jul 15, 2026
7 checks passed
@gsxdsm gsxdsm deleted the feature/linux-appimage-verify branch July 15, 2026 21:41
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.

1 participant