Skip to content

fix(workflows): detect compiled binaries via execPath fallback#963

Closed
leex279 wants to merge 1 commit intodevfrom
fix/binary-build-detection
Closed

fix(workflows): detect compiled binaries via execPath fallback#963
leex279 wants to merge 1 commit intodevfrom
fix/binary-build-detection

Conversation

@leex279
Copy link
Copy Markdown
Collaborator

@leex279 leex279 commented Apr 7, 2026

Summary

  • isBinaryBuild() only checked whether import.meta.dir lives under Bun's virtual FS (/$bunfs/ or B:\~BUN\). In CJS bytecode compiled binaries (bun build --compile with bytecode), import.meta.dir is empty/undefined and detection silently returned false.
  • Symptom in the released archon-linux-x64: installer prints Failed to read version: package.json not found (bad installation?) because the version command falls into the dev-mode getDevVersion() path that tries to read package.json from disk.
  • Adds a process.execPath basename check as a fallback: if execPath isn't bun/node, the process is a standalone compiled binary. Exported isCompiledExecPath() for testability.
  • Adds scripts/install-local.{ps1,sh} — local-file install harnesses for validating the install flow against a binary built from the current branch before cutting a release. Used to verify this fix end-to-end on Windows; the .sh variant is for Linux/Proxmox VM smoke testing.

Fixes #961

Test plan

  • bun test packages/workflows/src/defaults/bundled-defaults.test.ts — 18 pass, 0 fail (added 4 new tests for isCompiledExecPath covering bun/node interpreters and standalone binaries on Linux/macOS/Windows)
  • bun --filter @archon/workflows test — full suite green
  • bun --filter @archon/workflows type-check — clean
  • End-to-end smoke test on Windows: bun build --compileinstall-local.ps1archon version reports Build: binary (was reporting dev path before).
  • Linux smoke test on Proxmox VM via install-local.sh (pending hardware access)
  • CI green on dev

Related

Stacked alongside #962 (pino-pretty transport fix). Both are needed before the next binary release is usable end-to-end.

