Tiger Style v1.1 hardening: align policy, enforcement, and adoption truth#2
Merged
robertguss merged 15 commits intomainfrom Feb 9, 2026
Merged
Tiger Style v1.1 hardening: align policy, enforcement, and adoption truth#2robertguss merged 15 commits intomainfrom
robertguss merged 15 commits intomainfrom
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ✏️ Tip: You can disable in-progress messages and the fortune message in your review settings. Tip You can customize the high-level summary generated by CodeRabbit.Configure the ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughImplements Tiger Style v1.1 hardening decisions, introducing a language-activation manifest, tier-based planning requirements (Tier 0–3), enhanced CI workflow with per-language gates, updated security/recursion/test-code policies, evidence validation semantics, and comprehensive test fixtures for validation. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 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: c78add95f3
ℹ️ 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".
Comment on lines
+161
to
+165
| if [[ -n "$RISK_SECTION" ]]; then | ||
| validate_section_fields "Risk Tier" "$RISK_SECTION" \ | ||
| "Tier|Tier" \ | ||
| "Rationale|Rationale" | ||
|
|
There was a problem hiding this comment.
The new semantic validation only runs when the section body is non-empty (if [[ -n "$RISK_SECTION" ]]). This means a PR can include all required headings but leave sections blank (no bullet fields), and the validator will skip field checks entirely and still pass. That contradicts the intent of the new enforcement and allows heading-only evidence to slip through. Consider treating an empty section body as an error (e.g., run validate_section_fields unconditionally or explicitly fail on empty bodies).
Useful? React with 👍 / 👎.
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
2 similar comments
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/contract-gates.yml:
- Around line 140-141: The availability checks (e.g., the step named "Ensure
Rust toolchain is available" and the corresponding Python/TypeScript
availability checks) must be preceded by setup steps that actually install the
required toolchains and linters; add a Rust setup step (use
dtolnay/rust-action@v1 with components rustfmt, clippy), a Python setup step
(actions/setup-python@v5 with python-version '3.x' and a run to pip install ruff
mypy pytest), and a Node setup step (actions/setup-node@v4 with node-version
'20' and a run to npm ci) before the existing "Ensure ... is available" checks
so the command -v checks succeed and the gates validate the tools rather than
failing on missing installs.
In `@09-upstream-tiger-style-v1.1-patch-list.md`:
- Around line 3-5: The file contains a user-specific hardcoded path
"/Users/robertguss/Projects/programming_tiger_style"; remove this line or
replace it with a neutral placeholder like "<upstream-repo-path>" (or simply
omit it) so the document no longer references a local, user-specific filesystem
location; update the line that currently contains
"/Users/robertguss/Projects/programming_tiger_style" accordingly.
In `@scripts/validate_evidence_packet.sh`:
- Around line 161-178: The current logic skips field validation when
RISK_SECTION is empty, letting a present heading with no body pass; update the
script to detect a present "Risk Tier" heading but empty/whitespace body and
treat that as an error: after the existing heading-presence check (the code that
ensures the heading exists) add a guard that if the heading exists but
RISK_SECTION is empty or only whitespace, emit an error and set errors=1; apply
the same pattern for RED_SECTION, GREEN_SECTION, and REFACTOR_SECTION so
headings without bodies fail validation (refer to the RISK_SECTION variable and
the validate_section_fields call to locate where to add the check).
🧹 Nitpick comments (3)
scripts/validate_tdd_cycle.sh (1)
60-82: Unreachable pattern detected by static analysis.Shellcheck correctly identifies that
resources/*.md|resources/*.txton line 69 will never match because*.mdand*.txton line 63 already match those paths first. The current code is functionally correct (all intended paths return 0), but theresources/*.md|resources/*.txtpatterns are dead code.♻️ Suggested fix to remove redundant patterns
is_non_executable_path() { local path="$1" case "$path" in *.md|*.rst|*.txt|LICENSE) return 0 ;; docs/*|contracts/*|templates/*|checklists/*|.evidence/*) return 0 ;; - resources/*.md|resources/*.txt|resources/*.pdf) + resources/*.pdf) return 0 ;; .github/pull_request_template.md|.gitignore|.prettierrc.json) return 0 ;; - README.md|CONTRACT_SYSTEM_V2.md|PLANS.md) - return 0 - ;; *) return 1 ;; esac }scripts/tests/test_validate_evidence_packet.sh (1)
19-21: Missing existence check for requiredexpect_exitfile.If a fixture directory lacks the
expect_exitfile, line 21 will fail with a confusing error due toset -e. Consider adding an existence check with a clear error message for fixture authors.Proposed fix
local expected_exit + if [[ ! -f "$fixture_dir/expect_exit" ]]; then + echo "FAIL [$fixture_name]: missing expect_exit file" >&2 + failures=$((failures + 1)) + return + fi expected_exit="$(<"$fixture_dir/expect_exit")"scripts/tests/test_validate_tdd_cycle.sh (1)
19-20: Same missingexpect_exitexistence check as sibling script.For consistency, consider adding the same safeguard suggested for
test_validate_evidence_packet.sh.
Comment on lines
+140
to
+141
| - name: Ensure Rust toolchain is available | ||
| run: command -v cargo >/dev/null || (echo "Rust is active but cargo is unavailable in this runner." >&2; exit 1) |
There was a problem hiding this comment.
Tooling availability checks will fail without installation steps.
The language gates check for tool availability but don't install the tools. ubuntu-latest does not include ruff, mypy, pytest, or Rust's clippy by default. The gates will fail at the availability check rather than at the actual validation step.
Consider adding setup actions before the availability checks:
🔧 Suggested setup additions
For Rust gates (before line 140):
- name: Install Rust toolchain
uses: dtolnay/rust-action@v1
with:
components: rustfmt, clippyFor Python gates (before line 167):
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.x'
- name: Install Python tooling
run: pip install ruff mypy pytestFor TypeScript gates (before line 194):
- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: '20'
- name: Install dependencies
run: npm ciAlso applies to: 167-171, 194-198
🤖 Prompt for AI Agents
In @.github/workflows/contract-gates.yml around lines 140 - 141, The
availability checks (e.g., the step named "Ensure Rust toolchain is available"
and the corresponding Python/TypeScript availability checks) must be preceded by
setup steps that actually install the required toolchains and linters; add a
Rust setup step (use dtolnay/rust-action@v1 with components rustfmt, clippy), a
Python setup step (actions/setup-python@v5 with python-version '3.x' and a run
to pip install ruff mypy pytest), and a Node setup step (actions/setup-node@v4
with node-version '20' and a run to npm ci) before the existing "Ensure ... is
available" checks so the command -v checks succeed and the gates validate the
tools rather than failing on missing installs.
Comment on lines
+3
to
+5
| Target upstream repository: | ||
|
|
||
| - `/Users/robertguss/Projects/programming_tiger_style` |
There was a problem hiding this comment.
Remove or generalize the hardcoded local path.
The path /Users/robertguss/Projects/programming_tiger_style is user-specific and will confuse other contributors. Consider using a placeholder like <upstream-repo-path> or simply removing this line since the document title already indicates the target.
Proposed fix
Target upstream repository:
-- `/Users/robertguss/Projects/programming_tiger_style`
+- `<upstream-tiger-style-repo>`🤖 Prompt for AI Agents
In `@09-upstream-tiger-style-v1.1-patch-list.md` around lines 3 - 5, The file
contains a user-specific hardcoded path
"/Users/robertguss/Projects/programming_tiger_style"; remove this line or
replace it with a neutral placeholder like "<upstream-repo-path>" (or simply
omit it) so the document no longer references a local, user-specific filesystem
location; update the line that currently contains
"/Users/robertguss/Projects/programming_tiger_style" accordingly.
Comment on lines
+161
to
+178
| if [[ -n "$RISK_SECTION" ]]; then | ||
| validate_section_fields "Risk Tier" "$RISK_SECTION" \ | ||
| "Tier|Tier" \ | ||
| "Rationale|Rationale" | ||
|
|
||
| tier_line="$(grep -Eim1 '^[[:space:]]*[-*][[:space:]]*Tier[[:space:]]*:' <<<"$RISK_SECTION" || true)" | ||
| if [[ -n "$tier_line" ]]; then | ||
| tier_value="$(trim_value "${tier_line#*:}")" | ||
| if [[ -n "$tier_value" ]]; then | ||
| tier_clean="$(tr -d '[:space:]' <<<"$tier_value")" | ||
| tier_clean="${tier_clean//\`/}" | ||
| if ! [[ "$tier_clean" =~ ^[0-3]$ ]]; then | ||
| echo "Risk Tier section must include Tier as one of 0, 1, 2, or 3." >&2 | ||
| errors=1 | ||
| fi | ||
| fi | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Edge case: Empty section body after heading.
The validation at lines 161-178 (and similar blocks for Red/Green/Refactor) only runs if the section body is non-empty ([[ -n "$RISK_SECTION" ]]). However, if a heading exists but the section content is blank or only whitespace, get_section_body returns empty and field validation is skipped entirely.
The heading check at lines 84-89 ensures the heading exists, but an author could pass that check with just ## Risk Tier and no content below it.
Proposed fix to require non-empty section content
if [[ -n "$RISK_SECTION" ]]; then
validate_section_fields "Risk Tier" "$RISK_SECTION" \
"Tier|Tier" \
"Rationale|Rationale"
+else
+ echo "Risk Tier section is empty or contains only whitespace." >&2
+ errors=1
+fiApply similar patterns for RED_SECTION, GREEN_SECTION, and REFACTOR_SECTION.
🤖 Prompt for AI Agents
In `@scripts/validate_evidence_packet.sh` around lines 161 - 178, The current
logic skips field validation when RISK_SECTION is empty, letting a present
heading with no body pass; update the script to detect a present "Risk Tier"
heading but empty/whitespace body and treat that as an error: after the existing
heading-presence check (the code that ensures the heading exists) add a guard
that if the heading exists but RISK_SECTION is empty or only whitespace, emit an
error and set errors=1; apply the same pattern for RED_SECTION, GREEN_SECTION,
and REFACTOR_SECTION so headings without bodies fail validation (refer to the
RISK_SECTION variable and the validate_section_fields call to locate where to
add the check).
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
2 similar comments
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
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.
Objective
Risk Tier
1Scope
scripts/validate_tdd_cycle.sh,scripts/validate_evidence_packet.sh, and fixture-based validator tests.Red
scripts/tests/test_validate_tdd_cycle.shfixtures (stray_green_after_complete_cycle,trailing_open_cycle) andscripts/tests/test_validate_evidence_packet.shsemantic-missing fixtures.bash scripts/tests/test_validate_tdd_cycle.shandbash scripts/tests/test_validate_evidence_packet.shGREEN, open trailing cycle, and missing semantic evidence fields).Green
bash scripts/tests/test_validate_tdd_cycle.shandbash scripts/tests/test_validate_evidence_packet.shRefactor
bash -n scripts/validate_tdd_cycle.sh,bash -n scripts/validate_evidence_packet.sh,bash scripts/validate_tdd_cycle.sh --base HEAD,bash scripts/validate_evidence_packet.sh --pr-body .github/pull_request_template.mdInvariants
RED|GREEN|REFACTOR|DOCS|CHORE|BUILD|TEST; evidence headings remain fixed contract surface.Security Impact
Performance Impact
Assumptions
Open Questions
activeimmediately incontracts/ACTIVE_LANGUAGE_CONTRACTS.mdversus keeping all inactive as baseline template state.Rollback Plan
Validation Commands
Exceptions
no(set toyesonly with explicit approval)Checklist
checklists/PR_CONTRACT_CHECKLIST.md.checklists/ADVERSARIAL_REVIEW_CHECKLIST.mdif Tier 2/Tier 3.Summary by CodeRabbit
Documentation
Chores