Skip to content

Conversation

@amikofalvy
Copy link
Collaborator

Summary

Updated the GitHub Actions CI workflow to prevent in-progress jobs from being cancelled when new commits are pushed to main/master branches. The workflow now only cancels in-progress jobs for pull request events, allowing push events to complete their CI runs for isolated deployments.

Linear Ticket

Closes https://linear.app/inkeep/issue/PRD-5254/github-action-ci-ci-push-shouldnt-be-cancelled-if-another-commit-is

Changes

  • Modified .github/workflows/ci.yml to conditionally set cancel-in-progress based on event type
  • cancel-in-progress: ${{ github.event_name == 'pull_request' }} ensures:
    • Pull requests: Cancel previous runs when new commits are pushed (standard PR behavior)
    • Push events (main/master): Allow all CI jobs to complete (supports parallel deployments)
    • Merge group events: Allow all CI jobs to complete

Testing

  • YAML syntax validated
  • Change follows GitHub Actions best practices for concurrency control
  • Will be tested when workflow runs on this PR and subsequent pushes

Checklist

  • Tests added/updated (N/A - workflow configuration change)
  • Documentation updated (N/A - change is self-documenting)
  • All tests passing (will verify when CI runs)
  • Type checking passing (N/A - YAML change)
  • Linting passing (N/A - YAML change)

- Remove AI-generated markers from PR body template
- Add explicit instructions to NOT commit .claude-coordinator-* files
- Replace 'git add .' with 'git add <specific-files>'
- Add note about following AGENTS.md commit guidelines
- Prevents future PRs from including coordinator prompt files
Update CI workflow to only cancel in-progress jobs for pull requests.
Push events to main/master will now allow parallel CI jobs to complete,
ensuring isolated deployments are not disrupted by subsequent commits.

Related to PRD-5254
@changeset-bot
Copy link

changeset-bot bot commented Oct 17, 2025

⚠️ No Changeset found

Latest commit: bab96d8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Oct 17, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
agents-docs Ready Ready Preview Comment Oct 17, 2025 8:30pm
agents-manage-api Ready Ready Preview Comment Oct 17, 2025 8:30pm
agents-manage-ui Ready Ready Preview Comment Oct 17, 2025 8:30pm
agents-run-api Ready Ready Preview Comment Oct 17, 2025 8:30pm

💡 Enable Vercel Agent with $100 free credit for automated AI reviews

@claude
Copy link

claude bot commented Oct 17, 2025

Claude finished @amikofalvy's task —— View job


PR Review: GitHub Actions CI Job Cancellation Control

Review Progress

  • Examine changed files and understand the workflow modification
  • Analyze existing CI patterns and system architecture
  • Review engineering quality and potential gotchas
  • Assess customer/system impact and extensibility
  • Provide comprehensive feedback and recommendations

🎯 Change Analysis

This PR implements a conditional concurrency control for the CI workflow by changing:

# Before
cancel-in-progress: true

# After  
cancel-in-progress: ${{ github.event_name == 'pull_request' }}

Intent: Allow parallel CI jobs on push events to main/master while maintaining efficient job cancellation for PR iterations.


🏗️ System Architecture Assessment

Well-Architected Decisions

  1. Event-Driven Logic: Using github.event_name provides clean conditional behavior based on trigger context
  2. Granular Control: Maintains existing PR optimization while addressing deployment pipeline needs
  3. Backward Compatibility: No breaking changes to existing workflow behavior

⚠️ Areas for Consideration

1. Resource Management & Cost Implications

  • High Priority: Push events to main/master will now run all CI jobs to completion, even when subsequent commits are pushed rapidly
  • Impact: In scenarios with frequent commits (e.g., automated dependency updates, batch commits), multiple expensive CI jobs (build, test, typecheck) will run in parallel
  • Recommendation: Consider monitoring CI usage/costs and potentially adding safeguards for excessive parallel jobs

2. Deployment Pipeline Coordination

  • Medium Priority: The PR mentions "isolated deployments" but I don't see explicit deployment steps in the CI workflow
  • Current State: The CI workflow (.github/workflows/ci.yml) primarily handles testing/validation, while releases are handled separately in .github/workflows/release.yml
  • Consideration: Verify that downstream deployment workflows properly handle multiple simultaneous CI completions

