Contract System v2: core contracts, enforcement, and CI gates#1
Contract System v2: core contracts, enforcement, and CI gates#1robertguss merged 7 commits intomainfrom
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds Contract System v2: a governance suite of core contracts, language integration notes, templates, checklists, CI workflow, and validation scripts to enforce TDD (RED→GREEN→REFACTOR) and evidence-packet requirements across PRs and pushes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Author as PR Author
participant GitHub as GitHub Actions
participant Repo as Repository (scripts, templates, contracts)
participant Validator as Validation Scripts
participant Reviewer as Human Reviewer
Author->>GitHub: Open PR / Push to main
GitHub->>Repo: Checkout PR head (or branch)
GitHub->>Repo: Ensure scripts executable & lint
GitHub->>Validator: run validate_tdd_cycle.sh --base BASE_SHA
Validator-->>GitHub: TDD validation result
alt event is pull_request
GitHub->>Validator: run validate_evidence_packet.sh --pr-body PR_BODY
else push
GitHub->>Validator: if .evidence/EVIDENCE_PACKET.md present run validate_evidence_packet.sh --file path
end
Validator-->>GitHub: evidence validation result
GitHub->>Author: report CI status (pass/fail)
Author->>Reviewer: request review (if CI pass)
Reviewer->>Repo: review contracts/templates and evidence
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c2383d4da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.github/workflows/contract-gates.yml:
- Around line 30-42: Replace the HEAD~1 logic with the push event's before SHA
so the full pushed commit range is validated: when github.event_name is
"pull_request" keep BASE_SHA="$(jq -r '.pull_request.base.sha'
"$GITHUB_EVENT_PATH")", when it's "push" set BASE_SHA="$(jq -r '.before'
"$GITHUB_EVENT_PATH")" and if that value is empty or all-zero fallback to the
repository root (use git rev-list --max-parents=0 HEAD | tail -n 1) before
invoking scripts/validate_tdd_cycle.sh; keep the existing fallback used for
other events. Reference BASE_SHA, GITHUB_EVENT_PATH and
scripts/validate_tdd_cycle.sh when making the change.
In `@contracts/core/RISK_TIER_POLICY.md`:
- Around line 18-29: Update the "Evidence packet" row in the RISK_TIER_POLICY.md
controls table so Tier 0 reads "Required" (not "Optional") to match the Auditing
section and CI validation; locate the "Evidence packet" table row (and any
duplicate instances around the other table at lines ~51-53) and change the Tier
0 cell to "Required" so policy and enforcement are consistent.
In `@contracts/core/TDD_ENFORCEMENT_CONTRACT.md`:
- Around line 84-86: The "## Exceptions" heading conflicts with the sentence "No
TDD exception is allowed without explicit approval and documented risk
controls"; update the section to remove ambiguity by either deleting the "##
Exceptions" heading and statement if exceptions are absolutely forbidden, or
expand the section to define the exception process: replace the single sentence
with clear criteria and steps for requesting an exception, define what "explicit
approval" means (e.g., required approvers by role such as Tech
Lead/Architect/Compliance, how to record sign-off), list required documented
risk controls and where to store them, and include any timeboxed review/renewal
conditions so the behavior of the functions/phrases "## Exceptions" and "No TDD
exception is allowed without explicit approval and documented risk controls" are
consistent.
In `@scripts/validate_evidence_packet.sh`:
- Around line 54-94: Normalize CRLF by creating a temporary LF-only copy of the
input and use that copy for all validations: read "$INPUT_PATH", strip CR
characters (e.g., via sed -e 's/\r$//' or tr -d '\r') into a temp file (e.g.,
normalized_input), run the REQUIRED_HEADINGS loop and the grep placeholder check
against the normalized file instead of "$INPUT_PATH", and ensure the temp file
is removed on exit (use a trap to cleanup); update references to INPUT_PATH in
the heading check and placeholder check to point to the normalized file while
keeping the existing variables REQUIRED_HEADINGS, missing, and the grep patterns
unchanged.
🧹 Nitpick comments (12)
RESOURCES.md (1)
7-8: Clarify file locations for foundational references.The references to
TIGER_STYLE.mdandP10.pdflack location information. Consider adding:
- Repository paths if these files exist in the repo
- External URLs if they're hosted elsewhere
- A note indicating where to obtain them if they're proprietary/restricted
This will help users quickly access these foundational documents.
📝 Example of clearer reference format
## Foundations -1. `TIGER_STYLE.md` -2. `P10.pdf` (NASA/JPL Power of Ten) +1. `TIGER_STYLE.md` (located at `/docs/TIGER_STYLE.md`) +2. `P10.pdf` (NASA/JPL Power of Ten) - [available at NASA website](https://example.com) or `/docs/P10.pdf`templates/TASK_PACKET_TEMPLATE.md (1)
54-54: Optional: Consider hyphenation for compound modifier.Static analysis suggests hyphenating "Data handling" to "Data-handling" when used as a compound modifier before "constraints." However, "data handling" as two words is also commonly accepted. This is a minor style preference.
Optional refinement
-- Data handling constraints. +- Data-handling constraints.contracts/core/EVIDENCE_REQUIREMENTS.md (2)
11-14: Consider clarifying when to use repository files vs PR body.The guidance states evidence can be in either PR description or a repository file "for example
.evidence/EVIDENCE_PACKET.md". Consider adding criteria for when to use each approach (e.g., "Use repository files for complex multi-commit work or when evidence exceeds GitHub's character limits").
18-32: Consider flexibility for Tier 0 changes.The contract mandates 13 sections for "every evidence packet" without mentioning exemptions or simplified paths. If Tier 0 changes (docs-only, typo fixes) exist, they may not need full evidence packets. Consider adding a sentence acknowledging exemptions or referencing RISK_TIER_POLICY.md for scoping requirements by tier.
CONTRACT_SYSTEM_V2.md (2)
9-14: Add cross-references to core contracts for key concepts.Lines 11, 12, 13, and 14 mention "Risk-tier policy", "Evidence packet requirements", "Adversarial review model", and "CI automation" without linking to the detailed contracts. Consider adding inline references like:
- Risk-tier policy →
contracts/core/RISK_TIER_POLICY.md- Evidence packet requirements →
contracts/core/EVIDENCE_REQUIREMENTS.md- Adversarial review model →
checklists/ADVERSARIAL_REVIEW_CHECKLIST.mdThis improves discoverability for readers encountering these concepts for the first time.
23-28: Clarify adoption sequence timeline and current state.The "Adoption Sequence" describes a phased rollout but doesn't indicate:
- Whether these phases are sequential or can overlap
- What the current adoption state is (e.g., "Phase 1 complete, Phase 2 in progress")
- Who decides when to move between phases
Consider adding a status indicator or timeline, or explicitly stating this is a forward-looking roadmap.
contracts/core/TDD_ENFORCEMENT_CONTRACT.md (1)
25-32: Clarify how supporting commit prefixes interact with mandatory R-G-R sequence.Lines 25-30 allow
DOCS:,CHORE:,BUILD:,TEST:commits as "supporting commits", and line 32 requires "at least one complete Red -> Green -> Refactor sequence in each change set". Consider clarifying:
- Can supporting commits appear before the first RED: commit?
- Can supporting commits appear between R-G-R stages?
- Do supporting commits reset the R-G-R sequence requirement?
Example guidance: "Supporting commits may precede or follow R-G-R sequences but must not interrupt an incomplete sequence."
contracts/core/SECURITY_CONTRACT.md (2)
9-9: Provide guidance on threat modeling format and depth.Line 9 requires "Threat model required for Tier 2/Tier 3 changes" but doesn't specify format, depth, or tooling. Consider adding:
- Reference to a threat modeling template or framework (STRIDE, PASTA, etc.)
- Minimum required artifacts (attack trees, trust boundaries, threat list)
- Whether informal description is acceptable or formal model required
This would help ensure consistent threat model quality across PRs.
21-26: Consider defining "sensitive data" or providing classification guidance.Lines 21-26 require classifying and protecting "sensitive data" but don't define what qualifies. Consider adding:
- Examples (PII, credentials, financial data, health records)
- Reference to a data classification policy
- Inline note like "See RISK_TIER_POLICY.md for data sensitivity criteria"
This reduces ambiguity in applying the data protection requirements.
PYTHON_CODING_CONTRACT.md (1)
14-18: Consider clarifying "stricter rule" conflict resolution.Line 18 states "If rules conflict, apply the stricter rule and document rationale in evidence." This is pragmatic but potentially ambiguous when:
- Both rules are equally strict but contradictory (e.g., "always use X" vs "never use X")
- "Stricter" is subjective (performance vs safety tradeoffs)
Consider adding guidance like: "When strictness is unclear, escalate to reviewer/architect and document the resolution rationale."
.github/pull_request_template.md (1)
1-79: Consider adding inline guidance or examples for each section.The template provides structural placeholders but minimal guidance on what to write in each section. For first-time users, consider:
- Adding brief inline comments with examples (e.g., under
## Red:<!-- Example: "Test test_user_login_failure failed with AuthenticationError as expected" -->)- Linking to a filled example evidence packet in documentation
- Adding a "How to use this template" comment block at the top
This would improve adoption and reduce PR review cycles for guidance clarification.
contracts/core/AI_AGENT_CORE_CONTRACT.md (1)
91-91: Consider more concise phrasing.The phrase "all of the following" could be shortened to "all the following" for brevity.
✏️ Optional style improvement
-Exceptions require all of the following: +Exceptions require all the following:
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/validate_tdd_cycle.sh`:
- Around line 65-70: The case pattern list inside the while loop currently has
the broad "*.md" pattern before the specific "README.md" pattern, causing
README.md to never hit its explicit branch and triggering shellcheck
SC2221/SC2222; edit the case in scripts/validate_tdd_cycle.sh (the block that
checks variable DOC_ONLY and iterates with while IFS= read -r path; do ... case
"$path" in) and reorder the patterns so specific filenames like README.md appear
before the wildcard "*.md" (move "*.md" to the end of that pattern list) to
ensure README.md matches its explicit pattern and eliminate the ShellCheck
warnings.
🧹 Nitpick comments (1)
README.md (1)
75-78: Add the Contract System v2 overview to References for discoverability.Since
CONTRACT_SYSTEM_V2.mdis the conceptual entry point, listing it alongside references will help readers find it quickly.📝 Suggested addition
### References - `resources/RESOURCES.md` + - `CONTRACT_SYSTEM_V2.md`
|
Addressed requested review items (recommended threads 2-6) in commit e835755:\n\n1. Updated push-range TDD validation to use push event |
|
Follow-up: addressed and pushed additional review fixes.\n\nNew commits:\n- e835755: workflow base-SHA range fix, policy alignment, TDD exceptions clarification, CRLF normalization\n- 82e848c: shellcheck pattern-order cleanup in validate_tdd_cycle.sh\n\nResolved all outstanding review threads on the PR. |
1 participant
Objective
and CI gates.
Risk Tier
1Scope
CI workflow, and integration updates to existing language contracts/README.
Red
scripts/validate_tdd_cycle.sh --base mainandscripts/validate_evidence_packet.sh --pr-body .github/pull_request_template.md.Green
scripts/workflow to satisfy validation goals.
scripts/validate_tdd_cycle.sh --base mainandscripts/validate_evidence_packet.sh --pr-body .github/pull_request_template.md.Refactor
CONTRACT_SYSTEM_V2.mdto clarify system intent and adoption.bash -n scripts/validate_tdd_cycle.shandbash -n scripts/validate_evidence_packet.sh.Invariants
Security Impact
mandatory evidence sections.
Performance Impact
Assumptions
Open Questions
Rollback Plan
Validation Commands
Summary by CodeRabbit
Documentation
Chores
Tests