fix(cli): build SDK before native release artifacts#3795
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e541dc0db
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| * @returns {void} | ||
| */ | ||
| export function ensureNodeSdkBuild(run = runCommand) { | ||
| run('pnpm', ['--prefix', sdkWorkspaceRoot, 'run', 'generate'], 'Generate SDK artifacts for CLI runtime'); |
There was a problem hiding this comment.
When any CLI lifecycle script that calls ensure-superdoc-build runs (for example pretest, prebuild, prebuild:native, or pretypecheck in apps/cli/package.json), this invokes the SDK workspace's mutating generate script. That generator writes tracked outputs outside the CLI path, including apps/mcp/src/generated/*, packages/sdk/langs/browser/src/*, and prompt/dispatch artifacts under packages/sdk/tools, so a routine CLI build or test can dirty the checkout or silently update unrelated MCP/browser/Python SDK artifacts whenever the contract or prompt inputs drift. Please use a non-mutating currentness check or a Node-SDK-only generation/prep step here.
Useful? React with 👍 / 👎.
|
This pull request landed on |
Code Review by Qodo
1. SDK tools dir not prepared
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| export function ensureNodeSdkBuild(run = runCommand) { | ||
| run('pnpm', ['--prefix', sdkWorkspaceRoot, 'run', 'generate'], 'Generate SDK artifacts for CLI runtime'); | ||
| run('pnpm', ['--prefix', nodeSdkRoot, 'run', 'build'], 'Build Node SDK for CLI runtime'); | ||
| } |
There was a problem hiding this comment.
1. Sdk tools dir not prepared 🐞 Bug ≡ Correctness
ensureNodeSdkBuild builds the Node SDK dist but never creates the package-level tools/ directory that the SDK presets read at runtime; doc.preset.* can throw TOOLS_ASSET_NOT_FOUND on clean checkouts/native binaries. This breaks CLI preset operations because the SDK resolves tool JSON relative to @superdoc-dev/sdk’s package root, while codegen writes tools to packages/sdk/tools instead.
Agent Prompt
### Issue description
`ensureNodeSdkBuild()` runs SDK generation and `@superdoc-dev/sdk` build, but it does not ensure that `packages/sdk/langs/node/tools/` exists. At runtime, the Node SDK preset loaders resolve tool artifacts via `.../tools/*.json` relative to the SDK package root and throw `TOOLS_ASSET_NOT_FOUND` if they are missing. In a clean checkout / release build environment, codegen writes tools to `packages/sdk/tools` (workspace root), not to the Node SDK package’s `tools/` directory.
### Issue Context
- CLI preset operations (`doc.preset.*`) execute the Node SDK preset machinery in-process via `@superdoc-dev/sdk`.
- The Node SDK preset code reads JSON artifacts from `<sdk package root>/tools`.
- The Node SDK package has `prepack/postpack` scripts that *are* responsible for copying/symlinking tools into the package root, but `ensureNodeSdkBuild()` does not run them.
### Fix Focus Areas
- apps/cli/scripts/ensure-superdoc-build.js[31-34]
### Suggested fix
After `pnpm --prefix packages/sdk run generate`, add a step that makes the Node SDK package’s `tools/` directory available before building/compiling the CLI, for example:
- Run the Node SDK’s `postpack` script to create the symlink: `pnpm --prefix packages/sdk/langs/node run postpack` (creates `tools -> ../../tools`).
- If you need portability beyond *nix, replace `ln -s` in `postpack` with a Node-based symlink/copy script.
- Alternatively, add a small Node script invoked from `ensureNodeSdkBuild()` that ensures `packages/sdk/langs/node/tools` exists (symlink or copy) and points at `packages/sdk/tools`.
Make sure this step runs **before** `bun build --compile` so preset commands can load tool catalogs in the compiled binary.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| expect(calls).toEqual([ | ||
| { | ||
| command: 'pnpm', | ||
| args: ['--prefix', expect.stringContaining('/packages/sdk'), 'run', 'generate'], | ||
| label: 'Generate SDK artifacts for CLI runtime', | ||
| }, | ||
| { | ||
| command: 'pnpm', | ||
| args: ['--prefix', expect.stringContaining('/packages/sdk/langs/node'), 'run', 'build'], | ||
| label: 'Build Node SDK for CLI runtime', |
There was a problem hiding this comment.
2. Os-dependent path assertions 🐞 Bug ☼ Reliability
The new ensureNodeSdkBuild test hard-codes POSIX fragments ("/packages/sdk"), but production code
uses path.join(...) for --prefix, which yields platform-specific separators. This will fail on
Windows (and is fragile across path normalization differences).
Agent Prompt
### Issue description
The test for `ensureNodeSdkBuild()` asserts that `--prefix` arguments contain strings like `'/packages/sdk'`, but the implementation builds those paths via `path.join(repoRoot, ...)`, which will produce `\packages\sdk` on Windows.
### Issue Context
This is a unit test fragility that can break Windows CI/local runs even though the code is correct.
### Fix Focus Areas
- apps/cli/scripts/__tests__/ensure-superdoc-build.test.ts[41-50]
### Suggested fix
Update assertions to be separator-agnostic, e.g.:
- Import `node:path` in the test and assert against `path.join('packages','sdk')` and `path.join('packages','sdk','langs','node')`.
- Or normalize both sides: `expect(path.normalize(args[1])).toContain(path.normalize('/packages/sdk'))`.
- Or use a regex that accepts both separators: `/[\\/]packages[\\/]sdk/`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
🎉 This PR is included in superdoc-cli v0.21.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.20.0 |
|
🎉 This PR is included in @superdoc-dev/mcp v0.16.0 The release is available on GitHub release |
2 participants
The CLI native release build now prepares the Node SDK before Bun compiles CLI binaries.
doc.preset.*imports SDK preset code, so a fresh release checkout needs generated SDK sources and the SDK dist package beforebun build --compile.