Skip to content

Comments

fix: remove stdin timeout that caused race condition with piped input#16

Merged
gulivan merged 1 commit intomainfrom
fix/stdin-pipe-detection
Oct 23, 2025
Merged

fix: remove stdin timeout that caused race condition with piped input#16
gulivan merged 1 commit intomainfrom
fix/stdin-pipe-detection

Conversation

@gulivan
Copy link
Contributor

@gulivan gulivan commented Oct 23, 2025

The 10ms timeout in readStdin() was firing before piped data could arrive when chaining contextcalc commands (e.g., contextcalc --output ast | contextcalc). This caused the second command to skip stdin and scan the current directory instead.

Since we only call readStdin() when !process.stdin.isTTY (stdin is piped), we can safely wait for data without an arbitrary timeout. The function now properly waits for stdin to end naturally.

Fixes the issue where: bunx contextcalc . --output ast | bunx contextcalc would not detect piped stdin data.

Summary by CodeRabbit

  • Refactor
    • Streamlined stdin data collection in the CLI, removing legacy timeout logic and simplifying stream handling for improved efficiency.

The 10ms timeout in readStdin() was firing before piped data could arrive
when chaining contextcalc commands (e.g., contextcalc --output ast | contextcalc).
This caused the second command to skip stdin and scan the current directory instead.

Since we only call readStdin() when !process.stdin.isTTY (stdin is piped),
we can safely wait for data without an arbitrary timeout. The function now
properly waits for stdin to end naturally.

Fixes the issue where: bunx contextcalc . --output ast | bunx contextcalc
would not detect piped stdin data.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 23, 2025

Walkthrough

The readStdin function in src/cli.ts was simplified by removing a 10ms timeout fallback mechanism and the hasData tracking variable. The function now relies solely on collecting data chunks and resolving on the end-of-stream event, eliminating the early empty-string resolution path.

Changes

Cohort / File(s) Summary
stdin input simplification
src/cli.ts
Removed short-timeout (10ms) and hasData tracking from readStdin function, replacing conditional early-resolution logic with direct end-of-stream handling

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A timeout removed, the logic runs clean,
No early escape where data's unseen,
Stream waits for the end, as it should,
Simpler code, just like it should,
Less is more—hooray for that good! 🎉

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "fix: remove stdin timeout that caused race condition with piped input" directly and accurately describes the main change in the pull request. The title clearly identifies the specific problem being addressed (the 10ms timeout causing a race condition) and the solution (removing the timeout). It is concrete and specific rather than vague, making it immediately clear to someone scanning the repository history that this addresses a stdin handling issue with piped input. The title aligns perfectly with the stated PR objectives and the actual code changes, which removed the timeout mechanism from the readStdin function.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/stdin-pipe-detection

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e641d5a and bb4b366.

📒 Files selected for processing (1)
  • src/cli.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Do not use dotenv; Bun loads .env automatically
Use Bun.serve() for HTTP/WebSocket/HTTPS routes; do not use Express
Use bun:sqlite for SQLite; do not use better-sqlite3
Use Bun.redis for Redis; do not use ioredis
Use Bun.sql for Postgres; do not use pg or postgres.js
Use built-in WebSocket; do not use ws
Prefer Bun.file over node:fs readFile/writeFile
Use Bun.$ for shelling out instead of execa

Files:

  • src/cli.ts
src/cli.ts

📄 CodeRabbit inference engine (CLAUDE.md)

ContextCalc CLI entry point is src/cli.ts

Files:

  • src/cli.ts
🔇 Additional comments (1)
src/cli.ts (1)

39-53: Excellent fix for the stdin race condition!

Removing the 10ms timeout is the correct solution. Since readStdin() is only invoked when !process.stdin.isTTY (line 160), stdin is guaranteed to be piped and will emit an 'end' event naturally when the upstream process completes. The arbitrary timeout could fire prematurely before piped data arrived, causing the consumer to fall through to directory scanning instead of processing stdin.

The simplified implementation is cleaner and eliminates the race condition entirely.


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.

@gulivan gulivan merged commit ef6187b into main Oct 23, 2025
4 checks passed
github-actions bot pushed a commit that referenced this pull request Oct 23, 2025
## [1.4.2](v1.4.1...v1.4.2) (2025-10-23)

### Bug Fixes

* remove stdin timeout that caused race condition with piped input ([#16](#16)) ([ef6187b](ef6187b))
@github-actions
Copy link

🎉 This PR is included in version 1.4.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant