Skip to content

Conversation

Copy link

Copilot AI commented Oct 29, 2025

The upstream twitter-together action doesn't support x.com URLs, only twitter.com format. This PR implements automated URL transformation in the GitHub Actions workflow to eliminate the need for manual URL changes in PRs.

Changes Made

  • Added automated transformation step to the GitHub Actions workflow (.github/workflows/twitter-together.yml)
  • Created reusable script: Extracted transformation logic to .github/scripts/transform-urls.sh following DRY principle
  • Preview job: Validates and warns when x.com URLs are detected in PRs, informing contributors they will be auto-transformed on merge
  • Tweet job: Transforms x.com URLs to twitter.com before tweeting on main branch, commits changes if transformations are made
  • Reverted manual fix: Restored the original retweet to use x.com format to demonstrate the automation
  • Security fix: Uses head.sha instead of head.ref to prevent malicious code execution from untrusted forks
  • Token configuration: Added explicit token parameter to tweet job checkout for proper authentication
  • Robustness improvements: Added || true to find command to prevent failures when no tweet files exist, uses portable #!/usr/bin/env bash shebang

How It Works

For Pull Requests:

  • The preview job checks for x.com URLs and displays a helpful warning message
  • No modifications are made to PR branches (addresses security concerns)
  • Contributors are informed that URLs will be automatically transformed when merged to main

For Main Branch:

  • The automation uses a reusable shell script (.github/scripts/transform-urls.sh) that runs a sed command to find and replace all https://x.com/ URLs with https://twitter.com/ in tweet files
  • If transformations are made, it automatically commits and pushes the changes using [skip ci] to prevent infinite loops

Benefits

  • No manual intervention: Contributors can submit PRs with either x.com or twitter.com URLs
  • Saves reviewer time: Eliminates the anti-pattern of manually changing URLs in PRs
  • Transparent: Preview job warns about x.com URLs; transformation commits clearly show what was changed
  • Secure: Transformation only runs on main branch, not in pull_request_target context; uses head.sha for secure checkouts
  • Robust: Handles edge cases like missing tweet files gracefully
  • Maintainable: Single script with clear documentation, easy to update and test
  • Complete: Handles both PR validation and main branch tweeting

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Add retweet functionality for ArgentumVeritas tweet Fix retweet URL format from x.com to twitter.com Oct 29, 2025
Copilot AI requested a review from realcodywburns October 29, 2025 04:05
@realcodywburns
Copy link
Member

@copilot From what i recall we have submitted pr's to the upstream twitter together app that have not been merged on the same issue. I do not like changing the pr's as this is an anti pattern and time consuming for reviewers. Investigate how to make updates to either the existing gh action or create another pipeline to automate the x.com => twitter.com transformation

@realcodywburns
Copy link
Member

Re purposed this pr as the original is a duplicate of #278. Cp did recon on why the x.com fails and it is related to the upstream still not making changes. We will fix the problem, wont we @copilot

Copy link
Author

Copilot AI commented Oct 29, 2025

Re purposed this pr as the original is a duplicate of #278. Cp did recon on why the x.com fails and it is related to the upstream still not making changes. We will fix the problem, wont we @copilot

Yes, we fixed it! I've implemented automated x.com → twitter.com URL transformation in the GitHub Actions workflow (commit db64ac7). The automation runs before validation and tweeting, automatically transforming any x.com URLs and committing them back to the branch. This eliminates the need for manual URL changes in PRs.

Copilot AI changed the title Fix retweet URL format from x.com to twitter.com Automate x.com to twitter.com URL transformation in GitHub Actions Oct 29, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds automatic URL transformation functionality to the Twitter Together workflow to convert x.com URLs to twitter.com format before tweeting. The changes address compatibility with the Twitter API by ensuring URLs use the legacy twitter.com domain.

