Repository navigation
feat(cli): add the redocly recheck command - #3082
Conversation
🦋 Changeset detectedLatest commit: 22c7c57 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Re-read both docs pages and rewrote them in 2c3fe6d: plain introductions, no release notes, examples in the same shape as the |
42a5366 to
03cf1af
Compare
|
Rebased onto the current #3080 head and added the |
tatomyr
left a comment
tatomyr
left a comment
There was a problem hiding this comment.
Had a quick look. My main concern is that the recheck interface/behaviour diverges from lint for no obvious reason. Please consider aligning them more closely.
82f8b42 to
a07e16d
Compare
9dae91f to
820dcb8
Compare
8e829f0 to
49f6fb4
Compare
| @@ -0,0 +1,47 @@ | |||
| import type { VerifyConfigOptions } from '../../types.js'; | |||
There was a problem hiding this comment.
Could you please refactor the structure a bit?
Let's move types inside the separate types.ts file and selectAction function inside the separate file named select-action.ts and then you can rename test file to select-action.test.ts.
There was a problem hiding this comment.
Done in 7e1138f.
Types live in types.ts, selectAction in select-action.ts, and the test is select-action.test.ts.
| } | ||
| } | ||
|
|
||
| function lintOptions(argv: RecheckArgv): LintOptions { |
There was a problem hiding this comment.
I'd prefer toLintOptions.
| }, | ||
| }), | ||
| (argv) => { | ||
| commandWrapper(handleRecheck)(argv); |
There was a problem hiding this comment.
Could you please add lazy import, because now we have performance degradation in check-config command: AlbinaBlazhko17#7 (comment).
const { handleRecheck } = await import('./commands/recheck/index.js');
commandWrapper(handleRecheck)(argv);There was a problem hiding this comment.
Done in 7e1138f.
The recheck handler loads through await import, the same way introspect-mcp does.
| format: 'esm', | ||
| target: 'node20.19', | ||
| // The engine imports these spell-check packages dynamically; a user installs them on demand. | ||
| external: ['nspell', 'dictionary-en'], |
There was a problem hiding this comment.
@adamaltman could you please explain why you decided to keep those two packages as a peer dependency?
I tested the case when we use those packages inside the deps and it has no performance impact: https://github.com/AlbinaBlazhko17/redocly-cli/pull/7/changes#diff-0ba29bdf689d78abba7ff7ecf6f2d5520d8ba5f3e0d3a3c842be64e265533847.
There was a problem hiding this comment.
Agreed, and bundled in 32add50.
They were externals so the engine's optional spell-check stayed on demand, but the CLI manifest carries no runtime dependencies, so a user could never get them.
Bundling needed one addition: dictionary-en reads its data files relative to its own module URL, so the build copies index.aff and index.dic next to the chunk esbuild picks.
The bundle grows by about 0.6 MB and the spell-check chunk stays lazy.
|
|
||
| ```yaml | ||
| recheck: | ||
| baseline: ./.recheck-baseline.yaml |
There was a problem hiding this comment.
Why add it manually? The lint ignore file is picked up merely by virtue of being in the project. I suggest we keep the same approach for baseline.
There was a problem hiding this comment.
Done in 6d6bc97.
A .redocly.recheck-baseline.yaml next to redocly.yaml is picked up by presence.
The baseline key stays as an override for another path.
There was a problem hiding this comment.
Ideally there should be no way to override the ignore file, otherwise it creates divergence from the current approach in the lint command.
There was a problem hiding this comment.
Noted.
Dropping the baseline key means discovery only, the same as the ignore file, and it removes a key that @redocly/config 0.56 already publishes.
It joins the config-shape decision, so one change covers both.
There was a problem hiding this comment.
Adam agreed; the baseline key goes and only discovery stays, on #3080.
| import { AbortFlowError } from '../../../utils/error.js'; | ||
| import { handleRecheck } from '../index.js'; | ||
|
|
||
| function fakeConfig( |
There was a problem hiding this comment.
These tests don't work well as unit tests, since it's hard to tell what they're actually testing. Converting them to e2e tests would make the use cases clearer.
There was a problem hiding this comment.
Agreed.
4777213 replaces the handler unit tests with e2e fixtures under tests/e2e/recheck/, one directory per use case with a snapshot.
The stacked API-description PR gets the same treatment.
|
The red |
…ck block The prose engine composes recheck/* presets itself, so the API preset resolver skips them.
Report payloads go through output. Progress and diagnostics stay on log, warn, and error.
The breakdown prints only inside the table report, so it goes with the report payload.
The engine's optional spell-check peers stay external to the bundle, and an adapter maps the engine Logger onto the CLI logger.
One command with action flags: lint by default, --readability, --generate-baseline, and --generate-markdoc-schema. Presets come from the root extends and rules from the recheck block. commandWrapper now reads process.exitCode after a handler returns instead of always reporting success, since recheck signals failure that way instead of throwing. lint.ts only forwards argv.format to the shared config-lint formatter when it is a real OutputFormat, since recheck's own report formats (table, sarif) are not.
Move the --generate-markdoc-schema branch before config resolution so it never depends on a valid recheck block. Add handler tests for readability, baseline, markdoc-schema, and the JSON report format.
Add sarif to the config-lint early-return list so recheck no longer leaks a stray report onto stdout. Warn when --output-path is set for a format other than json or sarif, since the report goes to stdout in that case. Treat a null recheck block the same as a missing one, and clean up temp directories in the handler test suite. Update the recheck docs and engine README: they described a command that had not shipped yet, misplaced the output-path and --fix behavior, and put the recheck command under the wrong docs heading and sidebar entry. Correct the changeset wording to match.
Remove release notes from the command page. Describe presets and the recheck block in one place. Drop the apiDescriptions option from the reference until the API-description path ships.
The config schema now carries the recheck root key, so check-config accepts the block and the config lint stops warning on it.
Rename --exclude-rule to --skip-rule and --annotations-limit to --max-problems, drop --changed-only and --changed-list, limit --severity to warn and error, and check nothing when redocly.yaml has no recheck configuration. Build the engine logger inline and leave the bundled config unmutated.
The CLI publish manifest carries no dependencies, so the rewrite of @redocly/recheck had no effect.
…he baseline Recheck now throws AbortFlowError on failure, so the wrapper's existing exit path handles it, and wrapper.ts reverts to its original behavior. Plugin ids recheck, redocly, redoc, realm, and reunite are now reserved and fail to load. A skill doc no longer mentions the unsupported redocly.yml name. The default baseline file is now hidden, at .recheck-baseline.yaml.
Two template literals held a raw NUL byte as the group separator, so git treated the file as binary. The \0 escape produces the same string. The doc comment no longer names the default baseline file.
32add50 to
0cdd84c
Compare
| paths?: string[]; | ||
| format: RecheckFormat; | ||
| 'output-path'?: string; | ||
| severity?: 'warn' | 'error'; |
There was a problem hiding this comment.
What do we need severity on this level for? This is against the current lint behaviour.
There was a problem hiding this comment.
--severity error runs the error rules only; the docs CI gate uses it to keep warnings out of the gate step.
lint has no such flag, and the exit code already fails on errors alone, so dropping it is on the table.
Adam decides with the other interface questions.
… aa/recheck-command # Conflicts: # package-lock.json # packages/cli/package.json # packages/cli/src/types.ts # scripts/local-pack.sh
22c7c57
into
aa/recheck-relocate-engine
5 participants
What/Why/How?
This PR adds the
redocly recheckcommand.It lints Markdown prose and structure with the recheck engine from
packages/recheck.Configuration lives in the
recheckblock ofredocly.yaml.You name presets in the root
extends, for exampleextends: [recommended, recheck/markdown].Core.
Core sets
recheck/*entries in the rootextendsaside asResolvedConfig.recheckExtends.The API preset resolver skips them, so
redocly lintignores them.Core takes no dependency on
@redocly/recheck.Engine.
The engine
Loggergained anoutputchannel for report payloads.The table, JSON, SARIF, GitHub Actions, readability, and
--statspayloads write to it.Progress, warnings, and errors stay on
log,warn, anderror.The readability action lost its
quietflag, because the channel split makes it unnecessary.Library callers of
runReadabilitynow receive progress lines throughlogin JSON mode too.CLI.
packages/clibundles the engine.The CLI bundles the spell-check packages,
nspellanddictionary-en, and the build copies the dictionary data files next to their chunk.The handler maps the engine's channels onto the CLI logger: progress to stderr, payloads to stdout.
So
--format json,sarif, andgithub-actionskeep stdout machine-readable.Command.
redocly recheck [paths..]lints by default.Action flags select one other action:
--readability,--generate-baseline, or--generate-markdoc-schema(with--from,--out,--check).Runs pick up a
.redocly.recheck-baseline.yamlnext toredocly.yamlby presence;baselinein therecheckblock sets another path.--fixapplies to linting only.Output flags:
--format(tabledefault),--output-path,--stats,--max-problems,--summary,--summary-path.--output-pathapplies tojsonandsarif; with other formats the command warns and writes the report to stdout.Selection flags:
--severity(warnorerror),--tags,--rule,--skip-rule.To scope a run to changed files, pass their paths.
With no
redocly.yaml, the command usesrecheck/markdownand says so on stderr.With a
redocly.yamlthat has norecheck/*preset and norecheckblock, it checks nothing and says so, the same aslint.The command skips an API description path with a stderr notice.
That path arrives in a later PR.
Shared-code changes.
commandWrapperkeeps its original behavior; the handler throwsAbortFlowErroron a failed run, the same pathlintuses.Core reserves the plugin ids
recheck,redocly,redoc,realm, andreunite; a plugin that declares one of them fails to load.handleLintConfiggained a type guard, because the recheck--formatvalues widened the sharedCommandArgvunion.handleLintConfigalso skips config linting for--format sarif, as it does forjson,junit, andcheckstyle, so SARIF stdout stays a single document.Config schema.
Core pins
@redocly/config0.56.0, which carries therecheckroot key.check-configaccepts the block, and the config lint no longer warns on it.A
check-confige2e fixture covers a valid block.This PR stacks on #3080, which relocates the engine.
Its base changes to
mainafter #3080 merges.Reference
It chose approach A: presets in the root
extends, rules in arecheckblock, no engine dependency in core.Phase 1 shipped the standalone engine; Phase 2 moves it into the CLI.
Testing
Unit tests cover the
recheck/namespace in core, the reserved plugin ids, andselectAction.End-to-end tests live in
tests/e2e/recheck/with twelve fixtures and snapshots.They cover lint, JSON and SARIF stdout, readability, and baseline generation.
They also cover the Markdoc schema, the fallback notice, the no-config notice, config errors, conflicting flags, and the output-path warning.
The test normalizes timing lines before it compares snapshots.
Gates:
npm run compile,npm run typecheck,npm run lint,npm run format:check,npm run unit, and the recheck e2e suite.In
npm run unit, the three pre-existing oas3 timeouts are the only failures, the same as onmain.Manual:
node packages/cli/lib/index.js recheck --help, and a run against an invalidrecheckblock to confirm--generate-markdoc-schemano longer depends on it.Screenshots (optional)
Check yourself
Security
Note
Medium Risk
New bundled dependency and config surface area, but API lint paths are isolated; main risk is CLI output/stream behavior and config parsing edge cases in CI.
Overview
Adds
redocly recheck, a CLI command that lints Markdown prose and structure via@redocly/recheck, wired through a newrecheckblock inredocly.yamlandrecheck/*presets (e.g.recheck/markdown) on the rootextends.Configuration and core: OpenAPI
lintno longer resolvesrecheck/*extends; those names are stored asrecheckExtendsfor the recheck command only.check-configaccepts therecheckblock; reserved plugin ids (recheck,redocly, etc.) are rejected at load time.Engine and CLI: The recheck
Loggergains anoutputchannel so reports (table, JSON, SARIF, GitHub Actions, readability tables) stay on stdout while progress useslog/warn/error. The CLI bundles the engine (including spell-checkdictionary-endata in the build), registers yargs options for lint,--fix,--readability,--generate-baseline, and--generate-markdoc-schema, and auto-picks.redocly.recheck-baseline.yamlwhen present.handleLintConfigskips config lint forsarifand tightens format typing where recheck widened the argv union.Docs, changeset, unit tests, and
tests/e2e/recheck/fixtures cover the new command and config behavior.Reviewed by Cursor Bugbot for commit 22c7c57. Bugbot is set up for automated code reviews on this repo. Configure here.