feat(openapi-zod-ts): add --check-drift CI gate for generated output - #400
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughAdds a Changes--check-drift CI gate
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Fallow audit reportFound 18 findings. Dependencies (3)
Duplication (15)
Generated by fallow. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ) | ||
| expect(result.action).toBe('run') | ||
| if (result.action === 'run') { | ||
| expect(result.checkDrift).toBe(true) |
Contributor
There was a problem hiding this comment.
warn fallow/code-duplication: Code clone group 4 (7 lines, 2 instances)
warn fallow/code-duplication: Code clone group 5 (5 lines, 3 instances)
| errSpy.mockRestore() | ||
| }) | ||
|
|
||
| it('emits ::error file=<filename>:: annotation when github=true for stale files (no outputDir)', () => { |
Contributor
There was a problem hiding this comment.
warn fallow/code-duplication: Code clone group 7 (7 lines, 3 instances)
warn fallow/code-duplication: Code clone group 6 (6 lines, 2 instances)
warn fallow/code-duplication: Code clone group 8 (6 lines, 4 instances)
| errSpy.mockRestore() | ||
| }) | ||
|
|
||
| it('emits ::error file=<filename>:: annotation when github=true for missing files (no outputDir)', () => { |
Contributor
There was a problem hiding this comment.
warn fallow/code-duplication: Code clone group 8 (6 lines, 4 instances)
| errSpy.mockRestore() | ||
| }) | ||
|
|
||
| it('prefixes annotation file path with outputDir when provided', () => { |
Contributor
There was a problem hiding this comment.
warn fallow/code-duplication: Code clone group 7 (7 lines, 3 instances)
warn fallow/code-duplication: Code clone group 8 (6 lines, 4 instances)
| }) | ||
|
|
||
| it('does NOT emit ::error annotations when github=false even if GITHUB_ACTIONS is set', () => { | ||
| process.env['GITHUB_ACTIONS'] = 'true' |
Contributor
There was a problem hiding this comment.
warn fallow/code-duplication: Code clone group 9 (5 lines, 2 instances)
| errSpy.mockRestore() | ||
| }) | ||
|
|
||
| it('does not write GITHUB_STEP_SUMMARY when github=true but env var is not set', () => { |
Contributor
There was a problem hiding this comment.
warn fallow/code-duplication: Code clone group 7 (7 lines, 3 instances)
warn fallow/code-duplication: Code clone group 8 (6 lines, 4 instances)
warn fallow/code-duplication: Code clone group 6 (6 lines, 2 instances)
|
|
||
| it('does not write GITHUB_STEP_SUMMARY when github=true but env var is not set', () => { | ||
| // GITHUB_STEP_SUMMARY is not set (cleared in beforeEach). | ||
| const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined) |
Contributor
There was a problem hiding this comment.
warn fallow/code-duplication: Code clone group 10 (6 lines, 2 instances)
| // This runs BEFORE any mkdir/writeFile so the output directory is left untouched on | ||
| // failure. zod.ts (the user-owned schema file) is intentionally excluded: the generator | ||
| // never overwrites it after the first bootstrap, so it must not be in the expected set. | ||
| if (checkDrift) { |
Contributor
There was a problem hiding this comment.
warn fallow/code-duplication: Code clone group 3 (84 lines, 2 instances)
| driftPlan.schemaExists && | ||
| !resetSchema | ||
| ) { | ||
| const schemaPath = resolve(cwd, config.input_schema) |
Contributor
There was a problem hiding this comment.
warn fallow/code-duplication: Code clone group 11 (11 lines, 2 instances)
Contributor
There was a problem hiding this comment.
Resolved in 7b443b1.
| console.log( | ||
| `${prefix}Skipping ${config.input_schema}: already exists (edit freely, it's yours).` | ||
| ) | ||
| console.log(`${prefix}Skipping ${config.input_schema}: already exists (edit freely, it's yours).`) |
Contributor
There was a problem hiding this comment.
warn fallow/code-duplication: Code clone group 11 (11 lines, 2 instances)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/src/content/docs/guides/drift-detection.mdx`:
- Around line 62-63: The drift-check documentation is overstating the GitHub
Actions behavior in reportDrift(): the ::error annotations and
$GITHUB_STEP_SUMMARY output happen only on failure when drift is detected, not
on every run with GITHUB_ACTIONS=true. Update the wording in the drift-detection
guide to make those side effects explicitly failure-only, and keep the success
path described as a clean run that only logs the success message.
In `@packages/openapi-zod-ts/src/drift-check.ts`:
- Around line 137-153: The GitHub annotation helper only covers stale and
missing drift, so extra files are not annotated in PRs. Update
emitGithubAnnotations in drift-check.ts to also iterate over report.extra and
emit a matching ::error workflow command using the same prefix handling and the
entry.filename symbol, so all drift types found by reportDrift() produce inline
GitHub Actions annotations.
In `@packages/openapi-zod-ts/src/generator.ts`:
- Around line 204-205: The fix hint in generator.ts only appends --config via
fixCommand, so it drops active --input/--output overrides for the no-config
path. Update the fix command construction in generator.ts to include the same
input/output values used by the generator when opts.configPath is undefined,
alongside --check-drift, so the suggested openapi-zod-ts command matches the
current target; use the existing configDesc/fixCommand logic as the place to
preserve these overrides.
- Around line 148-219: The combined check path in generator.ts is being bypassed
because the earlier check-only return exits before the `checkDrift` branch runs.
Update the control flow in `generate` so `--check --check-drift` still executes
the drift validation logic (including `compareOutput` and `reportDrift`) before
any early exit, while preserving the existing `check`-only behavior and using
the existing `checkDrift`, `reportDrift`, and `generate` symbols to locate the
fix.
- Around line 148-152: The drift check in generator.ts is incorrectly treating
co-located input_schema files as extra because they are removed from expectedMap
but still discovered by compareOutput(). Update the drift comparison flow in the
checkDrift branch so user-owned schema files are excluded from the actual
filesystem scan as well, or otherwise filtered out before reporting. Make sure
the schemas.ts to ./schemas.js path handled by the generator logic (including
the input_schema handling around the schema generation setup) no longer produces
report.extra for the schema file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: acd1dc9c-72e0-4f0d-adb1-a76b7b3757d3
📒 Files selected for processing (10)
docs/astro.config.mjsdocs/src/content/docs/guides/drift-detection.mdxpackages/openapi-zod-ts/package.jsonpackages/openapi-zod-ts/src/__tests__/cli-args.test.tspackages/openapi-zod-ts/src/__tests__/drift-check.test.tspackages/openapi-zod-ts/src/__tests__/generator-schema.test.tspackages/openapi-zod-ts/src/cli-args.tspackages/openapi-zod-ts/src/cli.tspackages/openapi-zod-ts/src/drift-check.tspackages/openapi-zod-ts/src/generator.ts
Comment on lines
+62
to
+63
| Add a dedicated drift check step to your workflow. When `GITHUB_ACTIONS=true`, the command also emits `::error file=...::` workflow commands so GitHub renders inline annotations on the PR, and appends a summary table to the step summary panel. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Qualify the GitHub Actions side effects as failure-only.
reportDrift() emits ::error annotations and writes $GITHUB_STEP_SUMMARY only when drift is detected; clean runs return after logging the success message. Wording this as unconditional for GITHUB_ACTIONS=true overstates the behavior and will mislead users looking for annotations on passing runs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/src/content/docs/guides/drift-detection.mdx` around lines 62 - 63, The
drift-check documentation is overstating the GitHub Actions behavior in
reportDrift(): the ::error annotations and $GITHUB_STEP_SUMMARY output happen
only on failure when drift is detected, not on every run with
GITHUB_ACTIONS=true. Update the wording in the drift-detection guide to make
those side effects explicitly failure-only, and keep the success path described
as a clean run that only logs the success message.
Comment on lines
+137
to
+153
| function emitGithubAnnotations( | ||
| report: DriftReport, | ||
| fixCommand: string, | ||
| outputDir: string | undefined | ||
| ): void { | ||
| const prefix = outputDir !== undefined && outputDir !== '' ? `${outputDir}/` : '' | ||
| for (const entry of report.stale) { | ||
| console.log( | ||
| `::error file=${prefix}${entry.filename}::Drift detected: ${entry.filename} is stale. Run '${fixCommand}' and commit the result.` | ||
| ) | ||
| } | ||
| for (const entry of report.missing) { | ||
| console.log( | ||
| `::error file=${prefix}${entry.filename}::Drift detected: ${entry.filename} is missing. Run '${fixCommand}' and commit the result.` | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Annotate EXTRA files in GitHub Actions too.
reportDrift() fails on extra entries as well, but this helper only emits workflow commands for stale and missing. That leaves one class of drift without inline PR annotations even though the file path is known.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/openapi-zod-ts/src/drift-check.ts` around lines 137 - 153, The
GitHub annotation helper only covers stale and missing drift, so extra files are
not annotated in PRs. Update emitGithubAnnotations in drift-check.ts to also
iterate over report.extra and emit a matching ::error workflow command using the
same prefix handling and the entry.filename symbol, so all drift types found by
reportDrift() produce inline GitHub Actions annotations.
Comment on lines
+204
to
+205
| const configDesc = opts.configPath !== undefined ? `--config ${opts.configPath}` : '' | ||
| const fixCommand = ['openapi-zod-ts', configDesc].filter(Boolean).join(' ') |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve active --input/--output overrides in the fix hint.
fixCommand only carries --config. For the supported no-config path (--input ... --output ... --check-drift), the suggested openapi-zod-ts command won't reproduce the same target and can fail outright.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/openapi-zod-ts/src/generator.ts` around lines 204 - 205, The fix
hint in generator.ts only appends --config via fixCommand, so it drops active
--input/--output overrides for the no-config path. Update the fix command
construction in generator.ts to include the same input/output values used by the
generator when opts.configPath is undefined, alongside --check-drift, so the
suggested openapi-zod-ts command matches the current target; use the existing
configDesc/fixCommand logic as the place to preserve these overrides.
Adds a --check-drift flag that regenerates all output in memory, applies the same Prettier formatting as the write path, and compares against committed files on disk. Exits non-zero with per-file diagnostics when any file is stale, missing, or extra. Writes nothing to disk. When GITHUB_ACTIONS=true, emits ::error file=<name>:: annotations per drifted file and appends a markdown table to GITHUB_STEP_SUMMARY. Exports a reusable ./drift-check subpath (compareOutput, reportDrift, DriftReport) so openapi-server, openapi-react-query, and openapi-msw can adopt the same gate in follow-up PRs. Claude-Session: https://claude.ai/code/session_01SZmVypA9rE5H3HTreJgyE2
benjamineckstein
force-pushed
the
feat/cli-drift-check
branch
from
June 27, 2026 20:40
141db78 to
b891208
Compare
Merged
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.
What
Adds
npx openapi-zod-ts --check-drift, a read-only CI gate for consumers who commit generated output. It regenerates in memory, compares against the committed files, writes nothing, and exits non-zero with per-file diagnostics when any file is stale, missing, or extra. Under GitHub Actions it also emits::error file=...::annotations (pinned to the PR diff) and a$GITHUB_STEP_SUMMARYpanel, so a drift failure is actionable at a glance.This is distinct from the existing
--check, which validates hand-writteninput_schemaZod coverage.--check-driftvalidates the generated files themselves.Consumer usage
```yaml
```
Reusable API
New
openapi-zod-ts/drift-checksubpath export (`compareOutput`, `reportDrift`, `DriftReport`), so `openapi-server`, `openapi-react-query`, and `openapi-msw` can adopt the same gate in fast-follow PRs.Correctness notes
Docs
New guide at `guides/drift-detection` covering one-command usage, a copy-paste GitHub Actions snippet, and how this differs from `--check`.
Follow-ups (not in this PR)
Testing
921 tests pass. `drift-check.ts` at 100% branch coverage. `fallow:audit` exit 0, lint clean.
https://claude.ai/code/session_01SZmVypA9rE5H3HTreJgyE2
Summary by CodeRabbit
New Features
--check-driftmode to validate generated output against the committed filesystem, with CI-friendly failure behavior.Bug Fixes
Tests
--check-driftmode.