Skip to content

fix(ci): use env vars instead of secrets in step if conditions#14711

Merged
davidkonigsberg merged 1 commit intomainfrom
devin/1775578572-fix-ci-secrets-context
Apr 7, 2026
Merged

fix(ci): use env vars instead of secrets in step if conditions#14711
davidkonigsberg merged 1 commit intomainfrom
devin/1775578572-fix-ci-secrets-context

Conversation

@davidkonigsberg
Copy link
Copy Markdown
Contributor

Description

Fixes broken CI and Seed workflows on main introduced by #14676. Both ci.yml and seed.yml fail at workflow validation with:

Unrecognized named-value: 'secrets'. Located at position 1 within expression: secrets.DOCKER_USERNAME_PUBLIC_READONLY != ''

The secrets context cannot be referenced directly in step-level if expressions (likely due to workflow_call being a trigger in seed.yml, which changes secrets context availability, and stricter validation in ci.yml).

Changes Made

  • ci.yml (test-ete job, line 183): Changed if: ${{ secrets.DOCKER_USERNAME_PUBLIC_READONLY != '' }}if: ${{ env.DOCKERHUB_USERNAME != '' }} with a step-level env mapping the secret to an env var
  • seed.yml (benchmark job, line 869): Same fix

The with blocks still reference secrets directly for the actual login credentials, which is valid — only the if expression needed the workaround.

Review Checklist

  • Confirm step-level env is evaluated before the step's if condition (per GitHub docs, it is)
  • Verify that secrets in with: blocks still works correctly (it should — with inputs are evaluated in a different context than if)
  • Note: the composite action (cached-seed) already avoids this issue by accepting credentials as inputs, not referencing secrets directly

Testing

  • Workflow YAML syntax is valid
  • Actual validation can only be confirmed by CI running successfully after merge

Link to Devin session: https://app.devin.ai/sessions/59c37d5afae04814afa3f96c447a28ab
Requested by: @davidkonigsberg

The secrets context cannot be used directly in step-level if expressions.
GitHub Actions rejects this with 'Unrecognized named-value: secrets'.
Use env vars to pass the secret value and check the env var instead.

Co-Authored-By: David Konigsberg <davidakonigsberg@gmail.com>
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 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
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

🌱 Seed Test Selector

Select languages to run seed tests for:

  • Python
  • TypeScript
  • Java
  • Go
  • Ruby
  • C#
  • PHP
  • Swift
  • Rust
  • OpenAPI

How to use: Click the ⋯ menu above → "Edit" → check the boxes you want → click "Update comment". Tests will run automatically and snapshots will be committed to this PR.

@davidkonigsberg davidkonigsberg deleted the devin/1775578572-fix-ci-secrets-context branch April 7, 2026 16:21
@davidkonigsberg davidkonigsberg restored the devin/1775578572-fix-ci-secrets-context branch April 7, 2026 16:25
@davidkonigsberg davidkonigsberg enabled auto-merge (squash) April 7, 2026 16:29
@davidkonigsberg davidkonigsberg merged commit c32e032 into main Apr 7, 2026
113 of 115 checks passed
@davidkonigsberg davidkonigsberg deleted the devin/1775578572-fix-ci-secrets-context branch April 7, 2026 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants