-
Notifications
You must be signed in to change notification settings - Fork 181
Revert "chore: Revert updates to honor maintenance mode " #94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This reverts commit 3df2e10.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Reintroduces the updated GitHub Skills “exercise” experience for Secure your repository's supply chain (reverting a prior revert), including new onboarding and workflow-driven step progression.
Changes:
- Updates learner-facing docs (README + step markdowns) to the newer “copy exercise + issue-comment guidance” format.
- Replaces the legacy step-tracking workflow system with
skills/exercise-toolkit-based workflows (Step 0–4) and removes the old Step 0 welcome workflow/files. - Refreshes repo hygiene/legal metadata (MIT license text formatting,
.gitignoreaddition).
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Switches to “exercise” onboarding with copy link + troubleshooting details. |
| LICENSE | Updates to standard MIT header/formatting and 2025 attribution. |
| .gitignore | Ignores .temp/** sample payload files. |
| .github/workflows/0-start-exercise.yml | Adds new Step 0 to initialize the exercise via exercise-toolkit. |
| .github/workflows/1-dependency-graph.yml | Migrates Step 1 automation to exercise-toolkit issue-comment pattern. |
| .github/workflows/2-dependabot-alerts.yml | Migrates Step 2 automation; adds scripted validation logic for minimist updates. |
| .github/workflows/3-dependabot-security.yml | Migrates Step 3 automation; adds scripted validation logic for axios updates. |
| .github/workflows/4-dependabot-versions.yml | Migrates Step 4 automation; posts review content and finishes exercise. |
| .github/workflows/0-welcome.yml | Removes legacy Step 0 welcome workflow. |
| .github/steps/1-dependency-graph.md | Updates Step 1 instructions (new UI paths + comment-driven progression). |
| .github/steps/2-dependabot-alerts.md | Updates Step 2 instructions (new UI paths + comment-driven progression). |
| .github/steps/3-dependabot-security.md | Updates Step 3 instructions (new UI paths + comment-driven progression). |
| .github/steps/4-dependabot-versions.md | Updates Step 4 instructions; adds note about Dependabot PR closure behavior. |
| .github/steps/x-review.md | Updates final review content/resources. |
| .github/steps/0-welcome.md | Removes legacy step marker file. |
| .github/steps/-step.txt | Removes legacy numeric step tracking file. |
Comments suppressed due to low confidence (1)
.github/steps/x-review.md:9
- Grammar: “view and use dependency graph” is missing an article. Consider “view and use the dependency graph”.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| file="$PACKAGE_LOCK_JSON" | ||
| keyphrase="minimist-(?!1\\.2\\.[0-5])(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)(?:-((?:0|[1-9][0-9]*|[0-9]*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9][0-9]*|[0-9]*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?" | ||
| minimum_occurrences=1 | ||
| found_occurrences=$(grep -o "$keyphrase" "$file" | wc -l) | ||
| if [ "$found_occurrences" -lt "$minimum_occurrences" ]; then |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue as above: this pattern uses PCRE-only syntax (?!...) but is evaluated with plain grep. On bash -eo pipefail, a no-match will fail the step immediately, which prevents the intended “update … to a valid minimist version” messaging. Use grep -P (and handle no-match safely) or parse package-lock.json with a structured tool.
| # Verify all checks passed | ||
| passed=$(echo $checks | jq '. | all(.passed?)') | ||
| # Flatten to an array for returning. Allows iteration during rendering. | ||
| results=$(echo $checks | jq 'to_entries | map({name: .key} + .value)') |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
passed=$(... | jq '. | all(.passed?)') doesn’t actually evaluate the nested .passed fields in this object shape, and passed/results are never used to decide whether to fail or comment back to the learner. Either remove these unused outputs or compute passed correctly and gate the rest of the workflow on it (with a clear issue comment on failure).
|
|
||
| # START: Check practical exercise | ||
|
|
||
| # Search for the comment about registration validation |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment appears to be a copy/paste artifact and doesn’t match the step logic (this job is checking package-lock.json for follow-redirects). Please update/remove it so future maintainers don’t get misled when editing the workflow.
| # Search for the comment about registration validation | |
| # Verify follow-redirects version in package-lock.json |
|
|
||
| # START: Check practical exercise | ||
|
|
||
| # Search for the comment about registration validation |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment appears to be a copy/paste artifact and doesn’t match the step logic (this job is checking .github/dependabot.yml for a nuget entry). Please update/remove it to avoid confusion.
| # Search for the comment about registration validation | |
| # Verify that .github/dependabot.yml contains the expected nuget entry |
| # Check for minimist version in package.json | ||
| file="$PACKAGE_JSON" |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This inline comment says “minimist” but the check is for axios. Please rename the comment(s) so they match what’s being validated.
| 1. Dependency graph | ||
| 2. Dependency alerts | ||
| 3. Dependency security updates | ||
| 4. Dependency versions updates |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling/grammar: “Dependency versions updates” reads incorrectly. Consider changing it to “Dependency version updates”.
| 4. Dependency versions updates | |
| 4. Dependency version updates |
| disable_workflows: | ||
| name: Disable workflows | ||
| runs-on: ubuntu-latest | ||
|
|
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The disable_workflows job runs unconditionally on every push to main, including in the template repository. Since it calls gh workflow disable across .github/workflows, merging this will disable the workflows in the template repo itself. Add the same if: !github.event.repository.is_template guard (and/or a more specific condition) to this job so workflow disabling only occurs in learner copies.
| file="$PACKAGE_JSON" | ||
| keyphrase="\"minimist\":[\ \\n\\r\\t]*\"\\^(?!1\\.2\\.[0-5])(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)(?:-((?:0|[1-9][0-9]*|[0-9]*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9][0-9]*|[0-9]*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?\"" | ||
| minimum_occurrences=1 | ||
| found_occurrences=$(grep -o "$keyphrase" "$file" | wc -l) | ||
| if [ "$found_occurrences" -lt "$minimum_occurrences" ]; then |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This regex uses PCRE constructs (e.g., negative lookahead (?!...)) but the command uses plain grep -o (BRE), so it will never match on ubuntu-latest. Also, with bash -eo pipefail, a no-match from grep will fail the whole step before you can emit your friendly message. Use a parser-based check (e.g., jq/node) or switch to grep -P and ensure the no-match case doesn’t abort the step prematurely.
| 1. Navigate to the **Settings** tab and select **Code security and analysis**. | ||
| 1. Locate "Dependabot version updates" and click **Configure** to open a new file editor with pre-poplulated contents. The file is called `dependabot.yml`. | ||
| 1. Navigate to the **Settings** tab and select **Advanced Security**. | ||
| 1. Locate **Dependabot version updates** and click **Configure** to open a new file editor with pre-poplulated contents. The file is called `dependabot.yml`. |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: “pre-poplulated” should be “pre-populated”.
| 1. Locate **Dependabot version updates** and click **Configure** to open a new file editor with pre-poplulated contents. The file is called `dependabot.yml`. | |
| 1. Locate **Dependabot version updates** and click **Configure** to open a new file editor with pre-populated contents. The file is called `dependabot.yml`. |
| 1. Navigate to the **Pull requests** repository tab and select the newly created pull request that updates axios from version 0.21.1 to a patched version. | ||
| 1. Navigate to the **Settings** tab and select **Advanced Security**. | ||
| 1. Enable **Dependabot security updates**. You may need to wait 30-60 seconds before you see any new pull requests. | ||
| 1. Navigate to the **Pull requests** repository tab to view the what Dependabot has found. |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Grammar: “to view the what Dependabot has found” should be “to view what Dependabot has found”.
| 1. Navigate to the **Pull requests** repository tab to view the what Dependabot has found. | |
| 1. Navigate to the **Pull requests** repository tab to view what Dependabot has found. |
Reverts #85