Conversation
…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>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
End-to-end binary verification done locally on darwin-arm64:
All four targets compiled successfully (darwin-arm64 60M, darwin-x64 65M, linux-arm64 96M, linux-x64 97M). |
PR Review Summary (multi-agent)Important Issues
Suggestions
Strengths
Documentation
VerdictNEEDS MINOR FIXES — core approach is sound and well-motivated. Address the two silent-failure patterns (logger try/catch, drop |
- 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).
…eam00#982) Projects with docs outside `docs/` (e.g., `packages/docs-web/src/content/docs/`) get broken bundled commands because the path is hardcoded. Add `docs.path` to `.archon/config.yaml` and thread it through the workflow engine as `$DOCS_DIR` (default: `docs/`), following the same pipeline as `$BASE_BRANCH`. Changes: - Add `docs.path` to RepoConfig and `docsPath` to MergedConfig/WorkflowConfig - Thread `docsDir` through executor-shared, executor, and dag-executor - Update bundled commands to use `$DOCS_DIR` instead of hardcoded `docs/` - Add optional docs path prompt to `archon setup` - Add variable reference and configuration documentation - Resolve pre-existing merge conflicts in server/api.ts Fixes coleam00#982
…eam00#982) Projects with docs outside `docs/` (e.g., `packages/docs-web/src/content/docs/`) get broken bundled commands because the path is hardcoded. Add `docs.path` to `.archon/config.yaml` and thread it through the workflow engine as `$DOCS_DIR` (default: `docs/`), following the same pipeline as `$BASE_BRANCH`. Changes: - Add `docs.path` to RepoConfig and `docsPath` to MergedConfig/WorkflowConfig - Thread `docsDir` through executor-shared, executor, and dag-executor - Update bundled commands to use `$DOCS_DIR` instead of hardcoded `docs/` - Add optional docs path prompt to `archon setup` - Add variable reference and configuration documentation - Resolve pre-existing merge conflicts in server/api.ts Fixes coleam00#982
…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>
…eam00#982) Projects with docs outside `docs/` (e.g., `packages/docs-web/src/content/docs/`) get broken bundled commands because the path is hardcoded. Add `docs.path` to `.archon/config.yaml` and thread it through the workflow engine as `$DOCS_DIR` (default: `docs/`), following the same pipeline as `$BASE_BRANCH`. Changes: - Add `docs.path` to RepoConfig and `docsPath` to MergedConfig/WorkflowConfig - Thread `docsDir` through executor-shared, executor, and dag-executor - Update bundled commands to use `$DOCS_DIR` instead of hardcoded `docs/` - Add optional docs path prompt to `archon setup` - Add variable reference and configuration documentation - Resolve pre-existing merge conflicts in server/api.ts Fixes coleam00#982
…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>
Summary
Replaces runtime binary-build detection with a build-time
BUNDLED_IS_BINARYconstant in@archon/paths/bundled-build.ts, rewritten byscripts/build-binaries.sh(with an EXIT trap to restore dev defaults). Also rewrites@archon/paths/logger.tsto usepino-prettyas a destination stream instead of a worker-thread transport — eliminating the/$bunfs/require.resolvecrash entirely. Same code path runs in dev and compiled binaries; no environment detection in the logger at all.Closes #960, #961, #979. Supersedes #962 and #963.
Why
Runtime detection in #962/#963 needed two ANDed heuristics per package (
import.meta.dirprefix + exec basename) because Bun has two compile modes (ESM vs CJS bytecode) and only one signal works per mode. Heuristics are brittle (Windows backslash, case sensitivity, renamed interpreters), duplicated across@archon/pathsand@archon/workflows, and will silently break on future Bun compile modes. Archon already uses build-time constants forBUNDLED_VERSION— extending that pattern is the principled fix.For the logger:
pino-prettyas a worker-thread transport callsrequire.resolve('pino-pretty')inside Bun's/\$bunfs/virtual filesystem and crashes. As a destination stream, the formatter runs synchronously on the main thread — no worker, no resolve, no detection.Changes
packages/paths/src/bundled-build.ts—BUNDLED_IS_BINARY,BUNDLED_VERSION,BUNDLED_GIT_COMMIT(dev defaults)packages/paths/src/bundled-build.test.ts— assert dev defaultspackages/paths/src/logger.ts—pretty()as destination streampackages/paths/package.json— promotepino-prettyfrom optionalDependencies to dependenciespackages/paths/src/index.ts— re-export new constantspackages/workflows/src/defaults/bundled-defaults.ts— deleteisBunVirtualFs();isBinaryBuild()is now a one-line wrapper around the constant (kept forspyOnback-compat inloader.test.ts)packages/workflows/src/defaults/bundled-defaults.test.ts— drop deleted-function testspackages/cli/src/commands/version.ts— import from@archon/pathspackages/cli/src/commands/bundled-version.ts— supersededscripts/build-binaries.sh— rewrite new file path, setBUNDLED_IS_BINARY=true, restore via EXIT trapTest plan
Credit
Thanks to @leex279 for catching the original bugs in #960/#961 and the initial fixes in #962/#963 — this PR replaces them with a more principled architecture but the diagnosis is theirs.