Skip to content

fix: add parentheses to fix operator precedence in co-author check#1199

Open
FuturizeRush wants to merge 1 commit intoanthropics:mainfrom
FuturizeRush:fix/coauthor-operator-precedence
Open

fix: add parentheses to fix operator precedence in co-author check#1199
FuturizeRush wants to merge 1 commit intoanthropics:mainfrom
FuturizeRush:fix/coauthor-operator-precedence

Conversation

@FuturizeRush
Copy link
Copy Markdown

Summary

One-character fix: adds parentheses to correct operator precedence in the co-author line construction.

Bug

src/create-prompt/index.ts:398?? has lower precedence than !==:

// Current (buggy): parses as triggerDisplayName ?? (triggerUsername !== "Unknown")
(githubData.triggerDisplayName ?? context.triggerUsername !== "Unknown")

// Fixed: parses as (triggerDisplayName ?? triggerUsername) !== "Unknown"
((githubData.triggerDisplayName ?? context.triggerUsername) !== "Unknown")

When triggerDisplayName is "" (empty string — not null/undefined), ?? returns "" which is falsy, so the co-author line is incorrectly omitted.

Verified with JavaScript

displayName=""  username="someuser"
  buggy: ""                          ← co-author skipped incorrectly
  fixed: "Co-authored-by: someuser"  ← correct

Test plan

  • tsc --noEmit passes
  • Tested all 6 input combinations in Node.js — only displayName="" diverges
  • Fix is backward compatible (all other cases produce identical output)

`??` has lower precedence than `!==`, so the expression:
  triggerDisplayName ?? triggerUsername !== "Unknown"
parses as:
  triggerDisplayName ?? (triggerUsername !== "Unknown")

When triggerDisplayName is an empty string "", the condition
evaluates to "" (falsy), incorrectly skipping the co-author line
even though the user is not "Unknown".

Add parentheses to get the intended behavior:
  (triggerDisplayName ?? triggerUsername) !== "Unknown"
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.

1 participant