docs(#5521): add CI workflow conventions guide#5523
docs(#5521): add CI workflow conventions guide#5523fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
Create docs/contributing/ci-workflows.md covering concurrency group patterns, timeout policy, trigger completeness (merge_group), checkout SHA pinning, reusable workflow contracts, permissions, and additional conventions. Uses checklist format for reliable agent consumption during review. Add a row to the AGENTS.md topic-specific guidance table linking the new doc for .github/workflows/ changes. Conventions are derived from patterns already established in the repo's existing workflows (e2e.yml, functional-tests.yml, mint-cf-worker-test.yml) and from review feedback on PR #5482 that caught a redundant concurrency prefix, missing timeout-minutes, and missing merge_group trigger. Note: pre-commit could not run in sandbox (network restriction on git fetch). The post-script runs pre-commit authoritatively on the runner. Closes #5521
|
🤖 Finished Review · ✅ Success · Started 10:14 AM UTC · Completed 10:30 AM UTC |
Site previewPreview: https://dbddee96-site.fullsend-ai.workers.dev Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ReviewFindingsMedium
Low
Labels: PR adds CI workflow conventions documentation under docs/contributing/ and registers it in AGENTS.md |
|
|
||
| ## Concurrency groups | ||
|
|
||
| - [ ] Use `${{ github.workflow }}` as the workflow identifier — never duplicate the workflow name as a hardcoded string prefix. |
There was a problem hiding this comment.
[medium] API behavior claim
The concurrency groups section states 'Use ${{ github.workflow }} as the workflow identifier' as a blanket rule. In reusable workflows (on: workflow_call), github.workflow resolves to the caller's workflow name, not the reusable workflow's own name. Multiple different reusable workflows called from the same caller would share a concurrency group and cancel each other. The repo's 7 reusable workflows all correctly use hardcoded role-specific prefixes (e.g., fullsend-code-agent-). The document needs a carve-out for reusable workflows.
Suggested fix: Add an exception note: reusable workflows (on: workflow_call) must use a hardcoded role-specific prefix instead of github.workflow, because github.workflow resolves to the caller's name in workflow_call context.
| | [Design Decisions](docs/contributing/design-decisions.md) | Understanding architectural principles and key decisions | | ||
| | [Vouch System](docs/contributing/vouch-system.md) | Working with the contributor vouch gate or PR workflows | | ||
| | [Tier Conventions](docs/contributing/tier-conventions.md) | Using the term "tier" in code or docs — covers the three distinct tier contexts | | ||
| | [CI Workflows](docs/contributing/ci-workflows.md) | Adding or modifying GitHub Actions workflows under `.github/workflows/` | |
There was a problem hiding this comment.
[medium] protected-path
AGENTS.md is a protected governance file. The PR adds a table entry for the new CI Workflows doc. The change is authorized by issue #5521 and the PR description explains the rationale. Human approval is required for all protected-path changes.
| - [ ] For `pull_request_target` workflows, use a PR-number-scoped group for that event and fall back to the standard pattern for other triggers: | ||
| ```yaml | ||
| concurrency: | ||
| group: >- |
There was a problem hiding this comment.
[low] edge-case
The pull_request_target concurrency example omits cancel-in-progress entirely, defaulting to false. The repo's actual pull_request_target workflows (e2e.yml, functional-tests.yml) use a sophisticated cancel-in-progress expression gating on event type and label. A reader following the documented example would not get cancellation behavior for superseded PR runs.
Summary
Add
docs/contributing/ci-workflows.mdto document CI workflow conventions that currently exist only as tribal knowledge. Register it in the AGENTS.md topic-specific guidance table so the review agent references these conventions when reviewing.github/workflows/changes.Related Issue
Closes #5521
Changes
docs/contributing/ci-workflows.mdcovering all 5 convention areas from the issue:${{ github.workflow }}, avoid redundant hardcoded prefixes)timeout-minutes)merge_group:on path-filtered workflows with path-relevance guard)pinactfor verification)CI Workflows | Adding or modifying GitHub Actions workflows under .github/workflows/Testing
make lint-md-linkspasses — all links in new doc resolvedocs/contributing/viagetMarkdownFiles()— no manual config neededChecklist
!for breaking changes)Closes #5521
Post-script verification
agent/5521-ci-workflow-conventions)054111862bdd61201a722bc4c28f3fa0c382bdcf..HEAD)