You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Both PRs work and fix the bugs. They are not the cleanest possible fix. This issue replaces them with a more principled approach.
What the existing PRs do
Both use runtime detection: at process startup, sniff environment signals (import.meta.dir path prefix, process.execPath basename) to decide whether the process is running as a compiled binary. The result feeds into:
Each detection function has two heuristics ANDed together because Bun has two compile modes (ESM vs CJS bytecode) and only one signal works per mode.
Why runtime detection is the wrong tool
Two signals required to cover both compile modes — already a smell. The fact that we need two heuristics means each one is incomplete on its own.
Signals are environmental and string-based — import.meta.dir path matching, exec basename comparison with .exe stripping. Each comparison has subtle edge cases (Windows backslash, case sensitivity, renamed bun interpreters, weird PATH setups).
Future compile mode changes will silently break — if Bun 2.0 introduces a third compile mode, we discover it via another P0 bug report.
Duplicated detection logic between @archon/paths and @archon/workflows (because @archon/paths has zero @archon/* deps and can't import from @archon/workflows). Two copies that can drift.
Patches the symptom, not the cause — the cause is "we don't know at runtime whether we're a binary". The fix is to declare it at build time, not detect it at runtime.
The principled alternative — build-time constants
Archon already does this pattern for version. packages/cli/src/commands/bundled-version.ts:
scripts/build-binaries.sh overwrites this file at compile time with real values. The build script is the source of truth — it knows whether it is building a binary, period. No runtime guessing required.
Apply the same pattern to binary detection. Add BUNDLED_IS_BINARY to a generated file in @archon/paths (the bottom of the dep graph) so any package can import it without dep cycles.
Tradeoff: pino-pretty as a destination stream runs synchronously on the main thread instead of in a worker. For high-volume server logging this could be a perf hit. For CLI logging (1-100 msgs/sec) it is invisible. The server is not compiled into a binary today (no archon serve), so the perf cost only matters in dev mode where we do not care. If #978 ships and we measure a real bottleneck, we can revisit.
Proposed implementation — concrete plan
Phase 1 — Build-time constants in @archon/paths
New file: packages/paths/src/bundled-build.ts
/** * Build-time constants embedded into compiled binaries. * * In dev/test mode, the placeholders below are used and BUNDLED_IS_BINARY * is false. Compiled binaries get this file overwritten by * scripts/build-binaries.sh before `bun build --compile` is invoked. * * Lives in @archon/paths (the bottom of the dep graph) so any package can * import these constants without creating dep cycles. */exportconstBUNDLED_IS_BINARY=false;exportconstBUNDLED_VERSION='dev';exportconstBUNDLED_GIT_COMMIT='unknown';
Updated: scripts/build-binaries.sh — change BUNDLED_VERSION_FILE path from packages/cli/src/commands/bundled-version.ts to packages/paths/src/bundled-build.ts, add BUNDLED_IS_BINARY = true to the heredoc. Add git checkout of the file at the end (or before, with a trap) so the dev tree is not left dirty.
Phase 2 — Migrate isBinaryBuild() to read the constant
import{BUNDLED_IS_BINARY}from'@archon/paths';/** * @deprecated callers should import BUNDLED_IS_BINARY from '@archon/paths' * directly. Kept as a function wrapper for backwards compat with existing * test spies (loader.test.ts uses spyOn for per-test mocking). */exportfunctionisBinaryBuild(): boolean{returnBUNDLED_IS_BINARY;}
Delete isBunVirtualFs() and isCompiledExecPath() (no other callers after this change).
Why keep the function wrapper: loader.test.ts uses spyOn(bundledDefaults, 'isBinaryBuild').mockReturnValue(true) for per-test behavior. If we delete the function entirely and change all 15+ callers to import the constant directly, the spy mocking pattern breaks. The wrapper is one-line overhead that preserves test mockability.
Phase 3 — Migrate version command + delete the old generated file
~3-4 hours of focused work, including local binary verification and PR submission. The biggest unknown is the pino destination-stream refactor (Phase 4) — if pino-pretty has ESM/CJS interop quirks at the version archon uses, there may be import-shape debugging.
Risks
Build script restore: if the script overwrites bundled-build.ts without restoring the dev placeholders, the dev tree shows BUNDLED_IS_BINARY=true until git checkout. Mitigation: add a trap or stash logic to the build script.
pino-pretty as direct import: the import shape may need to be import pretty from 'pino-pretty' or import * as pretty depending on the package version's ESM/CJS interop. Test in dev first.
Cross-package import resolution in bun build --compile: verify the constant from @archon/paths/bundled-build.ts actually gets inlined into the binary. Should work — bun handles this — but worth confirming with a smoke test build.
Test mock pollution: if test files import isBunVirtualFs or isCompiledExecPath (which are about to be deleted), they break. Search before deleting.
Alternatives considered and rejected
"Just merge #962 and #963 as-is and refactor later" — rejected because: (a) once code lands, it tends to stay; (b) the duplicated detection logic between two packages is the kind of subtle drift bug that bites in 6 months; (c) the right pattern (build-time constants) already exists for BUNDLED_VERSION, so doing this "later" means deferring what should obviously happen now.
"Set NODE_ENV=production at compile time" — partially solves the pino issue (the existing NODE_ENV !== 'production' check would do the job) but does not address the isBinaryBuild problem at all, and forces compiled binaries to always emit NDJSON even in TTY mode (less friendly for debugging compiled binaries by hand).
"Use pino-pretty as transport but ship a --no-pretty flag" — rejected because users should not need a flag to make their CLI work.
The BUNDLED_VERSION precedent in packages/cli/src/commands/bundled-version.ts — same pattern, this issue extends it
Out of scope
Implementing telemetry — separate issue, but the same bundled-build.ts will hold the build-time PostHog API key
Restructuring @archon/paths beyond adding the new file — this is a focused refactor
Migrating all 15+ isBinaryBuild() callers to import the constant directly — that can be a follow-up; the function wrapper preserves backwards compatibility for now
Context
PRs #962 and #963 (from @leex279) fix two real bugs in the compiled binary install path:
archoncrashes on every TTY invocation withunable to determine transport target for "pino-pretty"because pino's transport worker can'trequire.resolveinside Bun's/$bunfs/virtual filesystem.archon versionreportspackage.json not found (bad installation?)becauseisBinaryBuild()only checkedimport.meta.dirfor/$bunfs/prefix, which is empty/undefined in CJS bytecode compiled binaries.Both PRs work and fix the bugs. They are not the cleanest possible fix. This issue replaces them with a more principled approach.
What the existing PRs do
Both use runtime detection: at process startup, sniff environment signals (
import.meta.dirpath prefix,process.execPathbasename) to decide whether the process is running as a compiled binary. The result feeds into:packages/paths/src/logger.ts— skip pino-pretty transport in compiled binaries (fix(paths): skip pino-pretty transport in compiled binaries #962)packages/workflows/src/defaults/bundled-defaults.ts— returntruefromisBinaryBuild()for both ESM and CJS bytecode compile modes (fix(workflows): detect compiled binaries via execPath fallback #963)Each detection function has two heuristics ANDed together because Bun has two compile modes (ESM vs CJS bytecode) and only one signal works per mode.
Why runtime detection is the wrong tool
import.meta.dirpath matching, exec basename comparison with.exestripping. Each comparison has subtle edge cases (Windows backslash, case sensitivity, renamed bun interpreters, weird PATH setups).@archon/pathsand@archon/workflows(because@archon/pathshas zero@archon/*deps and can't import from@archon/workflows). Two copies that can drift.The principled alternative — build-time constants
Archon already does this pattern for version.
packages/cli/src/commands/bundled-version.ts:scripts/build-binaries.shoverwrites this file at compile time with real values. The build script is the source of truth — it knows whether it is building a binary, period. No runtime guessing required.Apply the same pattern to binary detection. Add
BUNDLED_IS_BINARYto a generated file in@archon/paths(the bottom of the dep graph) so any package can import it without dep cycles.Why this is better
@archon/paths+@archon/workflowsBUNDLED_IS_BINARY === falsein devThe pino logger fix can also be cleaner
PR #962's approach: detect compiled binary, skip pretty transport.
Better approach: don't use pino-pretty as a transport at all. Use it as a destination stream, which avoids the worker thread entirely:
Why this is better than the runtime detection in #962:
require.resolve→ no/$bunfs/problem at all@archon/pathsTradeoff: pino-pretty as a destination stream runs synchronously on the main thread instead of in a worker. For high-volume server logging this could be a perf hit. For CLI logging (1-100 msgs/sec) it is invisible. The server is not compiled into a binary today (no
archon serve), so the perf cost only matters in dev mode where we do not care. If #978 ships and we measure a real bottleneck, we can revisit.Proposed implementation — concrete plan
Phase 1 — Build-time constants in
@archon/pathsNew file:
packages/paths/src/bundled-build.tsUpdated:
packages/paths/src/index.ts— add re-exports.Updated:
scripts/build-binaries.sh— changeBUNDLED_VERSION_FILEpath frompackages/cli/src/commands/bundled-version.tstopackages/paths/src/bundled-build.ts, addBUNDLED_IS_BINARY = trueto the heredoc. Addgit checkoutof the file at the end (or before, with a trap) so the dev tree is not left dirty.Phase 2 — Migrate
isBinaryBuild()to read the constantUpdated:
packages/workflows/src/defaults/bundled-defaults.tsDelete
isBunVirtualFs()andisCompiledExecPath()(no other callers after this change).Why keep the function wrapper:
loader.test.tsusesspyOn(bundledDefaults, 'isBinaryBuild').mockReturnValue(true)for per-test behavior. If we delete the function entirely and change all 15+ callers to import the constant directly, the spy mocking pattern breaks. The wrapper is one-line overhead that preserves test mockability.Phase 3 — Migrate version command + delete the old generated file
Updated:
packages/cli/src/commands/version.tsDeleted:
packages/cli/src/commands/bundled-version.ts— onlyversion.tsimports it; safe to remove.Phase 4 — Pino logger refactor (destination stream)
Updated:
packages/paths/src/logger.tsbuildLoggerOptions()withbuildLogger()returningpino.Loggerdirectlypretty()as a destination stream, not as a transport target stringisCompiledBinary()entirely (no longer needed)pino-prettyis inpackages/paths/package.jsondependencies(notdevDependencies)Phase 5 — Tests
New:
packages/paths/src/bundled-build.test.ts— assert defaults in dev mode.Updated:
packages/workflows/src/defaults/bundled-defaults.test.tsdescribe('isBunVirtualFs', ...)block (function gone)describe('isCompiledExecPath', ...)block (function gone)describe('isBinaryBuild', ...)to one assertionExisting tests that keep working without modification:
loader.test.ts(usesspyOn— implementation-agnostic)api.workflows.test.ts,api.health.test.ts,workflow-mock-factories.ts(mock the function, do not care about implementation)workflow-discovery.ts,executor-shared.ts,validator.ts,api.ts,version.ts(import path unchanged)Phase 6 — End-to-end binary verification
Before submitting the PR, build a binary locally and run it:
If the binary reports
BUNDLED_IS_BINARY = true(viaBuild: binary) and successfully loads the bundled defaults, the refactor is correct end-to-end.Phase 7 — Land
fix(paths): use build-time constants for binary detection + pretty stream loggerCo-Authored-By: leex279 <leex279@users.noreply.github.com>Files changed summary
packages/paths/src/bundled-build.tspackages/paths/src/index.tspackages/paths/src/logger.tspackages/paths/src/bundled-build.test.tspackages/workflows/src/defaults/bundled-defaults.tspackages/workflows/src/defaults/bundled-defaults.test.tspackages/cli/src/commands/version.tspackages/cli/src/commands/bundled-version.tsscripts/build-binaries.shNet: smaller codebase, fewer concepts, simpler tests.
Total effort
~3-4 hours of focused work, including local binary verification and PR submission. The biggest unknown is the pino destination-stream refactor (Phase 4) — if pino-pretty has ESM/CJS interop quirks at the version archon uses, there may be import-shape debugging.
Risks
bundled-build.tswithout restoring the dev placeholders, the dev tree showsBUNDLED_IS_BINARY=trueuntilgit checkout. Mitigation: add a trap or stash logic to the build script.import pretty from 'pino-pretty'orimport * as prettydepending on the package version's ESM/CJS interop. Test in dev first.bun build --compile: verify the constant from@archon/paths/bundled-build.tsactually gets inlined into the binary. Should work — bun handles this — but worth confirming with a smoke test build.isBunVirtualFsorisCompiledExecPath(which are about to be deleted), they break. Search before deleting.Alternatives considered and rejected
"Just merge #962 and #963 as-is and refactor later" — rejected because: (a) once code lands, it tends to stay; (b) the duplicated detection logic between two packages is the kind of subtle drift bug that bites in 6 months; (c) the right pattern (build-time constants) already exists for
BUNDLED_VERSION, so doing this "later" means deferring what should obviously happen now."Set NODE_ENV=production at compile time" — partially solves the pino issue (the existing
NODE_ENV !== 'production'check would do the job) but does not address theisBinaryBuildproblem at all, and forces compiled binaries to always emit NDJSON even in TTY mode (less friendly for debugging compiled binaries by hand)."Use pino-pretty as transport but ship a
--no-prettyflag" — rejected because users should not need a flag to make their CLI work.Related
isBinaryBuildfalse negative (fixed by this issue)BUNDLED_VERSIONprecedent inpackages/cli/src/commands/bundled-version.ts— same pattern, this issue extends itOut of scope
bundled-build.tswill hold the build-time PostHog API key@archon/pathsbeyond adding the new file — this is a focused refactorisBinaryBuild()callers to import the constant directly — that can be a follow-up; the function wrapper preserves backwards compatibility for now