feat(cli): revamp webviewer app scaffolding#151
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
@proofkit/better-auth
@proofkit/cli
create-proofkit
@proofkit/fmdapi
@proofkit/fmodata
@proofkit/typegen
@proofkit/webviewer
commit: |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds FM HTTP support and WebViewer-focused scaffolding changes: non-interactive CLI mode, FM HTTP health checks and config injection, centralized typegen config handling, wide Vite WebViewer template refactor (Tailwind/shadcn), new skill docs (create-changeset + typegen guidance), and many CLI/installer updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as rgba(66,133,244,0.5) User
participant CLI as rgba(0,150,136,0.5) CLI
participant FMHTTP as rgba(255,193,7,0.5) FM_HTTP
participant Typegen as rgba(156,39,176,0.5) Typegen
participant Git as rgba(33,150,243,0.5) Git
User->>CLI: run `proofkit init / add --non-interactive` (webviewer)
CLI->>FMHTTP: GET /health
FMHTTP-->>CLI: status + connectedFiles
alt FM HTTP healthy & connected file
CLI->>Typegen: ensureWebviewerFmHttpConfig(connectedFile, baseUrl)
Typegen-->>CLI: config merged (.proofkit-typegen.config.jsonc)
else FM HTTP not available
CLI->>CLI: fallback to env vars or prompt/abort
end
CLI->>Git: initialize repo (guarded by non-interactive)
CLI->>Typegen: run codegen (if applicable)
CLI-->>User: scaffold created, next steps logged
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/cli/init.ts (1)
140-148:⚠️ Potential issue | 🟠 Major
--non-interactivestill prompts for the project name.Line 151 now skips the app-type prompt in non-interactive mode, but this branch still unconditionally calls
text()when[dir]is omitted. In automation,proofkit init --non-interactiveshould fail fast with a clear error here instead of blocking on stdin.Also applies to: 150-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/cli/init.ts` around lines 140 - 148, The init flow currently always calls text() to prompt for projectName (assigned to projectName via abortIfCancel(...).toString()), which blocks in --non-interactive mode; change the logic to first check the non-interactive flag (e.g., nonInteractive or options.nonInteractive) and if non-interactive and name is undefined, immediately fail with a clear error (throw or call abort with a message like "project name is required in non-interactive mode") instead of calling text(); otherwise preserve the existing behavior (use name or call abortIfCancel(text(...)) with validateAppName and DEFAULT_APP_NAME).
🧹 Nitpick comments (12)
packages/cli/template/vite-wv/_gitignore (1)
5-5: Broaden env file ignores to reduce accidental secret commits.Line 5 ignores only
.env; env variants can still leak.Proposed gitignore hardening
.env +.env.* +!.env.example🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/template/vite-wv/_gitignore` at line 5, Current .gitignore only ignores the literal ".env", which still allows many env file variants to be committed; update the ignore patterns to cover common env filename variants by adding broader patterns such as ".env*" and any other team-specific variants (for example ".env.local", ".env.development", "*.env", or ".env.*") so all variants are ignored—replace or augment the existing ".env" entry accordingly.packages/cli/src/helpers/logNextSteps.ts (1)
31-59: Consider extracting the package manager command formatter.The pattern
["npm", "bun"].includes(pkgManager) ? \${pkgManager} run` : pkgManager` is repeated multiple times. A helper would reduce duplication.♻️ Optional: Extract helper function
+const formatCommand = (pkgManager: string, script: string) => { + const prefix = ["npm", "bun"].includes(pkgManager) ? `${pkgManager} run` : pkgManager; + return ` ${prefix} ${script}`; +}; + export const logNextSteps = ({ // ... }) => { // ... logger.dim("\nStart the dev server to view your app in a browser:"); - if (["npm", "bun"].includes(pkgManager)) { - logger.info(` ${pkgManager} run dev`); - } else { - logger.info(` ${pkgManager} dev`); - } + logger.info(formatCommand(pkgManager, "dev"));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/helpers/logNextSteps.ts` around lines 31 - 59, Extract the repeated package-manager formatting into a small helper (e.g., formatRunCommand or getPmRunner) and replace the inline ternaries that build commands for dev, typegen, launch-fm, and proofkit; locate uses referencing pkgManager and logger.info in logNextSteps (the blocks that call `${pkgManager} run ...` vs `${pkgManager} ...`) and call the helper to return the correct prefix (e.g., "npm run" vs "pnpm" or "yarn") then append the subcommand names ("dev", "typegen", "launch-fm", "proofkit") before logging; keep state.appType checks and logger.dim calls unchanged.packages/cli/template/vite-wv/src/index.css (1)
27-47: Consider addingcolor-scheme: darkfor dark mode.The
.darkclass defines dark theme variables, butcolor-schemeis hardcoded tolightin thehtmlrule. This affects native form controls and scrollbars which won't adapt to dark mode.🎨 Proposed enhancement
html { color-scheme: light; } + + html.dark { + color-scheme: dark; + }Also applies to: 80-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/template/vite-wv/src/index.css` around lines 27 - 47, The html rule currently forces color-scheme: light so native controls and scrollbars never adopt dark mode; update the stylesheet to set color-scheme: dark when the .dark theme is active by adding color-scheme: dark to the .dark block (keep html color-scheme: light or change to color-scheme: light dark as appropriate) so native form controls and scrollbars follow the .dark CSS variables; target the .dark selector to implement this change.packages/cli/src/utils/getProofKitVersion.ts (1)
32-36: The webviewer package.json exists at the expected location; error handling is optional.The file
packages/webviewer/package.jsonis present in the repository and properly accessible at the expected path. However, adding error handling remains defensible: whilegetVersion()lacks try-catch (reading the CLI's own guaranteed-to-exist package.json),getNodeMajorVersion()in the same file demonstrates that defensive error handling is used for cases with less certainty. If the webviewer package might be excluded in any deployment scenario, the proposed try-catch wrapping with a "beta" fallback would gracefully handle that case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/utils/getProofKitVersion.ts` around lines 32 - 36, Wrap the logic in getProofkitWebviewerVersion in a try-catch: attempt to read packageJsonPath and return packageJsonContent.version when present, but if fs.readJSONSync throws or any other error occurs, catch it and return fallback "beta"; reference the existing symbols getProofkitWebviewerVersion, packageJsonPath, and fs.readJSONSync and ensure the catch silently returns "beta" (optionally log debug) to match getNodeMajorVersion's defensive pattern.packages/cli/src/helpers/fmHttp.ts (2)
39-42: Declare explicit return type for exported helperPlease add
: Promise<string | undefined>todetectConnectedFmFilefor a clearer public contract.As per coding guidelines, "Use explicit types for function parameters and return values when they enhance clarity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/helpers/fmHttp.ts` around lines 39 - 42, The exported helper detectConnectedFmFile currently lacks an explicit return type; update its signature to declare an explicit return type Promise<string | undefined> (e.g., export async function detectConnectedFmFile(baseUrl = defaultBaseUrl): Promise<string | undefined>), ensuring callers see the public contract; keep the body using getFmHttpStatus and defaultBaseUrl unchanged.
16-16: Avoid broad type assertion in JSON parserLine 16 force-casts parsed JSON to
T, which skips runtime validation. Prefer returningunknownfromreadJsonand narrowing at each call site.As per coding guidelines, "Leverage TypeScript's type narrowing instead of type assertions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/helpers/fmHttp.ts` at line 16, The readJson helper currently force-casts parsed JSON to T (returning (await response.json().catch(() => null)) as T | null); change readJson so it returns Promise<unknown | null> (remove the generic T cast) and return the raw parsed value without the type assertion (i.e., return await response.json().catch(() => null)); then update each call site to perform proper runtime/type narrowing or validation before treating the value as a specific type (adjust callers that assumed readJson<T> to validate/parse the unknown into the expected type).packages/cli/template/vite-wv/src/App.tsx (1)
6-22: Type and freeze thestepsconfigConsider adding an explicit immutable type (
as const+ shape typing) forstepsto keep icon/title/body contracts tight in this starter template.As per coding guidelines, "Use const assertions (
as const) for immutable values and literal types" and "Use explicit types for function parameters and return values when they enhance clarity."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/template/vite-wv/src/App.tsx` around lines 6 - 22, The steps constant should be made immutable and typed: define a Step type (with readonly icon, title, body) or a tuple/readonly array shape, then annotate the existing steps variable with that type and apply a const assertion (as const) to the literal so icons/titles/bodies become literal/readonly; update the variable name steps to use the new Step type and add as const to the array literal to enforce the contract for icon/title/body (symbols: steps, Step, icon, title, body).packages/cli/src/helpers/scaffoldProject.ts (1)
14-14: Add explicit return type to helperPlease make the return type explicit on Line 14 (
: string[]) to keep this filter contract clear across call sites.As per coding guidelines, "Use explicit types for function parameters and return values when they enhance clarity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/helpers/scaffoldProject.ts` at line 14, The function getMeaningfulDirectoryEntries currently lacks an explicit return type; update its signature to declare the return type as string[] (i.e., function getMeaningfulDirectoryEntries(projectDir: string): string[]) so callers clearly know it returns an array of strings and to satisfy the project's typing guidelines; ensure any related exports or usages still compile after adding the explicit return type.packages/cli/src/generators/fmdapi.ts (1)
269-280: Replaceconsole.logwith a proper logger.Per coding guidelines,
console.logstatements should be removed from production code. Consider using the project's logger utility instead.♻️ Suggested fix
+import { logger } from "~/utils/logger.js"; + export async function runCodegenCommand() { const projectDir = state.projectDir; const config = await readJsonConfigFile(path.join(projectDir, typegenConfigFileName)); if (!config) { - console.log("no typegen config found, skipping typegen"); + logger.info("no typegen config found, skipping typegen"); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/generators/fmdapi.ts` around lines 269 - 280, Replace the console.log in runCodegenCommand with the project's logger: when readJsonConfigFile(...) returns falsy, call the logger (e.g., import and use logger.info or logger.debug depending on level) instead of console.log to report "no typegen config found, skipping typegen"; ensure the module imports the logger and use state.projectDir, readJsonConfigFile, dotenvConfig, and generateTypedClients as-is so behavior is unchanged.packages/cli/tests/webviewer-apps.test.ts (1)
16-21: Consider if the 30-second timeout onbeforeEachis necessary.The
beforeEachhook only performs directory cleanup and creation, which should be nearly instantaneous. The 30-second timeout appears excessive for this operation. If the timeout is intended for the test itself, it should be on theit()block instead.♻️ Remove or reduce timeout
beforeEach(() => { if (existsSync(projectDir)) { rmSync(projectDir, { recursive: true, force: true }); } mkdirSync(testDir, { recursive: true }); - }, 30_000); + });If tests need extended time, apply the timeout to the individual
it()blocks that run CLI commands.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/tests/webviewer-apps.test.ts` around lines 16 - 21, The beforeEach hook currently has an excessive 30_000ms timeout for simple filesystem operations (beforeEach using existsSync, rmSync, mkdirSync on projectDir/testDir); remove the 30_000 timeout or reduce it to a very small value (or move any longer timeout to the specific it() tests that run slow CLI commands) so only slow tests get extended timeouts and the setup remains fast and unbounded.packages/cli/src/cli/add/data-source/filemaker.ts (1)
78-108: Consider simplifying the condition on line 84.After the check at line 78 throws for non-interactive mode without a server, the condition
!(opts.server || isNonInteractiveMode())on line 84 will always evaluate to!opts.server(sinceisNonInteractiveMode()would have thrown). The explicitisNonInteractiveMode()check is defensive but redundant.This is fine as-is for clarity, but could be simplified if preferred.
♻️ Optional simplification
- if (!(opts.server || isNonInteractiveMode())) { + if (!opts.server) { const fallbackAction = abortIfCancel(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/cli/add/data-source/filemaker.ts` around lines 78 - 108, The condition after the throw is redundant: simplify the guard in the interactive fallback by replacing the check "!(opts.server || isNonInteractiveMode())" with just "!opts.server" because isNonInteractiveMode() already caused a throw earlier; update the conditional around the p.select fallback (referencing opts.server and isNonInteractiveMode()) to use "!opts.server" so the fallback prompt only runs when no server flag is present.packages/cli/tests/browser-apps.test.ts (1)
39-44: Decouple this test from CI fallback behavior.This case is now explicitly about
--non-interactive; forcingCI: "true"on Line [61] can mask regressions if any CI-specific path remains. Prefer not overridingCIin this test.Suggested diff
expect(() => { execSync(command, { cwd: testDir, - env: { - ...process.env, - CI: "true", - }, + env: { ...process.env }, encoding: "utf-8", }); }).not.toThrow();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/tests/browser-apps.test.ts` around lines 39 - 44, The test "should create a browser project with FileMaker integration in non-interactive mode" is forcing CI via process.env.CI which can mask CI-specific behavior; remove any override that sets CI (or restore original CI after) so the test relies only on the --non-interactive flag. Locate the test block (the it(...) with that description) and delete the line that sets process.env.CI = "true" (or wrap it in a saved/original restore sequence) so the environment is not artificially changed while building the `command` using `cliPath` and `projectName`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/create-changeset/SKILL.md:
- Around line 82-83: Update the changeset example package scope by replacing the
string "@openameba/package-name" with this repo's actual package scope (or use a
neutral placeholder like "@your-org/package-name") wherever it appears in the
SKILL.md examples; specifically update the occurrences of the example entry
`"@openameba/package-name": major|minor|patch` (and the other matching examples)
so copy/pasted changesets use the correct repo scope.
In `@packages/cli/src/cli/init.ts`:
- Around line 151-152: The init flow sets state.appType = isNonInteractiveMode()
? "browser" which leaves state.dataSource undefined and causes select() to be
invoked in non-interactive mode; update the branching so that when
isNonInteractiveMode() returns true and you set state.appType to "browser" you
also set state.dataSource = "none" (or otherwise skip calling select()) for
WebViewer/browser projects; adjust the logic around the WebViewer/dataSource
defaults (the code that currently defaults WebViewer to "none") to ensure
state.dataSource is set before any call to select(), referencing state.appType,
state.dataSource, isNonInteractiveMode(), select(), and the WebViewer/browser
path.
In `@packages/cli/src/helpers/fmHttp.ts`:
- Around line 19-31: The getFmHttpStatus function currently performs network
calls (the fetch to `${baseUrl}/health` and the readJson call for
`${baseUrl}/connectedFiles`) without timeout protection; add an
AbortController-based timeout (e.g. const controller = new AbortController();
const id = setTimeout(() => controller.abort(), HTTP_PROBE_TIMEOUT_MS)) and pass
controller.signal into fetch (fetch(url, { signal: controller.signal })) for the
health probe (healthResponse) and for the connectedFiles request; ensure you
clearTimeout(id) after a successful response. If readJson does not accept a
signal, wrap the readJson call in a Promise.race against a timeout-rejecting
promise that is cancelled by the same timer so both the health fetch and the
connectedFiles read have the same HTTP_PROBE_TIMEOUT_MS behavior in
getFmHttpStatus.
In `@packages/cli/src/helpers/scaffoldProject.ts`:
- Around line 20-23: The code uses fs.statSync on entryPath which can throw
(e.g., broken symlink) and abort scaffolding; update the directory filtering in
scaffoldProject (where entryPath and stats are used) to call fs.lstatSync inside
a try/catch, treating any error as “skip this entry” (or fallback to
fs.existsSync check) instead of letting the exception bubble; after successful
lstat, continue to use stats.isFile() (and handle stats.isSymbolicLink() if
desired) to decide whether to skip entries starting with ".".
In `@packages/cli/src/index.ts`:
- Around line 45-46: The CLI currently logs a warning when
isNonInteractiveMode() is true but still calls runMenu(), which causes
non-interactive runs to hang; update the control flow in the entry point so that
when isNonInteractiveMode() returns true you do NOT call runMenu() (and
similarly for the other branch around lines 49-55), instead emit an error via
logger.error (or throw a CLI/Usage error) with an actionable message and
exit/fail (e.g., process.exit(1) or rethrow) so automated callers receive a
failure. Locate and modify the block that references isNonInteractiveMode() and
runMenu() to short-circuit on non-interactive mode and return/exit with a clear
error rather than proceeding into runMenu().
In `@packages/cli/src/installers/dependencyVersionMap.ts`:
- Around line 57-62: Replace the wildcard dependency for "@proofkit/typegen"
with a pinned version via a getter like the other entries: add a
getTypegenVersion() function that returns "1.1.0-beta.16" (or derive it from the
monorepo source) and change the entry from "@proofkit/typegen": "*" to
"@proofkit/typegen": `^${getTypegenVersion()}`; also update any template that
currently uses the "*" literal (e.g., the vite-wv template) to use the same
getter so installs are deterministic.
In `@packages/cli/template/vite-wv/scripts/filemaker.js`:
- Around line 29-38: The resolveFileMakerTarget function currently picks
connectedFiles[0] which is unsafe when multiple FileMaker solutions are
connected; update resolveFileMakerTarget to first try to match an explicit
target from environment (e.g., process.env.FM_DATABASE) or another provided
identifier against the list returned by getConnectedFiles(), return the matched
entry (with stripFileExtension and host/source as before), and if multiple
connections remain ambiguous (no match or multiple matches) throw or return a
clear failure so callers like scripts/upload.js and scripts/launch-fm.js do not
act on the wrong solution; ensure you reference and use getConnectedFiles,
connectedFiles, and stripFileExtension when implementing the matching and
ambiguity check.
In `@packages/cli/template/vite-wv/src/index.css`:
- Around line 87-92: The CSS font-family declaration contains an unnecessary
quoted single-word font "Inter"; update the font-family list in index.css where
font-family: "Instrument Sans", "Inter", ui-sans-serif, system-ui, sans-serif;
is declared by removing the quotes around Inter (keep quotes for "Instrument
Sans" because it contains a space) so it becomes "Instrument Sans", Inter,
ui-sans-serif, system-ui, sans-serif to follow CSS conventions.
In `@packages/cli/template/vite-wv/src/main.tsx`:
- Around line 6-10: Replace the non-null assertion on
document.getElementById("root") by first assigning it to a const (narrowing its
type) and explicitly handling the null case: check if the element is null and
throw a descriptive Error (e.g., "Root element with id 'root' not found") before
calling ReactDOM.createRoot; then pass the narrowed element to
ReactDOM.createRoot(...).render(...) so the call sites ReactDOM.createRoot and
document.getElementById are safe under strict TypeScript.
In `@packages/typegen/typegen.schema.json`:
- Around line 296-314: The schema "__schema15" currently only accepts an object
and will break existing configs that set fmHttp as a boolean; update
"__schema15" to accept either a boolean or the detailed object to preserve
compatibility (use oneOf or type union to allow {type: "boolean"} OR the
existing object with properties enabled, scriptName, baseUrl, connectedFileName
and additionalProperties: false). Ensure the boolean option is explicitly
allowed so existing fmHttp: true/false passes validation while the object form
continues to validate against the current property set.
In `@packages/webviewer/skills/webviewer-integration/SKILL.md`:
- Around line 30-34: Replace the non-deterministic dependency pinning for
`@proofkit/typegen` in the SKILL.md install instruction (the "pnpm add -D
`@proofkit/typegen`@*" command) with a bounded beta-compatible range; e.g., use
`@proofkit/typegen`@beta or an explicit compatible range like
`@proofkit/typegen`@^1.1.0-beta.0 so installs are reproducible and aligned with
the WebViewer 3.0.7-beta.0 release.
---
Outside diff comments:
In `@packages/cli/src/cli/init.ts`:
- Around line 140-148: The init flow currently always calls text() to prompt for
projectName (assigned to projectName via abortIfCancel(...).toString()), which
blocks in --non-interactive mode; change the logic to first check the
non-interactive flag (e.g., nonInteractive or options.nonInteractive) and if
non-interactive and name is undefined, immediately fail with a clear error
(throw or call abort with a message like "project name is required in
non-interactive mode") instead of calling text(); otherwise preserve the
existing behavior (use name or call abortIfCancel(text(...)) with
validateAppName and DEFAULT_APP_NAME).
---
Nitpick comments:
In `@packages/cli/src/cli/add/data-source/filemaker.ts`:
- Around line 78-108: The condition after the throw is redundant: simplify the
guard in the interactive fallback by replacing the check "!(opts.server ||
isNonInteractiveMode())" with just "!opts.server" because isNonInteractiveMode()
already caused a throw earlier; update the conditional around the p.select
fallback (referencing opts.server and isNonInteractiveMode()) to use
"!opts.server" so the fallback prompt only runs when no server flag is present.
In `@packages/cli/src/generators/fmdapi.ts`:
- Around line 269-280: Replace the console.log in runCodegenCommand with the
project's logger: when readJsonConfigFile(...) returns falsy, call the logger
(e.g., import and use logger.info or logger.debug depending on level) instead of
console.log to report "no typegen config found, skipping typegen"; ensure the
module imports the logger and use state.projectDir, readJsonConfigFile,
dotenvConfig, and generateTypedClients as-is so behavior is unchanged.
In `@packages/cli/src/helpers/fmHttp.ts`:
- Around line 39-42: The exported helper detectConnectedFmFile currently lacks
an explicit return type; update its signature to declare an explicit return type
Promise<string | undefined> (e.g., export async function
detectConnectedFmFile(baseUrl = defaultBaseUrl): Promise<string | undefined>),
ensuring callers see the public contract; keep the body using getFmHttpStatus
and defaultBaseUrl unchanged.
- Line 16: The readJson helper currently force-casts parsed JSON to T (returning
(await response.json().catch(() => null)) as T | null); change readJson so it
returns Promise<unknown | null> (remove the generic T cast) and return the raw
parsed value without the type assertion (i.e., return await
response.json().catch(() => null)); then update each call site to perform proper
runtime/type narrowing or validation before treating the value as a specific
type (adjust callers that assumed readJson<T> to validate/parse the unknown into
the expected type).
In `@packages/cli/src/helpers/logNextSteps.ts`:
- Around line 31-59: Extract the repeated package-manager formatting into a
small helper (e.g., formatRunCommand or getPmRunner) and replace the inline
ternaries that build commands for dev, typegen, launch-fm, and proofkit; locate
uses referencing pkgManager and logger.info in logNextSteps (the blocks that
call `${pkgManager} run ...` vs `${pkgManager} ...`) and call the helper to
return the correct prefix (e.g., "npm run" vs "pnpm" or "yarn") then append the
subcommand names ("dev", "typegen", "launch-fm", "proofkit") before logging;
keep state.appType checks and logger.dim calls unchanged.
In `@packages/cli/src/helpers/scaffoldProject.ts`:
- Line 14: The function getMeaningfulDirectoryEntries currently lacks an
explicit return type; update its signature to declare the return type as
string[] (i.e., function getMeaningfulDirectoryEntries(projectDir: string):
string[]) so callers clearly know it returns an array of strings and to satisfy
the project's typing guidelines; ensure any related exports or usages still
compile after adding the explicit return type.
In `@packages/cli/src/utils/getProofKitVersion.ts`:
- Around line 32-36: Wrap the logic in getProofkitWebviewerVersion in a
try-catch: attempt to read packageJsonPath and return packageJsonContent.version
when present, but if fs.readJSONSync throws or any other error occurs, catch it
and return fallback "beta"; reference the existing symbols
getProofkitWebviewerVersion, packageJsonPath, and fs.readJSONSync and ensure the
catch silently returns "beta" (optionally log debug) to match
getNodeMajorVersion's defensive pattern.
In `@packages/cli/template/vite-wv/_gitignore`:
- Line 5: Current .gitignore only ignores the literal ".env", which still allows
many env file variants to be committed; update the ignore patterns to cover
common env filename variants by adding broader patterns such as ".env*" and any
other team-specific variants (for example ".env.local", ".env.development",
"*.env", or ".env.*") so all variants are ignored—replace or augment the
existing ".env" entry accordingly.
In `@packages/cli/template/vite-wv/src/App.tsx`:
- Around line 6-22: The steps constant should be made immutable and typed:
define a Step type (with readonly icon, title, body) or a tuple/readonly array
shape, then annotate the existing steps variable with that type and apply a
const assertion (as const) to the literal so icons/titles/bodies become
literal/readonly; update the variable name steps to use the new Step type and
add as const to the array literal to enforce the contract for icon/title/body
(symbols: steps, Step, icon, title, body).
In `@packages/cli/template/vite-wv/src/index.css`:
- Around line 27-47: The html rule currently forces color-scheme: light so
native controls and scrollbars never adopt dark mode; update the stylesheet to
set color-scheme: dark when the .dark theme is active by adding color-scheme:
dark to the .dark block (keep html color-scheme: light or change to
color-scheme: light dark as appropriate) so native form controls and scrollbars
follow the .dark CSS variables; target the .dark selector to implement this
change.
In `@packages/cli/tests/browser-apps.test.ts`:
- Around line 39-44: The test "should create a browser project with FileMaker
integration in non-interactive mode" is forcing CI via process.env.CI which can
mask CI-specific behavior; remove any override that sets CI (or restore original
CI after) so the test relies only on the --non-interactive flag. Locate the test
block (the it(...) with that description) and delete the line that sets
process.env.CI = "true" (or wrap it in a saved/original restore sequence) so the
environment is not artificially changed while building the `command` using
`cliPath` and `projectName`.
In `@packages/cli/tests/webviewer-apps.test.ts`:
- Around line 16-21: The beforeEach hook currently has an excessive 30_000ms
timeout for simple filesystem operations (beforeEach using existsSync, rmSync,
mkdirSync on projectDir/testDir); remove the 30_000 timeout or reduce it to a
very small value (or move any longer timeout to the specific it() tests that run
slow CLI commands) so only slow tests get extended timeouts and the setup
remains fast and unbounded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6d5ddf47-b07a-4da7-b0cf-1552ac5747a0
⛔ Files ignored due to path filters (2)
packages/cli/template/vite-wv/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (60)
.agents/skills/create-changeset/SKILL.mdapps/docs/content/docs/cli/webviewer/getting-started.mdxapps/docs/content/docs/cli/webviewer/overview.mdxapps/docs/content/docs/typegen/config.mdxpackages/cli/src/cli/add/auth.tspackages/cli/src/cli/add/data-source/filemaker.tspackages/cli/src/cli/add/data-source/index.tspackages/cli/src/cli/add/index.tspackages/cli/src/cli/add/page/index.tspackages/cli/src/cli/init.tspackages/cli/src/cli/remove/data-source.tspackages/cli/src/cli/typegen/index.tspackages/cli/src/generators/fmdapi.tspackages/cli/src/globalOptions.tspackages/cli/src/helpers/createProject.tspackages/cli/src/helpers/fmHttp.tspackages/cli/src/helpers/git.tspackages/cli/src/helpers/logNextSteps.tspackages/cli/src/helpers/scaffoldProject.tspackages/cli/src/index.tspackages/cli/src/installers/dependencyVersionMap.tspackages/cli/src/installers/react-email.tspackages/cli/src/state.tspackages/cli/src/utils/getProofKitVersion.tspackages/cli/template/vite-wv/_gitignorepackages/cli/template/vite-wv/components.jsonpackages/cli/template/vite-wv/index.htmlpackages/cli/template/vite-wv/package.jsonpackages/cli/template/vite-wv/postcss.config.cjspackages/cli/template/vite-wv/proofkit-typegen.config.jsoncpackages/cli/template/vite-wv/proofkit.jsonpackages/cli/template/vite-wv/scripts/filemaker.jspackages/cli/template/vite-wv/scripts/launch-fm.jspackages/cli/template/vite-wv/scripts/launch-fm.shpackages/cli/template/vite-wv/scripts/upload.jspackages/cli/template/vite-wv/src/App.tsxpackages/cli/template/vite-wv/src/components/AppLogo.tsxpackages/cli/template/vite-wv/src/components/full-screen-loader.tsxpackages/cli/template/vite-wv/src/config/env.tspackages/cli/template/vite-wv/src/config/theme/globals.csspackages/cli/template/vite-wv/src/config/theme/mantine-theme.tspackages/cli/template/vite-wv/src/index.csspackages/cli/template/vite-wv/src/lib/utils.tspackages/cli/template/vite-wv/src/main.tsxpackages/cli/template/vite-wv/src/routeTree.gen.tspackages/cli/template/vite-wv/src/routes/__root.tsxpackages/cli/template/vite-wv/src/routes/index.tsxpackages/cli/template/vite-wv/src/routes/secondary.tsxpackages/cli/template/vite-wv/src/utils/notification-helpers.tspackages/cli/template/vite-wv/src/utils/styles.tspackages/cli/template/vite-wv/tsconfig.jsonpackages/cli/template/vite-wv/vite.config.tspackages/cli/tests/browser-apps.test.tspackages/cli/tests/webviewer-apps.test.tspackages/fmdapi/skills/fmdapi-client/SKILL.mdpackages/fmodata/package.jsonpackages/fmodata/skills/fmodata-client/SKILL.mdpackages/typegen/src/types.tspackages/typegen/typegen.schema.jsonpackages/webviewer/skills/webviewer-integration/SKILL.md
💤 Files with no reviewable changes (13)
- packages/cli/template/vite-wv/src/components/AppLogo.tsx
- packages/cli/template/vite-wv/src/routes/__root.tsx
- packages/cli/template/vite-wv/src/utils/styles.ts
- packages/cli/template/vite-wv/src/routes/index.tsx
- packages/cli/template/vite-wv/postcss.config.cjs
- packages/cli/template/vite-wv/src/routes/secondary.tsx
- packages/cli/template/vite-wv/src/config/theme/globals.css
- packages/cli/template/vite-wv/scripts/launch-fm.sh
- packages/cli/template/vite-wv/src/utils/notification-helpers.ts
- packages/cli/template/vite-wv/src/config/theme/mantine-theme.ts
- packages/cli/template/vite-wv/src/components/full-screen-loader.tsx
- packages/cli/template/vite-wv/src/config/env.ts
- packages/cli/template/vite-wv/src/routeTree.gen.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/cli/src/utils/getProofKitVersion.ts (1)
32-35: Consider removing the biome-ignore comment for consistency.The
biome-ignorecomment on line 33 is inconsistent withgetFmdapiVersionandgetProofkitBetterAuthVersion, which reference similar build-time injected globals without suppression comments. Since__WEBVIEWER_VERSION__is declared inglobals.d.tsalongside the other version constants, Biome should recognize it.If the suppression is necessary here, consider adding it to the other two functions for consistency; otherwise, remove it.
♻️ Suggested fix (if suppression is unnecessary)
export const getProofkitWebviewerVersion = () => { - // biome-ignore lint/correctness/noUndeclaredVariables: replaced at build time via tsdown return __WEBVIEWER_VERSION__; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/utils/getProofKitVersion.ts` around lines 32 - 35, The biome-ignore comment in getProofkitWebviewerVersion is inconsistent with getFmdapiVersion and getProofkitBetterAuthVersion; remove the "// biome-ignore lint/correctness/noUndeclaredVariables: replaced at build time via tsdown" line from the getProofkitWebviewerVersion function so it matches the other version helpers, relying on the declaration of __WEBVIEWER_VERSION__ in globals.d.ts; alternatively, if suppression is truly required, add the same biome-ignore comment to getFmdapiVersion and getProofkitBetterAuthVersion for consistency.
🤖 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/utils/getProofKitVersion.ts`:
- Around line 32-35: The biome-ignore comment in getProofkitWebviewerVersion is
inconsistent with getFmdapiVersion and getProofkitBetterAuthVersion; remove the
"// biome-ignore lint/correctness/noUndeclaredVariables: replaced at build time
via tsdown" line from the getProofkitWebviewerVersion function so it matches the
other version helpers, relying on the declaration of __WEBVIEWER_VERSION__ in
globals.d.ts; alternatively, if suppression is truly required, add the same
biome-ignore comment to getFmdapiVersion and getProofkitBetterAuthVersion for
consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5f0180c1-f464-4c3c-9383-f97dc3edacf1
📒 Files selected for processing (4)
.changeset/cli-webviewer-scaffold-fmhttp.mdpackages/cli/src/globals.d.tspackages/cli/src/utils/getProofKitVersion.tspackages/cli/tsdown.config.ts
eluce2
changed the title
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
.agents/skills/create-changeset/SKILL.md (1)
28-30: Scope existing-changeset detection to the current branch diff.Current instruction checks for any
.changeset/*.md, which can falsely imply an existing changeset for the current work. Use branch-scoped detection (e.g., files changed inmain...HEAD) before prompting.Suggested doc tweak
-Before creating a changeset, check if a changeset file already exists for the current changes: -- Look for `.changeset/*.md` files (excluding README.md) -- If exists, ask user: "A changeset already exists. Create another one?" +Before creating a changeset, check if a changeset file already exists for the current branch changes: +- Check `.changeset/*.md` files changed in `main...HEAD` (excluding `.changeset/README.md`) +- If one exists, ask user: "A changeset already exists for this branch. Create another one?"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/create-changeset/SKILL.md around lines 28 - 30, The current check that looks for any `.changeset/*.md` should be replaced with a branch-scoped check that only considers files changed in the current branch diff (e.g., main...HEAD); update the logic in the create-changeset SKILL.md step that currently scans for `.changeset/*.md` (excluding README.md) so it runs a git diff like `git diff --name-only main...HEAD -- .changeset/*.md` (or equivalent git API) and filters out README.md, and only when that result is non-empty prompt the user with "A changeset already exists. Create another one?"; ensure the check lives where the instruction describes existing-changeset detection so the prompt is only shown for changesets in this branch's changes.packages/cli/src/helpers/logNextSteps.ts (1)
5-5: Decouple next-step rendering from globalstateLine 5 + Line 37 make
logNextStepsdepend on shared mutable state. Prefer passingappTypeintologNextStepsso behavior is explicit and easier to test.♻️ Proposed refactor
-import { state } from "~/state.js"; - export const logNextSteps = ({ projectName = DEFAULT_APP_NAME, noInstall, -}: Pick<InstallerOptions, "projectName" | "noInstall">) => { + appType, +}: Pick<InstallerOptions, "projectName" | "noInstall" | "appType">) => { const pkgManager = getUserPkgManager(); @@ - if (state.appType === "webviewer") { + if (appType === "webviewer") { logger.dim("\nWhen you're ready to generate FileMaker clients:"); logger.info(` ${formatRunCommand(pkgManager, "typegen")}`);Also applies to: 37-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/helpers/logNextSteps.ts` at line 5, The function logNextSteps currently imports and reads shared mutable state via the symbol state; change logNextSteps to accept an explicit appType parameter instead (e.g., change signature of logNextSteps to logNextSteps(appType: string | AppType)) and remove direct reads of state inside the function, using the passed appType to drive conditional rendering; update every caller of logNextSteps to pass the appropriate appType (where state.appType was used) and adjust any tests to inject appType directly so the function is decoupled from global state and easier to unit test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/create-changeset/SKILL.md:
- Around line 100-105: Update the multi-package example so the package names are
consistent: either change the frontmatter entries "@proofkit/cli" and
"@proofkit/typegen" to the spindle-* names used in the bullets, or update the
bullet list items "spindle-ui" and "spindle-tokens" to use the matching
"@proofkit/..." scope; ensure the same package identifiers appear in both the
frontmatter and the bullet descriptions to prevent copy/paste errors.
In `@packages/cli/src/cli/add/data-source/filemaker.ts`:
- Around line 40-75: The branch that handles local FM HTTP detection exits after
calling addPackageDependency, ensureWebviewerFmHttpConfig, and optionally
addLayout, but never persists the new data source into the shared settings (the
later settings.dataSources.push(...) / setSettings(settings) path is skipped);
fix by adding the same data-source persistence logic before the early return:
construct the FileMaker data source object (matching the shape used elsewhere),
push it into settings.dataSources and call setSettings(settings) (or invoke the
shared helper that does this) so that ensureWebviewerFmHttpConfig/addLayout
changes are reflected in project config; reference functions/variables:
ensureWebviewerFmHttpConfig, addLayout, addPackageDependency,
localDataSourceName, settings.dataSources.push, setSettings(settings).
- Around line 54-75: Validate the pair of flags before the branch: if exactly
one of opts.layoutName or opts.schemaName is provided, fail fast with an error
(do not fall through to p.note). Add a check above the existing if (...) block
that detects the half-specified case and either throw a clear Error or call the
CLI error/exit helper (e.g., p.error or similar) with a message instructing the
user to supply both --layoutName and --schemaName; keep the existing successful
path that calls addLayout({ ..., schemas: [...] }) when both are present and the
p.note path only for neither flag.
In `@packages/cli/src/cli/init.ts`:
- Around line 235-238: The ternary defaults make WebViewer always use "none" and
skip promptForFileMakerDataSource; change dataSource computation so WebViewer
does not force "none" when cliOptions.dataSource is undefined—either remove the
special-case for state.appType === "webviewer" and use the same fallback
(cliOptions.dataSource ?? (nonInteractive ? "none" : undefined)) or, if you want
explicit handling, keep the branch but only default to "none" when
nonInteractive is true so promptForFileMakerDataSource() can run when dataSource
is undefined; update references to dataSource, state.appType,
cliOptions.dataSource, nonInteractive and ensure promptForFileMakerDataSource()
remains reachable for WebViewer init.
In `@packages/cli/src/generators/fmdapi.ts`:
- Around line 255-263: The current merge at configArray[existingConfigIndex]
spreads ...newConfig after ...existingConfig which lets newConfig’s undefined
values wipe out existing datasource fields (e.g.,
existingConfig.fmHttp.envNames); change the merge logic to perform a
non-destructive deep-merge for fmHttp and datasource fields so existingConfig
values are preserved unless newConfig provides an explicit value — specifically,
build fmHttp by starting with existingConfig.fmHttp (or {}), then overlay only
defined keys from newConfig.fmHttp, and ensure envNames and other custom options
from existingConfig are carried through when newConfig omits them; do this where
configArray[existingConfigIndex] is assigned (using existingConfig, newConfig,
layouts, and fmHttp identifiers).
In `@packages/cli/src/helpers/scaffoldProject.ts`:
- Around line 28-30: The current check only ignores dot-prefixed regular files
(stats.isFile() && entry.startsWith(".")) so dot-prefixed directories (e.g.,
.git) still count as non-empty; change the predicate to ignore any dot-prefixed
entry regardless of type by testing entry.startsWith(".") (or moving that check
before the stats.isFile() gate) so both files and directories beginning with "."
are treated as hidden and skipped in the logic that determines whether a
directory is empty.
In `@packages/cli/src/installers/dependencyVersionMap.ts`:
- Around line 36-43: The dependency map pins `@tailwindcss/vite` to a non-existent
version (^4.2.1); update the value for "@tailwindcss/vite" to "^4.1.18" to match
the published package and avoid install failures, and while here align related
packages by updating "tailwindcss" to "^4.2.1" and "@tailwindcss/postcss" to
"^4.1.18" in the dependencyVersionMap object so the Tailwind ecosystem versions
are consistent.
---
Nitpick comments:
In @.agents/skills/create-changeset/SKILL.md:
- Around line 28-30: The current check that looks for any `.changeset/*.md`
should be replaced with a branch-scoped check that only considers files changed
in the current branch diff (e.g., main...HEAD); update the logic in the
create-changeset SKILL.md step that currently scans for `.changeset/*.md`
(excluding README.md) so it runs a git diff like `git diff --name-only
main...HEAD -- .changeset/*.md` (or equivalent git API) and filters out
README.md, and only when that result is non-empty prompt the user with "A
changeset already exists. Create another one?"; ensure the check lives where the
instruction describes existing-changeset detection so the prompt is only shown
for changesets in this branch's changes.
In `@packages/cli/src/helpers/logNextSteps.ts`:
- Line 5: The function logNextSteps currently imports and reads shared mutable
state via the symbol state; change logNextSteps to accept an explicit appType
parameter instead (e.g., change signature of logNextSteps to
logNextSteps(appType: string | AppType)) and remove direct reads of state inside
the function, using the passed appType to drive conditional rendering; update
every caller of logNextSteps to pass the appropriate appType (where
state.appType was used) and adjust any tests to inject appType directly so the
function is decoupled from global state and easier to unit test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 37b21e21-ebb1-49ea-beba-6d06e252eca2
📒 Files selected for processing (21)
.agents/skills/create-changeset/SKILL.mdbiome.jsonpackages/cli/src/cli/add/data-source/filemaker.tspackages/cli/src/cli/init.tspackages/cli/src/generators/fmdapi.tspackages/cli/src/helpers/fmHttp.tspackages/cli/src/helpers/logNextSteps.tspackages/cli/src/helpers/scaffoldProject.tspackages/cli/src/index.tspackages/cli/src/installers/dependencyVersionMap.tspackages/cli/src/utils/getProofKitVersion.tspackages/cli/template/vite-wv/_gitignorepackages/cli/template/vite-wv/package.jsonpackages/cli/template/vite-wv/scripts/filemaker.jspackages/cli/template/vite-wv/src/App.tsxpackages/cli/template/vite-wv/src/index.csspackages/cli/template/vite-wv/src/main.tsxpackages/cli/tests/browser-apps.test.tspackages/cli/tests/webviewer-apps.test.tspackages/typegen/typegen.schema.jsonpackages/webviewer/skills/webviewer-integration/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/cli/template/vite-wv/src/App.tsx
- packages/webviewer/skills/webviewer-integration/SKILL.md
- packages/typegen/typegen.schema.json
- packages/cli/template/vite-wv/scripts/filemaker.js
- packages/cli/src/utils/getProofKitVersion.ts
- packages/cli/tests/webviewer-apps.test.ts
- packages/cli/src/helpers/fmHttp.ts
Comment on lines
36
to
+43
| tailwindcss: "^4.1.10", | ||
| postcss: "^8.4.41", | ||
| "@tailwindcss/postcss": "^4.1.10", | ||
| "@tailwindcss/vite": "^4.2.1", | ||
| "class-variance-authority": "^0.7.1", | ||
| clsx: "^2.1.1", | ||
| "tailwind-merge": "^3.3.1", | ||
| "tw-animate-css": "^1.3.4", | ||
| "tailwind-merge": "^3.5.0", | ||
| "tw-animate-css": "^1.4.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the latest version of @tailwindcss/vite package?
💡 Result:
The latest published version of @tailwindcss/vite is 4.1.18 (as of March 17, 2026). [1][2]
🌐 Web query:
What is the latest version of tailwindcss package and @tailwindcss/postcss package
💡 Result:
- tailwindcss: v4.2.1 ([1])
- @tailwindcss/postcss: v4.1.18 ([2])
Sources: [1] [2]
Critical: @tailwindcss/vite is pinned to a non-existent version.
@tailwindcss/vite is pinned to ^4.2.1, but the latest and only available version is 4.1.18. Version 4.2.1 does not exist. This will cause package installation to fail or resolve unexpectedly. Update to ^4.1.18 to match the actual latest version. Additionally, consider updating tailwindcss to ^4.2.1 and @tailwindcss/postcss to ^4.1.18 for consistency across the Tailwind ecosystem.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/installers/dependencyVersionMap.ts` around lines 36 - 43,
The dependency map pins `@tailwindcss/vite` to a non-existent version (^4.2.1);
update the value for "@tailwindcss/vite" to "^4.1.18" to match the published
package and avoid install failures, and while here align related packages by
updating "tailwindcss" to "^4.2.1" and "@tailwindcss/postcss" to "^4.1.18" in
the dependencyVersionMap object so the Tailwind ecosystem versions are
consistent.
eluce2
force-pushed
the
codex/webviewer-first-cli-revamp
branch
from
b5ad8bf to
5544f68
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
.agents/skills/create-changeset/SKILL.md (1)
100-105:⚠️ Potential issue | 🟡 MinorKeep package identifiers consistent between frontmatter and bullets.
The frontmatter uses
@proofkit/*while the bullet descriptions usespindle-*. This is still a copy/paste risk in the example body.Suggested doc tweak
-- spindle-ui: Add new Button variant for secondary actions -- spindle-tokens: Fix color token contrast ratio +- `@proofkit/cli`: Add new Button variant for secondary actions +- `@proofkit/typegen`: Fix color token contrast ratio🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/create-changeset/SKILL.md around lines 100 - 105, The frontmatter package identifiers ("@proofkit/cli", "@proofkit/typegen") are inconsistent with the bullet list package names ("spindle-ui", "spindle-tokens"); update either the frontmatter or the bullets so both use the same package namespace (e.g., change the bullets to "@proofkit/ui" and "@proofkit/tokens" or change the frontmatter to "spindle-ui" and "spindle-tokens") and ensure the version bumps (minor/patch) align with the corrected package identifiers in the SKILL.md content.packages/cli/src/generators/fmdapi.ts (1)
215-227:⚠️ Potential issue | 🟠 Major
envNames: undefinedwill overwrite existing config values.In
ensureWebviewerFmHttpConfig,newConfigexplicitly setsenvNames: undefined(line 221). When this is spread in the merge at lines 255-257, it will overwrite any existingenvNamesthat the user may have configured. This is a destructive merge issue.Suggested fix - omit undefined values from newConfig
const newConfig: FmdapiDataSourceConfig = { type: "fmdapi", path: `./src/config/schemas/${dataSourceName}`, clearOldFiles: true, clientSuffix: "Layout", webviewerScriptName: "ExecuteDataApi", - envNames: undefined, layouts: [], fmHttp: { enabled: true, ...(baseUrl ? { baseUrl } : {}), ...(connectedFileName ? { connectedFileName } : {}), }, };Or use a more explicit merge that preserves existing values:
configArray[existingConfigIndex] = { ...existingConfig, - ...newConfig, + type: newConfig.type, + path: newConfig.path, + clearOldFiles: newConfig.clearOldFiles, + clientSuffix: newConfig.clientSuffix, + webviewerScriptName: newConfig.webviewerScriptName, layouts: existingConfig.layouts ?? [], fmHttp: { enabled: true, ...(existingConfig.fmHttp ?? {}), ...(newConfig.fmHttp ?? {}), }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/generators/fmdapi.ts` around lines 215 - 227, The newConfig object in ensureWebviewerFmHttpConfig sets envNames: undefined which will overwrite any existing envNames during the merge; remove the explicit envNames property when it's undefined (i.e. only include envNames in newConfig when a real value exists) or change the merge logic that combines newConfig with existing config to ignore undefined values so existing config.envNames is preserved; update ensureWebviewerFmHttpConfig/newConfig creation or the merge routine to conditionally add envNames or filter out undefined fields before merging.packages/cli/src/installers/dependencyVersionMap.ts (1)
39-39:⚠️ Potential issue | 🔴 CriticalPin
@tailwindcss/viteto a published release.npm currently lists
@tailwindcss/viteat4.1.13, so keeping this entry at^4.2.1makes scaffolded installs request a version that is not published yet. (npmjs.com)This queries the npm registry for the package dist-tag and checks whether
4.2.1exists. Expected result:lateststays on4.1.x, andhas_4_2_1isFalse. (npmjs.com)#!/bin/bash set -euo pipefail python - <<'PY' import json, urllib.parse, urllib.request pkg = "@tailwindcss/vite" url = f"https://registry.npmjs.org/{urllib.parse.quote(pkg, safe='')}" with urllib.request.urlopen(url) as resp: data = json.load(resp) print(f"latest={data['dist-tags']['latest']}") print(f"has_4_2_1={'4.2.1' in data['versions']}") PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/installers/dependencyVersionMap.ts` at line 39, The dependencyVersionMap entry for "@tailwindcss/vite" references an unpublished 4.2.1 release; update the version in dependencyVersionMap.ts to a published tag (e.g., change the "@tailwindcss/vite": "^4.2.1" entry to the current published version such as "^4.1.13" or the registry's latest) so scaffolded installs request an available release; locate the string key "@tailwindcss/vite" in the dependencyVersionMap object and update its version value accordingly.packages/cli/src/cli/add/data-source/filemaker.ts (1)
54-75:⚠️ Potential issue | 🟠 MajorReject half-specified layout scaffolding flags.
Passing only one of
--layoutNameor--schemaNamestill falls through to the note path and exits successfully, so scripted runs silently drop the requested layout generation.Suggested fix
+ const hasLayoutName = Boolean(opts.layoutName); + const hasSchemaName = Boolean(opts.schemaName); + if (hasLayoutName !== hasSchemaName) { + throw new Error("Both --layoutName and --schemaName are required when scaffolding a WebViewer layout."); + } + - if (opts.layoutName && opts.schemaName) { + if (hasLayoutName && hasSchemaName) { await addLayout({ projectDir, dataSourceName: localDataSourceName,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/cli/add/data-source/filemaker.ts` around lines 54 - 75, The code currently ignores cases where only one of opts.layoutName or opts.schemaName is provided and quietly falls back to the note path; change this to explicitly reject half-specified layout scaffolding flags by detecting the case where XOR(opts.layoutName, opts.schemaName) is true and failing early (e.g., call p.fail or throw a descriptive Error) explaining that both --layoutName and --schemaName must be provided together; keep the existing addLayout(...) call for the both-provided case and the p.note path for neither-provided case.
🧹 Nitpick comments (4)
.agents/skills/create-changeset/SKILL.md (1)
28-31: Scope existing-changeset detection to the current branch diff.Checking for any
.changeset/*.mdfile is too broad and can prompt unnecessarily due to unrelated files already in the repo. Limit this check to files introduced/changed onmain...HEADso the prompt only appears for branch-relevant changesets.Suggested doc tweak
-Before creating a changeset, check if a changeset file already exists for the current changes: -- Look for `.changeset/*.md` files (excluding README.md) +Before creating a changeset, check if a changeset file already exists for the current branch changes: +- Check `git diff --name-only main...HEAD` for `.changeset/*.md` files (excluding `.changeset/README.md`) - If exists, ask user: "A changeset already exists. Create another one?"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/create-changeset/SKILL.md around lines 28 - 31, The existing check that looks for any `.changeset/*.md` should be restricted to only files in the branch diff; update the logic (e.g., in the changeset creation flow or function responsible for detection such as createChangeset/detectExistingChangeset) to run a git diff name-only against main...HEAD (git diff --name-only main...HEAD), filter the results for paths matching `.changeset/*.md` excluding `README.md`, and only prompt "A changeset already exists. Create another one?" when that filtered list is non-empty; ensure the detection uses that diff output instead of scanning the entire repo.packages/cli/src/helpers/logNextSteps.ts (1)
5-5: Prefer passingappTypeintologNextStepsinstead of reading global state.Line 5 and Line 37 introduce a hidden dependency on mutable global process state, which makes this helper harder to test deterministically and reuse.
♻️ Suggested refactor
-import { state } from "~/state.js"; @@ export const logNextSteps = ({ projectName = DEFAULT_APP_NAME, noInstall, -}: Pick<InstallerOptions, "projectName" | "noInstall">) => { + appType, +}: Pick<InstallerOptions, "projectName" | "noInstall" | "appType">) => { @@ - if (state.appType === "webviewer") { + if (appType === "webviewer") {Also applies to: 37-37
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/helpers/logNextSteps.ts` at line 5, The helper logNextSteps currently reads mutable global state via the imported state (state.appType) which hides a dependency; change logNextSteps to accept an explicit appType parameter (e.g., function logNextSteps(appType: string, ...) or add appType to its params), stop using state.appType inside the function, and remove the direct dependency on the imported state in this module; then update all call sites that previously relied on global state to pass the appropriate appType value into logNextSteps (and remove the now-unused state import from this file).packages/cli/template/vite-wv/package.json (1)
24-35: Consider updating Vite to version 8.x.The devDependencies are generally well-chosen. A few observations:
Biome exact pin:
@biomejs/biomeis pinned exactly at"2.4.7"while other dependencies use caret ranges. This is a standard practice for formatters/linters to ensure consistent formatting across teams.Vite version: This template uses
^7.3.1, while Vite 8.x (released with full Rolldown integration) is now available. Sincevite-plugin-singlefilehas no peer dependency constraints, migrating to Vite 8 should not be blocked by plugin compatibility. Consider upgrading if the benefits (improved build performance, Rolldown) align with the template's goals.Beta dependency:
@proofkit/typegenuses a beta version (^1.1.0-beta.16), which is expected for internal tooling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/template/vite-wv/package.json` around lines 24 - 35, The package.json devDependencies pin Vite to "^7.3.1" and exactly pins "@biomejs/biome" to "2.4.7"; update the Vite entry to a compatible 8.x range (e.g., "^8.0.0") in the "devDependencies" and change the biome pin to a caret range (e.g., "@biomejs/biome": "^2.4.7") for consistency with other deps; after updating, run the template build/dev scripts and the single-file plugin checks to verify compatibility (reference the "vite" and "vite-plugin-singlefile" entries in package.json and the "@biomejs/biome" entry) and fix any resulting peer/dependency issues.packages/cli/tests/webviewer-apps.test.ts (1)
44-44: Hardcoded version will cause test brittleness.The assertion
expect(packageJson.devDependencies["@proofkit/typegen"]).toBe("^1.1.0-beta.16")will break whenever the typegen version is updated. Consider using a pattern match or simply verifying the dependency exists.Suggested alternatives
- expect(packageJson.devDependencies["@proofkit/typegen"]).toBe("^1.1.0-beta.16"); + // Verify dependency exists with a semver-compatible version + expect(packageJson.devDependencies["@proofkit/typegen"]).toMatch(/^\^?\d+\.\d+\.\d+/);Or simply check for existence:
- expect(packageJson.devDependencies["@proofkit/typegen"]).toBe("^1.1.0-beta.16"); + expect(packageJson.devDependencies["@proofkit/typegen"]).toBeDefined();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/tests/webviewer-apps.test.ts` at line 44, The test currently asserts a hardcoded version string via expect(packageJson.devDependencies["@proofkit/typegen"]).toBe("^1.1.0-beta.16"), which will break on version bumps; change the assertion in packages/cli/tests/webviewer-apps.test.ts to either verify the dependency exists (e.g., check packageJson.devDependencies hasOwnProperty or use expect(packageJson.devDependencies).toHaveProperty("@proofkit/typegen")) or use a looser pattern match (e.g., expect(packageJson.devDependencies["@proofkit/typegen"]).toMatch(/^\\^?\\d+\\.\\d+\\.\\d+/)) so the test no longer depends on a specific version string.
🤖 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/cli/add/data-source/filemaker.ts`:
- Around line 35-52: The branch that sets localDataSourceName currently uses
opts.name ?? "filemaker" and bypasses the same reserved-name, uniqueness and
validateAppName checks used later; instead derive localDataSourceName by running
the same validation/normalization used elsewhere (i.e. call validateAppName on
opts.name || defaultName and enforce the reserved-name and uniqueness checks
against existing data sources), and only pass the validated/unique name into
ensureWebviewerFmHttpConfig and other calls (update the localDataSourceName
assignment and any downstream uses to reference the validated result).
In `@packages/cli/src/utils/getProofKitVersion.ts`:
- Around line 36-45: getTypegenVersion is building the wrong path (PKG_ROOT
currently points to packages/cli), causing the try block to always fail and
return the fallback; update the path construction in getTypegenVersion to
resolve to the repository packages/typegen/package.json (for example by joining
PKG_ROOT with ".." and "typegen" or using path.resolve to reach
../typegen/package.json) so the fs.readJSONSync reads the real package.json and
returns packageJsonContent.version instead of always falling back to
"1.1.0-beta.16"; keep references to getTypegenVersion and PKG_ROOT when making
the change.
---
Duplicate comments:
In @.agents/skills/create-changeset/SKILL.md:
- Around line 100-105: The frontmatter package identifiers ("@proofkit/cli",
"@proofkit/typegen") are inconsistent with the bullet list package names
("spindle-ui", "spindle-tokens"); update either the frontmatter or the bullets
so both use the same package namespace (e.g., change the bullets to
"@proofkit/ui" and "@proofkit/tokens" or change the frontmatter to "spindle-ui"
and "spindle-tokens") and ensure the version bumps (minor/patch) align with the
corrected package identifiers in the SKILL.md content.
In `@packages/cli/src/cli/add/data-source/filemaker.ts`:
- Around line 54-75: The code currently ignores cases where only one of
opts.layoutName or opts.schemaName is provided and quietly falls back to the
note path; change this to explicitly reject half-specified layout scaffolding
flags by detecting the case where XOR(opts.layoutName, opts.schemaName) is true
and failing early (e.g., call p.fail or throw a descriptive Error) explaining
that both --layoutName and --schemaName must be provided together; keep the
existing addLayout(...) call for the both-provided case and the p.note path for
neither-provided case.
In `@packages/cli/src/generators/fmdapi.ts`:
- Around line 215-227: The newConfig object in ensureWebviewerFmHttpConfig sets
envNames: undefined which will overwrite any existing envNames during the merge;
remove the explicit envNames property when it's undefined (i.e. only include
envNames in newConfig when a real value exists) or change the merge logic that
combines newConfig with existing config to ignore undefined values so existing
config.envNames is preserved; update ensureWebviewerFmHttpConfig/newConfig
creation or the merge routine to conditionally add envNames or filter out
undefined fields before merging.
In `@packages/cli/src/installers/dependencyVersionMap.ts`:
- Line 39: The dependencyVersionMap entry for "@tailwindcss/vite" references an
unpublished 4.2.1 release; update the version in dependencyVersionMap.ts to a
published tag (e.g., change the "@tailwindcss/vite": "^4.2.1" entry to the
current published version such as "^4.1.13" or the registry's latest) so
scaffolded installs request an available release; locate the string key
"@tailwindcss/vite" in the dependencyVersionMap object and update its version
value accordingly.
---
Nitpick comments:
In @.agents/skills/create-changeset/SKILL.md:
- Around line 28-31: The existing check that looks for any `.changeset/*.md`
should be restricted to only files in the branch diff; update the logic (e.g.,
in the changeset creation flow or function responsible for detection such as
createChangeset/detectExistingChangeset) to run a git diff name-only against
main...HEAD (git diff --name-only main...HEAD), filter the results for paths
matching `.changeset/*.md` excluding `README.md`, and only prompt "A changeset
already exists. Create another one?" when that filtered list is non-empty;
ensure the detection uses that diff output instead of scanning the entire repo.
In `@packages/cli/src/helpers/logNextSteps.ts`:
- Line 5: The helper logNextSteps currently reads mutable global state via the
imported state (state.appType) which hides a dependency; change logNextSteps to
accept an explicit appType parameter (e.g., function logNextSteps(appType:
string, ...) or add appType to its params), stop using state.appType inside the
function, and remove the direct dependency on the imported state in this module;
then update all call sites that previously relied on global state to pass the
appropriate appType value into logNextSteps (and remove the now-unused state
import from this file).
In `@packages/cli/template/vite-wv/package.json`:
- Around line 24-35: The package.json devDependencies pin Vite to "^7.3.1" and
exactly pins "@biomejs/biome" to "2.4.7"; update the Vite entry to a compatible
8.x range (e.g., "^8.0.0") in the "devDependencies" and change the biome pin to
a caret range (e.g., "@biomejs/biome": "^2.4.7") for consistency with other
deps; after updating, run the template build/dev scripts and the single-file
plugin checks to verify compatibility (reference the "vite" and
"vite-plugin-singlefile" entries in package.json and the "@biomejs/biome" entry)
and fix any resulting peer/dependency issues.
In `@packages/cli/tests/webviewer-apps.test.ts`:
- Line 44: The test currently asserts a hardcoded version string via
expect(packageJson.devDependencies["@proofkit/typegen"]).toBe("^1.1.0-beta.16"),
which will break on version bumps; change the assertion in
packages/cli/tests/webviewer-apps.test.ts to either verify the dependency exists
(e.g., check packageJson.devDependencies hasOwnProperty or use
expect(packageJson.devDependencies).toHaveProperty("@proofkit/typegen")) or use
a looser pattern match (e.g.,
expect(packageJson.devDependencies["@proofkit/typegen"]).toMatch(/^\\^?\\d+\\.\\d+\\.\\d+/))
so the test no longer depends on a specific version string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a8f646e9-5f6f-4fb5-97ec-1d1e99d6159d
⛔ Files ignored due to path filters (2)
packages/cli/template/vite-wv/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (62)
.agents/skills/create-changeset/SKILL.md.changeset/cli-webviewer-scaffold-fmhttp.mdapps/docs/content/docs/cli/webviewer/getting-started.mdxapps/docs/content/docs/cli/webviewer/overview.mdxapps/docs/content/docs/typegen/config.mdxbiome.jsonpackages/cli/src/cli/add/auth.tspackages/cli/src/cli/add/data-source/filemaker.tspackages/cli/src/cli/add/data-source/index.tspackages/cli/src/cli/add/index.tspackages/cli/src/cli/add/page/index.tspackages/cli/src/cli/init.tspackages/cli/src/cli/remove/data-source.tspackages/cli/src/cli/typegen/index.tspackages/cli/src/generators/fmdapi.tspackages/cli/src/globalOptions.tspackages/cli/src/globals.d.tspackages/cli/src/helpers/createProject.tspackages/cli/src/helpers/fmHttp.tspackages/cli/src/helpers/git.tspackages/cli/src/helpers/logNextSteps.tspackages/cli/src/helpers/scaffoldProject.tspackages/cli/src/index.tspackages/cli/src/installers/dependencyVersionMap.tspackages/cli/src/installers/react-email.tspackages/cli/src/state.tspackages/cli/src/utils/getProofKitVersion.tspackages/cli/template/vite-wv/_gitignorepackages/cli/template/vite-wv/components.jsonpackages/cli/template/vite-wv/index.htmlpackages/cli/template/vite-wv/package.jsonpackages/cli/template/vite-wv/postcss.config.cjspackages/cli/template/vite-wv/proofkit-typegen.config.jsoncpackages/cli/template/vite-wv/proofkit.jsonpackages/cli/template/vite-wv/scripts/filemaker.jspackages/cli/template/vite-wv/scripts/launch-fm.jspackages/cli/template/vite-wv/scripts/launch-fm.shpackages/cli/template/vite-wv/scripts/upload.jspackages/cli/template/vite-wv/src/App.tsxpackages/cli/template/vite-wv/src/components/AppLogo.tsxpackages/cli/template/vite-wv/src/components/full-screen-loader.tsxpackages/cli/template/vite-wv/src/config/env.tspackages/cli/template/vite-wv/src/config/theme/globals.csspackages/cli/template/vite-wv/src/config/theme/mantine-theme.tspackages/cli/template/vite-wv/src/index.csspackages/cli/template/vite-wv/src/lib/utils.tspackages/cli/template/vite-wv/src/main.tsxpackages/cli/template/vite-wv/src/routeTree.gen.tspackages/cli/template/vite-wv/src/routes/__root.tsxpackages/cli/template/vite-wv/src/routes/index.tsxpackages/cli/template/vite-wv/src/routes/secondary.tsxpackages/cli/template/vite-wv/src/utils/notification-helpers.tspackages/cli/template/vite-wv/src/utils/styles.tspackages/cli/template/vite-wv/tsconfig.jsonpackages/cli/template/vite-wv/vite.config.tspackages/cli/tests/browser-apps.test.tspackages/cli/tests/webviewer-apps.test.tspackages/cli/tsdown.config.tspackages/fmdapi/skills/fmdapi-client/SKILL.mdpackages/fmodata/package.jsonpackages/fmodata/skills/fmodata-client/SKILL.mdpackages/webviewer/skills/webviewer-integration/SKILL.md
💤 Files with no reviewable changes (13)
- packages/cli/template/vite-wv/src/utils/styles.ts
- packages/cli/template/vite-wv/src/config/theme/globals.css
- packages/cli/template/vite-wv/scripts/launch-fm.sh
- packages/cli/template/vite-wv/src/utils/notification-helpers.ts
- packages/cli/template/vite-wv/src/routes/secondary.tsx
- packages/cli/template/vite-wv/src/routeTree.gen.ts
- packages/cli/template/vite-wv/postcss.config.cjs
- packages/cli/template/vite-wv/src/routes/index.tsx
- packages/cli/template/vite-wv/src/config/env.ts
- packages/cli/template/vite-wv/src/routes/__root.tsx
- packages/cli/template/vite-wv/src/config/theme/mantine-theme.ts
- packages/cli/template/vite-wv/src/components/AppLogo.tsx
- packages/cli/template/vite-wv/src/components/full-screen-loader.tsx
🚧 Files skipped from review as they are similar to previous changes (23)
- packages/cli/src/cli/add/index.ts
- packages/cli/template/vite-wv/tsconfig.json
- packages/fmdapi/skills/fmdapi-client/SKILL.md
- packages/cli/src/globals.d.ts
- packages/cli/template/vite-wv/proofkit-typegen.config.jsonc
- packages/cli/src/helpers/createProject.ts
- .changeset/cli-webviewer-scaffold-fmhttp.md
- packages/cli/template/vite-wv/_gitignore
- packages/cli/template/vite-wv/scripts/filemaker.js
- packages/webviewer/skills/webviewer-integration/SKILL.md
- packages/cli/src/cli/add/page/index.ts
- packages/cli/src/index.ts
- packages/cli/template/vite-wv/src/App.tsx
- packages/cli/tsdown.config.ts
- packages/cli/template/vite-wv/scripts/launch-fm.js
- packages/cli/src/cli/add/auth.ts
- apps/docs/content/docs/cli/webviewer/getting-started.mdx
- packages/fmodata/package.json
- packages/cli/template/vite-wv/components.json
- packages/cli/src/cli/add/data-source/index.ts
- packages/cli/template/vite-wv/src/lib/utils.ts
- packages/cli/src/installers/react-email.ts
- packages/cli/template/vite-wv/index.html
Comment on lines
+35
to
+52
| if (settings.appType === "webviewer") { | ||
| const fmHttpStatus = await getFmHttpStatus(); | ||
| const connectedFileName = fmHttpStatus.connectedFiles[0]; | ||
| const localDataSourceName = opts.name ?? "filemaker"; | ||
|
|
||
| if (!opts.server && fmHttpStatus.healthy && connectedFileName) { | ||
| addPackageDependency({ | ||
| projectDir, | ||
| dependencies: ["@proofkit/fmdapi"], | ||
| devMode: false, | ||
| }); | ||
|
|
||
| await ensureWebviewerFmHttpConfig({ | ||
| projectDir, | ||
| connectedFileName, | ||
| dataSourceName: localDataSourceName, | ||
| baseUrl: fmHttpStatus.baseUrl, | ||
| }); |
There was a problem hiding this comment.
Reuse the normal datasource-name validation in this branch.
opts.name ?? "filemaker" bypasses the reserved-name, uniqueness, and validateAppName checks used later in this function. In the local FM HTTP path that lets --name write an invalid or duplicate datasource name, and the default "filemaker" can collide when the project already has another FM source.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/cli/add/data-source/filemaker.ts` around lines 35 - 52, The
branch that sets localDataSourceName currently uses opts.name ?? "filemaker" and
bypasses the same reserved-name, uniqueness and validateAppName checks used
later; instead derive localDataSourceName by running the same
validation/normalization used elsewhere (i.e. call validateAppName on opts.name
|| defaultName and enforce the reserved-name and uniqueness checks against
existing data sources), and only pass the validated/unique name into
ensureWebviewerFmHttpConfig and other calls (update the localDataSourceName
assignment and any downstream uses to reference the validated result).
Comment on lines
+36
to
+45
| export const getTypegenVersion = () => { | ||
| const packageJsonPath = path.join(PKG_ROOT, "packages", "typegen", "package.json"); | ||
|
|
||
| try { | ||
| const packageJsonContent = fs.readJSONSync(packageJsonPath) as PackageJson; | ||
| return packageJsonContent.version ?? "1.1.0-beta.16"; | ||
| } catch { | ||
| return "1.1.0-beta.16"; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the directory structure and PKG_ROOT resolution
echo "=== Checking directory structure ==="
fd -t f "package.json" packages --max-depth 2
echo ""
echo "=== Checking PKG_ROOT definition ==="
rg -n "PKG_ROOT" packages/cli/src/consts.ts
echo ""
echo "=== Verifying typegen package.json location ==="
ls -la packages/typegen/package.json 2>/dev/null || echo "packages/typegen/package.json not found at expected location"Repository: proofgeist/proofkit
Length of output: 635
🏁 Script executed:
#!/bin/bash
# Get the full consts.ts file to understand PKG_ROOT and distPath
echo "=== Full consts.ts file ==="
cat -n packages/cli/src/consts.ts
echo ""
echo "=== Check if there are any tests or usages of getTypegenVersion ==="
rg -n "getTypegenVersion" --type tsRepository: proofgeist/proofkit
Length of output: 2219
Incorrect path construction — version lookup will always fall back.
PKG_ROOT resolves to packages/cli/ at runtime (since the code executes from the compiled dist/ directory). The path path.join(PKG_ROOT, "packages", "typegen", "package.json") would resolve to packages/cli/packages/typegen/package.json, which doesn't exist. The actual package.json is at packages/typegen/package.json.
This means the try block will always fail and the hardcoded fallback "1.1.0-beta.16" will always be returned.
Proposed fix
export const getTypegenVersion = () => {
- const packageJsonPath = path.join(PKG_ROOT, "packages", "typegen", "package.json");
+ const packageJsonPath = path.join(PKG_ROOT, "..", "typegen", "package.json");
try {
const packageJsonContent = fs.readJSONSync(packageJsonPath) as PackageJson;
return packageJsonContent.version ?? "1.1.0-beta.16";
} catch {
return "1.1.0-beta.16";
}
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/utils/getProofKitVersion.ts` around lines 36 - 45,
getTypegenVersion is building the wrong path (PKG_ROOT currently points to
packages/cli), causing the try block to always fail and return the fallback;
update the path construction in getTypegenVersion to resolve to the repository
packages/typegen/package.json (for example by joining PKG_ROOT with ".." and
"typegen" or using path.resolve to reach ../typegen/package.json) so the
fs.readJSONSync reads the real package.json and returns
packageJsonContent.version instead of always falling back to "1.1.0-beta.16";
keep references to getTypegenVersion and PKG_ROOT when making the change.
…ations - Added __TYPEGEN_VERSION__ to the globals in biome.json and updated the CLI configuration to read the Typegen version. - Enhanced the FileMaker data source handling by persisting new data source settings. - Improved error handling in the FileMaker data source prompt. - Cleaned up the project scaffolding logic for better directory entry checks. - Refactored getTypegenVersion to directly return the global TYPEGEN_VERSION.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/cli/src/cli/add/data-source/filemaker.ts (2)
41-87:⚠️ Potential issue | 🟠 MajorFail on incomplete layout flags before mutating the project.
The new error at Line 87 is still too late: by then
package.json,proofkit-typegen.config.jsonc, andproofkit.jsonhave already been updated. A command with only one of--layoutNameor--schemaNameshould fail before any of those side effects, otherwise the repo is left partially scaffolded even though the CLI exited with an error.Suggested fix
const connectedFileName = fmHttpStatus.connectedFiles[0]; const localDataSourceName = opts.name ?? "filemaker"; + const hasLayoutName = Boolean(opts.layoutName); + const hasSchemaName = Boolean(opts.schemaName); + + if (hasLayoutName !== hasSchemaName) { + throw new Error("Both --layoutName and --schemaName must be provided together."); + } if (!opts.server && fmHttpStatus.healthy && connectedFileName) { addPackageDependency({ @@ - if (opts.layoutName && opts.schemaName) { + if (hasLayoutName && hasSchemaName) { await addLayout({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/cli/add/data-source/filemaker.ts` around lines 41 - 87, The CLI currently validates the presence of both opts.layoutName and opts.schemaName after making side-effect changes (addPackageDependency, ensureWebviewerFmHttpConfig, and settings.dataSources.push), which can leave the repo partially modified; move the validation for the pair (opts.layoutName && opts.schemaName) up to run before any mutations (i.e., before calling addPackageDependency, ensureWebviewerFmHttpConfig, or pushing to settings.dataSources), and if only one is provided throw the Error("Both --layoutName and --schemaName must be provided together.") early; keep references to localDataSourceName, settings.dataSources.push, addPackageDependency, and ensureWebviewerFmHttpConfig so you update the correct block.
38-72:⚠️ Potential issue | 🟡 MinorReuse the hosted datasource-name validation before writing config.
This branch still persists
opts.name ?? "filemaker"directly, so it skips the reserved-name, uniqueness, andvalidateAppNamechecks used by the hosted flow. The default"filemaker"can collide on reruns, and invalid names get written intoproofkit-typegen.config.jsoncandproofkit.json.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/cli/add/data-source/filemaker.ts` around lines 38 - 72, The code currently sets localDataSourceName = opts.name ?? "filemaker" and persists it directly, skipping the hosted-flow checks (reserved-name, uniqueness, validateAppName). Replace that assignment with the same validation/normalization routine used by the hosted flow: call the hosted validation function (e.g., validateAppName or the datasource-name validator used in the hosted path) on opts.name or the default, enforce reserved-name checks, and ensure uniqueness against settings.dataSources.map(d => d.name); if validation fails or the name collides, surface an error (or compute a safe alternative) before calling ensureWebviewerFmHttpConfig and pushing to settings.dataSources so invalid or duplicate names are never written via dataSourceSchema or setSettings.
🧹 Nitpick comments (1)
packages/cli/src/helpers/scaffoldProject.ts (1)
12-22: Remove redundant hidden-entry filtering state.Line 12’s
AGENT_METADATA_DIRShas no runtime effect because Line 20 already drops every dot-prefixed entry. Keeping both adds maintenance noise.♻️ Proposed simplification
-const AGENT_METADATA_DIRS = new Set([".agents", ".claude", ".clawed", ".clinerules", ".cursor", ".windsurf"]); - function getMeaningfulDirectoryEntries(projectDir: string): string[] { return fs.readdirSync(projectDir).filter((entry) => { - if (AGENT_METADATA_DIRS.has(entry)) { - return false; - } - if (entry.startsWith(".")) { return false; } return true; }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/helpers/scaffoldProject.ts` around lines 12 - 22, AGENT_METADATA_DIRS is redundant because getMeaningfulDirectoryEntries already filters dot-prefixed names with entry.startsWith("."), so remove the unused AGENT_METADATA_DIRS constant and the conditional that checks AGENT_METADATA_DIRS.has(entry) inside getMeaningfulDirectoryEntries; keep the existing entry.startsWith(".") filter and overall return logic intact to simplify maintenance and avoid dead state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/create-changeset/SKILL.md:
- Line 117: Replace the unsafe shell-redirection placeholder used in the command
examples (e.g., the snippet Execute `pnpm textlint .changeset/<filename>.md`)
with a safe, non-angle-bracket placeholder such as `pnpm textlint
.changeset/{filename}.md` or `pnpm textlint .changeset/filename.md`; update all
occurrences of the `<filename>` style (including the related examples around the
`pnpm textlint .changeset/<filename>.md` snippet and the other occurrences
mentioned) so examples are copy/paste-safe.
In `@packages/cli/src/cli/add/data-source/filemaker.ts`:
- Around line 35-37: The code unconditionally calls getFmHttpStatus() for
webviewer runs which probes the local FM HTTP proxy; change the condition so the
local probe only runs when no external server was provided (i.e., guard the call
with the server flag check). Specifically, update the block that checks
settings.appType === "webviewer" to also require absence of a supplied server
(e.g., settings.server or the CLI server option) before calling
getFmHttpStatus() and using fmHttpStatus.connectedFiles[0] (connectedFileName);
leave hosted/--server flows to skip getFmHttpStatus() so fallback/hosted logic
can run.
---
Duplicate comments:
In `@packages/cli/src/cli/add/data-source/filemaker.ts`:
- Around line 41-87: The CLI currently validates the presence of both
opts.layoutName and opts.schemaName after making side-effect changes
(addPackageDependency, ensureWebviewerFmHttpConfig, and
settings.dataSources.push), which can leave the repo partially modified; move
the validation for the pair (opts.layoutName && opts.schemaName) up to run
before any mutations (i.e., before calling addPackageDependency,
ensureWebviewerFmHttpConfig, or pushing to settings.dataSources), and if only
one is provided throw the Error("Both --layoutName and --schemaName must be
provided together.") early; keep references to localDataSourceName,
settings.dataSources.push, addPackageDependency, and ensureWebviewerFmHttpConfig
so you update the correct block.
- Around line 38-72: The code currently sets localDataSourceName = opts.name ??
"filemaker" and persists it directly, skipping the hosted-flow checks
(reserved-name, uniqueness, validateAppName). Replace that assignment with the
same validation/normalization routine used by the hosted flow: call the hosted
validation function (e.g., validateAppName or the datasource-name validator used
in the hosted path) on opts.name or the default, enforce reserved-name checks,
and ensure uniqueness against settings.dataSources.map(d => d.name); if
validation fails or the name collides, surface an error (or compute a safe
alternative) before calling ensureWebviewerFmHttpConfig and pushing to
settings.dataSources so invalid or duplicate names are never written via
dataSourceSchema or setSettings.
---
Nitpick comments:
In `@packages/cli/src/helpers/scaffoldProject.ts`:
- Around line 12-22: AGENT_METADATA_DIRS is redundant because
getMeaningfulDirectoryEntries already filters dot-prefixed names with
entry.startsWith("."), so remove the unused AGENT_METADATA_DIRS constant and the
conditional that checks AGENT_METADATA_DIRS.has(entry) inside
getMeaningfulDirectoryEntries; keep the existing entry.startsWith(".") filter
and overall return logic intact to simplify maintenance and avoid dead state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0a505ab8-015d-467a-9de1-aca58c500546
📒 Files selected for processing (8)
.agents/skills/create-changeset/SKILL.mdbiome.jsonpackages/cli/src/cli/add/data-source/filemaker.tspackages/cli/src/cli/init.tspackages/cli/src/globals.d.tspackages/cli/src/helpers/scaffoldProject.tspackages/cli/src/utils/getProofKitVersion.tspackages/cli/tsdown.config.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- biome.json
- packages/cli/src/globals.d.ts
- packages/cli/tsdown.config.ts
|
|
||
| ### 5. Lint Changeset | ||
|
|
||
| Execute `pnpm textlint .changeset/<filename>.md` to validate the changeset file. |
There was a problem hiding this comment.
Avoid shell-redirection placeholders in runnable commands
Using <filename> in command examples is unsafe for copy/paste execution (<...> is interpreted as input redirection). Please switch to a concrete variable style to avoid broken runs.
Suggested doc fix
-Execute `pnpm textlint .changeset/<filename>.md` to validate the changeset file.
+Execute `pnpm textlint ".changeset/${CHANGESET_FILE}.md"` to validate the changeset file.
@@
-git add .changeset/<filename>.md
+git add ".changeset/${CHANGESET_FILE}.md"
git commit -m "chore: add changeset"Also applies to: 138-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/create-changeset/SKILL.md at line 117, Replace the unsafe
shell-redirection placeholder used in the command examples (e.g., the snippet
Execute `pnpm textlint .changeset/<filename>.md`) with a safe, non-angle-bracket
placeholder such as `pnpm textlint .changeset/{filename}.md` or `pnpm textlint
.changeset/filename.md`; update all occurrences of the `<filename>` style
(including the related examples around the `pnpm textlint
.changeset/<filename>.md` snippet and the other occurrences mentioned) so
examples are copy/paste-safe.
Comment on lines
+35
to
+37
| if (settings.appType === "webviewer") { | ||
| const fmHttpStatus = await getFmHttpStatus(); | ||
| const connectedFileName = fmHttpStatus.connectedFiles[0]; |
There was a problem hiding this comment.
Only probe local FM HTTP when no server was supplied.
The unconditional getFmHttpStatus() makes the local proxy part of every webviewer run, even hosted --server flows. In packages/cli/src/helpers/fmHttp.ts:32-50, that helper awaits the health and connected-files calls without local recovery, so a down proxy can short-circuit this command before the hosted path or fallback logic runs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/cli/add/data-source/filemaker.ts` around lines 35 - 37, The
code unconditionally calls getFmHttpStatus() for webviewer runs which probes the
local FM HTTP proxy; change the condition so the local probe only runs when no
external server was provided (i.e., guard the call with the server flag check).
Specifically, update the block that checks settings.appType === "webviewer" to
also require absence of a supplied server (e.g., settings.server or the CLI
server option) before calling getFmHttpStatus() and using
fmHttpStatus.connectedFiles[0] (connectedFileName); leave hosted/--server flows
to skip getFmHttpStatus() so fallback/hosted logic can run.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.agents/skills/create-changeset/SKILL.md (2)
117-117:⚠️ Potential issue | 🟡 MinorResolve the unresolved past review comment: unsafe shell-redirection placeholder.
The
<filename>placeholder in the command example can be misinterpreted as shell input redirection, making it unsafe for copy/paste execution. This issue was previously flagged but remains unaddressed.🛡️ Suggested fix
-Execute `pnpm textlint .changeset/<filename>.md` to validate the changeset file. +Execute `pnpm textlint .changeset/{CHANGESET_FILE}.md` to validate the changeset file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/create-changeset/SKILL.md at line 117, Replace the unsafe "<filename>" shell-redirection placeholder in the example command string "pnpm textlint .changeset/<filename>.md" with a safe, non-redirection placeholder or a concrete example (e.g., use "your-changeset.md" or "example-changeset.md") or instruct users to quote the path (e.g., ".changeset/<filename>.md") so the shell won't treat < as input redirection; update the SKILL.md line containing the exact text "pnpm textlint .changeset/<filename>.md" accordingly.
138-139:⚠️ Potential issue | 🟡 MinorReplace unsafe shell-redirection placeholder in git commands.
The
<filename>placeholder can be misinterpreted as shell input redirection. Use a safer placeholder format for copy/paste safety.🛡️ Suggested fix
-git add .changeset/<filename>.md +git add .changeset/{CHANGESET_FILE}.md git commit -m "chore: add changeset"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/create-changeset/SKILL.md around lines 138 - 139, Replace the unsafe shell-redirection-style placeholder in the git commands (the lines containing "git add .changeset/<filename>.md" and the corresponding commit step) with a safe copy/paste-friendly token such as "{filename}.md" or "FILENAME.md"; update the add command to "git add .changeset/{filename}.md" (or use .changeset/FILENAME.md) and keep the commit command unchanged except ensuring the message references the same placeholder, so users won't accidentally trigger shell redirection when copying the snippet.
🧹 Nitpick comments (3)
packages/cli/src/helpers/scaffoldProject.ts (1)
60-66: Redundantspinner.start()call after force mode clears directory.When
forceis true and the directory is cleared,spinner.start()is called on line 65. Then line 123 callsspinner.start()again unconditionally. Whileorahandles this gracefully (it's a no-op if already spinning), the redundant call is unnecessary.♻️ Remove the spinner.start() inside the force block
if (force) { spinner.info( `${chalk.yellow("Force mode enabled:")} clearing ${chalk.cyan.bold(projectName)} before scaffolding...\n`, ); fs.emptyDirSync(projectDir); - spinner.start(); // continue to scaffold after clearing } else if (isNonInteractiveMode()) {Also applies to: 123-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/helpers/scaffoldProject.ts` around lines 60 - 66, The spinner.start() call inside the force branch is redundant because spinner.start() is already called later unconditionally; in the scaffoldProject function remove the inner spinner.start() (the call immediately after fs.emptyDirSync(projectDir)) so spinner is only started once, leaving the clearing logic (fs.emptyDirSync(projectDir)) and spinner.info(...) intact; reference symbols: force, spinner.start(), fs.emptyDirSync(projectDir), projectName, scaffoldProject.packages/cli/src/helpers/createProject.ts (2)
47-72: Considersatisfiesfor compile-time validation of dependency names.The
as AvailableDependencies[]type assertion bypasses compile-time validation. If a dependency name is misspelled, TypeScript won't catch it. Usingsatisfiespreserves type safety while keeping the array type.♻️ Proposed change for one array (apply to others similarly)
- const VITE_SHADCN_BASE_DEPS = [ + const VITE_SHADCN_BASE_DEPS = [ "@radix-ui/react-slot", "@tailwindcss/vite", "@proofkit/fmdapi", "@proofkit/webviewer", "class-variance-authority", "clsx", "lucide-react", "tailwind-merge", "tailwindcss", "tw-animate-css", "zod", - ] as AvailableDependencies[]; + ] as const satisfies readonly AvailableDependencies[];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/helpers/createProject.ts` around lines 47 - 72, Replace the unsafe type assertions for dependency arrays with TypeScript's `satisfies` operator so the arrays (NEXT_SHADCN_BASE_DEPS, VITE_SHADCN_BASE_DEPS, SHADCN_BASE_DEV_DEPS, VITE_SHADCN_BASE_DEV_DEPS) are validated at compile time against AvailableDependencies[]; remove the trailing `as AvailableDependencies[]` assertions and use `satisfies AvailableDependencies[]` for each constant to preserve literal types while ensuring misspelled dependency names are caught by the type checker.
12-19: UnusedappRouterparameter in function signature.The
appRouterproperty is defined in theCreateProjectOptionsinterface but is not destructured or used withincreateBareProject. If it's needed by callers for type compatibility but not used here, consider documenting that; otherwise, remove it to avoid confusion.♻️ If unused, remove from destructuring or document intent
interface CreateProjectOptions { projectName: string; packages: PkgInstallerMap; scopedAppName: string; noInstall: boolean; force: boolean; - appRouter: boolean; }Or if needed for type compatibility elsewhere, add a comment:
interface CreateProjectOptions { projectName: string; packages: PkgInstallerMap; scopedAppName: string; noInstall: boolean; force: boolean; /** Used by callers; not consumed in createBareProject */ appRouter: boolean; }Also applies to: 21-27
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/helpers/createProject.ts` around lines 12 - 19, The CreateProjectOptions interface includes an unused appRouter property; update it and usages to avoid confusion: either remove appRouter from CreateProjectOptions and any call sites that only exist for this param, or keep it but mark it explicit with a comment (e.g., "/** Used by callers; not consumed in createBareProject */") so callers know it’s intentionally unused; locate the interface named CreateProjectOptions and the function createBareProject to adjust the interface and/or add the explanatory comment accordingly and ensure TypeScript types and call sites remain compatible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.agents/skills/create-changeset/SKILL.md:
- Line 117: Replace the unsafe "<filename>" shell-redirection placeholder in the
example command string "pnpm textlint .changeset/<filename>.md" with a safe,
non-redirection placeholder or a concrete example (e.g., use "your-changeset.md"
or "example-changeset.md") or instruct users to quote the path (e.g.,
".changeset/<filename>.md") so the shell won't treat < as input redirection;
update the SKILL.md line containing the exact text "pnpm textlint
.changeset/<filename>.md" accordingly.
- Around line 138-139: Replace the unsafe shell-redirection-style placeholder in
the git commands (the lines containing "git add .changeset/<filename>.md" and
the corresponding commit step) with a safe copy/paste-friendly token such as
"{filename}.md" or "FILENAME.md"; update the add command to "git add
.changeset/{filename}.md" (or use .changeset/FILENAME.md) and keep the commit
command unchanged except ensuring the message references the same placeholder,
so users won't accidentally trigger shell redirection when copying the snippet.
---
Nitpick comments:
In `@packages/cli/src/helpers/createProject.ts`:
- Around line 47-72: Replace the unsafe type assertions for dependency arrays
with TypeScript's `satisfies` operator so the arrays (NEXT_SHADCN_BASE_DEPS,
VITE_SHADCN_BASE_DEPS, SHADCN_BASE_DEV_DEPS, VITE_SHADCN_BASE_DEV_DEPS) are
validated at compile time against AvailableDependencies[]; remove the trailing
`as AvailableDependencies[]` assertions and use `satisfies
AvailableDependencies[]` for each constant to preserve literal types while
ensuring misspelled dependency names are caught by the type checker.
- Around line 12-19: The CreateProjectOptions interface includes an unused
appRouter property; update it and usages to avoid confusion: either remove
appRouter from CreateProjectOptions and any call sites that only exist for this
param, or keep it but mark it explicit with a comment (e.g., "/** Used by
callers; not consumed in createBareProject */") so callers know it’s
intentionally unused; locate the interface named CreateProjectOptions and the
function createBareProject to adjust the interface and/or add the explanatory
comment accordingly and ensure TypeScript types and call sites remain
compatible.
In `@packages/cli/src/helpers/scaffoldProject.ts`:
- Around line 60-66: The spinner.start() call inside the force branch is
redundant because spinner.start() is already called later unconditionally; in
the scaffoldProject function remove the inner spinner.start() (the call
immediately after fs.emptyDirSync(projectDir)) so spinner is only started once,
leaving the clearing logic (fs.emptyDirSync(projectDir)) and spinner.info(...)
intact; reference symbols: force, spinner.start(), fs.emptyDirSync(projectDir),
projectName, scaffoldProject.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 29ef0a3a-60b7-47cb-879e-7531c92b771f
📒 Files selected for processing (13)
.agents/skills/create-changeset/SKILL.mdbiome.jsonpackages/cli/src/cli/add/data-source/filemaker.tspackages/cli/src/cli/init.tspackages/cli/src/globals.d.tspackages/cli/src/helpers/createProject.tspackages/cli/src/helpers/scaffoldProject.tspackages/cli/src/installers/dependencyVersionMap.tspackages/cli/src/utils/getProofKitVersion.tspackages/cli/src/utils/proofkitReleaseChannel.tspackages/cli/template/vite-wv/vite.config.tspackages/cli/tests/release-channel.test.tspackages/cli/tsdown.config.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- biome.json
- packages/cli/template/vite-wv/vite.config.ts
- packages/cli/src/utils/getProofKitVersion.ts
- packages/cli/src/cli/add/data-source/filemaker.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/cli/tests/setup.ts (1)
9-9: Prefer a named default constant and preserve explicit env overridesLine 9 hard-codes
"1"and unconditionally overrides any pre-set value. Consider defaulting with??=so targeted tests can still opt in/out explicitly.♻️ Proposed refactor
beforeAll(() => { // Ensure test environment variables are loaded dotenv.config({ path: path.resolve(__dirname, "../.env.test") }); - process.env.PROOFKIT_SKIP_VERSION_CHECK = "1"; + const SKIP_VERSION_CHECK = "1" as const; + process.env.PROOFKIT_SKIP_VERSION_CHECK ??= SKIP_VERSION_CHECK; });As per coding guidelines, "Use meaningful variable names instead of magic numbers - extract constants with descriptive names" and "Use optional chaining (
?.) and nullish coalescing (??) for safer property access".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/tests/setup.ts` at line 9, Replace the unconditional hard-coded environment assignment to PROOFKIT_SKIP_VERSION_CHECK with a named default constant (e.g., DEFAULT_PROOFKIT_SKIP_VERSION_CHECK) and only set the env var when it is undefined using the nullish-coalescing-assignment operator (??=) so existing external overrides are preserved; locate the assignment to process.env.PROOFKIT_SKIP_VERSION_CHECK in tests/setup.ts and change it to use the constant and ??= semantics.packages/cli/src/installers/dependencyVersionMap.ts (1)
106-106: Consider pinning Zod to a more specific version range.The broad
^4range will accept any Zod 4.x release. Given that Zod 4 introduces significant API changes (unified error customization, new schema types, etc.), a more specific range like^4.0.0or^4.3could provide more predictable scaffolding for new projects.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/installers/dependencyVersionMap.ts` at line 106, The dependency entry for Zod in the dependencyVersionMap currently uses the broad version specifier "zod: \"^4\"" which allows any 4.x and can introduce breaking API drift; update the value for the "zod" key in dependencyVersionMap to a narrower range such as "^4.3" or "^4.0.0" (pick the desired minimum) so generated projects get a more predictable Zod release; locate the "zod" key in the dependencyVersionMap export and replace "^4" with the chosen specific semver range.
🤖 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/cli/init.ts`:
- Around line 241-244: The dataSource fallback uses only cliOptions.server for
non-interactive webviewer init which causes flags like
--adminApiKey/--dataApiKey/--fileName/--layoutName/--schemaName to be ignored;
hoist the existing predicate you compute later (the "has FileMaker flags"
boolean used to decide prompting, e.g., the one referenced alongside
promptForFileMakerDataSource) into a variable and reuse it in the dataSource
assignment so nonInteractive && !hasFileMakerFlags (not just !cliOptions.server)
yields "none"; update the dataSource ternary that references state.appType,
cliOptions, and nonInteractive to check this hoisted hasFileMakerFlags predicate
instead of cliOptions.server and ensure promptForFileMakerDataSource() will run
when those flags are present.
In `@packages/cli/src/helpers/scaffoldProject.ts`:
- Around line 20-22: The filter that rejects all dotfiles via the check
entry.startsWith(".") incorrectly treats an existing .gitignore as
non-meaningful; update the logic in the directory-entry filter to either allow
".gitignore" through (i.e., return true when entry === ".gitignore") or detect
an existing ".gitignore" before the template rename step (the code path that
writes/renames "_gitignore" at Line 127) and preserve/merge it instead of
overwriting—specifically, change the entry filter to special-case ".gitignore"
or add a pre-write check for an existing ".gitignore" and skip/merge the
template rename when present.
---
Nitpick comments:
In `@packages/cli/src/installers/dependencyVersionMap.ts`:
- Line 106: The dependency entry for Zod in the dependencyVersionMap currently
uses the broad version specifier "zod: \"^4\"" which allows any 4.x and can
introduce breaking API drift; update the value for the "zod" key in
dependencyVersionMap to a narrower range such as "^4.3" or "^4.0.0" (pick the
desired minimum) so generated projects get a more predictable Zod release;
locate the "zod" key in the dependencyVersionMap export and replace "^4" with
the chosen specific semver range.
In `@packages/cli/tests/setup.ts`:
- Line 9: Replace the unconditional hard-coded environment assignment to
PROOFKIT_SKIP_VERSION_CHECK with a named default constant (e.g.,
DEFAULT_PROOFKIT_SKIP_VERSION_CHECK) and only set the env var when it is
undefined using the nullish-coalescing-assignment operator (??=) so existing
external overrides are preserved; locate the assignment to
process.env.PROOFKIT_SKIP_VERSION_CHECK in tests/setup.ts and change it to use
the constant and ??= semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1eb0c5ae-7a47-490a-b5d9-790c26c42f67
📒 Files selected for processing (11)
packages/cli/src/cli/init.tspackages/cli/src/helpers/scaffoldProject.tspackages/cli/src/index.tspackages/cli/src/installers/dependencyVersionMap.tspackages/cli/tests/setup.tspackages/cli/tests/webviewer-apps.test.tspackages/cli/vitest.config.tspackages/typegen/bin/intent.jspackages/typegen/src/typegen.tspackages/typegen/src/types.tspackages/typegen/typegen.schema.json
✅ Files skipped from review due to trivial changes (2)
- packages/typegen/bin/intent.js
- packages/typegen/src/types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli/tests/webviewer-apps.test.ts
Comment on lines
+241
to
+244
| let dataSource = | ||
| state.appType === "webviewer" | ||
| ? (cliOptions.dataSource ?? (nonInteractive && !cliOptions.server ? "none" : "filemaker")) | ||
| : (cliOptions.dataSource ?? (nonInteractive ? "none" : undefined)); |
There was a problem hiding this comment.
Don't fall back to "none" when other FileMaker flags are present.
In non-interactive WebViewer init this branch only checks server, so --adminApiKey, --dataApiKey, --fileName, --layoutName, or --schemaName are silently ignored and promptForFileMakerDataSource() never runs. You're already computing the broader predicate at Lines 289-296; hoist and reuse it here instead of keying off server alone.
💡 Suggested adjustment
+ const hasExplicitFileMakerInputs = Boolean(
+ cliOptions.server ||
+ cliOptions.adminApiKey ||
+ cliOptions.dataApiKey ||
+ cliOptions.fileName ||
+ cliOptions.layoutName ||
+ cliOptions.schemaName,
+ );
+
let dataSource =
state.appType === "webviewer"
- ? (cliOptions.dataSource ?? (nonInteractive && !cliOptions.server ? "none" : "filemaker"))
+ ? (cliOptions.dataSource ?? (nonInteractive && !hasExplicitFileMakerInputs ? "none" : "filemaker"))
: (cliOptions.dataSource ?? (nonInteractive ? "none" : undefined));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let dataSource = | |
| state.appType === "webviewer" | |
| ? (cliOptions.dataSource ?? (nonInteractive && !cliOptions.server ? "none" : "filemaker")) | |
| : (cliOptions.dataSource ?? (nonInteractive ? "none" : undefined)); | |
| const hasExplicitFileMakerInputs = Boolean( | |
| cliOptions.server || | |
| cliOptions.adminApiKey || | |
| cliOptions.dataApiKey || | |
| cliOptions.fileName || | |
| cliOptions.layoutName || | |
| cliOptions.schemaName, | |
| ); | |
| let dataSource = | |
| state.appType === "webviewer" | |
| ? (cliOptions.dataSource ?? (nonInteractive && !hasExplicitFileMakerInputs ? "none" : "filemaker")) | |
| : (cliOptions.dataSource ?? (nonInteractive ? "none" : undefined)); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/cli/init.ts` around lines 241 - 244, The dataSource fallback
uses only cliOptions.server for non-interactive webviewer init which causes
flags like --adminApiKey/--dataApiKey/--fileName/--layoutName/--schemaName to be
ignored; hoist the existing predicate you compute later (the "has FileMaker
flags" boolean used to decide prompting, e.g., the one referenced alongside
promptForFileMakerDataSource) into a variable and reuse it in the dataSource
assignment so nonInteractive && !hasFileMakerFlags (not just !cliOptions.server)
yields "none"; update the dataSource ternary that references state.appType,
cliOptions, and nonInteractive to check this hoisted hasFileMakerFlags predicate
instead of cliOptions.server and ensure promptForFileMakerDataSource() will run
when those flags are present.
This was referenced Mar 18, 2026
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.
feat(cli): revamp webviewer app scaffolding
Add create-changeset skill
fix(cli): stop generating cursor rules
fix(cli): harden non-interactive init prompts
fix(cli): ignore hidden files during init
feat(cli): install typegen skills locally
Summary by CodeRabbit
New Features
Bug Fixes
Documentation