Skip to content

Fix: Complete rewrite of check-media-attachments workflow with all review fixes#695

Closed
Copilot wants to merge 4 commits into
mainfrom
copilot/fix-a8588417-3ca9-4e18-8953-1244f2e7876a
Closed

Fix: Complete rewrite of check-media-attachments workflow with all review fixes#695
Copilot wants to merge 4 commits into
mainfrom
copilot/fix-a8588417-3ca9-4e18-8953-1244f2e7876a

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 5, 2025

Summary

This PR completely fixes the check-media-attachments.yml workflow that was originally introduced in PR #670 to resolve issue #571 (SyntaxError: Unexpected token '|'). The original PR had 13+ review comments from CodeRabbit and Copilot that needed to be addressed. This PR implements all requested fixes and follows GitHub Actions best practices.

Issues Fixed

Critical Issues (4)

  1. SyntaxError resolved - Fixed all JavaScript syntax errors in the workflow script
  2. Upgraded to actions/github-script@v7 - The original used outdated v6 which was flagged by actionlint
  3. Fixed image alt detection regex - Changed from \salt= to \balt= (word boundary) as requested in review
  4. Fixed figure caption detection - Completely rewrote to properly parse figure blocks instead of using an incorrect negative lookahead in the opening tag

YAML Linting Issues (3)

  1. Quoted 'on' keyword - Prevents YAML from treating it as a boolean value
  2. Removed all trailing whitespace - Cleaned up 28+ lines with trailing spaces
  3. Added EOF newline - File now ends with a proper newline character

Performance & Security Improvements (5)

  1. Added file pagination - Uses github.paginate() to handle PRs with more than 30 files
  2. Removed unnecessary checkout step - The workflow fetches content via API, so checkout is not needed
  3. Added permissions block - Implements least-privilege access (read for contents, write for PRs/issues)
  4. Added concurrency control - Prevents duplicate workflow runs and cancels outdated runs
  5. Added paths filter - Only triggers on HTML file changes, reducing unnecessary runs

Enhanced Media Detection (5)

  1. Enhanced media detection - Now checks poster attributes and properly parses srcset entries
  2. Added data: URI filtering - Excludes base64-encoded inline images from media file counts
  3. Added content truncation handling - Falls back to git.getBlob for files larger than 1MB
  4. Extended file type support - Now handles .html, .htm, and .xhtml files (case-insensitive)
  5. Linked issue media detection - Automatically detects and reports media attachments found in GitHub issues linked to the PR (e.g., "Fixes darkmode/about #123", "Closes combine the navigation so it’s not separate for mobile and desktop fixes #456")

Key Changes

Workflow Trigger

"on":
  pull_request:
    types: [opened, synchronize, reopened]
    paths:
      - '**/*.html'
      - '**/*.htm'
      - '**/*.xhtml'

concurrency:
  group: check-media-attachments-${{ github.event.pull_request.number }}
  cancel-in-progress: true

Security & Permissions

permissions:
  contents: read
  pull-requests: write
  issues: write

Enhanced Detection Logic

Before:

const mediaMatches = content.match(/(src|href)=["'][^"']*\.(jpg|jpeg|png|...)["']/gi);

After:

// Direct media attributes (src, href, poster)
const direct = [...content.matchAll(/\b(?:src|href|poster)\s*=\s*["']([^"']+\.(jpg|jpeg|...))(?:\?[^"']*)?["']/gi)]
  .map(m => m[1])
  .filter(u => !/^data:/i.test(u));

// Srcset entries
const srcsetUrls = [...content.matchAll(/\bsrcset\s*=\s*["']([^"']+)["']/gi)]
  .flatMap(m => m[1].split(',').map(s => s.trim().split(/\s+/)[0]))
  .filter(u => /\.(jpg|jpeg|png|gif|webp|svg)$/i.test(u) && !/^data:/i.test(u));

const allMedia = [...new Set([...direct, ...srcsetUrls])];

Linked Issue Detection

The workflow now automatically detects issues linked in the PR description:

This helps track media files referenced in related issues when HTML files are modified.

Testing

All validation tests passed:

  • ✅ YAML syntax validated with Python's yaml.safe_load()
  • ✅ JavaScript syntax verified with Node.js --check flag
  • ✅ Regex patterns tested against realistic HTML samples
  • ✅ API calls follow GitHub's Octokit best practices

Impact

This workflow will now:

  • Run only when HTML files are modified (more efficient)
  • Handle large PRs correctly with pagination
  • Follow security best practices with proper permissions
  • Detect more accessibility issues (alt text, figcaptions)
  • Provide more comprehensive media file analysis
  • Avoid duplicate runs with concurrency control
  • Track media attachments in linked GitHub issues

The workflow is informational only and does not fail PRs. It posts a comment with:

  • Images missing alt attributes
  • Figures missing figcaptions
  • All media file references in changed HTML files
  • Media file references found in linked issues
  • Suggestions to use WebP format for better performance

Related Issues

Fixes #571
Supersedes #670

Original prompt

please fix this @alphaonelabs/alphaonelabs-education-website/pull/670


💡 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.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 5, 2025

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Comment @coderabbitai help to get the list of available commands and usage tips.

Co-authored-by: A1L13N <193832434+A1L13N@users.noreply.github.com>
Copilot AI changed the title [WIP] please fix this @alphaonelabs/alphaonelabs-education-website/pull/670 Fix: Complete rewrite of check-media-attachments workflow with all review fixes Oct 5, 2025
Copilot AI requested a review from A1L13N October 5, 2025 18:56
@A1L13N A1L13N marked this pull request as ready for review October 5, 2025 22:28
@A1L13N
Copy link
Copy Markdown
Contributor

A1L13N commented Oct 8, 2025

@copilot we want this to detect attachments in the github issue if an html file was changed

Co-authored-by: A1L13N <193832434+A1L13N@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Oct 8, 2025

@copilot we want this to detect attachments in the github issue if an html file was changed

Added linked issue media detection in commit e0869f4. The workflow now:

  1. Extracts issue numbers from the PR body (e.g., "Fixes issue with checking for media attachments #571", "Closes darkmode/about #123")
  2. Fetches each linked issue and checks its body for media attachments
  3. Detects both HTML media tags and Markdown image syntax (e.g., ![alt](image.jpg))
  4. Reports findings in a separate "Media Files Found in Linked Issues" section

The comment will now show media from both changed HTML files and linked issues.

@A1L13N A1L13N closed this Oct 8, 2025
@A1L13N A1L13N deleted the copilot/fix-a8588417-3ca9-4e18-8953-1244f2e7876a branch October 8, 2025 15:34
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.

issue with checking for media attachments

2 participants