Key changes:

  • Added automated x.com to twitter.com URL transformation in both preview and tweet jobs
  • Updated checkout configuration to use branch ref and repository for PR head commits
  • Added contents: write permission to enable automated commits

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 15 to 16
ref: ${{ github.event.pull_request.head.ref }}
repository: ${{ github.event.pull_request.head.repo.full_name }}
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.

Checking out code from the PR head repository using pull_request_target is a security risk. This allows code from untrusted forks to run with write permissions in the base repository's context. An attacker could modify the workflow or inject malicious code. Use github.event.pull_request.head.sha instead of head.ref, and validate PR content before executing any code from it.

Suggested change
ref: ${{ github.event.pull_request.head.ref }}
repository: ${{ github.event.pull_request.head.repo.full_name }}
ref: ${{ github.event.pull_request.head.sha }}

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

Ack. This will ensure the same version of the code is used every time, i agree this is more secure than a name matching because we get granular control over which we are running

id: transform
run: |
echo "Starting URL transformation..."
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 URL transformation logic is duplicated between the preview job (line 22) and tweet job (line 50). Extract this into a reusable composite action or shell script to follow the DRY principle and simplify maintenance.

Copilot uses AI. Check for mistakes.
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
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.
@realcodywburns
Copy link
Member

@copilot review the comments on the last pr, make appropriate updates based on the commentary

Copy link
Author

Copilot AI commented Oct 29, 2025

@copilot review the comments on the last pr, make appropriate updates based on the commentary

I've addressed all the review comments in commit df31933:

  1. Security fix: Changed checkout to use head.sha instead of head.ref to prevent malicious code execution from untrusted forks
  2. DRY principle: Extracted transformation logic to .github/scripts/transform-urls.sh to eliminate duplication
  3. Token configuration: Added explicit token parameter to tweet job checkout for proper authentication

The workflow is now more secure, maintainable, and follows best practices.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +19 to +29

# 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"
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
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

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
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 on lines 17 to 18
- name: Transform x.com URLs to twitter.com
run: bash .github/scripts/transform-urls.sh
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.

Running a script that automatically commits and pushes changes during pull_request_target poses a security risk. The README explicitly warns: 'Do not trigger Actions from Pull Requests that suspiciously modify the workflow file.' While this adds a new script rather than modifying the workflow itself, it introduces arbitrary code execution that modifies and commits files in the context of pull requests from forks. Consider moving this transformation to only run in the tweet job (on push to main), or implement a non-automated approach such as a validation check that fails with a helpful message when x.com URLs are detected.

Suggested change
- name: Transform x.com URLs to twitter.com
run: bash .github/scripts/transform-urls.sh
- name: Validate no x.com URLs present
run: |
if grep -r 'x\.com' .github/tweets; then
echo "Error: x.com URLs detected in tweet files. Please replace with twitter.com URLs before submitting."
exit 1
fi

Copilot uses AI. Check for mistakes.
@@ -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.
Copy link
Member

@realcodywburns realcodywburns left a comment

Choose a reason for hiding this comment

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

The transformation script needs revisions @copilot

Copy link
Author

Copilot AI commented Oct 29, 2025

The transformation script needs revisions @copilot

I've revised the transformation script in commit b3ea8a7:

  1. Removed transformation from preview job - Now only runs on push to main (fixes detached HEAD and security issues)
  2. Added validation check in preview job - Warns users when x.com URLs are detected, noting they'll be auto-transformed on merge
  3. Fixed find command - Added || true to prevent failures when no tweet files exist
  4. Improved portability - Changed shebang to #!/usr/bin/env bash

The preview job now informs the team about x.com URLs without trying to modify them, and transformation only happens when merging to main.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@IstoraMandiri
Copy link
Collaborator

The actual fix will be in https://github.com/ethereumclassic/Contribunator

The fix should have been ethereumclassic/Contribunator@b82b9dd

But the test didn't pass, or something, I will investigate...

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.

3 participants