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 workflow cancellation still behaves as expected
• Added needs.check-fork-status.result == 'success' guard to prevent tests from running if the fork check job fails (when outputs would be empty strings)

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 a workflow run should properly cancel all jobs (not continue due to ! cancelled() logic)
• 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 same changes need to be applied to the elementary repo.

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

Summary by CodeRabbit

• Chores
  • Added fork-aware gating for pull request testing with an optional approval step to control running CI on forked PRs.
  • Streamlined a warehouse test workflow by removing a separate PR-approval gate so tests run with simplified conditions.
  • General CI trigger and flow refinements to make pull-request test runs safer and clearer.

_{✏️ 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 the elementary repository.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds fork-aware gating to .github/workflows/test-all-warehouses.yml: detects fork PRs, requires manual approval for fork PRs before running tests, and refines ref resolution for pull_request vs pull_request_target. Removes the fork-approval gate from .github/workflows/test-warehouse.yml.

Changes

Cohort / File(s)	Summary
Fork-aware gating /.github/workflows/test-all-warehouses.yml	Adds check-fork-status job to detect fork-origin PRs, adds approve-fork job to require manual approval for fork PRs, and updates test job dependencies/conditions so tests run only for non-forks or approved forks. Adjusts dbt-data-reliability-ref resolution to consider pull_request vs pull_request_target.
Approval gate removal /.github/workflows/test-warehouse.yml	Removes the check-if-requires-approval job and its outputs; removes needs and environment-based conditional logic from the test job so tests no longer require the prior approval gate.

Sequence Diagram(s)

sequenceDiagram
    actor GitHub
    participant CheckFork as "check-fork-status\n(runner)"
    participant Approve as "approve-fork\n(manual)"
    participant Test as "test job\n(runner)"

    GitHub->>CheckFork: Trigger workflow (pull_request / pull_request_target)
    CheckFork->>CheckFork: Determine if PR is from a fork
    CheckFork->>Approve: Emit fork status (needs approval if fork)

    alt PR is from a fork
        Approve->>Approve: Await manual approval
        Approve->>Test: Signal approval granted
    else PR is internal
        CheckFork->>Test: Bypass approval, proceed
    end

    Test->>Test: Run tests (conditional on approval/bypass)
    Test->>GitHub: Report results/status

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I scoped the branches, sniffed the trail,
A forked request paused the nightly gale,
Internal hops skip — external paws wait,
One gentle nod and we continue the gate,
I twirl a carrot, tests run — celebrate! 🎋

🚥 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 'Improve fork safety: consolidate approval and add pull_request trigger' accurately reflects the main changes: consolidating approval into test-all-warehouses.yml and adding pull_request trigger support.
Linked Issues check	✅ Passed	The PR successfully implements all three objectives from ELE-5221: consolidates approval in test-all-warehouses.yml [ELE-5221 objective 1], adds pull_request trigger with fork/non-fork logic [ELE-5221 objective 2], and removes approval requirement from pull_request [ELE-5221 objective 3].
Out of Scope Changes check	✅ Passed	All changes are within scope: test-all-warehouses.yml adds fork detection and approval gating as required; test-warehouse.yml removes the approval gate as intended per the consolidation objective. No unrelated changes detected.
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 workflow condition reads outputs from
needs.check-fork-status even when that job may have failed (leaving outputs
empty) which lets tests run; update the if condition to also verify the job
succeeded by adding an explicit check like needs.check-fork-status.result ==
'success' (in addition to the existing outputs checks) so the overall clause
becomes: always() && needs.check-fork-status.result == 'success' &&
needs.check-fork-status.outputs.should_skip != 'true' &&
(needs.check-fork-status.outputs.is_fork != 'true' || needs.approve-fork.result
== 'success'); this ensures tests only run when the fork-check job completed
successfully.

[haritamar]

This comment is redundant

[devin-ai-integration]

Removed the redundant comment. Applied the same fix to the elementary repo as well.