Conversation
…L gotchas Capture cross-project gotchas siloed in downstream project memory: - security-config.md: explicit `permissions: read-all` anti-pattern in Token-Permissions section (scores 0 vs 10 for explicit per-permission scopes); CodeQL Supported Languages subsection noting PHP is not supported (use `javascript-typescript` + `actions` matrix on PHP/TYPO3 repos). - tag-validation.md: expand the one-line bullets in "Batch PR Merging Gotchas" into proper subsections covering `gh pr merge --delete-branch` failing under merge queues (with detection snippet) and Contents API commits not satisfying `required_signatures` (with three workarounds: `--admin`, SSH push, GitHub App). Cross-reference auto-merge-guide.md for the stale-reviews / Copilot-race and signed-rebase cases that are already canonically documented there rather than restate them. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates documentation regarding GitHub Actions security configurations, CodeQL language support, and multi-repo merge strategies. It specifically warns against using permissions: read-all, clarifies that CodeQL does not support PHP, and provides workarounds for merge queue and commit signing issues. A review comment correctly pointed out that the suggested script for detecting merge queues used an incorrect API endpoint and lacked error handling, providing a corrected version using gh pr view.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Pull request overview
This PR expands the GitHub project reference docs with cross-project operational guidance around batch PR merging, branch protection, workflow permissions, and CodeQL configuration so consumers of the skill have the same gotchas documented centrally.
Changes:
- Expanded
tag-validation.mdwith fuller guidance for batch PR merges, including merge-queue and signed-commit caveats. - Added a new
permissions: read-allanti-pattern section tosecurity-config.md. - Added CodeQL guidance for PHP/TYPO3 repositories, including supported language matrix recommendations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
skills/github-project/references/tag-validation.md |
Reworked batch-merge gotchas into subsections and added merge-queue / required-signatures guidance with cross-references. |
skills/github-project/references/security-config.md |
Added workflow-permissions guidance and a new CodeQL subsection for PHP/TYPO3 repositories. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ction The original snippet queried `gh api "repos/$REPO"` for a `merge_queue` field that the REST endpoint does not actually return — it would always evaluate to null and never detect the queue, defeating the gotcha's whole purpose. Caught by Gemini Code Assist on PR #72. Use the GraphQL `Repository.mergeQueue` field instead (returns null when no queue is configured) and add explicit error handling on the API call. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
…ranch snippet Hardcoding `--merge` contradicts the bullet right above warning that repos may only allow squash or rebase. In a mixed fleet, copying the "fix" still produces "merge method not allowed" failures on squash-only or rebase-only repos. Wire in the same dynamic strategy detection used in auto-merge-guide.md so both the queue check and the strategy choice are per-repo. Caught by Copilot review on PR #72. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
|
Summary
Capture cross-project gotchas that were siloed in downstream project memory (
t3x-nr-passkeys-be'sMEMORY.md) so all consumers of the skill see them.security-config.md:permissions: read-all" subsection in the Token-Permissions area.read-allscores 0 on the Scorecard Token-Permissions check; explicit per-permission scopes score 10. Includes before/after snippet.javascript-typescript+actionsas the matrix to use on PHP/TYPO3 repos. Cites community discussion #158392 and notes that listingjavascript(the deprecated name) breaks v3+.tag-validation.md: promote one-line bullets in "Batch PR Merging Gotchas" into proper subsections:gh pr merge --delete-branchfails with merge queues" — withgh api repos/$REPO --jq '.merge_queue // null'detection snippet for batch loops.required_signatures" — explains theweb-flowcommitter identity issue, lists three workarounds:--adminbypass, SSH push of locally-signed commits, GitHub App with verified signing. Cross-referencesmulti-repo-operations.mdfor the related HTTP 409 angle.auto-merge-guide.md"Signed Commits and Merge Strategy Compatibility"; stale-reviews / Copilot race →auto-merge-guide.md"Auto-Approve Race Condition with Copilot Reviewer". Adds a real-world heterogeneity note (Netresearch fleet has repos with all three permutations of allowed merge methods).Two of the six lessons from the source
MEMORY.mdare intentionally not added: the per-repo merge-method detection snippet and thedismiss_stale_reviews+ Copilot race are already canonically documented inauto-merge-guide.md(which explicitly says it's the canonical home and project memories should cross-reference). Adding them again would duplicate exactly what those sections warn against.Test plan
references/security-config.md/references/tag-validation.mdsections — only additions and one bullet rewrite