Improve fork safety: consolidate approval and add pull_request trigger

[devin-ai-integration]

Summary

This PR improves the fork testing workflow by:

1. Adding a pull_request trigger for internal PRs (non-forks) so workflow changes can be tested immediately without waiting for merge
2. Keeping pull_request_target for fork PRs that need access to secrets
3. Moving the approval step from test-warehouse.yml to test-all-warehouses.yml so approval happens once instead of per-platform (reduces approval spam)
4. Adding logic to skip duplicate runs (internal PRs skip pull_request_target, fork PRs skip pull_request)

Trigger behavior after this change:

• Internal PRs: Run via pull_request (no approval needed)
• Fork PRs: Run via pull_request_target (single approval required)
• Manual/workflow_call: Run normally (no approval needed)

Fixes ELE-5221

Updates since last revision

• Changed always() to ! cancelled() in the test job condition so that cancelling the workflow still behaves as expected
• Added needs.check-fork-status.result == 'success' guard to prevent tests from running if the fork check job fails (when it fails, outputs are empty strings which could bypass the skip logic)

Review & Testing Checklist for Human

• Verify security: Confirm that fork PRs still require approval via elementary_test_env environment before accessing secrets - the approval gate should only trigger for pull_request_target from forks
• Verify skip logic: The should_skip condition should correctly skip pull_request events from forks and pull_request_target events from non-forks
• Verify cancellation behavior: Cancelling the workflow should properly cancel all jobs (not continue running due to ! cancelled())
• Test with actual PRs: Create a test internal PR and verify it runs via pull_request without approval; have someone create a fork PR to verify it requires approval
• Verify workflow_dispatch still works: Manual triggers should bypass all fork checks and run normally

Recommended test plan:

1. Merge this PR and observe the workflow behavior on subsequent internal PRs
2. Ask a contributor to open a fork PR to verify the approval flow works correctly with single approval

Notes

This is part of a two-repo change - the companion PR for dbt-data-reliability is elementary-data/dbt-data-reliability#919

Link to Devin run: https://app.devin.ai/sessions/268e854e9d814cd4a6e23511732beb37
Requested by: Itamar Hartstein (@haritamar)

Summary by CodeRabbit

• Chores
  • Updated CI/testing workflow: added fork detection and a manual approval gate for pull requests from forks so tests run only after approval when required.
  • Restored and refined gating and commit-reference handling across PR types to ensure correct and consistent test execution.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[linear]

ELE-5221 OSS - fork safety improvements

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[github-actions]

👋 @devin-ai-integration[bot]
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in this pull request.

[coderabbitai]

📝 Walkthrough

Walkthrough

Added centralized fork-detection and manual approval gating to the main test orchestration workflow; removed per-warehouse approval gating and updated workflow triggers/conditions to handle pull_request and pull_request_target event contexts.

Changes

Cohort / File(s)	Summary
Workflow orchestration with fork gating .github/workflows/test-all-warehouses.yml	Added check-fork-status job to detect fork PRs (exposes is_fork/should_skip); added approve-fork manual approval job; made test dependent on these jobs and added conditional if to run only when allowed; broadened PR head SHA handling to include both pull_request_target and pull_request; adjusted triggers to use pull_request for internal PRs and pull_request_target for forks.
Warehouse-level approval cleanup .github/workflows/test-warehouse.yml	Removed check-if-requires-approval job and removed dynamic environment/needs dependency from the test job so per-warehouse tests no longer perform approval gating (delegated to central workflow).

Sequence Diagram

sequenceDiagram
    participant GH as GitHub (Event)
    participant Checker as check-fork-status
    participant Approver as approve-fork
    participant Orchestrator as test-all-warehouses (test job)
    participant Warehouse as test-warehouse.yml

    GH->>Checker: PR event (pull_request or pull_request_target)
    activate Checker
    Checker->>Checker: determine is_fork / should_skip
    deactivate Checker

    alt PR is from a fork
        Checker->>Approver: requires approval
        activate Approver
        Approver->>Approver: wait for manual environment approval
        Approver-->>Orchestrator: approval granted
        deactivate Approver
    else internal PR
        Checker-->>Orchestrator: proceed without approval
    end

    activate Orchestrator
    Orchestrator->>Warehouse: invoke `test-warehouse.yml` (resolved elementary-ref / head SHA)
    Warehouse->>Warehouse: run warehouse tests
    Warehouse-->>Orchestrator: results
    deactivate Orchestrator

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 I hopped through YAML, sniffed each PR,
I marked the forks and watched from afar,
A gentle paw taps the approval gate,
When given the nod, I help run each crate,
Happy hops — CI dances under the star.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: improving fork safety through consolidating approval (moving it to test-all-warehouses.yml) and adding the pull_request trigger for internal PRs.
Linked Issues check	✅ Passed	The PR implements all key requirements from ELE-5221: consolidates approval in test-all-warehouses.yml, adds pull_request trigger for internal PRs, implements fork-aware gating to skip duplicate runs, and removes per-platform approval from test-warehouse.yml.
Out of Scope Changes check	✅ Passed	All changes are directly related to the PR objectives from ELE-5221. The workflow modifications for fork safety, approval consolidation, and dual-trigger setup are all within scope of the stated requirements.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In @.github/workflows/test-all-warehouses.yml:
- Around line 78-83: The current if condition for the test job uses always(),
which lets the job run even when check-fork-status failed and left outputs
empty; update the guard to require the check-fork-status job succeeded by adding
a result check (e.g., require needs.check-fork-status.result == 'success') to
the existing condition for the test job so that the test job only proceeds when
check-fork-status completed successfully; locate the test job's if block
referencing needs.check-fork-status.outputs and modify it to also assert
needs.check-fork-status.result == 'success' (or replace always() with success())
so the test job will be blocked when check-fork-status fails.