3. Merge Group Behavior

  • Medium Priority: The workflow triggers on merge_group events, which will now also avoid cancellation
  • Implication: GitHub merge queues may run multiple jobs simultaneously when processing batched merges
  • Assessment: This is likely correct behavior for merge queues, but worth validating

4. Race Conditions in Resource Usage

  • Medium Priority: Multiple parallel CI jobs may compete for:
    • Database files (workflow cleans *.db files but parallel jobs might interfere)
    • Cache keys (Turborepo cache uses github.sha which should isolate properly)
    • Test database conflicts (each job uses DB_FILE_NAME: test.db)

Recommendation: The database cleanup step suggests awareness of this issue, but consider adding job-specific DB file naming:

DB_FILE_NAME: test-${{ github.run_id }}.db

🔍 Engineering Quality Review

Code Quality: Excellent

  • Clean, semantic YAML syntax
  • Well-documented commit message explaining the rationale
  • Follows GitHub Actions best practices

Testing Strategy

  • Gap: No automated way to validate the conditional logic works correctly
  • Suggestion: Consider adding a workflow dispatch trigger for manual testing of different event types

Documentation & Observability

  • Good: Commit message clearly explains the change
  • Enhancement Opportunity: The CI summary step could log which cancellation mode is active:
- name: Log Concurrency Settings
  run: |
    echo "Event: ${{ github.event_name }}"
    echo "Cancellation enabled: ${{ github.event_name == 'pull_request' }}"

🚨 Gotchas & Common Pitfalls

1. Unintended Event Types

  • Risk: Future event types (e.g., workflow_dispatch, schedule) will default to no cancellation
  • Better Pattern:
cancel-in-progress: ${{ contains(fromJSON('["pull_request"]'), github.event_name) }}

This makes the allowed events explicit and defaults to non-cancellation for unknown events.

2. Branch Protection & Status Checks

  • Consideration: If branch protection rules wait for "CI" status, multiple parallel jobs may create race conditions in status reporting
  • Assessment: GitHub typically handles this well, but worth monitoring

3. Resource Exhaustion

  • Scenario: Rapid-fire commits to main could spawn many parallel CI jobs
  • Mitigation: GitHub has built-in concurrent job limits, but consider organizational implications

🎭 Customer & System Impact

Positive Impacts

  1. Deployment Reliability: Ensures each commit's CI completion for proper deployment tracking
  2. Parallel Development: Multiple teams can push to main without disrupting each other's CI
  3. Audit Trail: Complete CI history for all commits improving traceability

Potential Risks

  1. Resource Costs: Increased CI minutes usage during high-velocity periods
  2. Queue Saturation: Under extreme load, job queuing delays might increase
  3. Debugging Complexity: Multiple concurrent CI runs may complicate failure analysis

🔮 Future Extensibility

Well-Positioned For

  • Adding more sophisticated cancellation logic (e.g., time-based, commit author-based)
  • Integration with deployment status tracking
  • Branch-specific cancellation policies

Architectural Soundness

The conditional approach is clean and maintainable. If future requirements need more complex logic, this pattern scales well:

cancel-in-progress: ${{ 
  (github.event_name == 'pull_request') || 
  (github.event_name == 'push' && github.ref != 'refs/heads/main')
}}

📋 Final Recommendations

Immediate (Pre-Merge)

  1. Add monitoring for the new behavior - log cancellation mode in CI summary
  2. Verify merge group behavior aligns with expectations
  3. Consider database file naming to prevent parallel job conflicts

Post-Merge Monitoring

  1. Track CI resource usage for cost implications
  2. Monitor deployment pipeline behavior with multiple simultaneous completions
  3. Watch for any race conditions in high-velocity commit scenarios

Long-term Considerations

  1. Document the deployment architecture that necessitated this change
  2. Consider workflow-level resource limits if cost becomes an issue
  3. Evaluate extending this pattern to other workflows if successful

Overall Assessment: This is a well-thought-out change that addresses a real need in deployment pipelines. The implementation is clean and follows best practices. The main considerations are around resource management and monitoring the impact of increased parallel job execution.

Approval Status: ✅ Recommend Approval with post-merge monitoring of the mentioned areas.


Job Run: https://github.com/inkeep/agents/actions/runs/18604037081

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.

2 participants