Skip to content

Conversation

@aspiers
Copy link
Contributor

@aspiers aspiers commented Dec 15, 2025

Without this patch, the lex CLI exits with code 0 even when validation fails, causing CI to incorrectly report success.

This is a problem because validation failures should cause the CI workflow to fail, alerting developers to issues in their lexicon definitions.

This patch solves the problem by monkey-patching the lex CLI's util.js file to replace the // skip line with throw e;, ensuring that validation errors are properly propagated and cause the workflow to fail.

Co-authored-by: Cursor [email protected]

Summary by CodeRabbit

  • Chores
    • Updated CI workflow configuration to enhance test execution handling.

✏️ Tip: You can customize this high-level summary in your review settings.

Without this patch, the `lex` CLI exits with code 0 even when
validation fails, causing CI to incorrectly report success.

This is a problem because validation failures should cause the CI
workflow to fail, alerting developers to issues in their lexicon
definitions.

This patch solves the problem by monkey-patching the `lex` CLI's
`util.js` file to replace the `// skip` line with `throw e;`, ensuring
that validation errors are properly propagated and cause the workflow
to fail.

Co-authored-by: Cursor <[email protected]>
@coderabbitai
Copy link

coderabbitai bot commented Dec 15, 2025

Walkthrough

A CI workflow step is added to patch a runtime dependency by replacing "// skip" with "throw e;" in the @atproto/lex-cli utility module before running checks, modifying validation exit behavior during test execution.

Changes

Cohort / File(s) Summary
CI Workflow Patch
.github/workflows/test.yml
Added sed command in "Install dependencies" step to patch node_modules/@atproto/lex-cli/dist/util.js, replacing "// skip" with "throw e;" prior to test execution

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Attention points:
    • Verify the sed pattern correctly targets the intended lines without unintended side effects
    • Clarify why runtime patching of the dependency is necessary (consider documenting in PR description or comments)
    • Confirm the patch is idempotent and won't cause issues if dependencies are reinstalled

Poem

🐰 A patch in the night, so clever and quick,
We sed through the skips with a validation trick,
Errors now thrown where silence would hide,
The checks run stronger with nowhere to glide! ✨

Pre-merge checks and finishing touches

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)
Check name Status Explanation Resolution
Lexicon Documentation Sync ❌ Error ERD.puml contains 'contributor' and 'token' entities without corresponding lexicon JSON files, violating the Lexicon Documentation Sync requirement. Create lexicon files for missing entities, remove them from ERD if they're external, or add clarifying annotations to the diagram.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: patching the lex CLI to fix exit behavior on validation failures, which matches the PR objectives of ensuring CI fails on validation errors.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5b3015a and db59832.

📒 Files selected for processing (1)
  • .github/workflows/test.yml (1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: aspiers
Repo: hypercerts-org/hypercerts-lexicon PR: 34
File: lexicons/org/hypercerts/claim/evaluation.json:45-63
Timestamp: 2025-12-15T10:13:17.689Z
Learning: For the hypercerts-lexicon repository, CI automatically runs `npm run gen-api` and `npm run check` to regenerate TypeScript types and validate lexicon definitions, so manual reminders about running these commands are not needed in code reviews.
Learnt from: CR
Repo: hypercerts-org/hypercerts-lexicon PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T16:09:09.119Z
Learning: Applies to lexicons/**/*.json : Validate lexicon definitions and regenerate types using `npm run check` before deployment
📚 Learning: 2025-12-15T10:13:17.689Z
Learnt from: aspiers
Repo: hypercerts-org/hypercerts-lexicon PR: 34
File: lexicons/org/hypercerts/claim/evaluation.json:45-63
Timestamp: 2025-12-15T10:13:17.689Z
Learning: For the hypercerts-lexicon repository, CI automatically runs `npm run gen-api` and `npm run check` to regenerate TypeScript types and validate lexicon definitions, so manual reminders about running these commands are not needed in code reviews.

Applied to files:

  • .github/workflows/test.yml
📚 Learning: 2025-12-09T16:09:09.119Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-lexicon PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T16:09:09.119Z
Learning: Applies to lexicons/**/*.json : Validate lexicon definitions and regenerate types using `npm run check` before deployment

Applied to files:

  • .github/workflows/test.yml
📚 Learning: 2025-12-09T16:09:09.119Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-lexicon PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T16:09:09.119Z
Learning: Check code formatting using `npm run lint` or `npm run format:check` before submitting code

Applied to files:

  • .github/workflows/test.yml
📚 Learning: 2025-12-09T16:09:09.119Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-lexicon PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T16:09:09.119Z
Learning: Run `npm run format` before committing to ensure consistent code formatting

Applied to files:

  • .github/workflows/test.yml
📚 Learning: 2025-12-09T16:09:09.119Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-lexicon PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T16:09:09.119Z
Learning: Applies to lexicons/**/*.json : Regenerate TypeScript API types after modifying lexicon JSON files by running `npm run gen-api`

Applied to files:

  • .github/workflows/test.yml
🔇 Additional comments (1)
.github/workflows/test.yml (1)

25-27: This is a deliberate workaround for a known upstream bug, not a critical issue.

The @atproto/lex-cli library genuinely exits with code 0 on validation failures, preventing CI from detecting issues. The patch is explicitly documented in the commit history with full rationale. For a project of this scale, this pragmatic approach is appropriate, and the semver constraint (^0.9.5) allows automatic upgrades when the upstream bug is fixed.

No action required.

Likely an incorrect or invalid review comment.


Comment @coderabbitai help to get the list of available commands and usage tips.

@aspiers aspiers merged commit 99cc40a into hypercerts-org:main Dec 15, 2025
3 checks passed
@aspiers aspiers deleted the fix-lex-cli branch December 15, 2025 15:59
@aspiers
Copy link
Contributor Author

aspiers commented Dec 15, 2025

Submitted #39 to eliminate confusion from CodeRabbit and others.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant