Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new safe-outputs option to push a follow-up empty commit (with a non-GITHUB_TOKEN auth context) so CI workflows trigger as expected, and updates docs/schema to reflect the new option and the Playwright network config change.
Changes:
- Introduces
github-token-for-extra-empty-commitforcreate-pull-requestandpush-to-pull-request-branchand wires it into workflow env. - Adds an
extra_empty_commithelper (with cycle prevention + tests) and calls it after PR creation / branch push. - Updates JSON schema + docs; migrates Playwright allowlist docs from
tools.playwright.allowed_domainsto top-levelnetwork.allowed.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/push_to_pull_request_branch.go | Adds config field + parsing for the extra-empty-commit token. |
| pkg/workflow/create_pull_request.go | Adds config field and injects GH_AW_EXTRA_EMPTY_COMMIT_TOKEN into the create PR job env when configured. |
| pkg/workflow/compiler_safe_outputs_job.go | Injects GH_AW_EXTRA_EMPTY_COMMIT_TOKEN at consolidated safe-outputs job level. |
| pkg/parser/schemas/main_workflow_schema.json | Documents the new safe-outputs option in the schema. |
| actions/setup/js/extra_empty_commit.cjs | Implements the empty-commit push helper with cycle prevention and error handling. |
| actions/setup/js/extra_empty_commit.test.cjs | Adds unit tests for the helper behavior and failure modes. |
| actions/setup/js/create_pull_request.cjs | Calls the helper after PR creation. |
| actions/setup/js/push_to_pull_request_branch.cjs | Calls the helper after pushing changes. |
| docs/src/content/docs/troubleshooting/common-issues.md | Updates Playwright network allowlist troubleshooting guidance. |
| docs/src/content/docs/reference/tools.md | Updates Playwright reference to network.allowed. |
| docs/src/content/docs/reference/safe-outputs.md | Documents the new CI-trigger token option and behavior. |
| docs/src/content/docs/reference/frontmatter-full.md | Removes deprecated Playwright allowed_domains documentation. |
| docs/src/content/docs/reference/faq.md | Reframes CI-trigger guidance and documents the recommended approach. |
| docs/src/content/docs/reference/auth.mdx | Adds auth docs for the new token option. |
| docs/public/editor/autocomplete-data.json | Removes deprecated Playwright allowed_domains from editor autocomplete data. |
| AGENTS.md | Updates example config from allowed_domains to network.allowed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Add extra empty commit token if configured (for pushing an empty commit to trigger CI) | ||
| extraEmptyCommitToken := data.SafeOutputs.CreatePullRequests.GithubTokenForExtraEmptyCommit | ||
| if extraEmptyCommitToken != "" { | ||
| if extraEmptyCommitToken == "app" { | ||
| customEnvVars = append(customEnvVars, " GH_AW_EXTRA_EMPTY_COMMIT_TOKEN: ${{ steps.safe-outputs-app-token.outputs.token || '' }}\n") |
There was a problem hiding this comment.
This new env var wiring is untested. There are existing unit/integration tests around create-pull-request job env var generation; please add coverage for github-token-for-extra-empty-commit (explicit token and 'app' cases) to prevent regressions.
| // Add extra empty commit token if configured on create-pull-request or push-to-pull-request-branch. | ||
| // This token is used to push an empty commit after code changes to trigger CI events, | ||
| // working around the GITHUB_TOKEN limitation where events don't trigger other workflows. | ||
| if data.SafeOutputs != nil { | ||
| var extraEmptyCommitToken string |
There was a problem hiding this comment.
Please extend the existing TestBuildJobLevelSafeOutputEnvVars coverage to include GH_AW_EXTRA_EMPTY_COMMIT_TOKEN when github-token-for-extra-empty-commit is set (including 'app' and precedence between create-pull-request vs push-to-pull-request-branch).
| // Configure git remote with the token for authentication | ||
| const remoteUrl = `https://x-access-token:${token}@github.com/${repoOwner}/${repoName}.git`; |
There was a problem hiding this comment.
The remote URL for the extra empty commit is hard-coded to github.com, which will fail on GitHub Enterprise Server (and is inconsistent with other codepaths that use GITHUB_SERVER_URL). Build the remote host from $GITHUB_SERVER_URL (or reuse the origin remote) so this works on GHES and GitHub.com.
| // Configure git remote with the token for authentication | |
| const remoteUrl = `https://x-access-token:${token}@github.com/${repoOwner}/${repoName}.git`; | |
| // Configure git remote with the token for authentication, supporting GitHub.com and GHES | |
| let serverUrl = process.env.GITHUB_SERVER_URL || "https://github.com"; | |
| // Normalize server URL (remove trailing slashes) | |
| serverUrl = serverUrl.replace(/\/+$/, ""); | |
| // Inject credentials into the server URL while preserving the scheme | |
| const authedServerUrl = serverUrl | |
| .replace(/^https:\/\//, `https://x-access-token:${token}@`) | |
| .replace(/^http:\/\//, `http://x-access-token:${token}@`); | |
| const remoteUrl = `${authedServerUrl}/${repoOwner}/${repoName}.git`; |
| const remoteUrl = `https://x-access-token:${token}@github.com/${repoOwner}/${repoName}.git`; | ||
|
|
||
| // Add a temporary remote with the token | ||
| try { | ||
| await exec.exec("git", ["remote", "remove", "ci-trigger"]); |
There was a problem hiding this comment.
This embeds the auth token directly in the git remote URL and then uses @actions/exec to run git commands. Since @actions/exec typically logs the full command, the token can leak to workflow logs (especially for GitHub App tokens which may not be auto-masked). Call core.setSecret(token) before use and/or run the remote add/push commands with silent output (or use a header-based auth approach) so the token never appears in logs.
|
Hey @dsyme 👋 — thanks for working on the CI triggering enhancement! This looks like useful functionality for the project. However, there are a few things that would help align this PR with the project's contribution guidelines:
This PR was created directly without following that process. 🔀 Mixed Changes: The PR mixes two unrelated changes:
These should be in separate PRs so reviewers can focus on one change at a time. ✅ What's Good:
Since this repository uses an agentic development workflow, the typical next step would be for a maintainer to decide whether to:
|
|
Hey @dsyme 👋 — thanks for working on the CI triggering enhancement! This looks like useful functionality for the project. However, there are a few things that would help align this PR with the project's contribution guidelines:
This PR was created directly without following that process. 🔀 Mixed Changes: The PR mixes two unrelated changes:
These should be in separate PRs so reviewers can focus on one change at a time. ✅ What's Good:
Since this repository uses an agentic development workflow, the typical next step would be for a maintainer to decide whether to accept this direct PR as an exception or ask you to close this and open an issue with an agentic plan instead.
|
Summary
github-token-for-extra-empty-commitoption tocreate-pull-requestandpush-to-pull-request-branchsafe outputs, which pushes an empty commit using an alternate token to work around theGITHUB_TOKENlimitation that prevents CI events from being triggeredtools.playwright.allowed_domainsfield in favour of the top-levelnetwork.allowedconfiguration