fix(ci): restore CI on docs-only changes to protect documentation contracts - #142
Merged
Merged
Conversation
…tracts tests/test_documentation_contract.py reads the CONTENT of docs/PRD.md, docs/TRD.md, docs/ERD.md, docs/TRACEABILITY.md, docs/adr/*.md, README.md, CHANGELOG.md, and other canonical documents, asserting specific required text (e.g. RFC citations, ADR links, PR-integration notes) — the same "contract tests pin ... prose" pattern this org's own central .github repo documents. PR #141's paths-ignore on ci.yml's push/pull_request triggers made the account-unification-tests job (which runs this contract suite) skip entirely on docs-only changes, so a docs-only PR that silently broke one of these assertions would never be caught by CI. The checked and unchecked paths crisscross within the same directories (all of docs/adr/ is contract-checked, but only 2 of 11 files under docs/doctoring/ are; 9 of 16 top-level docs/*.md files are checked and 7 aren't), and REQUIRED_DOCUMENTS/GOVERNING_ADRS are easy to extend without anyone remembering to also update a separate workflow paths-ignore list. A narrowed allowlist would reintroduce the same silent gap the next time a new document join the contract. Reverting the trigger paths-ignore is the safe default; codeql.yml's paths-ignore is untouched since CodeQL doesn't execute this suite and isn't a doc-currency mechanism. Verified: `uv run --locked --project services/account_unification --extra dev pytest tests/test_documentation_contract.py -q` (7 passed), plus ruff and interrogate, all green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPull request 이벤트에서 ChangesCI 트리거 업데이트
Estimated code review effort: 1 (매우 낮음) | ~2분 ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Safety issue
.github/workflows/ci.yml'spaths-ignore: ["docs/**", "*.md"](added in#141 to reduce org-wide Actions queue congestion) makes the
account-unification-testsjob — which runstests/test_documentation_contract.py— skip entirely on docs-only PRs.That suite reads the content of
docs/PRD.md,docs/TRD.md,docs/ERD.md,docs/TRACEABILITY.md,docs/adr/*.md,README.md,CHANGELOG.md, and other canonical documents and asserts specificrequired text is present (RFC citations, ADR cross-links, PR-integration
notes, etc.) — this repo's own instance of the "contract tests pin
workflows AND prose" pattern that
ContextualWisdomLab/.githubdocumentsfor itself. With the paths-ignore in place, a docs-only PR that silently
broke one of these assertions would never be caught by CI.
Why revert instead of narrow
The checked/unchecked paths crisscross within the same directories:
docs/adr/— all 10 files are contract-checked (8 governing ADRs +0013-mcp-oauth-client-authorization.md+ theREADME.mdindex).docs/doctoring/— only 2 of 11 files are checked(
product-technical-gap-baseline.md,mcp-oauth-authorization.md).docs/*.md— 9 of 16 are checked (PRD,TRD,ERD,TEST_STRATEGY,THREAT_MODEL,TRACEABILITY,UML,OPERABILITY,product-technical-gap-baseline.md); 7 aren't.*.md— 6 of 7 files are checked (README,CHANGELOG,DOCUMENTATION,ARCHITECTURE,AGENTS,CLAUDE); onlySECURITY.mdisn't.
REQUIRED_DOCUMENTS/GOVERNING_ADRSin the test file are easy to extendwithout anyone remembering to also update a separate workflow
paths-ignorelist — a narrowed allowlist would just reintroduce thesame silent gap the next time a document joins the contract. Reverting
the trigger
paths-ignoreinci.ymlis the safe default here.codeql.yml'spaths-ignoreis left untouched: CodeQL scans code, notdocs, and doesn't execute this test suite, so it isn't a doc-currency
mechanism and the original congestion-reduction rationale still holds
for it.
Other doc-currency mechanisms checked
Searched the full repo for any other test/workflow reading doc content
(
rgacross all file types, plus explicit checks oftest_links.py,test_deployment_contracts.py,test_implementation_completeness.py,and every
.github/workflows/*.ymlfor a docs-build/lint/link-checkerstep). None found —
tests/test_documentation_contract.pyis the onlymechanism, and it's now fully addressed.
Test plan
uv run --locked --project services/account_unification --extra dev pytest tests/test_documentation_contract.py -q— 7 passed (samecommand
ci.ymlruns)uv run ruff check app tests tools— all checks passeduv run interrogate .— 100% docstring coveragepython3 -c "import yaml; yaml.safe_load(open('.github/workflows/ci.yml'))"— valid YAMLci.yml's trigger block is now byte-identical to itspre-fix(ci): skip docs-only changes for ci and CodeQL workflows #141 state (commit
655ff06)🤖 Generated with Claude Code
Summary by CodeRabbit