Skip to content

Fix: Claude review workflow SIGPIPE error (exit code 141)#103

Merged
mollup merged 10 commits into
mainfrom
fix-claude-review-sigpipe
May 4, 2026
Merged

Fix: Claude review workflow SIGPIPE error (exit code 141)#103
mollup merged 10 commits into
mainfrom
fix-claude-review-sigpipe

Conversation

@mollup
Copy link
Copy Markdown
Collaborator

@mollup mollup commented May 3, 2026

Problem

The Claude PR review workflow (.github/workflows/claude-review.yml) was failing with exit code 141 on all pull requests. This error occurs in the "Collect diff and file list" step:

git diff --no-color "$BASE"..."$HEAD" | head -c 200000 > .claude-review/diff.patch

Root Cause

Exit code 141 is SIGPIPE (Broken Pipe). It happens when:

  1. git diff starts writing output to stdout
  2. head -c 200000 reads exactly 200KB and then closes the pipe
  3. git diff tries to write more data to the now-closed pipe
  4. The OS sends SIGPIPE to git diff, causing it to exit with code 141

The set -euo pipefail directive at the top of the script causes the workflow to fail on this error, even though it's intentional and expected - we want to truncate the diff at 200KB.

Solution

Modified line 91 to explicitly ignore SIGPIPE (exit code 141):

git diff --no-color "$BASE"..."$HEAD" | head -c 200000 > .claude-review/diff.patch || test $? -eq 141

This tells bash: "Run this command, and if it fails, check if the exit code is 141. If it is, treat it as success."

The set -euo pipefail directive still applies to other errors - only SIGPIPE is ignored.

Testing

This fix has been tested locally and follows standard bash best practices for handling intentional pipe closures.

Impact

References

mollup added 10 commits May 3, 2026 14:15
- Add score fields (player1Score, player2Score) to BracketMatch type
- Add tournamentWinner field to BracketResponse
- Implement PUT /api/tournaments/:id/matches/:matchId/score endpoint
- Add submitMatchScore function to store with validation
- Update getTournamentBracket to include tournament winner
- Add comprehensive test suite with 14 test cases covering:
  - Score submission and validation
  - Bracket advancement and winner determination
  - Real-time updates
  - Score history preservation
  - Edge cases (byes, completed matches)
- All 73 tests passing
- Add Replay type to types.ts with all required fields
- Create replay storage in store.ts with Maps for replays and replaysByTournament
- Implement createReplay, getReplayById, getReplaysByTournament, validateReplayFileSize functions
- Create src/routes/replays.ts with POST /api/replays and GET /api/replays/:id
- Add GET /api/tournaments/:id/replays endpoint in tournaments.ts
- Implement 2GB file size limit validation
- Add authentication checks (organizer-only upload)
- Ensure organizers can only upload to their own tournaments
- Replays accessible publicly without authentication
- Add comprehensive test suite with 14 tests covering all edge cases
- All 96 tests passing
- Add searchReplays function to store.ts with filtering and pagination
- Support filtering by game (case-insensitive), event_id, and player_name
- Implement pagination with default page size of 20, max 100
- Return paginated results with metadata (total, page, pageSize, totalPages)
- Results sorted in reverse-chronological order by default
- Add GET /api/replays route to replays.ts
- Handle edge cases for invalid pagination parameters
- Performance tested for datasets up to 100 replays (< 500ms)
- Add comprehensive test suite with 21 tests
- All 117 tests passing
- Add startDate, venue, city optional fields to Tournament interface
- Update createTournament to accept new location/date fields
- Add searchEvents function to store.ts with filtering by game and city
- Filter out past events by default (events with startDate < now)
- Sort results by startDate ascending (soonest first)
- Create EventResponse interface with entrantCount
- Create src/routes/events.ts with GET /api/events endpoint
- Add events router to app.ts
- Performance optimized for large datasets (< 400ms)
- Add comprehensive test suite with 17 tests
- All 134 tests passing
- Add getPointsForPlacement function with tiered points system (1st=100, 2nd=75, 3-4th=50, 5-8th=25, participation=10)
- Create getLeaderboard function to aggregate stats across finalized tournaments
- Filter by game (required, case-insensitive)
- Support player_id query to return specific player's rank
- Implement pagination with default page size 20, max 100
- Sort by points descending with tiebreakers (total wins, total tournaments, userId)
- Only include finalized tournaments in calculations
- Create LeaderboardEntry and LeaderboardResponse interfaces
- Create src/routes/leaderboard.ts with GET /api/leaderboard endpoint
- Add leaderboard router to app.ts
- Add comprehensive test suite with 14 tests
- All 148 tests passing
- Added Follow interface to types.ts with followerId, followingId, createdAt
- Implemented follow storage with Maps for follows, followersByUser, followingByUser in store.ts
- Created store functions: createFollow, deleteFollow, getUserFollowing, getUserFollowers, isFollowing
- Added POST /api/follows endpoint to create follow relationships
- Added DELETE /api/follows/:id endpoint to remove follows with proper authorization
- Added GET /api/users/:id/following endpoint with pagination
- Added GET /api/users/:id/followers endpoint with pagination
- Prevents self-follows and duplicate follow attempts (409 status)
- All endpoints require authentication
- All 20 test cases passing
- Full test suite: 168 tests passing
- Added Subscription interface to types.ts with status, priceId, expiryDate, clientSecret
- Implemented subscription storage with Maps for subscriptions and subscriptionsByUser in store.ts
- Created store functions: createSubscription, getSubscriptionById, getUserSubscription, activateSubscription, cancelSubscription, hasActivePremiumSubscription
- Added POST /api/subscriptions endpoint to create subscriptions (organizer only)
- Added POST /api/subscriptions/webhook endpoint for Stripe webhooks (payment success/failure, cancellation)
- Added GET /api/subscriptions/:userId endpoint to view subscription status
- Added DELETE /api/subscriptions/:id endpoint to cancel subscription
- Added requirePremium middleware to gate premium features
- Added /api/premium/features test endpoint demonstrating premium feature gating
- Prevents duplicate active/pending subscriptions (409 status)
- Handles subscription expiry automatically
- Mock Stripe payment intent format for testing
- All 20 test cases passing
- Full test suite: 188 tests passing
The git diff command was causing exit code 141 (SIGPIPE) when piped to head -c.
This is expected behavior when the downstream command closes the pipe early.

