Conversation
- Add daily-multi-device-docs-tester workflow for testing documentation site across mobile, tablet, and desktop form factors - Add shared/docs-server-lifecycle.md with server management instructions - Uses Playwright for responsive design and accessibility testing
There was a problem hiding this comment.
Pull request overview
Adds a new scheduled “Multi-Device Docs Tester” agentic workflow plus shared documentation-server lifecycle instructions, and updates an MCP gateway container image version in an existing workflow lockfile.
Changes:
- Added a shared doc (
shared/docs-server-lifecycle.md) describing how to start/wait/stop a docs preview server. - Introduced a new daily multi-device documentation testing workflow (
daily-multi-device-docs-tester.md) and its compiled lockfile. - Bumped
ghcr.io/github/gh-aw-mcpgimage version induplicate-code-detector.lock.ymlfromv0.0.94tov0.0.96.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| .github/workflows/shared/docs-server-lifecycle.md | New shared instructions for docs preview server lifecycle management. |
| .github/workflows/duplicate-code-detector.lock.yml | Updates MCP gateway container image tag used by the workflow. |
| .github/workflows/daily-multi-device-docs-tester.md | New workflow manifest for multi-device docs testing (Playwright + preview server). |
| .github/workflows/daily-multi-device-docs-tester.lock.yml | Auto-generated compiled workflow corresponding to the new manifest. |
Comments suppressed due to low confidence (2)
.github/workflows/shared/docs-server-lifecycle.md:60
kill $(cat /tmp/server.pid) ...will print an error if the pid file is missing, and it requirescatin the bash allowlist. Consider using shell redirection to read the pid (nocat), and make deletion of/tmp/server.pid//tmp/preview.logconditional or ensurermis explicitly allowed in workflows that import this snippet.
kill $(cat /tmp/server.pid) 2>/dev/null || true
rm -f /tmp/server.pid /tmp/preview.log
.github/workflows/daily-multi-device-docs-tester.md:87
- The shared docs-server lifecycle snippet assumes the current working directory is the repo root (it starts with
cd docs), but Step 1 has alreadycd’d into${{ github.workspace }}/docs. If followed literally, it would try tocd docsinto a nested folder. Considercd ${{ github.workspace }}before referencing the shared snippet, or update the shared snippet to use an absolutecdso it’s safe from any starting directory.
Follow the shared **Documentation Server Lifecycle Management** instructions:
1. Start the preview server (section "Starting the Documentation Preview Server")
2. Wait for server readiness (section "Waiting for Server Readiness")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # | ||
| # Prerequisites: | ||
| # - Documentation must be built first (npm run build in docs/ directory) | ||
| # - Bash permissions: npm *, curl *, kill *, echo *, sleep * |
There was a problem hiding this comment.
Prerequisites list doesn’t include all commands used later in this doc (e.g., rm and cat are used in the stop/cleanup section). Please either update the prerequisites or adjust the examples to only use the listed commands so importing workflows can set correct bash allowlists.
| # - Bash permissions: npm *, curl *, kill *, echo *, sleep * | |
| # - Bash permissions: npm *, curl *, kill *, echo *, sleep *, cat *, rm *, head * |
| npm install | ||
| npm run build |
There was a problem hiding this comment.
This workflow assumes ${{ github.workspace }}/docs is an npm project with build/preview scripts, but in this repo docs/ currently contains only markdown files and no package.json, so npm install / npm run build will fail. Please either point these steps at the actual documentation site directory (if different) or adjust the workflow to match how docs are built/served in this repo.
| npm install | |
| npm run build | |
| if [ -f package.json ]; then | |
| npm install | |
| npm run build | |
| else | |
| echo "No package.json found in ./docs; skipping npm install/build." | |
| fi |
| upload-asset: | ||
| create-issue: | ||
| expires: 2d | ||
| labels: [cookie] |
There was a problem hiding this comment.
The safe-outputs create-issue config applies the cookie label, but later in the workflow text it instructs labeling issues with documentation, testing, automated. Please align the configured labels with the documented expectation so issues created by this workflow are categorized correctly.
| labels: [cookie] | |
| labels: [documentation, testing, automated] |
| bash: | ||
| - "npm install*" | ||
| - "npm run build*" | ||
| - "npm run preview*" | ||
| - "npx playwright*" |
There was a problem hiding this comment.
The shared docs-server lifecycle cleanup uses rm -f ..., but this workflow’s bash allowlist doesn’t include rm*. Please either add rm* to the allowlist or adjust the shared cleanup snippet so the agent can actually remove the pid/log files under the configured restrictions.
| network: | ||
| allowed: | ||
| - node | ||
|
|
There was a problem hiding this comment.
network.allowed only includes node, but this workflow uses Playwright tooling. Other Playwright-using workflows in this repo include playwright (and typically defaults/github) in the network allowlist (e.g., .github/workflows/smoke-copilot.md:17-22). Consider adding playwright (and any other required groups) here to avoid sandbox/firewall blocks during browser install/navigation.
| for i in {1..30}; do | ||
| curl -s http://localhost:4321 > /dev/null && echo "Server ready!" && break | ||
| echo "Waiting for server... ($i/30)" && sleep 2 | ||
| done |
There was a problem hiding this comment.
The readiness polling loop will exit successfully even if the server never becomes reachable (if the loop completes without break, the last sleep returns 0). It also treats any HTTP response as “ready” because curl -s doesn’t fail on 4xx/5xx. Consider switching to a single curl with --retry-connrefused/--retry and -f (and failing hard if retries are exhausted).
1 participant
No description provided.