fix(event): keep bounded consume alive after stdin EOF#1183
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR refines the stdin EOF shutdown behavior in the event consumer. A new ChangesStdin EOF Shutdown Gating
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@cmd/event/consume.go`:
- Around line 375-377: The predicate shouldWatchStdinEOF currently uses strict
equality to zero for maxEvents and timeout which mismatches downstream semantics
where lifecycle bounds are active only when > 0; change the checks in
shouldWatchStdinEOF (the function determining whether to watch stdin EOF) to use
<= 0 for maxEvents and timeout so negative values are treated as unbounded like
consume.Options / listeningText / exitReason downstream, ensuring a consistent
decision about watching stdin EOF.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 50b2fd9e-b363-40a6-88cf-520f0450a4db
📒 Files selected for processing (2)
cmd/event/consume.gocmd/event/consume_stdin_test.go
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1183 +/- ##
=======================================
Coverage 68.92% 68.92%
=======================================
Files 629 629
Lines 58762 58764 +2
=======================================
+ Hits 40503 40505 +2
Misses 14952 14952
Partials 3307 3307 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@db4a09c361f9eb13fdb8ebb7d73f27e18846a902🧩 Skill updatenpx skills add sjh9714/cli#fix-event-consume-bounded-stdin -y -g |
Summary
Fixes #1131.
event consumedocumented--max-events/--timeoutas the way to run bounded non-interactive consumers, but the non-TTY stdin EOF watcher still canceled those runs immediately when a subprocess closed stdin. This keeps stdin EOF as the lifecycle signal only for unbounded non-TTY consumers, so bounded runs can exit by their own limit or timeout.Changes
--max-events, and--timeoutcases.Test Plan
lark-cli event consumeflow works as expected; not run because this local environment is not configured with Lark app credentials.Commands run:
go test ./cmd/event -run TestShouldWatchStdinEOF -count=1(failed before the fix)go test ./cmd/event -run 'TestShouldWatchStdinEOF|TestWatchStdinEOF' -count=1go test ./cmd/event -count=1make unit-testgo vet ./...gofmt -l .go mod tidy && git diff --exit-code -- go.mod go.summake buildgo run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev=origin/maingit diff --checkmake test(unit/vet/examples stages passed; integration tests then failed because locallark-cliconfig is not configured, witherror.type=config/message=not configured)Note: I used Codex while preparing this change, and I reviewed the final diff and ran the listed checks locally.
Summary by CodeRabbit
consumecommand no longer stops on stdin closure for bounded runs; stdin EOF is only observed for non-terminal sessions with no max-events or timeout set.