diff --git a/.claude/docs/analysis/CLAUDE_MD_UPDATES.md b/.claude/docs/analysis/CLAUDE_MD_UPDATES.md new file mode 100644 index 0000000000..64214ecac0 --- /dev/null +++ b/.claude/docs/analysis/CLAUDE_MD_UPDATES.md @@ -0,0 +1,346 @@ +# Concrete CLAUDE.md Updates Based on CI Breakage Analysis + +## Table of Contents + +- [Executive Summary of What Went Wrong](#executive-summary-of-what-went-wrong) +- [High Priority Updates](#high-priority-add-these-3-sections-immediately) + - [Section 1: Testing Build and Package Scripts](#section-1-testing-build-and-package-scripts) + - [Section 2: Master Branch Health Monitoring](#section-2-master-branch-health-monitoring) + - [Section 3: Managing File Paths in Configuration](#section-3-managing-file-paths-in-configuration) +- [Medium Priority Updates](#medium-priority-enhanced-merge-conflict-resolution) +- [Implementation Plan](#implementation-plan) +- [Validation](#validation) +- [Key Takeaways](#key-takeaways-for-claudemd-philosophy) + +## Executive Summary of What Went Wrong + +**Root Cause Chain:** + +1. PR #1830 (Sep 29) moved `node_package/` → `packages/react-on-rails/` +2. Path in `package-scripts.yml` was updated to `packages/react-on-rails/lib/ReactOnRails.full.js` +3. Later, structure was partially reverted to `lib/` at root, but `package-scripts.yml` wasn't updated +4. This broke `yalc publish` silently for ~7 weeks +5. When fixed in PR #2054, the CI safety check created a circular dependency +6. The safety check had a bug: checked `run.conclusion` which never updates after reruns + +**Key Failures:** + +- ❌ No verification that paths in package-scripts.yml were correct after structure change +- ❌ No manual testing of yalc publish to catch the breakage +- ❌ No monitoring of master health - failure went unnoticed for weeks +- ❌ CI safety mechanism created circular dependency instead of helping + +--- + +## HIGH PRIORITY: Add These 3 Sections Immediately + +### Section 1: Testing Build and Package Scripts + +Add this new section to CLAUDE.md (both root and .conductor/london-v1): + +````markdown +## Testing Build and Package Scripts + +**CRITICAL: Build scripts are infrastructure code that MUST be tested manually:** + +### Why This Matters + +- The `prepack`/`prepare` scripts in package.json/package-scripts.yml run during: + - `npm install` / `yarn install` (for git dependencies) + - `yalc publish` (critical for local development) + - `npm publish` + - Package manager prepare phase +- If these fail, users can't install or use the package +- Failures are often silent - they don't show up in normal CI + +### Mandatory Testing After ANY Changes + +**If you modify package.json, package-scripts.yml, or build configs:** + +1. **Test the prepack script:** + ```bash + yarn run prepack + # Should succeed without errors + ``` +```` + +2. **Test yalc publish (CRITICAL):** + + ```bash + yarn run yalc.publish + # Should publish successfully + ``` + +3. **Verify build artifacts exist at expected paths:** + + ```bash + # Check the path referenced in package-scripts.yml + ls -la lib/ReactOnRails.full.js + + # If package-scripts.yml references packages/*, check that too + ls -la packages/*/lib/*.js + ``` + +4. **Test clean install:** + ```bash + rm -rf node_modules + yarn install + # Should install without errors + ``` + +### When Directory Structure Changes + +If you rename/move directories that contain build artifacts: + +1. **Update ALL path references in package-scripts.yml** +2. **Test yalc publish BEFORE pushing** +3. **Test in a fresh clone to ensure no local assumptions** +4. **Consider adding a CI job to validate artifact paths** + +### Real Example: What Went Wrong + +In Sep 2024, we moved `node_package/` → `packages/react-on-rails/`. The path in +package-scripts.yml was updated to `packages/react-on-rails/lib/ReactOnRails.full.js`. +Later, the structure was partially reverted to `lib/` at root, but package-scripts.yml +wasn't updated. This broke yalc publish silently for 7 weeks. Manual testing of +`yarn run yalc.publish` would have caught this immediately. + +```` + +### Section 2: Master Branch Health Monitoring + +Add this new section to CLAUDE.md (both versions): + +```markdown +## Master Branch Health Monitoring + +**CRITICAL: Master staying broken affects the entire team. Don't let it persist.** + +### Immediate Actions After Your PR Merges + +Within 30 minutes of your PR merging to master: + +1. **Check CI status:** + ```bash + # View the merged PR's CI status + gh pr view --json statusCheckRollup + + # Or check recent master runs + gh run list --branch master --limit 5 +```` + +2. **If you see failures:** + - Investigate IMMEDIATELY + - Don't assume "someone else will fix it" + - You are responsible for ensuring your PR doesn't break master + +### When You Discover Master is Broken + +1. **Determine if it's from your PR:** + + ```bash + gh run list --branch master --limit 10 + ``` + +2. **Take immediate action:** + - If your PR broke it: Submit a fix PR within the hour, OR revert and resubmit + - If unsure: Investigate and communicate with team + - Never leave master broken overnight + +### Silent Failures are Most Dangerous + +Some failures don't show up in standard CI: + +- yalc publish failures +- Build artifact path issues +- Package installation problems + +**Always manually test critical workflows:** + +- If you changed package structure → test `yarn run yalc.publish` +- If you changed build configs → test `yarn build && ls -la lib/` +- If you changed generators → test `rake run_rspec:example_basic` + +### Understanding Workflow Reruns + +**Important limitation:** + +- Re-running a workflow does NOT change its `conclusion` in the GitHub API +- GitHub marks a run as "failed" even if a manual rerun succeeds +- Our CI safety checks (as of PR #2062) now handle this correctly +- But be aware: old commits with failed reruns may still block docs-only commits + +**What this means:** + +- If master workflows fail, reruns alone won't fix the circular dependency +- You need a new commit that passes to establish a clean baseline +- See PR #2065 for an example of breaking the cycle + +```` + +### Section 3: Managing File Paths in Configuration + +Add this new section to CLAUDE.md (both versions): + +```markdown +## Managing File Paths in Configuration Files + +**CRITICAL: Hardcoded paths are a major source of bugs, especially after refactors.** + +### Before Committing Path Changes + +1. **Verify the path actually exists:** + ```bash + ls -la +```` + +2. **Test operations that use the path:** + + ```bash + # If it's a build artifact path in package-scripts.yml: + yarn run prepack + yarn run yalc.publish + + # If it's a webpack output path: + yarn build && ls -la + ``` + +3. **Search for ALL references to old paths if renaming:** + ```bash + # Example: if renaming node_package/ to packages/ + grep -r "node_package" . --exclude-dir=node_modules --exclude-dir=.git + grep -r "packages/react-on-rails" . --exclude-dir=node_modules + ``` + +### Files That Commonly Have Path References + +**Always check these after directory structure changes:** + +- `package-scripts.yml` - build artifact paths in prepack/prepare scripts +- `package.json` - "files", "main", "types", "exports" fields +- `webpack.config.js` / `config/webpack/*` - output.path, resolve.modules +- `.github/workflows/*.yml` - cache paths, artifact paths, working directories +- `lib/generators/**/templates/**` - paths in generated code +- Documentation files - example paths and installation instructions + +### Post-Refactor Validation Checklist + +After any directory structure change, run this checklist: + +```bash +# 1. Find any lingering references to old paths +grep -r "old/path/name" . --exclude-dir=node_modules --exclude-dir=.git + +# 2. Verify build artifacts are in expected locations +yarn build +find . -name "ReactOnRails.full.js" -type f +find . -name "package.json" -type f + +# 3. Test package scripts +yarn run prepack +yarn run yalc.publish + +# 4. Test clean install +rm -rf node_modules && yarn install + +# 5. Run full test suite +rake +``` + +### Real Example: The package-scripts.yml Bug + +**What happened:** + +- Path was changed from `node_package/lib/` to `packages/react-on-rails/lib/` +- Later, the actual directory structure was reverted to `lib/` at root +- But package-scripts.yml still referenced `packages/react-on-rails/lib/` +- This caused yalc publish to fail silently for 7 weeks + +**How to prevent:** + +1. After changing directory structure, search for ALL references to old paths +2. Always run `yarn run yalc.publish` manually to verify it works +3. Check that paths in package-scripts.yml match actual file locations +4. Use `ls -la ` to verify paths exist before committing + +```` + +--- + +## MEDIUM PRIORITY: Enhanced Merge Conflict Resolution + +Update the existing "Merge Conflict Resolution Workflow" section with this addition: + +```markdown +### Merge Conflict Resolution Workflow +**CRITICAL**: When resolving merge conflicts, follow this exact sequence: + +1. **Resolve logical conflicts only** - don't worry about formatting +2. **VERIFY FILE PATHS** - if the conflict involved directory structure: + - Check if any hardcoded paths need updating + - Run: `grep -r "old/path" . --exclude-dir=node_modules` + - Pay special attention to package-scripts.yml, webpack configs, package.json + - **Test affected scripts:** If package-scripts.yml changed, run `yarn run prepack` +3. **Add resolved files**: `git add .` (or specific files) +4. **Auto-fix everything**: `rake autofix` +5. **Add any formatting changes**: `git add .` +6. **Continue rebase/merge**: `git rebase --continue` or `git commit` +7. **TEST CRITICAL SCRIPTS if build configs changed:** + ```bash + yarn run prepack # Test prepack script + yarn run yalc.publish # Test yalc publish if package structure changed + rake run_rspec:gem # Run relevant test suites +```` + +**❌ NEVER manually format during conflict resolution** - this causes formatting wars. +**❌ NEVER blindly accept path changes** - verify they're correct for current structure. +**❌ NEVER skip testing after resolving conflicts in build configs** - silent failures are dangerous. + +``` + +--- + +## Implementation Plan + +### Week 1 (Immediate) +- [ ] Add Section 1: Testing Build and Package Scripts to both CLAUDE.md files +- [ ] Add Section 2: Master Branch Health Monitoring to both CLAUDE.md files +- [ ] Add Section 3: Managing File Paths in Configuration to both CLAUDE.md files + +### Week 2 +- [ ] Update merge conflict resolution section in both CLAUDE.md files +- [ ] Add checklist for large refactors (optional - can wait) + +### Future Improvements +- [ ] Consider adding a CI job that validates build artifact paths +- [ ] Consider pre-commit hook to validate package-scripts.yml paths exist +- [ ] Document this incident in team knowledge base + +--- + +## Validation + +After adding these sections, verify they work by: + +1. Having a team member review the new sections +2. Testing that the instructions are clear and actionable +3. Referencing these sections during next PR that touches build configs +4. Updating based on feedback after first real-world use + +--- + +## Key Takeaways for CLAUDE.md Philosophy + +**What makes good CLAUDE.md content:** +- ✅ Specific commands to run, not vague advice +- ✅ Real examples of what went wrong and how to prevent it +- ✅ Checklists and step-by-step instructions +- ✅ Clear "why this matters" context +- ✅ Mandatory testing after specific types of changes + +**What to avoid:** +- ❌ Generic advice like "be careful with paths" +- ❌ Unclear responsibilities ("someone should check") +- ❌ Assumptions that CI will catch everything +- ❌ Missing the "silent failure" scenarios +``` diff --git a/.claude/docs/analysis/README.md b/.claude/docs/analysis/README.md new file mode 100644 index 0000000000..265d836e7c --- /dev/null +++ b/.claude/docs/analysis/README.md @@ -0,0 +1,32 @@ +# Analysis Documentation + +This directory contains root cause analysis and implementation guides from past incidents. + +## Files + +### CLAUDE_MD_UPDATES.md + +Concrete implementation guide created after the Nov 2024 CI circular dependency incident. Contains copy-paste ready sections that were applied to CLAUDE.md to prevent similar issues. + +### claude-md-improvements.md + +Original root cause analysis of the CI breakage, including timeline of events, what went wrong, and comprehensive recommendations for prevention. + +## Purpose + +These documents serve as: + +- Historical record of incidents and how they were resolved +- Templates for future documentation improvements +- Learning resources for understanding project-specific failure modes +- Reference for similar issues in the future + +## Related + +The analysis in these files led to the creation of: + +- `.claude/docs/testing-build-scripts.md` +- `.claude/docs/master-health-monitoring.md` +- `.claude/docs/managing-file-paths.md` + +See also PR #2062 and PR #2065 for the full context. diff --git a/.claude/docs/analysis/claude-md-improvements.md b/.claude/docs/analysis/claude-md-improvements.md new file mode 100644 index 0000000000..4a46cf44f6 --- /dev/null +++ b/.claude/docs/analysis/claude-md-improvements.md @@ -0,0 +1,219 @@ +# Recommended CLAUDE.md Improvements + +Based on the CI breakage analysis, here are specific additions to prevent similar issues: + +## 1. Add Section on Large Refactors and Directory Structure Changes + +**CRITICAL: When working on PRs that change directory structure or move files:** + +1. **Create a comprehensive checklist** of all places that reference the old paths: + + - Search for hardcoded paths in all config files (grep -r "old/path" .) + - Check package.json, package-scripts.yml, webpack configs, CI workflows + - Check documentation and example code + - Check generator templates + +2. **Run ALL test suites** after directory changes: + + - `rake` (all tests) + - `rake run_rspec:example_basic` (generator tests) + - Manual test of `yalc publish` if changing package structure + - Build and test in a fresh clone to catch path issues + +3. **Add temporary validation** in CI: + + - Add a CI job that explicitly tests the changed paths + - Keep this validation for 2-3 releases after the refactor + +4. **Document the change** in CHANGELOG.md with migration instructions + +5. **Consider breaking large refactors into smaller PRs**: + - PR 1: Prepare new structure (create new dirs, copy files) + - PR 2: Update references + - PR 3: Remove old structure + - This makes it easier to catch issues and roll back if needed + +## 2. Add Section on Build Script Testing + +**CRITICAL: Build scripts in package.json and package-scripts.yml need validation:** + +1. **The prepack/prepare scripts are CRITICAL** - they run during: + + - `npm install` / `yarn install` (for git dependencies) + - `yalc publish` + - `npm publish` + - Package manager prepare phase + +2. **Always test these scripts manually after changes:** + + ```bash + # Test the prepack script + yarn run prepack + + # Test yalc publish (critical for local development) + yarn run yalc.publish + + # Verify build artifacts exist at expected paths + ls -la lib/ReactOnRails.full.js + ls -la packages/*/lib/ + ``` + +3. **If you change directory structure:** + + - Update ALL path checks in package-scripts.yml + - Test with a clean install: `rm -rf node_modules && yarn install` + - Test yalc publish to ensure it works for users + +4. **Add tests for critical build paths:** + - Consider adding a CI job that validates expected build artifacts exist + - Add tests that verify package.json/package-scripts.yml paths are correct + +## 3. Add Section on Monitoring Master Health + +**CRITICAL: Don't let master stay broken:** + +1. **If CI fails on master after your PR merges:** + + - Check GitHub Actions within 30 minutes of merge + - Run `gh pr view --json statusCheckRollup` after merge + - Set up GitHub notifications for master branch failures + +2. **If you discover master is broken:** + + - Investigate IMMEDIATELY - don't assume "someone else will fix it" + - Use `gh run list --branch master --limit 10` to see recent failures + - Check if it's a recurring failure or a new issue + - If the failure is from your PR, either: + - Submit a fix PR immediately, OR + - Consider reverting your PR and re-submitting with the fix + +3. **Silent failures are the most dangerous:** + + - yalc publish failures won't show up in most CI runs + - Build artifact path issues may only surface during actual usage + - Always test the actual use case (yalc publish, npm install from git, etc.) + +4. **When manually re-running workflows:** + - Re-running a workflow DOES NOT change its conclusion in the GitHub API + - Our CI safety checks now handle this, but be aware of the limitation + - If a rerun succeeds, monitor that subsequent commits don't get blocked + +## 4. Update the Merge Conflict Resolution Section + +**Enhancement to existing section:** + +```markdown +### Merge Conflict Resolution Workflow + +**CRITICAL**: When resolving merge conflicts, follow this exact sequence: + +1. **Resolve logical conflicts only** - don't worry about formatting +2. **VERIFY FILE PATHS** - if the conflict involved directory structure: + - Check if any hardcoded paths need updating + - Grep for old paths: `grep -r "old/path" .` + - Pay special attention to package-scripts.yml, webpack configs +3. **Add resolved files**: `git add .` (or specific files) +4. **Auto-fix everything**: `rake autofix` +5. **Add any formatting changes**: `git add .` +6. **Continue rebase/merge**: `git rebase --continue` or `git commit` +7. **TEST CRITICAL SCRIPTS**: If package-scripts.yml or build configs changed: + - Run `yarn run prepack` to verify build scripts work + - Run `yarn run yalc.publish` if package structure changed + - Run relevant test suites + +**❌ NEVER manually format during conflict resolution** - this causes formatting wars. +**❌ NEVER blindly accept path changes** - verify they're correct for current structure. +``` + +## 5. Add Section on Path Management + +**NEW SECTION:** + +````markdown +## Managing File Paths in Configuration + +**CRITICAL: Hardcoded paths are a major source of bugs:** + +1. **Before committing changes to any config file with paths:** + + - Verify the path actually exists: `ls -la ` + - Test that operations using the path work + - If changing package structure, search for ALL references to old paths + +2. **Common files with path references:** + + - `package-scripts.yml` - build artifact paths + - `package.json` - "files", "main", "types" fields + - Webpack configs - output.path, resolve.modules + - `.github/workflows/*.yml` - cache paths, artifact paths + - Generator templates - paths in generated code + - Documentation - example paths + +3. **After any directory structure change:** + + ```bash + # Find potential hardcoded paths (adjust as needed) + grep -r "node_package" . --exclude-dir=node_modules --exclude-dir=.git + grep -r "packages/react-on-rails" . --exclude-dir=node_modules + + # Verify build artifacts are in expected locations + yarn build + find . -name "ReactOnRails.full.js" -type f + ``` +```` + +4. **Use variables/constants when possible:** + - In JavaScript/TypeScript, use `__dirname` or import.meta.url + - In Ruby, use `Rails.root` or `File.expand_path` + - Avoid hardcoding relative paths in config files when possible + +``` + +--- + +## Root Cause of This CI Breakage + +**What happened:** + +1. PR #1830 (Sep 29) changed directory from `node_package/` to `packages/react-on-rails/` +2. The path in `package-scripts.yml` was updated to `packages/react-on-rails/lib/ReactOnRails.full.js` +3. Later, the directory structure was partially reverted back to `lib/` at the root +4. But `package-scripts.yml` wasn't updated, leaving the wrong path +5. This made yalc publish fail silently for weeks +6. When the path was fixed in PR #2054, the CI safety check blocked it due to previous failures +7. The safety check had a bug: it checked `run.conclusion` which never updates after reruns + +**What we learned:** + +- ✅ Large refactors need comprehensive path audits +- ✅ Build scripts (prepack/prepare) need manual testing +- ✅ Silent failures (yalc publish) are dangerous +- ✅ CI safety mechanisms need to handle manual reruns correctly +- ✅ Master health needs active monitoring + +**How the recommendations prevent this:** + +1. Section 1 (Large Refactors) - Would have caught incomplete path updates +2. Section 2 (Build Scripts) - Would have caught yalc publish failure early +3. Section 3 (Master Health) - Would have alerted team to broken master +4. Section 4 (Merge Conflicts) - Would prevent path regressions during merges +5. Section 5 (Path Management) - Would have caught the incorrect path + +--- + +## Implementation Priority + +**High Priority** (add to CLAUDE.md this week): +- Section 2: Testing Build and Package Scripts +- Section 3: Master Branch Health Monitoring +- Section 5: Managing File Paths in Configuration + +**Medium Priority** (add within 2 weeks): +- Section 1: Large Refactors and Directory Structure Changes +- Section 4: Enhanced Merge Conflict Resolution + +**Future Improvements** (consider later): +- Automated tests for path validation +- CI job to verify build artifacts exist at expected paths +- Pre-commit hook to validate package-scripts.yml paths +``` diff --git a/.claude/docs/managing-file-paths.md b/.claude/docs/managing-file-paths.md new file mode 100644 index 0000000000..aed21b9582 --- /dev/null +++ b/.claude/docs/managing-file-paths.md @@ -0,0 +1,80 @@ +# Managing File Paths in Configuration Files + +**CRITICAL: Hardcoded paths are a major source of bugs, especially after refactors.** + +## Before Committing Path Changes + +1. **Verify the path actually exists:** + + ```bash + ls -la + ``` + +2. **Test operations that use the path:** + + ```bash + # If it's a build artifact path in package-scripts.yml: + yarn run prepack + yarn run yalc.publish + + # If it's a webpack output path: + yarn build && ls -la + ``` + +3. **Search for ALL references to old paths if renaming:** + ```bash + # Example: if renaming node_package/ to packages/ + grep -r "node_package" . --exclude-dir=node_modules --exclude-dir=.git + grep -r "packages/react-on-rails" . --exclude-dir=node_modules + ``` + +## Files That Commonly Have Path References + +**Always check these after directory structure changes:** + +- `package-scripts.yml` - build artifact paths in prepack/prepare scripts +- `package.json` - "files", "main", "types", "exports" fields +- `webpack.config.js` / `config/webpack/*` - output.path, resolve.modules +- `.github/workflows/*.yml` - cache paths, artifact paths, working directories +- `lib/generators/**/templates/**` - paths in generated code +- Documentation files - example paths and installation instructions + +## Post-Refactor Validation Checklist + +After any directory structure change, run this checklist: + +```bash +# 1. Find any lingering references to old paths +grep -r "old/path/name" . --exclude-dir=node_modules --exclude-dir=.git + +# 2. Verify build artifacts are in expected locations +yarn build +find . -name "ReactOnRails.full.js" -type f +find . -name "package.json" -type f + +# 3. Test package scripts +yarn run prepack +yarn run yalc.publish + +# 4. Test clean install +rm -rf node_modules && yarn install + +# 5. Run full test suite +rake +``` + +## Real Example: The package-scripts.yml Bug + +**What happened:** + +- Path was changed from `node_package/lib/` to `packages/react-on-rails/lib/` +- Later, the actual directory structure was reverted to `lib/` at root +- But package-scripts.yml still referenced `packages/react-on-rails/lib/` +- This caused yalc publish to fail silently for 7 weeks + +**How to prevent:** + +1. After changing directory structure, search for ALL references to old paths +2. Always run `yarn run yalc.publish` manually to verify it works +3. Check that paths in package-scripts.yml match actual file locations +4. Use `ls -la ` to verify paths exist before committing diff --git a/.claude/docs/master-health-monitoring.md b/.claude/docs/master-health-monitoring.md new file mode 100644 index 0000000000..c389988a1b --- /dev/null +++ b/.claude/docs/master-health-monitoring.md @@ -0,0 +1,64 @@ +# Master Branch Health Monitoring + +**CRITICAL: Master staying broken affects the entire team. Don't let it persist.** + +## Immediate Actions After Your PR Merges + +Within 30 minutes of your PR merging to master: + +1. **Check CI status:** + + ```bash + # View the merged PR's CI status + gh pr view --json statusCheckRollup + + # Or check recent master runs + gh run list --branch master --limit 5 + ``` + +2. **If you see failures:** + - Investigate IMMEDIATELY + - Don't assume "someone else will fix it" + - You are responsible for ensuring your PR doesn't break master + +## When You Discover Master is Broken + +1. **Determine if it's from your PR:** + + ```bash + gh run list --branch master --limit 10 + ``` + +2. **Take immediate action:** + - If your PR broke it: Submit a fix PR within the hour, OR revert and resubmit + - If unsure: Investigate and communicate with team + - Never leave master broken overnight + +## Silent Failures are Most Dangerous + +Some failures don't show up in standard CI: + +- yalc publish failures +- Build artifact path issues +- Package installation problems + +**Always manually test critical workflows:** + +- If you changed package structure → test `yarn run yalc.publish` +- If you changed build configs → test `yarn build && ls -la lib/` +- If you changed generators → test `rake run_rspec:example_basic` + +## Understanding Workflow Reruns + +**Important limitation:** + +- Re-running a workflow does NOT change its `conclusion` in the GitHub API +- GitHub marks a run as "failed" even if a manual rerun succeeds +- Our CI safety checks (as of [PR #2062](https://github.com/shakacode/react_on_rails/pull/2062)) now handle this correctly +- But be aware: old commits with failed reruns may still block docs-only commits + +**What this means:** + +- If master workflows fail, reruns alone won't fix the circular dependency +- You need a new commit that passes to establish a clean baseline +- See [PR #2065](https://github.com/shakacode/react_on_rails/pull/2065) for an example of breaking the cycle diff --git a/.claude/docs/testing-build-scripts.md b/.claude/docs/testing-build-scripts.md new file mode 100644 index 0000000000..8c8b10855b --- /dev/null +++ b/.claude/docs/testing-build-scripts.md @@ -0,0 +1,65 @@ +# Testing Build and Package Scripts + +**CRITICAL: Build scripts are infrastructure code that MUST be tested manually:** + +## Why This Matters + +- The `prepack`/`prepare` scripts in package.json/package-scripts.yml run during: + - `npm install` / `yarn install` (for git dependencies) + - `yalc publish` (critical for local development) + - `npm publish` + - Package manager prepare phase +- If these fail, users can't install or use the package +- Failures are often silent - they don't show up in normal CI + +## Mandatory Testing After ANY Changes + +**If you modify package.json, package-scripts.yml, or build configs:** + +1. **Test the prepack script:** + + ```bash + yarn run prepack + # Should succeed without errors + ``` + +2. **Test yalc publish (CRITICAL):** + + ```bash + yarn run yalc.publish + # Should publish successfully + ``` + +3. **Verify build artifacts exist at expected paths:** + + ```bash + # Check the path referenced in package-scripts.yml + ls -la lib/ReactOnRails.full.js + + # If package-scripts.yml references packages/*, check that too + ls -la packages/*/lib/*.js + ``` + +4. **Test clean install:** + ```bash + rm -rf node_modules + yarn install + # Should install without errors + ``` + +## When Directory Structure Changes + +If you rename/move directories that contain build artifacts: + +1. **Update ALL path references in package-scripts.yml** +2. **Test yalc publish BEFORE pushing** +3. **Test in a fresh clone to ensure no local assumptions** +4. **Consider adding a CI job to validate artifact paths** + +## Real Example: What Went Wrong + +In Sep 2024, we moved `node_package/` → `packages/react-on-rails/`. The path in +package-scripts.yml was updated to `packages/react-on-rails/lib/ReactOnRails.full.js`. +Later, the structure was partially reverted to `lib/` at root, but package-scripts.yml +wasn't updated. This broke yalc publish silently for 7 weeks. Manual testing of +`yarn run yalc.publish` would have caught this immediately. diff --git a/.github/actions/ensure-master-docs-safety/action.yml b/.github/actions/ensure-master-docs-safety/action.yml index 4620d8e3ef..5dd25ddf3f 100644 --- a/.github/actions/ensure-master-docs-safety/action.yml +++ b/.github/actions/ensure-master-docs-safety/action.yml @@ -1,5 +1,5 @@ name: Ensure master docs-only skips are safe -description: Fails if the previous master commit still has failing workflow runs when current push is docs-only +description: Prevents docs-only commits from skipping CI when previous master commit has unresolved test failures inputs: docs-only: description: 'String output from ci-changes-detector ("true" or "false")' diff --git a/CLAUDE.md b/CLAUDE.md index 0f6390a54a..da0775bba7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -211,12 +211,25 @@ When making changes, update the **appropriate changelog(s)**: **CRITICAL**: When resolving merge conflicts, follow this exact sequence: 1. **Resolve logical conflicts only** - don't worry about formatting -2. **Add resolved files**: `git add .` (or specific files) -3. **Auto-fix everything**: `rake autofix` -4. **Add any formatting changes**: `git add .` -5. **Continue rebase/merge**: `git rebase --continue` or `git commit` +2. **VERIFY FILE PATHS** - if the conflict involved directory structure: + - Check if any hardcoded paths need updating + - Run: `grep -r "old/path" . --exclude-dir=node_modules` + - Pay special attention to package-scripts.yml, webpack configs, package.json + - **Test affected scripts:** If package-scripts.yml changed, run `yarn run prepack` +3. **Add resolved files**: `git add .` (or specific files) +4. **Auto-fix everything**: `rake autofix` +5. **Add any formatting changes**: `git add .` +6. **Continue rebase/merge**: `git rebase --continue` or `git commit` +7. **TEST CRITICAL SCRIPTS if build configs changed:** + ```bash + yarn run prepack # Test prepack script + yarn run yalc.publish # Test yalc publish if package structure changed + rake run_rspec:gem # Run relevant test suites + ``` **❌ NEVER manually format during conflict resolution** - this causes formatting wars between tools. +**❌ NEVER blindly accept path changes** - verify they're correct for current structure. +**❌ NEVER skip testing after resolving conflicts in build configs** - silent failures are dangerous. ### Debugging Formatting Issues - Check current formatting: `yarn start format.listDifferent` @@ -236,6 +249,18 @@ When making changes, update the **appropriate changelog(s)**: - **Gem-only tests**: `rake run_rspec:gem` - **All tests except examples**: `rake all_but_examples` +## Testing Build and Package Scripts + +@.claude/docs/testing-build-scripts.md + +## Master Branch Health Monitoring + +@.claude/docs/master-health-monitoring.md + +## Managing File Paths in Configuration Files + +@.claude/docs/managing-file-paths.md + ## Project Architecture ### Monorepo Structure