Skip to content

fix(paths): skip pino-pretty transport in compiled binaries#962

Closed
leex279 wants to merge 1 commit intodevfrom
fix/logger-pino-pretty-in-binary
Closed

fix(paths): skip pino-pretty transport in compiled binaries#962
leex279 wants to merge 1 commit intodevfrom
fix/logger-pino-pretty-in-binary

Conversation

@leex279
Copy link
Copy Markdown
Collaborator

@leex279 leex279 commented Apr 7, 2026

Summary

  • Compiled bun build --compile binaries crash on every TTY invocation with error: unable to determine transport target for "pino-pretty" because Pino transports do a dynamic require.resolve('pino-pretty') that cannot work inside Bun's /$bunfs/ virtual filesystem.
  • This PR makes the logger emit NDJSON unconditionally inside compiled binaries, so the transport worker is never started.
  • Detection is inlined in @archon/paths because that package has zero @archon/* dependencies (per CLAUDE.md). It checks both import.meta.dir (ESM compiled binaries) and process.execPath basename (CJS bytecode binaries where import.meta.dir may be empty).

Fixes #960

Test plan

  • bun --filter @archon/paths test — 83 pass, 0 fail
  • End-to-end smoke test: bun build --compile --outfile dist/archon-test.exe packages/cli/src/cli.ts && ./dist/archon-test.exe version now prints Build: binary instead of crashing.
  • bun --filter @archon/paths type-check — clean
  • eslint clean on changed file
  • CI green on dev

The Pino logger unconditionally configured `transport: { target: 'pino-pretty' }`
whenever stdout is a TTY. Pino transports spawn a worker that performs a dynamic
`require.resolve('pino-pretty')` at runtime, which cannot resolve inside Bun's
`/$bunfs/` virtual filesystem in `bun build --compile` binaries. As a result,
every CLI command crashed on startup in a real terminal:

    error: unable to determine transport target for "pino-pretty"
        at I (/$bunfs/root/archon-linux-x64:7:35652)

100% of fresh binary installs were unusable. Piping output (e.g. `archon
version | cat`) masked the bug because `isTTY` became false.

Compiled binaries now always emit NDJSON. Detection is inlined here because
`@archon/paths` has zero `@archon/*` dependencies; it checks both
`import.meta.dir` (covers ESM compiled binaries) and `process.execPath`
basename (covers CJS bytecode binaries where `import.meta.dir` is empty).

Fixes #960
@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: 7759a1d2-ff62-4829-b990-6cc87e1f9e71

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/logger-pino-pretty-in-binary

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
- Remove duplicate 'returns null when no resumable run exists' test that was
  accidentally left in alongside the newly-added one
- Fix twoDaysAgo timestamp arithmetic (was 48 minutes, not 2 days)
- Add test for last_activity_at IS NULL branch in findResumableRun
- Change warn → error + add errorType in findResumableRun catch block
- Add errorType to workflow.ts signal cleanup catch
- Add errorType to cli.ts failOrphanedRuns startup catch
- Add inline comment clarifying nowMinusDays(3) uses param index, not day count
puvuglobal pushed a commit to puvuglobal/Archon that referenced this pull request Apr 8, 2026
…ix-issue-961

fix: prevent SIGTERM from orphaning CLI workflow runs
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
- Remove duplicate 'returns null when no resumable run exists' test that was
  accidentally left in alongside the newly-added one
- Fix twoDaysAgo timestamp arithmetic (was 48 minutes, not 2 days)
- Add test for last_activity_at IS NULL branch in findResumableRun
- Change warn → error + add errorType in findResumableRun catch block
- Add errorType to workflow.ts signal cleanup catch
- Add errorType to cli.ts failOrphanedRuns startup catch
- Add inline comment clarifying nowMinusDays(3) uses param index, not day count
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
…ix-issue-961

fix: prevent SIGTERM from orphaning CLI workflow runs
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
- Remove duplicate 'returns null when no resumable run exists' test that was
  accidentally left in alongside the newly-added one
- Fix twoDaysAgo timestamp arithmetic (was 48 minutes, not 2 days)
- Add test for last_activity_at IS NULL branch in findResumableRun
- Change warn → error + add errorType in findResumableRun catch block
- Add errorType to workflow.ts signal cleanup catch
- Add errorType to cli.ts failOrphanedRuns startup catch
- Add inline comment clarifying nowMinusDays(3) uses param index, not day count
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
…ix-issue-961

fix: prevent SIGTERM from orphaning CLI workflow runs
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