-
Notifications
You must be signed in to change notification settings - Fork 1.1k
ci: enhance documentation check for running on all PR's (testing) #21325
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
…es and remove labeled condition
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
This PR enhances the documentation check workflow to run on all pull requests, adding intelligent differentiation between new PRs and PR updates. The workflow now installs the Coder CLI, creates AI-powered tasks to analyze documentation needs, and streams real-time task output to GitHub Actions.
Key changes:
- Added Coder CLI installation and authentication steps for task execution
- Implemented trigger type detection to provide different analysis prompts for new PRs versus PR updates
- Added real-time task output streaming and automatic workspace cleanup
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if: always() | ||
| env: |
Copilot
AI
Dec 18, 2025
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 command coder ssh "${TASK_NAME}" -- timeout 600 curl -sN http://localhost:3284/events could fail silently due to the || true at the end of the while loop. While this prevents workflow failure, it means connection issues, authentication problems, or other SSH failures won't be visible. Consider logging the exit status or adding more specific error handling before falling back to || true.
| if: always() | |
| env: | |
| done | |
| STREAM_EXIT_STATUS=${PIPESTATUS[0]} | |
| if [[ "${STREAM_EXIT_STATUS}" -ne 0 ]]; then | |
| echo "Warning: coder ssh stream exited with status ${STREAM_EXIT_STATUS}" >&2 | |
| fi |
.github/workflows/doc-check.yaml
Outdated
| echo "---" | ||
| fi | ||
| # Check for status change to stable (task complete) | ||
| STATUS=$(echo "$DATA" | jq -r '.status // empty' 2>/dev/null) | ||
| if [[ "$STATUS" == "stable" ]]; then | ||
| echo "" |
Copilot
AI
Dec 18, 2025
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 jq parsing on line 389 uses jq -r '.message // empty' with error redirection to /dev/null. If the JSON parsing fails (malformed JSON), the error is silently ignored, potentially causing missed output. Consider checking if jq parsing succeeds or logging parse failures to help debug issues with the event stream format.
| exit 1 | ||
| fi | ||
| sleep 5 |
Copilot
AI
Dec 18, 2025
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 status check logic only handles "failed" and "canceled" workspace states, but there might be other error states that should cause the workflow to exit (e.g., "stopped", "deleting", "error"). Consider checking for any non-success states more comprehensively, or document why only these two states are checked.
| sleep 5 | |
| if [[ "$WORKSPACE_STATUS" == "failed" || "$WORKSPACE_STATUS" == "canceled" || "$WORKSPACE_STATUS" == "stopped" || "$WORKSPACE_STATUS" == "deleting" || "$WORKSPACE_STATUS" == "error" ]]; then |
| SETUPEOF | ||
| ) | ||
| # Full analysis comment format (for new PRs) | ||
| COMMENT_FORMAT=$(cat <<'COMMENTEOF' | ||
| FULL ANALYSIS COMMENT FORMAT (for new PRs): | ||
| <!-- doc-check-analysis --> |
Copilot
AI
Dec 18, 2025
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 SETUP_INSTRUCTIONS heredoc uses an unquoted delimiter (SETUPEOF) which allows PR_NUMBER to expand. While this appears intentional, if PR_NUMBER contains any shell metacharacters, it could lead to unexpected behavior when this instruction is included in the task prompt. Consider validating PR_NUMBER is numeric before using it in the heredoc, or using a safer substitution method.
.github/workflows/doc-check.yaml
Outdated
| steps: | ||
| - name: Install Coder CLI | ||
| run: | | ||
| curl -fsSL "${{ secrets.DOC_CHECK_CODER_URL }}/install.sh" | sh |
Copilot
AI
Dec 18, 2025
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 Coder CLI installation script is executed from a URL stored in secrets without verifying its integrity (e.g., checksum verification). This could be a security risk if the secret is compromised or if there's a man-in-the-middle attack. Consider adding checksum verification or using a pinned version from a trusted source.
| curl -fsSL "${{ secrets.DOC_CHECK_CODER_URL }}/install.sh" | sh | |
| curl -fsSL "${{ secrets.DOC_CHECK_CODER_URL }}/install.sh" -o coder-install.sh | |
| echo "${{ secrets.DOC_CHECK_CODER_INSTALL_SHA256 }} coder-install.sh" | sha256sum -c - | |
| sh coder-install.sh |
.github/workflows/doc-check.yaml
Outdated
| if [[ "$line" == data:* ]]; then | ||
| DATA="${line#data:}" | ||
Copilot
AI
Dec 18, 2025
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 timeout value of 600 seconds (10 minutes) for the curl command is hardcoded. This should be consistent with or related to other timeout values in the workflow, or extracted as a configurable parameter. Additionally, consider whether 10 minutes is sufficient for complex PR analyses.
| if [[ "$line" == data:* ]]; then | |
| DATA="${line#data:}" | |
| # Configure stream timeout (seconds), defaulting to 600 if not set | |
| STREAM_TIMEOUT_SECONDS="${STREAM_TIMEOUT_SECONDS:-600}" | |
| # Stream events from AgentAPI via SSH | |
| # This shows real-time AI conversation output | |
| coder ssh "${TASK_NAME}" -- timeout "${STREAM_TIMEOUT_SECONDS}" curl -sN http://localhost:3284/events 2>/dev/null | \ |
| else | ||
| echo "No task name found, skipping cleanup" | ||
| fi | ||
Copilot
AI
Dec 18, 2025
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 task deletion command uses 2>&1 || echo "Task deletion failed or already deleted", which masks the actual error that occurred. If the deletion fails for reasons other than "already deleted" (e.g., permissions issues, network problems), this could hide important debugging information. Consider capturing and logging the actual error message, or at least distinguishing between different failure modes.
| if ! coder exp task delete -y "${TASK_NAME}" 2>&1; then | |
| status=$? | |
| echo "Task deletion command failed with exit code ${status}." >&2 | |
| echo "The task may already be deleted, or there may be another issue (e.g., permissions or network problems)." >&2 | |
| exit "${status}" | |
| fi |
.github/workflows/doc-check.yaml
Outdated
| # Parse SSE events | ||
| if [[ "$line" == data:* ]]; then | ||
| DATA="${line#data:}" | ||
Copilot
AI
Dec 18, 2025
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 hardcoded localhost URL http://localhost:3284/events assumes a specific port and endpoint for the AgentAPI. If this configuration changes or varies between environments, the streaming will fail silently due to the || true. Consider making the port and endpoint configurable, or at least document why this specific endpoint is used.
| AGENTAPI_EVENTS_URL="${AGENTAPI_EVENTS_URL:-http://localhost:3284/events}" | |
| coder ssh "${TASK_NAME}" -- timeout 600 curl -sN "${AGENTAPI_EVENTS_URL}" 2>/dev/null | \ |
| Review PR #${PR_NUMBER} and determine if documentation needs updating or creating. | ||
| This is a NEW PR - perform a complete analysis from scratch. | ||
| PR URL: ${PR_URL} | ||
| WORKFLOW: | ||
| 1. Setup (repo is pre-cloned at ~/coder) | ||
| cd ~/coder | ||
| git fetch origin pull/${PR_NUMBER}/head:pr-${PR_NUMBER} | ||
| git checkout pr-${PR_NUMBER} | ||
| ${SETUP_INSTRUCTIONS} | ||
| 2. Get PR info | ||
| WORKFLOW: | ||
| 1. Get PR info | ||
| Use GitHub MCP tools to get PR title, body, and diff | ||
| Or use: git diff main...pr-${PR_NUMBER} | ||
| 3. Understand Changes | ||
| Read the diff and identify what changed | ||
| 2. Understand ALL Changes | ||
| Read the entire diff and identify what changed | ||
| Ask: Is this user-facing? Does it change behavior? Is it a new feature? | ||
| 4. Search for Related Docs | ||
| 3. Search for Related Docs | ||
| cat ~/coder/docs/manifest.json | jq '.routes[] | {title, path}' | head -50 | ||
| grep -ri "relevant_term" ~/coder/docs/ --include="*.md" | ||
| 5. Decide | ||
| 4. Decide | ||
| NEEDS DOCS if: New feature, API change, CLI change, behavior change, user-visible | ||
| NO DOCS if: Internal refactor, test-only, already documented, non-user-facing, dependency updates | ||
| FIRST check: Did this PR already update docs? If yes and complete, say "No Changes Needed" | ||
| 6. Comment on the PR using this format | ||
| 5. Comment on the PR | ||
| - This is a new PR, so CREATE a new comment with the marker | ||
| - Use the format below | ||
| - Keep headings clean (no emojis in headings) | ||
| - Use status indicators sparingly (✓ ⚠ ✗) | ||
| COMMENT FORMAT: | ||
| ## 📚 Documentation Check | ||
| ${COMMENT_FORMAT} | ||
| ### ✅ Updates Needed | ||
| - **[docs/path/file.md](github_link)** - Brief what needs changing | ||
| DOCS STRUCTURE: | ||
| Read ~/coder/docs/manifest.json for the complete documentation structure. | ||
| Common areas include: reference/, admin/, user-guides/, ai-coder/, install/, tutorials/ | ||
| ### 📝 New Docs Needed | ||
| - **docs/suggested/location.md** - What should be documented | ||
| EOF | ||
| ) | ||
| ### ✨ No Changes Needed | ||
| [Reason: Documents already updated in PR | Internal changes only | Test-only | No user-facing impact] | ||
| else | ||
| echo "Building PR UPDATE prompt (incremental analysis)" | ||
Copilot
AI
Dec 18, 2025
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 heredoc uses an unquoted delimiter (EOF) which allows variable expansion. While this appears intentional for PR_NUMBER, it creates a risk of command injection if any of the environment variables (PR_URL, PR_NUMBER, TRIGGER_TYPE) contain malicious content. Since these come from GitHub context, they should be properly sanitized or the heredoc should use quoted delimiters with explicit variable substitution only for trusted values.
| PR #${PR_NUMBER} has been UPDATED with new commits (trigger: ${TRIGGER_TYPE}). | ||
| PR URL: ${PR_URL} | ||
| ${SETUP_INSTRUCTIONS} | ||
| IMPORTANT CONTEXT: | ||
| - This PR was previously analyzed (there may be an earlier doc-check comment) | ||
| - New commits have been pushed since then | ||
| - Your job is to provide a BRIEF update, not repeat the full analysis | ||
| WORKFLOW: | ||
| 1. Check recent commits | ||
| git log --oneline -5 | ||
| See what was added/changed recently | ||
| 2. Quick assessment | ||
| - Do the new changes affect documentation needs? | ||
| - Were docs added/updated in the new commits? | ||
| - Is this a significant change or minor fix? | ||
| 3. Post a SHORT update comment | ||
| - DO NOT update or edit previous comments | ||
| - Create a NEW, BRIEF comment (see format below) | ||
| - Keep it conversational and minimal | ||
| - Only do a full re-analysis if changes are substantial | ||
| UPDATE COMMENT FORMAT (keep it SHORT!): | ||
| <!-- doc-check-update --> | ||
| ### Doc Check Update | ||
| **Commits reviewed:** [X new commits] | ||
| [Pick ONE status line based on situation:] | ||
| ✓ **No changes needed** - [brief reason: minor fix / internal change / etc.] | ||
| ✓ **Docs updated** - [what was added/changed in docs] | ||
| ⚠ **Still needs docs** - [brief reminder of what's outstanding] | ||
| ⚠ **Updated but requesting changes** - [acknowledge changes but note what's still missing] | ||
| ✗ **New issues found** - [if new commits introduce new doc requirements] | ||
| --- | ||
| *This comment was generated by an AI Agent through [Coder Tasks](https://coder.com/docs/ai-coder/tasks)* | ||
| *[Coder Tasks](https://coder.com/docs/ai-coder/tasks)* | ||
| DOCS STRUCTURE: | ||
| Read ~/coder/docs/manifest.json for the complete documentation structure. | ||
| Common areas include: reference/, admin/, user-guides/, ai-coder/, install/, tutorials/ | ||
| But check manifest.json - it has everything. | ||
| GUIDELINES: | ||
| - Be concise! 2-4 lines is ideal | ||
| - Don't repeat previous analysis | ||
| - Focus only on what CHANGED | ||
| - Use emojis sparingly (only ✓ ⚠ ✗ for status) | ||
| - No emojis in headings | ||
| - If nothing documentation-relevant changed, just say so briefly | ||
| - Only do full analysis if the PR direction changed significantly | ||
| EOF | ||
| ) | ||
| fi | ||
| # Output the prompt | ||
| { |
Copilot
AI
Dec 18, 2025
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 same security concern applies here - the unquoted heredoc delimiter allows variable expansion of all environment variables, which could lead to command injection if PR_URL, PR_NUMBER, or TRIGGER_TYPE contain malicious content. Consider using a quoted delimiter ('EOF') and explicitly substituting only the trusted variables.
Documentation CheckAnalyzed: 2025-12-18 22:30 UTC No Changes NeededThis PR modifies the internal CI workflow (
Reason: Internal CI/CD changes only. No user-facing features, APIs, CLI commands, or product behavior changes. This workflow is used by maintainers/contributors, not end users of Coder. Analysis by Coder Tasks - Updates will appear as new comments |
Documentation CheckAnalyzed: 2025-12-18 No Changes NeededThis PR modifies the
Reason: Internal CI workflow changes only - no user-facing documentation impact. Analysis by Coder Tasks - Updates will appear as new comments |
- Add explicit STEP markers with CHECKPOINT requirements - Require showing command output as evidence - Make commenting the FINAL step only after analysis - Add warnings against skipping steps or making assumptions - Require specific file/line references in decisions
Coder extracts display_name from first line of prompt. Now shows 'Doc Check: PR #XXXX - Full Analysis' or 'Doc Check: PR #XXXX - Update' instead of random Docker-style names like 'Agitated khayyam'
Doc Check UpdateCommits reviewed: 3 new commits (006ac48, c1c987a, 4be7d7c) ✓ No changes needed - Recent commits only modify |
Doc Check UpdateCommits reviewed: 3 new commits (006ac48, c1c987a, 4be7d7c) ✓ No changes needed - Recent commits only modified |
Doc Check UpdateCommits reviewed: 5 new commits (d779f42 through 4be7d7c) ✓ No changes needed - All recent commits are CI workflow improvements to |
Doc Check UpdateCommits reviewed: 3 new commits ✓ No changes needed - CI workflow refinements only (doc-check formatting/reporting improvements), no feature or documentation changes. |
Doc Check UpdateCommits reviewed: 5 new commits ✓ No changes needed - Recent commits only modify CI workflow configuration ( |
Doc Check UpdateCommits reviewed: 5 new commits ✓ No changes needed - All recent commits are CI workflow improvements (doc-check.yaml formatting and reporting enhancements). No feature or documentation changes. |
Doc Check UpdateCommits reviewed: 5 new commits (3466cdb...f7c760b) ✓ No changes needed - Recent commits are CI workflow improvements to the doc-check process itself (). No changes to PR content or documentation files. |
.github/workflows/doc-check.yaml
Outdated
| steps: | ||
| - name: Install Coder CLI | ||
| run: | | ||
| curl -fsSL "${{ secrets.DOC_CHECK_CODER_URL }}/install.sh" | sh |
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.
you could also use https://github.com/coder/setup-action
No description provided.