Modified the command to ignore SIGPIPE (exit code 141) specifically while
still failing on other errors due to 'set -euo pipefail'.

Fixes: Exit code 141 error on PR CI checks
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 3, 2026

Claude automated code review

Summary

This PR fixes a SIGPIPE (exit code 141) error in the Claude review GitHub workflow by explicitly handling the expected broken-pipe condition when head truncates the diff at 200KB. The PR also introduces comprehensive new backend features (replays, event discovery, leaderboards, follows, subscriptions), extensive test coverage for these features, and a substantial automation architecture document.

Risk assessment

High — The PR bundles a critical workflow fix with substantial new backend features and 561 lines of generated automation documentation. While the workflow fix is low-risk, the scope makes it difficult to validate all changes holistically, and the generated documentation may need manual review.

Findings

[major] .github/workflows/claude-review.yml:96** — The SIGPIPE handling uses || test $? -eq 141which works but is non-standard. Clearer to use|| [ $? -eq 141 ]` or wrap in a function. Current syntax is fragile across shells.

[major] src/store.ts:522-630** — Missing input validation on searchReplays()andgetLeaderboard()pagination params: no explicit bounds-checking before array slicing. Maliciouspage` values could cause unexpected behavior.

[major] src/routes/replays.ts:56-61 and `src/routes/tournaments.ts:324-381** — Score submission endpoints allow tournament organizers to submit scores unilaterally without verification that the match is actually ready or that both players have checked in. No audit log.

[major] `AUTOMATION-ARCHITECTURE-SUMMARY.md:1-561** — This is auto-generated/LLM-produced content with outdated references (e.g., "April 20, 2026"), formatting inconsistencies, and unclear ownership. Should be human-reviewed or removed; if kept, mark as aspirational.

[minor] src/subscription.test.ts:152-169, 268-284** — Tests mock webhook behavior by directly calling POST /api/subscriptions/webhook`, but no signature verification is implemented. Real Stripe webhooks require HMAC validation; this could be exploited if exposed.

[minor] src/types.ts:69-72** — Added player1Scoreandplayer2ScoretoBracketMatch` but are nullable. Unclear when/why they're null during bracket lifecycle; should document the state machine.

[nit] `src/store.ts:512** — Comment says "Replay Functions" but code is in subscription reset section. Move or clarify.

Test coverage

Good coverage for new features (replays, leaderboards, follows, subscriptions, events); tests include happy paths, auth/authz checks, edge cases, and pagination. However:

  • Missing: tests for replay/subscription audit logs
  • Missing: tests for score submission when match status ≠ "ready"
  • Missing: tests for leaderboard tiebreaker edge cases (three-way ties)
  • No integration tests validating bracket advancement after score submission

Security

  • Authorization: Score submission only checks organizer role, not ownership of tournament or match readiness. Any organizer can submit scores to any tournament.
  • Input validation: Pagination params in searchReplays and getLeaderboard lack explicit bounds; could lead to memory exhaustion on large datasets.
  • Secrets: No issues identified; API keys are not exposed.
  • Webhook validation: Subscription webhook (POST /api/subscriptions/webhook) has no signature verification. Missing Stripe HMAC check.

Suggested follow-ups

  • Simplify SIGPIPE handling to a more portable idiom or wrap in a bash function.
  • Validate match state (status === "ready") before accepting score submissions.
  • Add HMAC signature verification to subscription webhook (see Stripe docs).
  • Remove or heavily revise AUTOMATION-ARCHITECTURE-SUMMARY.md; it reads like placeholder content.
  • Add audit logging for score changes (who submitted, when, previous/new values).
  • Document player1Score/player2Score null semantics in types or bracket module.

Advisory only — a human reviewer still approves the merge. Re-run with /claude-review in a comment.

@mollup mollup merged commit b5aa93c into main May 4, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant