Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Address PR review comments: fix security, add DRY principle, add miss…
…ing token

Co-authored-by: realcodywburns <13103499+realcodywburns@users.noreply.github.com>
  • Loading branch information
Copilot and realcodywburns committed Oct 29, 2025
commit df319334d84b37cfc73a7b4deabcc753a4b9ac4f
31 changes: 31 additions & 0 deletions .github/scripts/transform-urls.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/bin/bash
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider using #!/usr/bin/env bash instead of #!/bin/bash for better portability across different systems where bash might be installed in different locations.

Suggested change
#!/bin/bash
#!/usr/bin/env bash

Copilot uses AI. Check for mistakes.
# Transform x.com URLs to twitter.com format in tweet files
# This script is used by the GitHub Actions workflow to ensure compatibility
# with the twitter-together action which only supports twitter.com URLs

set -e

echo "Starting URL transformation..."

# Find all tweet files and transform x.com URLs to twitter.com
find tweets -name "*.tweet" -type f -exec sed -i 's|https://x\.com/|https://twitter.com/|g' {} +
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

The find command with -exec ... {} + will fail if no .tweet files are found, causing the script to exit with an error due to set -e. Add a check to verify the tweets directory exists and contains files, or use || true to prevent failures when no files match.

Suggested change
find tweets -name "*.tweet" -type f -exec sed -i 's|https://x\.com/|https://twitter.com/|g' {} +
find tweets -name "*.tweet" -type f -exec sed -i 's|https://x\.com/|https://twitter.com/|g' {} + || true

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ack. This will be useful in cases like the current pr where the code is not a new tweet and the check is un necessarily blocking


Comment thread
realcodywburns marked this conversation as resolved.
# Check if any changes were made
if git diff --quiet; then
echo "No URL transformations needed"
exit 0
else
echo "Transformed x.com URLs to twitter.com"

# Configure git user
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"

# Commit and push changes
git add tweets/
git commit -m "Auto-transform x.com URLs to twitter.com format [skip ci]"
git push

echo "Changes committed and pushed"
Comment on lines +21 to +31
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

The git push command will fail in the pull_request_target context. When the preview job checks out github.event.pull_request.head.sha, it results in a detached HEAD state without a proper branch reference. Additionally, for external contributors, the workflow cannot push to their fork. Consider removing automatic commits from the preview job and only performing transformations in the tweet job when merging to main, or implement a different approach such as commenting on the PR to request manual fixes.

Suggested change
# Configure git user
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"
# Commit and push changes
git add tweets/
git commit -m "Auto-transform x.com URLs to twitter.com format [skip ci]"
git push
echo "Changes committed and pushed"

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The intent of using sha was to ensure we were retrieving the correct. If the transformation is only for the push to main this will make sense. We should have the action remind the team of this in the pr before the final push to main if feasible

exit 0
fi
36 changes: 5 additions & 31 deletions .github/workflows/twitter-together.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,10 @@ jobs:
- name: checkout pull request
uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.ref }}
repository: ${{ github.event.pull_request.head.repo.full_name }}
ref: ${{ github.event.pull_request.head.sha }}
token: ${{ secrets.GITHUB_TOKEN }}
- name: Transform x.com URLs to twitter.com
id: transform
run: |
echo "Starting URL transformation..."
find tweets -name "*.tweet" -type f -exec sed -i 's|https://x\.com/|https://twitter.com/|g' {} +

if git diff --quiet; then
echo "transformed=false" >> $GITHUB_OUTPUT
echo "No URL transformations needed"
else
echo "transformed=true" >> $GITHUB_OUTPUT
echo "Transformed x.com URLs to twitter.com"
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"
git add tweets/
git commit -m "Auto-transform x.com URLs to twitter.com format [skip ci]"
git push
fi
run: bash .github/scripts/transform-urls.sh
- name: Validate Tweets
uses: IstoraMandiri/twitter-together@etc
env:
Expand All @@ -44,19 +27,10 @@ jobs:
steps:
- name: checkout main
uses: actions/checkout@v3
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

The checkout action in the tweet job should specify token parameter with appropriate permissions to enable the subsequent git push operation. Without explicit token configuration, the push may fail or use unexpected credentials.

Suggested change
uses: actions/checkout@v3
uses: actions/checkout@v3
with:
token: ${{ secrets.GITHUB_TOKEN }}

Copilot uses AI. Check for mistakes.
with:
token: ${{ secrets.GITHUB_TOKEN }}
- name: Transform x.com URLs to twitter.com
run: |
echo "Starting URL transformation..."
find tweets -name "*.tweet" -type f -exec sed -i 's|https://x\.com/|https://twitter.com/|g' {} +

if ! git diff --quiet; then
echo "Transformed x.com URLs to twitter.com"
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"
git add tweets/
git commit -m "Auto-transform x.com URLs to twitter.com format [skip ci]"
git push
fi
run: bash .github/scripts/transform-urls.sh
- name: Tweet
uses: IstoraMandiri/twitter-together@etc
env:
Expand Down