feat(packages/cli): add check command for config validation and deadlink detection#14
feat(packages/cli): add check command for config validation and deadlink detection#14zrosenbauer merged 5 commits intomainfrom
check command for config validation and deadlink detection#14Conversation
…dlink detection Introduces `zpress check` — a standalone command that validates the zpress config and detects broken internal links via a Rspress build with captured output. The `build` command now includes checks by default (opt out with `--no-check`). Key changes: - New `check` command with clean lint-style output for broken links - `defineConfig` is now a pure type-helper passthrough (no validation/exit) - `loadConfig` validates via `validateConfig` and returns Result tuples - `buildSiteForCheck` captures stdout/stderr to parse Rspress deadlink diagnostics - `build --check` (default) runs config validation + deadlink detection Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 9c06a6e The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
📝 WalkthroughWalkthroughAdds a new Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as "zpress CLI"
participant Loader as "loadConfig"
participant Validator as "validateConfig"
participant Sync as "sync (quiet)"
participant Rspress as "buildSiteForCheck"
participant Parser as "parseDeadlinks"
participant Presenter as "presentResults"
User->>CLI: run `zpress check` or `zpress build` (with check)
CLI->>Loader: loadConfig(cwd)
Loader->>Validator: validateConfig(config)
Validator-->>Loader: ConfigResult
Loader-->>CLI: [config | error]
alt config invalid
CLI->>Presenter: presentResults(configResult, skipped build)
Presenter-->>CLI: success? false
CLI->>User: exit 1
else config valid
CLI->>Sync: sync(content, quiet:true)
Sync-->>CLI: pages written/skipped
CLI->>Rspress: buildSiteForCheck(capture stderr)
Rspress-->>Parser: captured stderr
Parser-->>CLI: BuildResult (deadlink groups or none)
CLI->>Presenter: presentResults(configResult, buildResult)
Presenter-->>CLI: success?
alt all checks pass
CLI->>User: log success
else
CLI->>User: exit 1
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/config.ts (1)
18-32:⚠️ Potential issue | 🟠 MajorWrap
c12LoadConfig()rejections in atry-catchand map toConfigError.The function advertises
Promise<ConfigResult<ZpressConfig>>, butc12LoadConfig()can reject (on file syntax errors, missing extends, permission failures, etc.). Any such rejection escapes this function and crashes all CLI callers expecting a tuple. Wrap the load and map any caught error into aConfigErrortuple to maintain the structured error contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/config.ts` around lines 18 - 32, Wrap the call to c12LoadConfig inside loadConfig with a try-catch: call c12LoadConfig(...) inside try, and in catch return a ConfigResult tuple mapping the thrown error to a ConfigError (use the existing configError helper, e.g. configError('load_failed', String(err) or err.message)) so callers always get [ConfigError, null] instead of an unhandled rejection; keep the subsequent checks (config and config.sections) and final return of validateConfig(config as ZpressConfig).packages/cli/src/commands/build.ts (1)
35-44:⚠️ Potential issue | 🟠 Major
runConfigCheckat line 44 is effectively dead code due to early exit.When
checkis true, the intent appears to be collecting config errors and presenting them viapresentResultsfor a unified lint-style output. However, the early exit at lines 36-39 runs unconditionally, soconfigErris alwaysnullby line 44, makingrunConfigCheckalways return{ passed: true, errors: [] }.Consider moving the early exit into the unchecked branch so the checked path can present config errors alongside deadlink results:
🐛 Proposed fix
const [configErr, config] = await loadConfig(paths.repoRoot) - if (configErr) { - ctx.logger.error(configErr.message) - process.exit(1) - } if (check) { // Checked build: validate config, sync, then run deadlink-detecting build ctx.logger.step('Validating config...') const configResult = runConfigCheck(config, configErr) + + // If config failed, skip sync/build but still present results + if (!configResult.passed) { + const buildResult = { passed: true, deadlinks: [] } + presentResults({ configResult, buildResult, logger: ctx.logger }) + ctx.logger.outro('Build failed') + process.exit(1) + } ctx.logger.step('Syncing content...') await sync(config, { paths, quiet: true }) // ... rest unchanged } else { // Unchecked build: standard sync + build (no validation, noisy output) + if (configErr) { + ctx.logger.error(configErr.message) + process.exit(1) + } await sync(config, { paths, quiet }) await buildSite({ config, paths }) ctx.logger.outro('Done') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/build.ts` around lines 35 - 44, The early exit after loadConfig causes configErr to be cleared for the checked path so runConfigCheck/config reporting never runs; change the control flow so that the unconditional process.exit(1) (and ctx.logger.error) only happens in the unchecked branch (when check is false) — keep the loadConfig call and preserve ctx.logger.step('Validating config...'), then in the check=true path call runConfigCheck(config, configErr) and pass its results to presentResults, while in the check=false path retain the existing immediate error log and process.exit(1) behavior; update references: loadConfig, configErr, runConfigCheck, presentResults, check, ctx.logger.error, process.exit.
🧹 Nitpick comments (3)
packages/cli/src/lib/check.ts (2)
79-86: MakerunConfigChecktake a single params object and own the null-config invariant.
runConfigCheck(null, null)currently returns{ passed: true }, so callers still need a second!configguard later. Collapsing these inputs into{ config, loadError }and failing whenconfigis absent keeps the API self-consistent and matches the repo's exported-function convention. As per coding guidelines, "Use object parameters for functions with 2+ parameters and include explicit return types."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/lib/check.ts` around lines 79 - 86, Change runConfigCheck to accept a single object parameter (e.g. runConfigCheck({ config, loadError }: { config: ZpressConfig | null; loadError?: ConfigError | null }): ConfigCheckResult) and make it own the null-config invariant by returning passed: false with an appropriate error when config is null or undefined; keep the existing explicit return type ConfigCheckResult and preserve the loadError handling (if loadError return it in errors), otherwise if config is missing return a failure error indicating missing config.
53-54: Remove the lint disable comment by rewriting withString.raw.The code currently passes lint with the disable comment in place. However, you can eliminate the need for the disable by using
String.rawwith theRegExpconstructor to avoid control character interpretation:Refactor
-// oxlint-disable-next-line prefer-regex-literals, no-control-regex -- regex literal is clearer for a well-known ANSI escape pattern -const ANSI_PATTERN = /\u001B\[[0-9;]*m/g +const ANSI_PATTERN = new RegExp(String.raw`\u001B\[[0-9;]*m`, 'g')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/lib/check.ts` around lines 53 - 54, Replace the regex literal and its lint-disable comment by constructing ANSI_PATTERN via the RegExp constructor with a raw string: call new RegExp with String.raw to supply the backslash/escape sequence for the ANSI CSI and the character class/quantifier, and pass the 'g' flag; update the constant named ANSI_PATTERN so the pattern is created from String.raw(...) and remove the oxlint-disable-next-line comment.packages/cli/src/lib/rspress.ts (1)
63-69: Encode the check-build mode instead of duplicatingbuildSite().
buildSiteForCheck()currently does the samecreateRspressConfig(options)+build(...)sequence asbuildSite(), so the new semantics only live in the docblock. If check builds really need different logging behavior, thread that choice through here or collapse both helpers into one private build function before the two paths drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/lib/rspress.ts` around lines 63 - 69, The buildSiteForCheck function duplicates buildSite; refactor by creating a single private helper (e.g., a new function like runBuild(options: ServerOptions, {checkMode?: boolean})) that calls createRspressConfig(options) and build(...), then have both buildSiteForCheck and buildSite call that helper with checkMode true/false (or thread a check flag through to alter logging behavior), or alternatively make buildSite accept an optional check flag and remove buildSiteForCheck; update calls to use the helper and remove duplicated logic while preserving unique logging behavior when checkMode is set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/lib/check.ts`:
- Around line 29-32: Replace the ad-hoc { passed, deadlinks } shape with an
explicit discriminated union so callers can represent skipped and errored builds
instead of faking deadlink entries: change BuildCheckResult into a union like {
status: 'passed'; deadlinks: DeadlinkInfo[] } | { status: 'skipped' } | {
status: 'error'; error: string } (or similar), update any producers to return
the appropriate variant instead of inserting fake DeadlinkInfo objects, and
update consumers (including the code that currently synthesizes a passing result
in the check command and the place that constructs fake deadlink entries) to
switch on status and handle deadlinks only for the 'passed' case. Ensure
DeadlinkInfo remains unchanged and adjust typings/usages accordingly.
---
Outside diff comments:
In `@packages/cli/src/commands/build.ts`:
- Around line 35-44: The early exit after loadConfig causes configErr to be
cleared for the checked path so runConfigCheck/config reporting never runs;
change the control flow so that the unconditional process.exit(1) (and
ctx.logger.error) only happens in the unchecked branch (when check is false) —
keep the loadConfig call and preserve ctx.logger.step('Validating config...'),
then in the check=true path call runConfigCheck(config, configErr) and pass its
results to presentResults, while in the check=false path retain the existing
immediate error log and process.exit(1) behavior; update references: loadConfig,
configErr, runConfigCheck, presentResults, check, ctx.logger.error,
process.exit.
In `@packages/core/src/config.ts`:
- Around line 18-32: Wrap the call to c12LoadConfig inside loadConfig with a
try-catch: call c12LoadConfig(...) inside try, and in catch return a
ConfigResult tuple mapping the thrown error to a ConfigError (use the existing
configError helper, e.g. configError('load_failed', String(err) or err.message))
so callers always get [ConfigError, null] instead of an unhandled rejection;
keep the subsequent checks (config and config.sections) and final return of
validateConfig(config as ZpressConfig).
---
Nitpick comments:
In `@packages/cli/src/lib/check.ts`:
- Around line 79-86: Change runConfigCheck to accept a single object parameter
(e.g. runConfigCheck({ config, loadError }: { config: ZpressConfig | null;
loadError?: ConfigError | null }): ConfigCheckResult) and make it own the
null-config invariant by returning passed: false with an appropriate error when
config is null or undefined; keep the existing explicit return type
ConfigCheckResult and preserve the loadError handling (if loadError return it in
errors), otherwise if config is missing return a failure error indicating
missing config.
- Around line 53-54: Replace the regex literal and its lint-disable comment by
constructing ANSI_PATTERN via the RegExp constructor with a raw string: call new
RegExp with String.raw to supply the backslash/escape sequence for the ANSI CSI
and the character class/quantifier, and pass the 'g' flag; update the constant
named ANSI_PATTERN so the pattern is created from String.raw(...) and remove the
oxlint-disable-next-line comment.
In `@packages/cli/src/lib/rspress.ts`:
- Around line 63-69: The buildSiteForCheck function duplicates buildSite;
refactor by creating a single private helper (e.g., a new function like
runBuild(options: ServerOptions, {checkMode?: boolean})) that calls
createRspressConfig(options) and build(...), then have both buildSiteForCheck
and buildSite call that helper with checkMode true/false (or thread a check flag
through to alter logging behavior), or alternatively make buildSite accept an
optional check flag and remove buildSiteForCheck; update calls to use the helper
and remove duplicated logic while preserving unique logging behavior when
checkMode is set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f57d1aa9-f7a0-4c30-befb-31a97d47ca2f
📒 Files selected for processing (10)
.changeset/add-check-command.mdpackages/cli/src/commands/build.tspackages/cli/src/commands/check.tspackages/cli/src/index.tspackages/cli/src/lib/check.tspackages/cli/src/lib/rspress.tspackages/core/src/config.tspackages/core/src/define-config.tspackages/core/src/index.tspackages/ui/src/config.ts
Replace the overloaded `{ passed, deadlinks }` shape with a tagged
union (`passed | failed | skipped | error`) so callers no longer need
to fake build results when the build was skipped or failed for
non-deadlink reasons.
Co-Authored-By: Claude <noreply@anthropic.com>
d6f6521 to
f60ad9c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/cli/src/lib/check.ts (1)
312-316: Avoid using.map()for side effects.Using
map()purely for side effects discards the returned array and obscures intent. UseforEach()or afor...ofloop for clarity.Proposed fix
- // oxlint-disable-next-line no-unused-expressions -- side-effect logging over config errors - configResult.errors.map((err) => { - logger.message(` ${err.message}`) - return null - }) + for (const err of configResult.errors) { + logger.message(` ${err.message}`) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/lib/check.ts` around lines 312 - 316, The snippet uses configResult.errors.map(...) for side effects which is misleading; replace the map call with configResult.errors.forEach(...) (or a for...of loop) so the intent is clear and no unused array is created, keeping the logger.message(...) calls unchanged; locate the usage of configResult.errors.map in the check.ts code and swap it to forEach while preserving the existing logging behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/lib/check.ts`:
- Around line 112-128: The interceptor created by createInterceptor
(interceptWrite) swallows write callbacks and never invokes them, breaking
callers that rely on the completion callback; update interceptWrite to detect
when a callback is provided (either as the second or third argument per Node's
write overloads), call that callback (with no error) after pushing the
chunkToString result into chunks, and preserve the original boolean return
value; ensure you handle both string and Uint8Array chunk types and match the
signature of process.stdout.write so existing callers receive their callback
invocation.
- Around line 80-88: The function runConfigCheck currently ignores its config
argument and accepts two positional params; change its signature to accept a
single object param (e.g., { config, loadError }) and validate that config is
non-null: if loadError is present return { passed: false, errors: [loadError] },
else if config is null return { passed: false, errors: [new ConfigError('missing
config')]} (or an appropriate ConfigError instance/message), otherwise return {
passed: true, errors: [] }; update any callers to pass an object and adjust the
return type to ConfigCheckResult accordingly.
---
Nitpick comments:
In `@packages/cli/src/lib/check.ts`:
- Around line 312-316: The snippet uses configResult.errors.map(...) for side
effects which is misleading; replace the map call with
configResult.errors.forEach(...) (or a for...of loop) so the intent is clear and
no unused array is created, keeping the logger.message(...) calls unchanged;
locate the usage of configResult.errors.map in the check.ts code and swap it to
forEach while preserving the existing logging behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8c4d634a-0257-4816-8e98-41f6aa2f23c6
📒 Files selected for processing (2)
packages/cli/src/commands/check.tspackages/cli/src/lib/check.ts
…te callbacks - runConfigCheck now takes a single object param per coding guidelines - Added null-config guard so missing config without a loadError is caught - Interceptor now invokes write-completion callbacks instead of swallowing them Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/cli/src/lib/check.ts (1)
329-334: ConsiderforEachfor side-effect-only iteration.Using
mapwith a discarded return value for side-effect logging works, butforEachwould be more idiomatic since no transformation is needed. This is a minor clarity improvement.♻️ Suggested refactor
- // oxlint-disable-next-line no-unused-expressions -- side-effect logging over config errors - configResult.errors.map((err) => { - logger.message(` ${err.message}`) - return null - }) + configResult.errors.forEach((err) => { + logger.message(` ${err.message}`) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/lib/check.ts` around lines 329 - 334, The code is using configResult.errors.map solely for side-effect logging which is non-idiomatic; replace the map call with a forEach to iterate without returning values: locate the configResult.errors.map(...) block and change it to configResult.errors.forEach(...) and keep the logger.message(` ${err.message}`) side-effect inside the callback (no return needed), ensuring the oxlint-disable comment remains if required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/cli/src/lib/check.ts`:
- Around line 329-334: The code is using configResult.errors.map solely for
side-effect logging which is non-idiomatic; replace the map call with a forEach
to iterate without returning values: locate the configResult.errors.map(...)
block and change it to configResult.errors.forEach(...) and keep the
logger.message(` ${err.message}`) side-effect inside the callback (no return
needed), ensuring the oxlint-disable comment remains if required.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fff1d655-b017-4eb0-8b1c-029ed3ea4101
📒 Files selected for processing (3)
packages/cli/src/commands/build.tspackages/cli/src/commands/check.tspackages/cli/src/lib/check.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli/src/commands/check.ts
Summary
zpress checkcommand that validates config and detects broken internal links via a Rspress build with captured outputbuildcommand via--checkflag (enabled by default, opt out with--no-check)defineConfigto be a pure type-helper passthrough — validation moves toloadConfigwhich returns structuredResulttuplesChanges
New files
packages/cli/src/commands/check.ts— ThecheckCLI commandpackages/cli/src/lib/check.ts— Shared check orchestration (config validation, build output capture, deadlink parsing, lint-style presentation)Modified files
packages/core/src/define-config.ts—defineConfigis now a passthrough;validateConfigexportedpackages/core/src/config.ts—loadConfigvalidates viavalidateConfigand returnsResulttuplespackages/core/src/index.ts— ExportsvalidateConfigpackages/ui/src/config.ts— Accepts optionallogLevelincreateRspressConfigpackages/cli/src/lib/rspress.ts— AddsbuildSiteForCheckfor deadlink detection buildspackages/cli/src/commands/build.ts— Adds--checkflag with config validation and deadlink detectionpackages/cli/src/index.ts— RegisterscheckCommandTest plan
pnpm buildcompiles all packagespnpm check(typecheck + lint + format) passeszpress checkon a valid project shows "All checks passed"zpress checkwith broken internal links detects and lists deadlinks with lint-style outputzpress checkwith invalid config (e.g. empty sections) shows friendly validation errorzpress buildruns checks by default and fails on deadlinkszpress build --no-checkskips validation and builds normallySummary by CodeRabbit
New Features
Improvements