fix(cli): exit 0 quietly when the output pipe closes (EPIPE) - #546
Conversation
When CLI output is piped and the reader closes early (archgate ... | head, or an agent harness tearing down mid-run), the next stream write emits an EPIPE error event. With no listener attached, Bun escalates it to a fatal uncaught exception, crashing the CLI and flooding Sentry (issue CLI-7). Install stream guards at CLI startup: stdout EPIPE exits 0 quietly (the pipeline convention - a consumer that stops reading means done), stderr EPIPE is swallowed since stdout may still have a live consumer, and every other stream error is rethrown so real bugs keep crashing loudly. The error boundaries (handleCommandError, main().catch) short-circuit a synchronously-thrown EPIPE the same way: exit 0, no logging, no Sentry. ARCH-002 now codifies the broken-pipe policy in its Decision section. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
|
Warning Review limit reached
Next review available in: 51 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe CLI now installs stdout and stderr error guards before validation and output. EPIPE errors receive quiet handling with exit code 0, while non-EPIPE errors remain fatal. The exit helpers classify EPIPE errors and record a cancelled 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
Deploying archgate-cli with
|
| Latest commit: |
039f655
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://19ca9ac0.archgate-cli.pages.dev |
| Branch Preview URL: | https://rhuanbarreto-cli-epipe-broke.archgate-cli.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/helpers/exit.ts`:
- Around line 132-134: Remove the generic isEpipeError handling from
handleCommandError() and the corresponding EPIPE branch in the CLI error path.
Preserve EPIPE-specific handling only in handleStdoutError() and
handleStderrError(), where the originating stream is known, including quiet exit
0 exclusively for stdout and swallowing stderr EPIPE without exiting.
In `@tests/integration/stream-guards.test.ts`:
- Around line 3-9: Shorten the overview comment above the integration test to
five or fewer contiguous narrative lines while preserving its essential
description of the spawned Bun process, synthetic EPIPE event, and platform
portability context.
- Line 37: Update the temporary script setup in the async runScript() flow to
replace writeFileSync(file, script) with awaited Bun.write() using the existing
file and script values.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b81de8ce-e8f9-4c91-ac01-c6c86c14d257
📒 Files selected for processing (8)
.archgate/adrs/ARCH-002-error-handling.md.claude/agent-memory/archgate-developer/MEMORY.mdsrc/cli.tssrc/helpers/exit.tssrc/helpers/stream-guards.tstests/helpers/exit.test.tstests/helpers/stream-guards.test.tstests/integration/stream-guards.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: Smoke Test (Windows) / Windows
- GitHub Check: Lint, Test & Check
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (csharp)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (13)
tests/**/*.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-009-platform-detection-helper.md)
In test files, use
_resetPlatformCache()to simulate different platforms instead of mocking or mutatingprocess.platformdirectly.
tests/**/*.ts: Use Bun's built-inbun testrunner for all tests; do not use Jest, Vitest, or custom assertions.
Mirror thesrc/directory structure intests/, and name test files<module-name>.test.ts.
Usemkdtempfor filesystem-test isolation, keep writes inside the temporary directory, and clean up temporary resources inafterEachorafterAll.
Test each module's public interface with descriptive names; do not test private internals.
Every runnable test must contain anexpect()assertion; usetest.skiportest.todofor placeholders and do not leave assertion-less or silently skipped tests.
Restore every captured environment variable withrestoreEnv(key, original)rather than assigning the captured value directly.
Mockos.homedir()via an imported module namespace andspyOn; do not overrideHOMEto control home-directory resolution. Environment overrides are valid only for code that readsBun.envat call time.
Mock first-party modules withimport * as modplusspyOn, restore them withmock.restore(), and never usemock.module()or an-implproduction split for first-party modules.
For HTTP mocking, saveglobalThis.fetchbefore replacing it and restore the direct assignment inafterEach; do not usemock.module("node:fetch").
Tests must not hit the network or touch real user-scope paths or other real state.
Wrap inlinespyOnormockImplementationlifecycles intry/finally, or manage them in hooks, somockRestore()always executes.
Close external SDK instances, servers, clients, and transports inafterEachorafterAll, not in test bodies.
Configuregit user.emailanduser.namelocally aftergit initand before committing in temporary repositories; never rely on global Git identity.
Inject small threshold values into threshold tests instead of generating thousands of file...
Files:
tests/helpers/exit.test.tstests/integration/stream-guards.test.tstests/helpers/stream-guards.test.ts
{src,tests}/**/*.ts
📄 CodeRabbit inference engine (.archgate/adrs/LEGAL-001-spdx-license-headers.md)
{src,tests}/**/*.ts: Every TypeScript source file insrc/andtests/must begin with// SPDX-License-Identifier: Apache-2.0followed by// Copyright 2026 Archgate.
If a TypeScript file has a shebang line (for example#!/usr/bin/env buninsrc/cli.ts), the SPDX license header must appear immediately after the shebang.
Use single-line//comments for the SPDX header; do not use block comments (/* */) or alternate license identifiers.
Files:
tests/helpers/exit.test.tstests/integration/stream-guards.test.tssrc/cli.tssrc/helpers/exit.tstests/helpers/stream-guards.test.tssrc/helpers/stream-guards.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-006-dependency-policy.md)
**/*.{ts,tsx}: Prefer Bun built-ins for file I/O, HTTP, globbing, testing, and subprocess execution; prefernode:built-in modules over npm alternatives when appropriate.
UseBun.spawnwith array-based arguments for all subprocess execution; do not useBun.$because it can hang on Windows.
Do not add npm packages for functionality already provided by Bun, such asglob,chalk, or utility libraries used for a single function.
Use Bun APIs such asBun.file()instead of Node.js-specific APIs such asfs.readFile()when Bun provides an equivalent.
Use relative imports with Bun's native module resolution; do not use TypeScript path aliases.Use TypeScript strict mode with ESNext and ES modules; derive schema types with
z.infer<>rather than defining separate interfaces.
Files:
tests/helpers/exit.test.tstests/integration/stream-guards.test.tssrc/cli.tssrc/helpers/exit.tstests/helpers/stream-guards.test.tssrc/helpers/stream-guards.ts
**/*.{rules.ts,ts}
📄 CodeRabbit inference engine (.archgate/adrs/LEGAL-002-dependency-license-compatibility.md)
Newly encountered permissive licenses may be added to the LEGAL-002 allowlist in the rule implementation, with a comment explaining any override for packages whose package.json lacks a license field.
Files:
tests/helpers/exit.test.tstests/integration/stream-guards.test.tssrc/cli.tssrc/helpers/exit.tstests/helpers/stream-guards.test.tssrc/helpers/stream-guards.ts
tests/**/*.test.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-025-idiomatic-bun-test-parametrization-and-matchers.md)
tests/**/*.test.ts: Usetest.each()for the same assertion logic against multiple independent inputs, anddescribe.each()when each input requires a group of related tests. Do not register tests or run independent assertions insidefor/.forEachloops.
Use array rows for positionaltest.each()arguments and object rows for named fields, with descriptive title placeholders such as%s,%p,%d, or$field.
Assert derived facts with specific matchers rather than collapsing booleans into.toBe(true)or.toBe(false): compare values directly with.toBe()/.toEqual(), use.toContain()or.toMatch()for membership and substrings,.toBeInstanceOf(Array)for array checks,.toHaveLength()for counts, and.find()with.toBeDefined()/.toBeUndefined()for predicate existence checks.
Do not precompute a boolean solely for assertion; assert directly on the underlying values so failures expose the expected and received values.
When converting a loop totest.each()ordescribe.each(), preserve every assertion that ran per iteration; do not drop or merge assertions across cases.
Files:
tests/helpers/exit.test.tstests/integration/stream-guards.test.tstests/helpers/stream-guards.test.ts
{src,tests,lint,scripts,shims}/**/*.ts
📄 CodeRabbit inference engine (.archgate/adrs/GEN-004-concise-forward-only-code-comments.md)
{src,tests,lint,scripts,shims}/**/*.ts: Project-authored TypeScript comments must be concise, describe current behavior only, and never narrate history, relocations, refactors, or how the code came to be.
A contiguous run of whole-line comments must contain at most five lines of narrative prose; longer rationale belongs in an ADR, agent-memory file, issue, or PR with a pointer. Tests and fixtures follow the same limit.
Use structural TSDoc tags such as@param,@returns,@throws,@example, and@seefor structured documentation; tagged sections are exempt from the five-line narrative bound, while@remarks,@description,@summary,@notes,@todo, and@fixmeremain counted as prose.
Files:
tests/helpers/exit.test.tstests/integration/stream-guards.test.tssrc/cli.tssrc/helpers/exit.tstests/helpers/stream-guards.test.tssrc/helpers/stream-guards.ts
**
⚙️ CodeRabbit configuration file
**: This project uses Archgate — an AI governance framework based on
Architecture Decision Records (ADRs). The ADRs in.archgate/adrs/
are the authoritative rules for this codebase. Each ADR has a companion
.rules.tsfile with automated checks that run viaarchgate check.When reviewing, you must:
- Treat ADR violations as blocking issues, not suggestions.
- Cite the specific ADR ID when flagging a violation (e.g., "Violates ARCH-006").
- Focus on semantic and contextual violations that automated rules cannot catch —
the.rules.tsfiles already cover syntactic/structural patterns.- If you are unsure whether something violates an ADR, flag it as a question
rather than approving it.
Files:
tests/helpers/exit.test.tstests/integration/stream-guards.test.tssrc/cli.tssrc/helpers/exit.tstests/helpers/stream-guards.test.tssrc/helpers/stream-guards.ts
src/**/!(*platform).ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-009-platform-detection-helper.md)
src/**/!(*platform).ts: Insrc/TypeScript source files, do not readprocess.platformdirectly; usesrc/helpers/platform.ts(isWindows(),isMacOS(),isLinux(),isWSL(),getPlatformInfo()) for all platform detection.
Use the centralized platform helper instead of duplicating OS/WSL detection logic inline anywhere insrc/TypeScript source.
When behavior differs between Linux and Windows, account for WSL by usingisWSL()rather than assuming `
Files:
src/cli.tssrc/helpers/exit.tssrc/helpers/stream-guards.ts
src/**/*.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-010-prefer-bun-built-in-json-parsing.md)
src/**/*.ts: Useawait Bun.file(path).json()when reading JSON files in Bun TypeScript source code; do not useJSON.parse(await Bun.file(path).text())orJSON.parse(fs.readFileSync(path, "utf-8"))for file reads.
UseBun.JSONC.parse()when reading files that may contain comments, such astsconfig.json, instead of plainJSON.parse()on file contents.
ReserveJSON.parse()for parsing JSON strings from non-file sources such as API responses or string variables; do not use it as the default for reading JSON files.
src/**/*.ts: In TypeScript source files undersrc/, do not create barrel files: anindex.tscontaining onlyexport,export type, orimport typestatements and no executable logic is forbidden.index.tsfiles containing real logic are permitted.
In TypeScript source files undersrc/, do not use re-exports such asexport { X } from './module'orexport type { X } from './module'; imports must point directly to the module that defines the symbol.
In TypeScript source files undersrc/, do not import from directory paths or barrel entry points; use explicit paths to the defining source module, such as../engine/loaderrather than../engineor../engine/index.
src/**/*.ts: All subprocess execution in Archgate source files MUST useBun.spawnwith array-based arguments; do not useBun.$, import$frombun, or usenode:child_process.
Do not use shell features such as pipes, redirects, or globbing inBun.spawnarguments; execute commands directly with separate array arguments.
When capturing subprocess output, configurestdoutandstderras pipes, consume streams withnew Response(...).text(), drain stdout and stderr concurrently, and awaitproc.exited.
Usestdout: "inherit"andstderr: "inherit"for subprocesses whose output belongs on the terminal, and pass command working directories through thecwdoption.
Wrap CLI availability checks intry/catchand return a boolean when the command may no...
Files:
src/cli.tssrc/helpers/exit.tssrc/helpers/stream-guards.ts
src/cli.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-001-command-structure.md)
src/cli.ts: Every command must be explicitly imported and registered insrc/cli.ts; command auto-discovery such asexecutableDir()is forbidden.
All asynchronous bootstrap logic insrc/cli.tsmust be wrapped in anasync function main()and invoked withmain().catch((err) => { logError(String(err)); process.exit(2); }); top-levelawaitis forbidden.Keep the CLI entry point compatible with Bun, including the
#!/usr/bin/env bunshebang and the minimum user-facing Bun version enforcement.
src/cli.ts: KeepinstallStreamErrorGuards()as the first pre-main guard after the Bun check so stream error listeners attach before any output is written.
Inmain().catch(), handleUserErroras an expected failure by callinglogError()and exiting with code 1 without Sentry capture; only non-UserErrorerrors should follow the exit-2 andcaptureException()path.
Files:
src/cli.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: For user-scope editors, resolve paths using the editor's actual path helper; do not assume Windows conventions. For opencode, mirrorxdg-basedir, which falls back to~/.configon all platforms.
For opencode-gated behavior, useisOpencodeAvailable()rather thanisOpencodeCliAvailable()alone because the Desktop distribution has no CLI binary and shares the config directory.
For Copilot-gated behavior, useisCopilotAvailable()rather thanisCopilotCliAvailable()alone because desktop and CLI distributions share~/.copilot/.
Files:
src/cli.tssrc/helpers/exit.tssrc/helpers/stream-guards.ts
src/helpers/stream-guards.ts
📄 CodeRabbit inference engine (.archgate/adrs/ARCH-002-error-handling.md)
Handle EPIPE as follows: stdout EPIPE exits quietly with code 0 without logging or Sentry capture; stderr EPIPE is swallowed without exiting; all other stream errors must be rethrown.
Files:
src/helpers/stream-guards.ts
.archgate/adrs/**/*.{md,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Read relevant self-governance ADRs before architectural changes; ADRs use YAML frontmatter and companion
.rules.tsfiles exporting a plain object satisfyingRuleSet.
Files:
.archgate/adrs/ARCH-002-error-handling.md
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: archgate/cli
Timestamp: 2026-08-05T06:47:20.551Z
Learning: Reviewers must ensure expected failures are distinguished from internal bugs: expected failures use exit code 1, unexpected failures use exit code 2, and actionable error messages are provided where possible.
📚 Learning: 2026-07-15T22:55:51.978Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 476
File: tests/helpers/telemetry-config.test.ts:24-28
Timestamp: 2026-07-15T22:55:51.978Z
Learning: In this Bun/TypeScript codebase, when a unit under test spawns subprocesses via Bun.spawn (e.g., running `git credential ...`), prefer overriding relevant env vars (such as `HOME`, `GIT_CONFIG_GLOBAL`, `GIT_CONFIG_NOSYSTEM`) using `process.env` in the test and restoring them with the test utility (e.g., `restoreEnv` from `tests/test-utils.ts`). Avoid relying on `spyOn(os, 'homedir')` for this purpose, because it only affects in-process calls and does not change the environment inherited by subprocesses; env-var overrides should be used for subprocess-level isolation and must be applied at call time.
Applied to files:
tests/helpers/exit.test.tstests/helpers/stream-guards.test.ts
📚 Learning: 2026-07-15T22:56:35.415Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 476
File: tests/commands/clean.test.ts:61-62
Timestamp: 2026-07-15T22:56:35.415Z
Learning: When reviewing tests that rely on src/helpers/paths.ts `internalPath()`, note that `internalPath()` intentionally reads `Bun.env.HOME ?? Bun.env.USERPROFILE` at call time and only uses `os.homedir()` if neither env var is set. Therefore, don’t suggest changing tests to `spyOn(os, "homedir")` for this behavior; instead, use per-test `Bun.env.HOME` / `Bun.env.USERPROFILE` overrides (as applicable) so the tests control `internalPath()`’s inputs.
Applied to files:
tests/helpers/exit.test.tstests/helpers/stream-guards.test.ts
📚 Learning: 2026-07-25T00:05:58.884Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 496
File: tests/helpers/auth.test.ts:38-46
Timestamp: 2026-07-25T00:05:58.884Z
Learning: When reviewing the Archgate CLI repository’s GEN-004 “concise forward-only narration” comments, don’t rely only on the automated phrase-based narration checks. Those checks can pass even when the comment wording describes historical/transfer semantics rather than current behavior (e.g., saying a prior restore “leaked” a value or a later subprocess “inherited it”). Manually verify that the comment describes the code’s current, forward behavior; flag or adjust comments that imply past/historical state transfer even if GEN-004 enforcement passes.
Applied to files:
tests/helpers/exit.test.tstests/integration/stream-guards.test.tssrc/cli.tssrc/helpers/exit.tstests/helpers/stream-guards.test.tssrc/helpers/stream-guards.ts
📚 Learning: 2026-07-25T00:05:59.109Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 496
File: src/cli.ts:0-0
Timestamp: 2026-07-25T00:05:59.109Z
Learning: Code comments may include a concise issue/PR reference (per GEN-004) when it’s used to point readers to fuller rationale instead of inlining that rationale. During review, flag surrounding comment prose that reads like historical context or narrates refactors/relocations; a bare GEN-004-style reference is allowed and should not be flagged by itself.
Applied to files:
tests/helpers/exit.test.tstests/integration/stream-guards.test.tssrc/cli.tssrc/helpers/exit.tstests/helpers/stream-guards.test.tssrc/helpers/stream-guards.ts
📚 Learning: 2026-08-04T19:58:05.877Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 543
File: src/helpers/copilot-user-settings.ts:0-0
Timestamp: 2026-08-04T19:58:05.877Z
Learning: In archgate/cli TypeScript code, use `Bun.file(path).exists()` only to check whether a file exists; it must not be used for directory existence checks. For helpers such as `isCopilotAvailable()` that need to detect a configuration directory, use an appropriate directory-aware check such as `existsSync` from `node:fs`.
Applied to files:
tests/helpers/exit.test.tstests/integration/stream-guards.test.tssrc/cli.tssrc/helpers/exit.tstests/helpers/stream-guards.test.tssrc/helpers/stream-guards.ts
📚 Learning: 2026-07-25T15:44:40.668Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 501
File: .archgate/adrs/ARCH-005-testing-standards.md:0-0
Timestamp: 2026-07-25T15:44:40.668Z
Learning: In Archgate CLI test code governed by ARCH-007, only allow `Bun.$` in test suites that are explicitly restricted to a single platform. Any cross-platform test that runs on Linux, macOS, and Windows must avoid `Bun.$` and instead use array-based `Bun.spawn`. For shared git setup used by tests, import and use the `git()` helper from `tests/test-utils.ts` rather than duplicating git setup logic.
Applied to files:
tests/helpers/exit.test.tstests/integration/stream-guards.test.tstests/helpers/stream-guards.test.ts
📚 Learning: 2026-07-25T22:03:14.216Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 501
File: .archgate/adrs/ARCH-002-error-handling.md:0-0
Timestamp: 2026-07-25T22:03:14.216Z
Learning: In Archgate boundary-wrapped CLI command actions (the handlers that rely on `handleCommandError()` for user-facing error output), expected-failure guards should signal user errors by throwing `new UserError(<message/details>)` rather than directly calling `logError()` followed by `exitWith(1)`. This keeps user-facing logging and the exit path centralized in `handleCommandError()`. For normal/computed command outcomes (e.g., `const exitCode = getExitCode(await runChecks(...))`), use `await exitWith(exitCode)` instead of calling `process.exit(exitCode)` so telemetry/Sentry flushing and outcome tagging still run.
Applied to files:
tests/helpers/exit.test.tstests/integration/stream-guards.test.tssrc/cli.tssrc/helpers/exit.tstests/helpers/stream-guards.test.tssrc/helpers/stream-guards.ts
📚 Learning: 2026-07-25T23:21:49.190Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 512
File: tests/engine/git-files.test.ts:98-100
Timestamp: 2026-07-25T23:21:49.190Z
Learning: When reviewing archgate/cli for ARCH-006 (per its ADR frontmatter), only enforce the production-dependency policy scoped to package.json. Do not treat test-only refactors or relocated `node:fs` fixture writes as an ARCH-006 violation (since ARCH-006 does not govern test-file I/O API selection). If there’s a broader/test-wide refactor that would migrate fixture writing to `Bun.write()`, evaluate it separately under the appropriate in-scope rule.
Applied to files:
tests/helpers/exit.test.tstests/integration/stream-guards.test.tstests/helpers/stream-guards.test.ts
📚 Learning: 2026-07-27T16:05:38.683Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 536
File: tests/commands/adr/sync-strict.test.ts:173-173
Timestamp: 2026-07-27T16:05:38.683Z
Learning: In this Bun + TypeScript repo, for rejected-promise assertions use the unawaited form: `expect(promise).rejects.toThrow(...)`. Do NOT add `await` to `expect(promise).rejects.toThrow(...)` (Bun’s types model this as `void`), because it will violate the type-aware oxlint rules `typescript(await-thenable)` and `typescript(no-confusing-void-expression)`. Only request an `await` if the repo adopts a typed, lint-compliant assertion helper or Bun’s typings change.
Applied to files:
tests/helpers/exit.test.tstests/integration/stream-guards.test.tstests/helpers/stream-guards.test.ts
📚 Learning: 2026-06-11T12:50:28.661Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 406
File: .claude/agent-memory/archgate-developer/feedback_prefer_tests_over_adr_rules.md:8-18
Timestamp: 2026-06-11T12:50:28.661Z
Learning: In `archgate/cli`, for markdown files under `.claude/agent-memory/`, follow the established convention: use YAML frontmatter (with a `name:` field used as the document title) and do not require a top-level `#` (H1) heading. During code review, do not flag missing first-line/first-top-level H1 headings (e.g., MD041) for these agent-memory files since markdownlint is not part of the repo’s `bun run validate` lint pipeline (oxlint/oxfmt only).
Applied to files:
.claude/agent-memory/archgate-developer/MEMORY.md
📚 Learning: 2026-07-25T00:05:20.592Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 496
File: .claude/agent-memory/archgate-developer/project_test_isolation_gotchas.md:10-10
Timestamp: 2026-07-25T00:05:20.592Z
Learning: When reviewing documentation/agent-memory entries under `.claude/agent-memory/**`, do not enforce GEN-004’s “forward-only” comment/narrative requirement. These entries are allowed to keep historical/past-tense incident narratives and dated markers (e.g., `Found YYYY-MM-DD`) because the context is intended to help future agents evaluate edge cases. Outside this scope, GEN-004’s forward-only rule should still apply.
Applied to files:
.claude/agent-memory/archgate-developer/MEMORY.md
📚 Learning: 2026-07-02T16:03:33.031Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 446
File: src/helpers/session-context-opencode.ts:81-100
Timestamp: 2026-07-02T16:03:33.031Z
Learning: For synchronous helper functions that use Bun’s sqlite sync API (i.e., they must remain synchronous), it’s acceptable to use `existsSync` from `node:fs` to check whether the SQLite database file exists. Avoid using `Bun.file(path).exists()` for this purpose because it’s async and would force the helper to become async (no equivalent synchronous Bun alternative). If the DB file is missing, throw/return a clear, actionable "No database found" error (per ARCH-006) rather than letting the sqlite open fail with a generic DB-open error.
Applied to files:
src/helpers/exit.tssrc/helpers/stream-guards.ts
📚 Learning: 2026-07-11T13:03:15.386Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 467
File: .archgate/adrs/ARCH-011-consistent-project-root-resolution.md:0-0
Timestamp: 2026-07-11T13:03:15.386Z
Learning: For Markdown files formatted by oxfmt (especially ADRs), avoid inline code spans that contain escaped backticks, e.g. `\`...\`` inside a single `` `...` `` span. oxfmt may mis-parse these and, on re-format, can collapse spaces after later inline code spans on the same line, effectively removing any manually re-added spacing. Instead, rephrase the text so the message stays plain quoted text, and put any embedded command/fragment that needs code formatting (e.g., `archgate init`) in its own separate inline code span; keep surrounding punctuation/spacing outside the code span.
Applied to files:
.archgate/adrs/ARCH-002-error-handling.md
📚 Learning: 2026-07-25T16:24:51.133Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 501
File: .archgate/adrs/ARCH-003-output-formatting.md:0-0
Timestamp: 2026-07-25T16:24:51.133Z
Learning: In Archgate ADRs (.archgate/adrs/*.md), omit quantitative claims (e.g., token savings, benchmarks, performance deltas) unless they are backed by a reproducible measurement and supported by a single cited reference. If you cannot satisfy both (reproducible measurement + exactly one cited reference), describe the benefit qualitatively and tie it to the relevant policy/requirements instead of using numeric estimates.
Applied to files:
.archgate/adrs/ARCH-002-error-handling.md
📚 Learning: 2026-07-25T22:03:17.073Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 501
File: .archgate/adrs/ARCH-015-cli-command-documentation-coverage.md:17-18
Timestamp: 2026-07-25T22:03:17.073Z
Learning: When updating an ADR that documents rule discovery/enforcement behavior, ensure the ADR’s stated discovery contract matches the implementation in code. If the rule only discovers commands by scanning `src/commands/*.ts` and `src/commands/*/index.ts`, the ADR must not claim it also inspects command registration calls elsewhere (e.g., `src/cli.ts`). Any ADR language that changes the documented contract should be treated as a normative change to behavior and aligned with the corresponding implementation/issue, not as prose-only documentation compression.
Applied to files:
.archgate/adrs/ARCH-002-error-handling.md
📚 Learning: 2026-07-26T13:09:49.888Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 533
File: .archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md:0-0
Timestamp: 2026-07-26T13:09:49.888Z
Learning: In archgate/cli rule ADRs, `ctx.scopedFiles` is computed from the ADR frontmatter `files` glob patterns before the rule context is constructed. For ARCH-020-style rules, ensure the ADR `files` frontmatter correctly scopes the allowed paths (e.g., `files: ["src/**/*.ts"]`); then rule-specific `.ts`/file filters should assume the incoming file list is already restricted and avoid re-applying the same path-prefix restriction inside individual rules.
Applied to files:
.archgate/adrs/ARCH-002-error-handling.md
🔇 Additional comments (8)
.archgate/adrs/ARCH-002-error-handling.md (1)
42-43: LGTM!Also applies to: 147-147
src/helpers/exit.ts (1)
105-115: LGTM!Also applies to: 152-160
tests/helpers/exit.test.ts (1)
9-9: LGTM!Also applies to: 71-93
src/helpers/stream-guards.ts (1)
1-80: LGTM!tests/helpers/stream-guards.test.ts (1)
1-95: LGTM!src/cli.ts (1)
42-42: LGTM!Also applies to: 60-65
.claude/agent-memory/archgate-developer/MEMORY.md (1)
29-29: LGTM!tests/integration/stream-guards.test.ts (1)
1-2: LGTM!Also applies to: 11-36, 38-103
Code Coverage
Full HTML report available in workflow artifacts. Per-directory breakdown
|
An EPIPE reaching handleCommandError() or main().catch() has no stream identity - it can come from a subprocess pipe or a socket, not only process.stdout - so treating it as success there masks real command failures from scripts and CI. The stream guards attached to process.stdout/stderr are the only place the origin is known, and they fully cover the crash mechanism (Bun delivers write EPIPE as a stream error event, never through the promise chain). Also use Bun.write() for the temp script in the async integration test helper. Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
# archgate ## [0.52.0](v0.51.0...v0.52.0) (2026-08-06) ### ⚠ BREAKING CHANGES * add --strict and --output <format> (SARIF), remove --json/--ci/--max-warnings from check (#536) ### Features * add --strict and --output <format> (SARIF), remove --json/--ci/--max-warnings from check ([#536](#536)) ([70b1ede](70b1ede)) * **dist:** distribute archgate via winget ([#552](#552)) ([93cb3a8](93cb3a8)), references [#544](#544) * **lint:** migrate to TypeScript 7 and adopt oxlint type-aware linting ([#534](#534)) ([29daad8](29daad8)), references [#529](#529) * **plugin:** install Copilot plugin declaratively, covering the desktop app ([#543](#543)) ([ae2b988](ae2b988)) ### Bug Fixes * bound scan loop, narrow install-method types, and extend ARCH-021 to markdown code spans ([#548](#548)) ([3793d80](3793d80)), closes [#541](#541) [#540](#540) [#515](#515), references [#541](#541) [#540](#540) [#515](#515) * **cli:** exit 0 quietly when the output pipe closes (EPIPE) ([#546](#546)) ([e7bfa19](e7bfa19)) * **hooks:** invoke hooks through package scripts instead of a shell ([#557](#557)) ([3b9e411](3b9e411)), references [#442](#442) [#441](#441) [#442](#442) --- This PR was generated with [simple-release](https://github.com/TrigenSoftware/simple-release). <details> <summary>📄 Cheatsheet</summary> <br> You can configure the bot's behavior through a pull request comment using the `!simple-release/set-options` command. ### Command Format ````md !simple-release/set-options ```json { "bump": {}, "publish": {} } ``` ```` ### Useful Parameters #### Bump | Parameter | Type | Description | |-----------|------|-------------| | `version` | `string` | Force set specific version | | `as` | `'major' \| 'minor' \| 'patch' \| 'prerelease'` | Release type | | `prerelease` | `string` | Pre-release identifier (e.g., "alpha", "beta") | | `firstRelease` | `boolean` | Whether this is the first release | | `skip` | `boolean` | Skip version bump | | `byProject` | `Record<string, object>` | Per-project bump options for monorepos | #### Publish | Parameter | Type | Description | |-----------|------|-------------| | `skip` | `boolean` | Skip publishing | | `access` | `'public' \| 'restricted'` | Package access level | | `tag` | `string` | Tag for npm publication | ### Usage Examples #### Force specific version ````md !simple-release/set-options ```json { "bump": { "version": "2.0.0" } } ``` ```` #### Force major bump ````md !simple-release/set-options ```json { "bump": { "as": "major" } } ``` ```` #### Create alpha pre-release ````md !simple-release/set-options ```json { "bump": { "prerelease": "alpha" } } ``` ```` #### Publish with specific access and tag ````md !simple-release/set-options ```json { "bump": { "prerelease": "beta" }, "publish": { "access": "public", "tag": "beta" } } ``` ```` ### Custom Changelog Preamble You can add custom markdown to the top of the changelog (right after the version header) using the `!simple-release/set-preamble` command. The markdown after the command line becomes the preamble. ```md !simple-release/set-preamble ## What's new? - The website was completely redesigned - The new API gives you awesome possibilities ``` In a monorepo, pass the full package name after the command to target a single package's changelog. Wrap the name in backticks so GitHub keeps it as text instead of a mention: ```md !simple-release/set-preamble `@your-org/core` ## Core changes - New plugin system ``` Use one comment per package, plus one without a name for the whole release. ### Access Restrictions The commands can only be used by users with permissions: - repository owner - organization member - collaborator ### Notes - The last comment with `!simple-release/set-options` command takes priority - The last `!simple-release/set-preamble` comment per package takes priority - JSON must be valid, otherwise the `set-options` command will be ignored - Parameters apply only to the current release execution - The commands can be updated by editing the comment or adding a new one </details> <!-- Please do not edit this comment. simple-release-pull-request: true simple-release-branch-from: release simple-release-branch-to: main --> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
What
Fixes the fatal
EPIPE: broken pipe, writecrash (Sentry issue CLI-7) that fires when CLI output is piped and the reader closes early — e.g.archgate adr list | head, or an agent harness tearing down mid-initwhile an Inquirer prompt is rendering.Why
Bun delivers a write-to-closed-pipe failure as an
errorevent onprocess.stdout/process.stderr. With no listener attached, the event escalates to a fatal uncaught exception (auto.node.onuncaughtexception), crashing the CLI and reporting a non-bug to Sentry. By pipeline convention, a consumer that stops reading means "done" — success, not an error.How
src/helpers/stream-guards.ts(new):errorlisteners on both streams. stdout EPIPE → quiet exit 0 (re-entrancy-guarded — Bun re-emits EPIPE on every subsequent write); stderr EPIPE → swallowed (stdout may still have a live consumer); any other stream error → rethrown so real bugs still crash loudly and reach Sentry.src/helpers/exit.ts:isEpipeError()+exitForBrokenPipe()(exit 0, telemetry outcomecancelled/broken_pipe, no Sentry capture, no logging — the output channel is gone).handleCommandError()short-circuits EPIPE.src/cli.ts: guards installed as the first pre-main step, before any output;main().catch()handles a synchronously-thrown EPIPE the same way..archgate/adrs/ARCH-002: Decision section now codifies the broken-pipe policy; Compliance checklist requires the guards to stay first incli.ts.Verification
| head -c 100): guarded → silent exit 0; unguarded → fatal crash reproducing the Sentry stack.errorevent on the real streams because a genuine OS-level pipe close is not portable to arrange from bun:test (a spawned child's cancelled stdout stays open on Windows).bun run validatepasses end-to-end (1963 tests, ADR check 50/50, knip, build check).Reviewer notes
git log | headsemantics.