PR workflow: Improve review status signals and reduce noise (#6938)
* Initial plan * Improve PR review workflow status signals and add agent personas - Add Job 4 (report-skipped) to explicitly report when visual review is skipped - Update resolve-review-urls.js to output skip status and reason - Add clear agent persona headers to all bot comments (Preview Bot, Doc Review Bot) - Reduce URL duplication by having visual review reference PR Preview comment - Update copilot-visual-review.md template with completion signal format - Add consistent status tables with emojis for clear at-a-glance status Co-authored-by: jstirnaman <212227+jstirnaman@users.noreply.github.com> * Address code review feedback - Remove unnecessary comment about fallback default in resolve-review-urls.js - Use plain text sanitization for skip reasons in preview comments - Add clarifying comment about string comparison in GitHub Actions outputs Co-authored-by: jstirnaman <212227+jstirnaman@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jstirnaman <212227+jstirnaman@users.noreply.github.com> Co-authored-by: Jason Stirnaman <jstirnaman@influxdata.com>copilot/fix-broken-docker-link
parent
21c9c9f191
commit
b6dc879bf4
|
|
@ -21,14 +21,64 @@ For each preview URL, verify:
|
|||
- [ ] **Navigation correct** — Sidebar entries link to the right pages and
|
||||
the page appears in the expected location
|
||||
|
||||
## Output
|
||||
## Output Format
|
||||
|
||||
**IMPORTANT:** Your response must follow this exact format for clear completion signaling.
|
||||
|
||||
### Response Header
|
||||
|
||||
Start your response with this header (fill in the values):
|
||||
|
||||
```markdown
|
||||
## 🔍 Visual Review Complete — Copilot
|
||||
|
||||
| Status | Details |
|
||||
|--------|---------|
|
||||
| **Result** | ✅ APPROVED / ⚠️ CHANGES REQUESTED / 🔍 NEEDS HUMAN REVIEW |
|
||||
| **Pages Reviewed** | X page(s) |
|
||||
| **Issues Found** | X BLOCKING, X WARNING, X INFO |
|
||||
| **Reviewed** | YYYY-MM-DD HH:MM UTC |
|
||||
```
|
||||
|
||||
### Result Rules
|
||||
|
||||
- **APPROVED** — Zero BLOCKING issues found
|
||||
- **CHANGES REQUESTED** — One or more BLOCKING issues found
|
||||
- **NEEDS HUMAN REVIEW** — Cannot determine severity or page didn't load
|
||||
|
||||
### Issue Format
|
||||
|
||||
List any issues found using this format:
|
||||
|
||||
```markdown
|
||||
### Issues Found
|
||||
|
||||
#### BLOCKING
|
||||
|
||||
- **[page-url]** — Description of the issue
|
||||
- Suggested fix: ...
|
||||
|
||||
#### WARNING
|
||||
|
||||
- **[page-url]** — Description of the issue
|
||||
|
||||
#### INFO
|
||||
|
||||
- **[page-url]** — Observation
|
||||
```
|
||||
|
||||
If no issues are found, write:
|
||||
|
||||
```markdown
|
||||
### Issues Found
|
||||
|
||||
No issues found. All pages pass visual review.
|
||||
```
|
||||
|
||||
## Severity Definitions
|
||||
|
||||
Follow the shared review comment format, severity definitions, and label
|
||||
mapping in
|
||||
[templates/review-comment.md](../templates/review-comment.md).
|
||||
|
||||
Adapt the "Files Reviewed" section to list preview URLs instead of file
|
||||
paths.
|
||||
|
||||
## Preview URLs
|
||||
|
||||
|
|
|
|||
|
|
@ -44,12 +44,14 @@ export function generatePreviewComment(options) {
|
|||
|
||||
const timestamp = new Date().toISOString().replace('T', ' ').split('.')[0] + ' UTC';
|
||||
|
||||
let body = `${COMMENT_MARKER}\n## PR Preview\n\n`;
|
||||
// Agent persona header for clear identification
|
||||
let body = `${COMMENT_MARKER}\n## 📦 PR Preview — Preview Bot\n\n`;
|
||||
|
||||
switch (status) {
|
||||
case 'success':
|
||||
body += `| Status | Details |\n`;
|
||||
body += `|--------|----------|\n`;
|
||||
body += `| **Result** | ✅ DEPLOYED |\n`;
|
||||
body += `| **Preview** | [View preview](${previewUrl}) |\n`;
|
||||
body += `| **Pages** | ${pages.length} page(s) deployed |\n`;
|
||||
if (buildTime) {
|
||||
|
|
@ -74,30 +76,42 @@ export function generatePreviewComment(options) {
|
|||
|
||||
case 'pending':
|
||||
if (needsInput) {
|
||||
body += `| Status | Details |\n`;
|
||||
body += `|--------|----------|\n`;
|
||||
body += `| **Result** | ⏳ NEEDS INPUT |\n`;
|
||||
body += `| **Checked** | ${timestamp} |\n\n`;
|
||||
body += `### Preview pages needed\n\n`;
|
||||
body += `This PR changes layout/asset files but doesn't specify which pages to preview.\n\n`;
|
||||
body += `**To generate a preview**, add documentation URLs to your PR description, for example:\n`;
|
||||
body += `\`\`\`\nPlease review:\n- https://docs.influxdata.com/influxdb3/core/get-started/\n- /telegraf/v1/plugins/\n\`\`\`\n\n`;
|
||||
body += `Then re-run the workflow or push a new commit.\n\n`;
|
||||
body += `---\n<sub>Last checked: ${timestamp}</sub>`;
|
||||
body += `Then re-run the workflow or push a new commit.`;
|
||||
} else {
|
||||
body += `⏳ **Preview building...**\n\n`;
|
||||
body += `---\n<sub>Started: ${timestamp}</sub>`;
|
||||
body += `| Status | Details |\n`;
|
||||
body += `|--------|----------|\n`;
|
||||
body += `| **Result** | ⏳ BUILDING |\n`;
|
||||
body += `| **Started** | ${timestamp} |\n\n`;
|
||||
body += `Preview is building...`;
|
||||
}
|
||||
break;
|
||||
|
||||
case 'failed':
|
||||
body += `### Preview failed\n\n`;
|
||||
body += `The preview build encountered an error:\n\n`;
|
||||
body += `| Status | Details |\n`;
|
||||
body += `|--------|----------|\n`;
|
||||
body += `| **Result** | ❌ FAILED |\n`;
|
||||
body += `| **Failed** | ${timestamp} |\n\n`;
|
||||
body += `### Build Error\n\n`;
|
||||
body += `\`\`\`\n${sanitizeForCodeBlock(errorMessage)}\n\`\`\`\n\n`;
|
||||
body += `[View workflow logs](https://github.com/influxdata/docs-v2/actions)\n\n`;
|
||||
body += `---\n<sub>Failed: ${timestamp}</sub>`;
|
||||
body += `[View workflow logs](https://github.com/influxdata/docs-v2/actions)`;
|
||||
break;
|
||||
|
||||
case 'skipped':
|
||||
body += `### Preview skipped\n\n`;
|
||||
body += `${sanitizeForCodeBlock(skipReason || 'No previewable changes detected.', 500)}\n\n`;
|
||||
body += `---\n<sub>Checked: ${timestamp}</sub>`;
|
||||
// Skip reasons are controlled strings from the workflow, plain text sanitization is sufficient
|
||||
const safeSkipReason = (skipReason || 'No previewable changes detected.').substring(0, 200);
|
||||
body += `| Status | Details |\n`;
|
||||
body += `|--------|----------|\n`;
|
||||
body += `| **Result** | ⏭️ SKIPPED |\n`;
|
||||
body += `| **Reason** | ${safeSkipReason} |\n`;
|
||||
body += `| **Checked** | ${timestamp} |`;
|
||||
break;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -7,6 +7,8 @@
|
|||
* Outputs (for GitHub Actions):
|
||||
* - urls: JSON array of URL paths
|
||||
* - url-count: Number of URLs
|
||||
* - skipped: Boolean indicating if review was skipped
|
||||
* - skip-reason: Reason for skipping (if applicable)
|
||||
*/
|
||||
|
||||
import { appendFileSync } from 'fs';
|
||||
|
|
@ -58,4 +60,17 @@ const urls = [...new Set([...homePageUrls, ...contentUrls])].slice(
|
|||
appendFileSync(GITHUB_OUTPUT, `urls=${JSON.stringify(urls)}\n`);
|
||||
appendFileSync(GITHUB_OUTPUT, `url-count=${urls.length}\n`);
|
||||
|
||||
// Output skip status for downstream jobs
|
||||
if (urls.length === 0) {
|
||||
appendFileSync(GITHUB_OUTPUT, `skipped=true\n`);
|
||||
const skipReason =
|
||||
changed.length === 0
|
||||
? 'No content files changed in this PR'
|
||||
: 'Changed files do not map to previewable URLs';
|
||||
appendFileSync(GITHUB_OUTPUT, `skip-reason=${skipReason}\n`);
|
||||
console.log(`Visual review skipped: ${skipReason}`);
|
||||
} else {
|
||||
appendFileSync(GITHUB_OUTPUT, `skipped=false\n`);
|
||||
}
|
||||
|
||||
console.log(`Detected ${urls.length} preview URLs`);
|
||||
|
|
|
|||
|
|
@ -38,6 +38,8 @@ jobs:
|
|||
outputs:
|
||||
urls: ${{ steps.detect.outputs.urls }}
|
||||
url-count: ${{ steps.detect.outputs.url-count }}
|
||||
skipped: ${{ steps.detect.outputs.skipped }}
|
||||
skip-reason: ${{ steps.detect.outputs.skip-reason }}
|
||||
steps:
|
||||
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
|
||||
with:
|
||||
|
|
@ -176,11 +178,6 @@ jobs:
|
|||
const prNumber = context.issue.number || Number(process.env.PR_NUMBER);
|
||||
const previewBase = `https://influxdata.github.io/docs-v2/pr-preview/pr-${prNumber}`;
|
||||
|
||||
// Build preview URL list
|
||||
const urlList = urls
|
||||
.map(u => `- [${u}](${previewBase}${u})`)
|
||||
.join('\n');
|
||||
|
||||
// Read the Copilot visual review template
|
||||
const template = fs.readFileSync(
|
||||
'.github/prompts/copilot-visual-review.md',
|
||||
|
|
@ -188,26 +185,33 @@ jobs:
|
|||
);
|
||||
|
||||
const marker = '<!-- doc-review-visual -->';
|
||||
const timestamp = new Date().toISOString().replace('T', ' ').split('.')[0] + ' UTC';
|
||||
const body = [
|
||||
marker,
|
||||
'## Preview Pages for Review',
|
||||
'## 🔍 Visual Review — Doc Review Bot',
|
||||
'',
|
||||
`${urls.length} page(s) changed in this PR:`,
|
||||
`| Status | Details |`,
|
||||
`|--------|---------|`,
|
||||
`| **Pages** | ${urls.length} page(s) to review |`,
|
||||
`| **Preview** | See PR Preview comment above for full URL list |`,
|
||||
`| **Requested** | ${timestamp} |`,
|
||||
'',
|
||||
'<details>',
|
||||
'<summary>Preview URLs</summary>',
|
||||
'',
|
||||
urlList,
|
||||
'',
|
||||
'</details>',
|
||||
'> **Note:** Preview URLs are listed in the **PR Preview** comment to avoid duplication.',
|
||||
'> Click the preview link there to view the deployed pages.',
|
||||
'',
|
||||
'---',
|
||||
'',
|
||||
`@github-copilot please review the preview pages listed above using the template below:`,
|
||||
`@github-copilot please review the ${urls.length} preview pages for this PR using the checklist below:`,
|
||||
'',
|
||||
template.trim(),
|
||||
'',
|
||||
urlList,
|
||||
'### Pages to Review',
|
||||
'',
|
||||
...urls.slice(0, 10).map(u => {
|
||||
const pageUrl = `${previewBase}${u}`;
|
||||
return `- \`${u}\` → [preview](${pageUrl})`;
|
||||
}),
|
||||
...(urls.length > 10 ? [`- ... and ${urls.length - 10} more (see PR Preview comment)`] : []),
|
||||
].join('\n');
|
||||
|
||||
// Update existing comment or create new one
|
||||
|
|
@ -245,15 +249,28 @@ jobs:
|
|||
script: |
|
||||
const prNumber = context.issue.number || Number(process.env.PR_NUMBER);
|
||||
const marker = '<!-- doc-review-visual-timeout -->';
|
||||
const timestamp = new Date().toISOString().replace('T', ' ').split('.')[0] + ' UTC';
|
||||
const body = [
|
||||
marker,
|
||||
'## Visual Review Skipped',
|
||||
'## 🔍 Visual Review — Doc Review Bot',
|
||||
'',
|
||||
'The PR preview deployment did not become available within 10 minutes.',
|
||||
'Visual review was skipped. The Copilot code review (Job 2) still ran.',
|
||||
`| Status | Details |`,
|
||||
`|--------|---------|`,
|
||||
`| **Result** | ⏱️ TIMED OUT |`,
|
||||
`| **Reason** | Preview deployment not available within 10 minutes |`,
|
||||
`| **Checked** | ${timestamp} |`,
|
||||
'',
|
||||
'To trigger visual review manually, re-run this workflow after the',
|
||||
'preview is deployed.',
|
||||
'### What This Means',
|
||||
'',
|
||||
'- Visual review was **skipped** because the preview was not deployed in time',
|
||||
'- Copilot code review (diff-based) still ran independently',
|
||||
'- Human reviewers should check the preview manually when available',
|
||||
'',
|
||||
'### Next Steps',
|
||||
'',
|
||||
'To trigger visual review:',
|
||||
'1. Wait for the PR Preview to deploy (check the PR Preview comment)',
|
||||
'2. Re-run this workflow from the Actions tab',
|
||||
].join('\n');
|
||||
|
||||
const { data: comments } = await github.rest.issues.listComments({
|
||||
|
|
@ -278,3 +295,72 @@ jobs:
|
|||
body,
|
||||
});
|
||||
}
|
||||
|
||||
# -----------------------------------------------------------------
|
||||
# Job 4: Report when visual review is skipped (no URLs to review)
|
||||
# -----------------------------------------------------------------
|
||||
report-skipped:
|
||||
runs-on: ubuntu-latest
|
||||
permissions:
|
||||
pull-requests: write
|
||||
needs: resolve-urls
|
||||
# GitHub Actions outputs are always strings; 'true' string comparison is intentional
|
||||
if: needs.resolve-urls.result == 'success' && needs.resolve-urls.outputs.skipped == 'true'
|
||||
steps:
|
||||
- name: Post skipped notice
|
||||
uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0
|
||||
env:
|
||||
PR_NUMBER: ${{ github.event.pull_request.number || inputs.pr_number }}
|
||||
SKIP_REASON: ${{ needs.resolve-urls.outputs.skip-reason }}
|
||||
with:
|
||||
script: |
|
||||
const prNumber = context.issue.number || Number(process.env.PR_NUMBER);
|
||||
const skipReason = process.env.SKIP_REASON || 'No previewable content changes detected';
|
||||
const marker = '<!-- doc-review-visual-skipped -->';
|
||||
const timestamp = new Date().toISOString().replace('T', ' ').split('.')[0] + ' UTC';
|
||||
const body = [
|
||||
marker,
|
||||
'## 🔍 Visual Review — Doc Review Bot',
|
||||
'',
|
||||
`| Status | Details |`,
|
||||
`|--------|---------|`,
|
||||
`| **Result** | ⏭️ SKIPPED |`,
|
||||
`| **Reason** | ${skipReason} |`,
|
||||
`| **Checked** | ${timestamp} |`,
|
||||
'',
|
||||
'### What This Means',
|
||||
'',
|
||||
'- No preview pages were identified for visual review',
|
||||
'- This is expected for PRs that only change non-content files',
|
||||
'- Copilot code review (diff-based) still runs independently',
|
||||
'',
|
||||
'### If You Expected Visual Review',
|
||||
'',
|
||||
'Check that your PR includes changes to files in `content/` that map to',
|
||||
'published documentation pages.',
|
||||
].join('\n');
|
||||
|
||||
const { data: comments } = await github.rest.issues.listComments({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
issue_number: prNumber,
|
||||
});
|
||||
const existing = comments.find(c => c.body.includes(marker));
|
||||
|
||||
if (existing) {
|
||||
await github.rest.issues.updateComment({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
comment_id: existing.id,
|
||||
body,
|
||||
});
|
||||
} else {
|
||||
await github.rest.issues.createComment({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
issue_number: prNumber,
|
||||
body,
|
||||
});
|
||||
}
|
||||
|
||||
core.info(`Posted visual review skipped notice: ${skipReason}`);
|
||||
|
|
|
|||
|
|
@ -58,11 +58,12 @@ jobs:
|
|||
const existing = comments.find(c => c.body.includes(marker));
|
||||
|
||||
if (!existing) {
|
||||
const timestamp = new Date().toISOString().replace('T', ' ').split('.')[0] + ' UTC';
|
||||
await github.rest.issues.createComment({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
issue_number: context.issue.number,
|
||||
body: `${marker}\n## 📝 PR Preview Not Available\n\nPR previews are not available for pull requests from forks due to GitHub Actions security restrictions.\n\nTo preview your changes locally, run:\n\`\`\`bash\nnpx hugo server\n\`\`\`\n\nOnce merged, your changes will be visible on the production site.`
|
||||
body: `${marker}\n## 📦 PR Preview — Preview Bot\n\n| Status | Details |\n|--------|----------|\n| **Result** | ⏭️ NOT AVAILABLE |\n| **Reason** | Fork PR (security restriction) |\n| **Checked** | ${timestamp} |\n\n### Local Preview\n\nPR previews are not available for pull requests from forks due to GitHub Actions security restrictions.\n\nTo preview your changes locally, run:\n\`\`\`bash\nnpx hugo server\n\`\`\`\n\nOnce merged, your changes will be visible on the production site.`
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue