Skip to content

chore(gha): prefer ci-protected env#9728

Merged
jmelahman merged 3 commits intomainfrom
jamison/ci-protected
Mar 30, 2026
Merged

chore(gha): prefer ci-protected env#9728
jmelahman merged 3 commits intomainfrom
jamison/ci-protected

Conversation

@jmelahman
Copy link
Copy Markdown
Contributor

@jmelahman jmelahman commented Mar 28, 2026

Description

How Has This Been Tested?

Captured by existing

Additional Options

  • [Optional] Override Linear Check

Summary by cubic

Standardized GitHub Actions to use the ci-protected environment so env-scoped secrets and protections apply consistently across tests, deploys, and scheduled tasks, including notify/failure paths.

  • Refactors
    • PR/deploy: pr-python-model-tests, preview deploy, Storybook deploy + failure handler.
    • Maintenance/scheduled: nightly LLM provider chat (failure notify), post-merge beta cherry-pick (success/failure notify), sync_foss, tag-nightly.

Written for commit 5a775cc. Summary will update on new commits.

@jmelahman jmelahman requested review from a team and justin-tahara as code owners March 28, 2026 00:03
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 28, 2026

Greptile Summary

This PR standardizes GitHub Actions workflow jobs to use the ci-protected environment, which applies environment-scoped secrets and protection rules. The change affects seven workflow files covering nightly tests, preview/storybook deploys, FOSS sync, cherry-pick notifications, and nightly tagging. All additions follow a consistent pattern and fit naturally into the existing CI structure.

Key changes:

  • environment: ci-protected added to notification, deploy, and scheduled maintenance jobs across all seven files
  • Jobs that only use github.token (e.g. cherry-pick-to-latest-release) are correctly left without the environment annotation
  • The reusable workflow caller in nightly-llm-provider-chat.yml is intentionally left unchanged, with only its notification job scoped to the environment

One notable consideration: in pr-python-model-tests.yml, the workflow-level env: block (lines 12–24) references several secrets. GitHub Actions only injects environment secrets within the job that specifies the environment key, not at workflow scope. If those secrets are migrated to be environment-only (not also repo-level), the workflow-level env: block would silently produce empty values. Moving those declarations inside the job's env: block would be the safe, future-proof approach.

Confidence Score: 5/5

Safe to merge — changes are purely additive CI configuration with no impact on application logic.

All seven changes are single-line additions of environment: ci-protected to job definitions. No application code is touched, no secrets are removed, and existing behaviour is preserved. The only finding is a P2 style note about workflow-level vs. job-level secret scoping in one file, which does not block functionality unless secrets are migrated to be environment-only.

.github/workflows/pr-python-model-tests.yml — workflow-level env: block may need to move inside the job if secrets are fully migrated to environment scope.

Important Files Changed

Filename Overview
.github/workflows/pr-python-model-tests.yml Adds environment: ci-protected to the model-check job; note that the workflow-level env: block (lines 12–24) references secrets that are only available within jobs with the environment specified, not at workflow scope
.github/workflows/nightly-llm-provider-chat.yml Adds environment: ci-protected to the notify-slack-on-failure job only; the upstream provider-chat-test reusable workflow job is unchanged and correctly scoped separately
.github/workflows/post-merge-beta-cherry-pick.yml Adds environment: ci-protected to the two Slack notification jobs; cherry-pick automation jobs that only use github.token are intentionally left without the environment
.github/workflows/preview.yml Adds environment: ci-protected to the Deploy-Preview job; Vercel secrets are now accessed via the protected environment
.github/workflows/storybook-deploy.yml Adds environment: ci-protected to both the Deploy-Storybook and notify-slack-on-failure jobs; consistent with the PR pattern
.github/workflows/sync_foss.yml Adds environment: ci-protected to the sync-foss job; the deploy key secret used for pushing to the FOSS repo is now gated behind the environment
.github/workflows/tag-nightly.yml Adds environment: ci-protected to the create-and-push-tag job; DEPLOY_KEY and MONITOR_DEPLOYMENTS_WEBHOOK secrets are correctly accessed within the job scope

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    ENV[ci-protected Environment\nSecrets + Protection Rules]

    subgraph nightly-llm-provider-chat.yml
        A1[provider-chat-test\nreusable workflow] --> A2[notify-slack-on-failure\n✅ ci-protected]
    end

    subgraph post-merge-beta-cherry-pick.yml
        B1[resolve-cherry-pick-request] --> B2[cherry-pick-to-latest-release]
        B2 --> B3[notify-slack-on-success\n✅ ci-protected]
        B1 --> B4[notify-slack-on-failure\n✅ ci-protected]
    end

    subgraph pr-python-model-tests.yml
        C1[model-check\n✅ ci-protected]
    end

    subgraph preview.yml
        D1[Deploy-Preview\n✅ ci-protected]
    end

    subgraph storybook-deploy.yml
        E1[Deploy-Storybook\n✅ ci-protected] --> E2[notify-slack-on-failure\n✅ ci-protected]
    end

    subgraph sync_foss.yml
        F1[sync-foss\n✅ ci-protected]
    end

    subgraph tag-nightly.yml
        G1[create-and-push-tag\n✅ ci-protected]
    end

    ENV -.->|secrets & rules| A2
    ENV -.->|secrets & rules| B3
    ENV -.->|secrets & rules| B4
    ENV -.->|secrets & rules| C1
    ENV -.->|secrets & rules| D1
    ENV -.->|secrets & rules| E1
    ENV -.->|secrets & rules| E2
    ENV -.->|secrets & rules| F1
    ENV -.->|secrets & rules| G1
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: .github/workflows/pr-python-model-tests.yml
Line: 12-24

Comment:
**Workflow-level `env:` can't access environment-scoped secrets**

GitHub Actions only makes environment secrets available within the job that specifies the `environment` key — they are not injected into workflow-level `env:` blocks. If any of these secrets (`AWS_ACCESS_KEY_ID`, `COHERE_API_KEY`, `LITELLM_API_KEY`, `OPENAI_API_KEY`, etc.) are stored exclusively as `ci-protected` environment secrets (rather than as repository-level secrets), this block would silently produce empty strings.

Since this PR's goal is to consolidate onto environment-scoped secrets, consider moving these declarations inside the `model-check` job's `env:` block (line 37) so they are resolved in the correct job scope:

```yaml
  model-check:
    runs-on: ...
    environment: ci-protected
    timeout-minutes: 45
    env:
      PYTHONPATH: ./backend
      # Bedrock
      AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
      AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
      AWS_REGION_NAME: ${{ vars.AWS_REGION_NAME }}
      # API keys for testing
      COHERE_API_KEY: ${{ secrets.COHERE_API_KEY }}
      LITELLM_API_KEY: ${{ secrets.LITELLM_API_KEY }}
      LITELLM_API_URL: ${{ secrets.LITELLM_API_URL }}
      OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
      AZURE_API_KEY: ${{ secrets.AZURE_API_KEY }}
      AZURE_API_URL: ${{ vars.AZURE_API_URL }}
```

If these secrets are (and will remain) repo-level secrets, the current placement works fine and no change is needed.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (3): Last reviewed commit: "fewer" | Re-trigger Greptile

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 15 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

@github-actions
Copy link
Copy Markdown
Contributor

🖼️ Visual Regression Report

Project Changed Added Removed Unchanged Report
admin 126 0 0 38 View Report
exclusive 5 0 0 3 View Report

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