Skip to content

Conversation

@vibaiher-qatium
Copy link

Summary

This PR fixes critical issues where Turnstyle wasn't properly waiting for previous workflow runs to complete, potentially causing concurrent deployments.

🔴 Issues Fixed

1. Missing Workflow Run States (src/github.ts)

  • Problem: Only queried 3 of 6+ active workflow states (in_progress, queued, waiting)
  • Missing: pending, requested, action_required
  • Impact: Turnstyle would skip runs in these states, allowing concurrent executions
  • Fix: Added queries for all missing states

2. Incorrect Filtering Logic (src/wait.ts)

  • Problem: Only filtered out runs with conclusion === 'success'
  • Impact: Workflows waited unnecessarily for failed/cancelled/timed-out runs
  • Fix: Changed to skip ALL completed runs (any conclusion), only wait for active runs (no conclusion)

3. Enhanced Test Coverage (tests/wait.test.ts)

  • Updated test to verify all 6 active states are properly handled
  • Added new test to verify failed/cancelled runs are correctly skipped

📊 Impact Assessment

  • Severity: High - Can cause concurrent deployments
  • Affected Scenarios:
    • Multiple quick commits/PRs triggering workflows
    • Workflows with approval steps (pending/action_required states)
    • Matrix builds that start in requested state
    • Repositories with high workflow trigger frequency

🧪 Testing

  • ✅ TypeScript compilation passes
  • ✅ Distribution bundle rebuilt
  • ⚠️ Unit tests have dependency issues (unrelated to changes)
  • Code changes are minimal and focused on the identified issues

📝 Changes

src/github.ts: Added API queries for pending, requested, and action_required states
src/wait.ts: Changed filtering logic to only wait for active runs (no conclusion)
tests/wait.test.ts: Enhanced test coverage for new states and filtering behavior
dist/index.js: Rebuilt distribution bundle with changes

🔗 References


🤖 Generated with Claude Code

vibaiher-qatium and others added 3 commits September 30, 2025 13:47
This commit addresses critical issues where Turnstyle wasn't properly
waiting for previous workflow runs to complete, potentially causing
concurrent deployments.

**Issues Fixed:**

1. **Missing workflow run states** (github.ts)
   - Added queries for `pending`, `requested`, and `action_required` states
   - Previously only queried `in_progress`, `queued`, and `waiting`
   - Missing states caused Turnstyle to skip active runs

2. **Incorrect filtering logic** (wait.ts)
   - Changed from only skipping successful runs to skipping all completed runs
   - Now only waits for active runs (those without a conclusion)
   - Previously waited unnecessarily for failed/cancelled runs

3. **Test coverage** (wait.test.ts)
   - Updated test to verify all 6 active states are handled
   - Added test for skipping failed/cancelled runs

**Impact:**
- Prevents concurrent workflow executions in high-frequency trigger scenarios
- Properly handles workflows with approval steps (pending/action_required states)
- No longer blocks on failed/cancelled runs

Refs: https://docs.github.com/en/rest/actions/workflow-runs

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
The test "will skip failed and cancelled runs" now expects 4 debug messages
instead of 2 because the wait() method is called twice in the test scenario:
- First iteration: filters out runs 1 & 2 (2 messages)
- Second iteration: filters out runs 1 & 2 again (2 more messages)

This reflects the actual behavior where completed runs are logged each time
they are filtered during the wait loop.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Resolved conflicts by combining:
- New workflow run states (pending, requested, action_required) from fix/workflow-run-states
- Console.log debugging statements from master
- Maintained both console.log for visibility and this.debug for test assertions

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@vibaiher-qatium
Copy link
Author

Let's wait for the debugging feature to see if this PR makes sense or not.

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