Skip to content

fix: set outputs/env vars for empty string field values#132

Merged
volodymyrZotov merged 1 commit into1Password:mainfrom
toga4:empty-strings
Jan 27, 2026
Merged

fix: set outputs/env vars for empty string field values#132
volodymyrZotov merged 1 commit into1Password:mainfrom
toga4:empty-strings

Conversation

@toga4
Copy link
Copy Markdown
Contributor

@toga4 toga4 commented Jan 23, 2026

  • Fix inconsistent behavior where empty string field values from 1Password caused outputs/env vars to be undefined
  • Now matches op run behavior: empty field values result in defined variables with empty string values
  • Skip setSecret for empty strings to avoid runner warning

Fixes #131

Empty string field values from 1Password were causing the action to skip setting outputs and environment variables entirely.
This was inconsistent with `op run` behavior, which sets the variable with an empty value.

- Change falsy check to explicit null/undefined check in extractSecret
- Skip setSecret for empty strings to avoid runner warning
- Add tests for empty string value handling
@toga4
Copy link
Copy Markdown
Contributor Author

toga4 commented Jan 27, 2026

Hi @volodymyrZotov, I hope you don't mind me reaching out directly. I noticed you've been reviewing PRs in this repo recently.
Would you be able to take a look at this one when you have time? It's a relatively small change to fix the empty string handling behavior. I'd appreciate any feedback.
Thank you!

@volodymyrZotov
Copy link
Copy Markdown
Contributor

/ok-to-test sha=13f927c

@github-actions
Copy link
Copy Markdown

✅ E2E tests passed.

View test run output

Copy link
Copy Markdown
Contributor

@volodymyrZotov volodymyrZotov left a comment

Choose a reason for hiding this comment

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

Looks good @toga4 ! Thank you for your input! 🌟

@volodymyrZotov volodymyrZotov merged commit 5fd6fbc into 1Password:main Jan 27, 2026
5 checks passed
@toga4
Copy link
Copy Markdown
Contributor Author

toga4 commented Jan 27, 2026

@volodymyrZotov Thanks for merging! 🎉
When you get a chance, could you cut a new release (v3.1.1) so users can pick up this fix? Let me know if there's anything else needed from my side. Thanks!

volodymyrZotov added a commit that referenced this pull request Jan 27, 2026
@toga4 toga4 deleted the empty-strings branch February 10, 2026 04:51
@toga4
Copy link
Copy Markdown
Contributor Author

toga4 commented Feb 10, 2026

Hi @volodymyrZotov, friendly follow-up on this.

I've been trying to use the fix from this PR by pinning to the merge commit, but I noticed that dist/ wasn't rebuilt as part of the merge — so the bundled code still has the old if (!secretValue) check, and empty string values are still being skipped at runtime.

Would it be possible to rebuild dist/ and cut a new release? Happy to open a PR for the rebuild if that would help move things along.

Thanks!

@volodymyrZotov
Copy link
Copy Markdown
Contributor

@toga4 correct. The change is not released yet. The team will make a new release shortly.

@toga4
Copy link
Copy Markdown
Contributor Author

toga4 commented Feb 10, 2026

@volodymyrZotov Thanks for confirming! Looking forward to the release.

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.

Empty field values result in undefined environment variables/outputs instead of empty strings

2 participants