`isBinaryBuild()` only checked whether `import.meta.dir` lives under Bun's
virtual filesystem (`/$bunfs/` or `B:\~BUN\`). In CJS bytecode compiled
binaries (`bun build --compile` with bytecode), `import.meta.dir` is empty
or undefined, so detection silently returned `false`.

Symptoms in the released `archon-linux-x64` binary:

  [WARN] Binary installed but version check failed:
  Error: Failed to read version: package.json not found (bad installation?)

That error is only thrown by `getDevVersion()` in
`packages/cli/src/commands/version.ts:33` — which the binary should never
reach. Any other code branching on `isBinaryBuild()` (default workflow
loading, embedded resources) was also taking the wrong path.

Add a `process.execPath` basename check as a fallback: if execPath is
neither `bun` nor `node`, the process is running as a standalone compiled
binary. Exported `isCompiledExecPath()` for testability and added unit
tests covering bun/node interpreters and standalone binaries on Linux,
macOS, and Windows.

Also adds `scripts/install-local.{ps1,sh}` — local-file install harnesses
for validating the install flow against a binary built from the current
branch before cutting a release. These were used to verify this fix
end-to-end on Windows; the Linux harness is documented for testing on
Proxmox / fresh Linux VMs.

Fixes #961
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 7, 2026

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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3315ea7e-b192-4511-a595-be4f6710c2f5

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
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/binary-build-detection

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.

Wirasm added a commit that referenced this pull request Apr 8, 2026
…stream logger

Replaces runtime detection of compiled binaries (env sniffing via
import.meta.dir / process.execPath) with a build-time BUNDLED_IS_BINARY
constant in @archon/paths/bundled-build.ts, rewritten by
scripts/build-binaries.sh and restored on EXIT via a trap.

Also rewrites @archon/paths/logger.ts to use pino-pretty as a destination
stream instead of a worker-thread transport. The formatter now runs on
the main thread, eliminating the require.resolve('pino-pretty') lookup
that crashes inside Bun's /\$bunfs/ virtual filesystem in compiled
binaries. The same code path runs in dev and binaries — no environment
detection in the logger at all.

isBinaryBuild() in @archon/workflows is kept as a one-line wrapper
around BUNDLED_IS_BINARY so existing spyOn-based test mocking in
loader.test.ts continues to work without modification.

Closes #960
Closes #961
Closes #979
Supersedes #962
Supersedes #963

Co-Authored-By: leex279 <leex279@users.noreply.github.com>
Wirasm added a commit that referenced this pull request Apr 8, 2026
…stream logger (#982)

* fix(build): use build-time constants for binary detection and pretty stream logger

Replaces runtime detection of compiled binaries (env sniffing via
import.meta.dir / process.execPath) with a build-time BUNDLED_IS_BINARY
constant in @archon/paths/bundled-build.ts, rewritten by
scripts/build-binaries.sh and restored on EXIT via a trap.

Also rewrites @archon/paths/logger.ts to use pino-pretty as a destination
stream instead of a worker-thread transport. The formatter now runs on
the main thread, eliminating the require.resolve('pino-pretty') lookup
that crashes inside Bun's /\$bunfs/ virtual filesystem in compiled
binaries. The same code path runs in dev and binaries — no environment
detection in the logger at all.

isBinaryBuild() in @archon/workflows is kept as a one-line wrapper
around BUNDLED_IS_BINARY so existing spyOn-based test mocking in
loader.test.ts continues to work without modification.

Closes #960
Closes #961
Closes #979
Supersedes #962
Supersedes #963

Co-Authored-By: leex279 <leex279@users.noreply.github.com>

* style(workflows): hoist BUNDLED_IS_BINARY import to top of file

* fix(build,logger): harden pretty init and trap restore

- logger: wrap pino-pretty init in try/catch and fall back to JSON so a
  broken TTY or missing peer can't crash module load.
- build-binaries.sh: drop '2>/dev/null || true' from the EXIT trap so a
  failed bundled-build.ts restore is visible instead of silently leaving
  the dev tree with BUNDLED_IS_BINARY=true.
- bundled-defaults: unmark isBinaryBuild() @deprecated and document why
  the wrapper is the intentional test seam (mock.module pollution in Bun).

---------

Co-authored-by: leex279 <leex279@users.noreply.github.com>
@Wirasm
Copy link
Copy Markdown
Collaborator

Wirasm commented Apr 8, 2026

Closing — superseded by #982 which replaces both this PR and #963 with a build-time constants refactor (e7789944 on dev).

Why the architectural redirect: the runtime detection approach needed two ANDed heuristics per package (import.meta.dir prefix + exec basename) because Bun has two compile modes (ESM vs CJS bytecode). Each heuristic has edge cases (Windows backslash, case sensitivity, renamed interpreters), the logic was duplicated across @archon/paths and @archon/workflows, and it would silently break on future Bun compile modes. #982 declares BUNDLED_IS_BINARY as a build-time constant written by scripts/build-binaries.sh — same pattern archon already uses for BUNDLED_VERSION. One source of truth, zero edge cases, no cross-package duplication.

For the logger specifically: #982 uses pino-pretty as a destination stream (synchronous, runs on main thread) instead of a worker-thread transport (which calls require.resolve inside /$bunfs/ and crashes). Same code path in dev and compiled binary — no environment detection at all.

Thank you @leex279 for catching the underlying bugs in #960/#961 and providing the initial fixes here. Your diagnosis drove the refactor; the new implementation credits you in its commit message. The install-local.{sh,ps1} scripts you added in #963 are useful QA infrastructure and were dropped from #982 to keep the diff focused on the fix itself — worth re-filing as a separate small PR if you want them back.

@Wirasm Wirasm closed this Apr 8, 2026
puvuglobal pushed a commit to puvuglobal/Archon that referenced this pull request Apr 8, 2026
Reused worktrees were never checked against the configured baseBranch,
so PRs could silently target the wrong base after a config change.

Changes:
- Add isAncestorOf() git helper using merge-base --is-ancestor
- Add baseBranch field to IsolationHints for opt-in validation
- Validate base branch in resolver checkExisting and findReusable paths
- Add base branch warning in CLI worktree reuse path
- Add tests for isAncestorOf and resolver base branch mismatch

Fixes coleam00#963
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
Reused worktrees were never checked against the configured baseBranch,
so PRs could silently target the wrong base after a config change.

Changes:
- Add isAncestorOf() git helper using merge-base --is-ancestor
- Add baseBranch field to IsolationHints for opt-in validation
- Validate base branch in resolver checkExisting and findReusable paths
- Add base branch warning in CLI worktree reuse path
- Add tests for isAncestorOf and resolver base branch mismatch

Fixes coleam00#963
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
…stream logger (coleam00#982)

* fix(build): use build-time constants for binary detection and pretty stream logger

Replaces runtime detection of compiled binaries (env sniffing via
import.meta.dir / process.execPath) with a build-time BUNDLED_IS_BINARY
constant in @archon/paths/bundled-build.ts, rewritten by
scripts/build-binaries.sh and restored on EXIT via a trap.

Also rewrites @archon/paths/logger.ts to use pino-pretty as a destination
stream instead of a worker-thread transport. The formatter now runs on
the main thread, eliminating the require.resolve('pino-pretty') lookup
that crashes inside Bun's /\$bunfs/ virtual filesystem in compiled
binaries. The same code path runs in dev and binaries — no environment
detection in the logger at all.

isBinaryBuild() in @archon/workflows is kept as a one-line wrapper
around BUNDLED_IS_BINARY so existing spyOn-based test mocking in
loader.test.ts continues to work without modification.

Closes coleam00#960
Closes coleam00#961
Closes coleam00#979
Supersedes coleam00#962
Supersedes coleam00#963

Co-Authored-By: leex279 <leex279@users.noreply.github.com>

* style(workflows): hoist BUNDLED_IS_BINARY import to top of file

* fix(build,logger): harden pretty init and trap restore

- logger: wrap pino-pretty init in try/catch and fall back to JSON so a
  broken TTY or missing peer can't crash module load.
- build-binaries.sh: drop '2>/dev/null || true' from the EXIT trap so a
  failed bundled-build.ts restore is visible instead of silently leaving
  the dev tree with BUNDLED_IS_BINARY=true.
- bundled-defaults: unmark isBinaryBuild() @deprecated and document why
  the wrapper is the intentional test seam (mock.module pollution in Bun).

---------

Co-authored-by: leex279 <leex279@users.noreply.github.com>
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
Reused worktrees were never checked against the configured baseBranch,
so PRs could silently target the wrong base after a config change.

Changes:
- Add isAncestorOf() git helper using merge-base --is-ancestor
- Add baseBranch field to IsolationHints for opt-in validation
- Validate base branch in resolver checkExisting and findReusable paths
- Add base branch warning in CLI worktree reuse path
- Add tests for isAncestorOf and resolver base branch mismatch

Fixes coleam00#963
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
…stream logger (coleam00#982)

* fix(build): use build-time constants for binary detection and pretty stream logger

Replaces runtime detection of compiled binaries (env sniffing via
import.meta.dir / process.execPath) with a build-time BUNDLED_IS_BINARY
constant in @archon/paths/bundled-build.ts, rewritten by
scripts/build-binaries.sh and restored on EXIT via a trap.

Also rewrites @archon/paths/logger.ts to use pino-pretty as a destination
stream instead of a worker-thread transport. The formatter now runs on
the main thread, eliminating the require.resolve('pino-pretty') lookup
that crashes inside Bun's /\$bunfs/ virtual filesystem in compiled
binaries. The same code path runs in dev and binaries — no environment
detection in the logger at all.

isBinaryBuild() in @archon/workflows is kept as a one-line wrapper
around BUNDLED_IS_BINARY so existing spyOn-based test mocking in
loader.test.ts continues to work without modification.

Closes coleam00#960
Closes coleam00#961
Closes coleam00#979
Supersedes coleam00#962
Supersedes coleam00#963

Co-Authored-By: leex279 <leex279@users.noreply.github.com>

* style(workflows): hoist BUNDLED_IS_BINARY import to top of file

* fix(build,logger): harden pretty init and trap restore

- logger: wrap pino-pretty init in try/catch and fall back to JSON so a
  broken TTY or missing peer can't crash module load.
- build-binaries.sh: drop '2>/dev/null || true' from the EXIT trap so a
  failed bundled-build.ts restore is visible instead of silently leaving
  the dev tree with BUNDLED_IS_BINARY=true.
- bundled-defaults: unmark isBinaryBuild() @deprecated and document why
  the wrapper is the intentional test seam (mock.module pollution in Bun).

---------

Co-authored-by: leex279 <leex279@users.noreply.github.com>
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