Skip to content

Add optional TRIGGERED_BY_EMAIL to mention who triggered the build - #9

Open
yousefdebaz-fivexlio wants to merge 1 commit into
masterfrom
feat/triggered-by-email-internal
Open

Add optional TRIGGERED_BY_EMAIL to mention who triggered the build#9
yousefdebaz-fivexlio wants to merge 1 commit into
masterfrom
feat/triggered-by-email-internal

Conversation

@yousefdebaz-fivexlio

Copy link
Copy Markdown
Contributor

Same-repo mirror of #7 (#7 from AndreiHippo:feat/triggered-by-email), opened so the AI code review workflow can run on it - fork PRs are excluded by design (see #8).

Original description from #7:

Problem

The approval message identifies two people: the commits committer and its author. Neither is reliably a person. On GitHub, every commit created by the platform itself - squash merges, merge commits, rebase merges, web-editor edits, API-created commits - is committed as GitHub <noreply@github.com>. On a protected branch that only ever receives merges through the merge button, that is every commit. The author is whoever opened the pull request, which is often a bot (Dependabot, a code-review app). So an approval request can arrive with no human mentioned at all, even though a person just pressed Merge and is waiting on the deploy.

Change

A new optional TRIGGERED_BY_EMAIL variable. When set, a Triggered by line is added to the details block, resolving the email to a Slack mention and falling back to the plain email - the same treatment the committer and author lines already get. When unset, the message is byte-for-byte what it is today.

Testing

  • flake8 and pylint -E clean, per lint.sh.
  • Rendered the details block through main.py with Slack and git stubbed, across all three paths: email resolvable in Slack (mention), email unknown to Slack (plain-text fallback), and variable unset (unchanged output).

Credit: AndreiHippo. Close #7 once this merges (or supersede it).

Commits are not always made by a person. On GitHub, squash merges and merge
commits made through the web UI are committed as `GitHub <noreply@github.com>`,
and the author is whoever opened the pull request - which can itself be a bot.
In that case neither of the two identities in the approval message points at
the person waiting on the deploy, so nobody gets mentioned.

Let CI pass that identity in via the new optional TRIGGERED_BY_EMAIL variable.
When set, a `Triggered by` line is added to the message, mentioning the person
if the email matches a Slack profile and falling back to the plain email if it
does not - same behaviour as the existing committer and author lines. When it
is unset, the message is unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@yousefdebaz-fivexlio
yousefdebaz-fivexlio force-pushed the feat/triggered-by-email-internal branch from d104ac1 to 4c8d09b Compare August 13, 2026 22:42
Comment thread main.py
author_slack_id = helpers_slack.user_id_by_email(app, author_email)
author_id = f'<@{author_slack_id}>' if author_slack_id is not None else author_email
triggered_by_id = None
if triggered_by_email:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The empty string check if triggered_by_email: passes for whitespace-only strings (e.g., ' ' or '\t'). These would fail the Slack lookup and fall back to displaying the raw whitespace value. Consider using if triggered_by_email and triggered_by_email.strip(): for more robust validation.

@fivexl-reviewer

Copy link
Copy Markdown

AI Code Review Summary

  • Findings posted: 1 inline, 0 thread replies
  • By dimension: business_logic: 1
  • Threads marked resolved: 0

Additional observations

  • Additional observations:
  • Line 61: Minor newline handling inconsistency - when triggered_by_id is present, you get double newlines between Author and time-based message.
  • Line 48: Slack lookup failures are silent (no logging), making production debugging harder.
  • No new security vulnerabilities introduced - the PR follows existing patterns for Slack message interpolation.

Note for AI coding agents: detailed findings are posted as inline review comments on the diff — read the unresolved review threads (e.g. gh api repos/<owner>/<repo>/pulls/<pr>/comments) before making changes; this summary is only an overview.

This summary was automatically generated by the AI code review workflow.

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