feat(cli): allow docs.yml redirects to reference a YAML file - #17311
feat(cli): allow docs.yml redirects to reference a YAML file#17311devin-ai-integration[bot] wants to merge 4 commits into
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| const absoluteFilepathToRedirects = resolve(dirname(absoluteFilepathToDocsConfig), RelativeFilePath.of(redirects)); | ||
| if (!(await doesPathExist(absoluteFilepathToRedirects))) { | ||
| throw new CliError({ | ||
| message: `Failed to load redirects: ${absoluteFilepathToRedirects} does not exist`, | ||
| code: CliError.Code.ParseError | ||
| }); | ||
| } |
There was a problem hiding this comment.
🟡 Pointing redirects at an absolute file path crashes the CLI with an internal error
The redirects path from docs.yml is required to be relative (RelativeFilePath.of(redirects) at packages/cli/configuration-loader/src/docs-yml/resolveRedirects.ts:32) before any friendly error handling runs, so users who give a full path see an internal crash instead of a clear message.
Impact: The command aborts with a raw internal error and no guidance about what is wrong in the configuration file.
Mechanism: RelativeFilePath.of throws a plain Error for absolute inputs, bypassing CliError handling
RelativeFilePath.of throws new Error("Filepath is not relative: " + value) when given an absolute path (packages/commons/fs-utils/src/RelativeFilePath.ts:11-13). resolveRedirects is invoked during workspace load (packages/cli/workspace/loader/src/loadDocsWorkspace.ts:124-127) and in parseDocsConfiguration, neither of which converts that generic error into a CliError, so every command that loads docs (check, generate, docs dev) fails with an unexpected-error stack rather than the intended Failed to load redirects: ... message.
The rest of the docs config resolves user-supplied paths with resolve(dirname(absoluteFilepathToDocsConfig), unresolvedFilepath) (packages/cli/configuration-loader/src/docs-yml/parseDocsConfiguration.ts:1694-1702), which tolerates absolute inputs; resolveRedirects deviates from that convention.
A related edge case: redirects: "" resolves to the containing directory, doesPathExist returns true, and readFile then throws an uncaught EISDIR.
| const absoluteFilepathToRedirects = resolve(dirname(absoluteFilepathToDocsConfig), RelativeFilePath.of(redirects)); | |
| if (!(await doesPathExist(absoluteFilepathToRedirects))) { | |
| throw new CliError({ | |
| message: `Failed to load redirects: ${absoluteFilepathToRedirects} does not exist`, | |
| code: CliError.Code.ParseError | |
| }); | |
| } | |
| const absoluteFilepathToRedirects = resolve(dirname(absoluteFilepathToDocsConfig), redirects); | |
| if (!(await doesPathExist(absoluteFilepathToRedirects))) { | |
| throw new CliError({ | |
| message: `Failed to load redirects: ${absoluteFilepathToRedirects} does not exist`, | |
| code: CliError.Code.ParseError | |
| }); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch — fixed in bf96a5b. resolveRedirects now uses resolve(dirname(docsConfig), redirects) directly (matching resolveFilepath), so absolute paths work instead of throwing a raw Filepath is not relative error. Also guarded the redirects: "" case and switched to doesPathExist(..., "file") so a directory reports is not a file rather than blowing up with EISDIR. Tests added for both.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Docs Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on
Docs generation runs |
SDK Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
Description
redirectsindocs.ymlnow accepts either the inline list it accepts today or a relative/absolute filepath to a YAML file that contains only aredirectslist:The file is read and validated at docs-workspace load time, so every downstream consumer (
parseDocsConfiguration,no-circular-redirects,missing-redirects,valid-markdown-link, theme stitching, publish) keeps seeing a plain list — no consumer had to learn about the filepath form.Changes Made
fern/apis/docs-yml/definition/docs.yml:redirects: optional<RedirectsConfiguration>whereRedirectsConfigurationis an undiscriminated union oflist<RedirectConfig> | string; regenerated the docs-config SDK and the JSON schemas.resolveRedirects(packages/cli/configuration-loader): resolves the path relative todocs.yml, parses the YAML, and validates it againstRedirectsFile— a strict object with a singleredirectskey holding strictRedirectConfigs. Errors on a missing file/non-file, malformed YAML, a bare list with noredirectskey, or an invalid redirect.loadRawDocsConfiguration,parseDocsConfiguration, and cli-v2'sLegacyDocsWorkspaceAdaptercall it;DocsWorkspace.config/ the docs-validator AST node are now typedDocsConfigurationWithResolvedRedirects(redirects?: RedirectConfig[]).Testing
packages/cli/configuration-loader/src/docs-yml/__test__/resolveRedirects.test.ts(inline passthrough, relative + absolute file loading, missing file, empty path, invalid redirect, bare list without theredirectskey).Docs for the new form live in
fern-api/docsand aren't updated here — happy to open that PR too.Link to Devin session: https://app.devin.ai/sessions/70d7967abc644ccaa6292cb3cf80bf73