Skip to content

fix(build): use build-time constants for binary detection and pretty stream logger (replaces #962/#963) #979

@Wirasm

Description

@Wirasm

Context

PRs #962 and #963 (from @leex279) fix two real bugs in the compiled binary install path:

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

  1. 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.
  2. Signals are environmental and string-basedimport.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).
  3. Future compile mode changes will silently break — if Bun 2.0 introduces a third compile mode, we discover it via another P0 bug report.
  4. 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.
  5. 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:

export const BUNDLED_VERSION = '0.2.0';
export const BUNDLED_GIT_COMMIT = 'unknown';

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.

Why this is better

Concern Runtime detection (current PRs) Build-time constant (this issue)
Edge cases Many (path prefixes, exec name, Windows, case) Zero — boolean is set at build time
Code duplication @archon/paths + @archon/workflows One file, imported anywhere
Future bun compile modes Need new signal added Still works (build script knows)
Test surface 5 platform-specific tests in #963 One assertion: BUNDLED_IS_BINARY === false in dev
Lines of code ~50 lines across two packages ~10 lines in one file
Architectural pattern New (runtime sniffing) Existing (already used for version)
Risk of regression Subtle string-comparison bugs None — constant is just a constant

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

import pino from 'pino';
import pretty from 'pino-pretty';

function buildLogger(): pino.Logger {
  const level = getInitialLevel();
  if (process.stdout.isTTY && process.env.NODE_ENV !== 'production') {
    const stream = pretty({ colorize: true, translateTime: 'SYS:HH:MM:ss.l', ignore: 'pid,hostname' });
    return pino({ level }, stream);
  }
  return pino({ level });
}

Why this is better than the runtime detection in #962:

  • No worker thread → no require.resolve → no /$bunfs/ problem at all
  • Same code path in dev and compiled binary — no environment detection needed
  • Eliminates the entire class of "this works in dev but not in binary" bugs
  • Removes the duplicated detection logic that fix(paths): skip pino-pretty transport in compiled binaries #962 inlined into @archon/paths

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.
 */
export const BUNDLED_IS_BINARY = false;
export const BUNDLED_VERSION = 'dev';
export const BUNDLED_GIT_COMMIT = 'unknown';

Updated: packages/paths/src/index.ts — add re-exports.

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

Updated: packages/workflows/src/defaults/bundled-defaults.ts

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).
 */
export function isBinaryBuild(): boolean {
  return BUNDLED_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

Updated: packages/cli/src/commands/version.ts

// before
import { BUNDLED_VERSION, BUNDLED_GIT_COMMIT } from './bundled-version';
// after
import { BUNDLED_VERSION, BUNDLED_GIT_COMMIT } from '@archon/paths';

Deleted: packages/cli/src/commands/bundled-version.ts — only version.ts imports it; safe to remove.

Phase 4 — Pino logger refactor (destination stream)

Updated: packages/paths/src/logger.ts

  • Replace buildLoggerOptions() with buildLogger() returning pino.Logger directly
  • Use pretty() as a destination stream, not as a transport target string
  • Delete isCompiledBinary() entirely (no longer needed)
  • Verify pino-pretty is in packages/paths/package.json dependencies (not devDependencies)

Phase 5 — Tests

New: packages/paths/src/bundled-build.test.ts — assert defaults in dev mode.

Updated: packages/workflows/src/defaults/bundled-defaults.test.ts

  • Delete describe('isBunVirtualFs', ...) block (function gone)
  • Delete describe('isCompiledExecPath', ...) block (function gone)
  • Simplify describe('isBinaryBuild', ...) to one assertion

Existing tests that keep working without modification:

  • loader.test.ts (uses spyOn — implementation-agnostic)
  • api.workflows.test.ts, api.health.test.ts, workflow-mock-factories.ts (mock the function, do not care about implementation)
  • All consumers in 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:

cd ~/Projects/cole/Archon
bash scripts/build-binaries.sh
./dist/binaries/archon-darwin-arm64 version       # → real version, "Build: binary"
./dist/binaries/archon-darwin-arm64 workflow list # → loads bundled workflows from embedded JSON
git checkout packages/paths/src/bundled-build.ts  # restore dev defaults

If the binary reports BUNDLED_IS_BINARY = true (via Build: binary) and successfully loads the bundled defaults, the refactor is correct end-to-end.

Phase 7 — Land

Files changed summary

File Action Lines
packages/paths/src/bundled-build.ts new ~10
packages/paths/src/index.ts modified +1
packages/paths/src/logger.ts rewritten ~30
packages/paths/src/bundled-build.test.ts new ~15
packages/workflows/src/defaults/bundled-defaults.ts refactored -50 / +5
packages/workflows/src/defaults/bundled-defaults.test.ts trimmed -50 / +0
packages/cli/src/commands/version.ts modified +1 / -1
packages/cli/src/commands/bundled-version.ts deleted -15
scripts/build-binaries.sh updated +5 / -3

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

  1. 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.
  2. 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.
  3. 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.
  4. 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.

Related

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    P1High priority - Address soon, next in queuearchitectureArchitectural changes and designbugSomething is broken

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions