Add direct wrapper tests for create-proofkit#165
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Collaborator
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (292)
📝 WalkthroughWalkthroughAdds a new standalone CLI package under Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as User
participant CLI as CLI
participant Otto as OttoFMS
participant FM as FileMaker Data API
participant FS as Project FS
User->>CLI: run "add data-source" (FileMaker)
CLI->>Otto: open login URL / poll for token
Otto-->>CLI: return auth token
CLI->>Otto: start demo file deployment (optional)
Otto-->>CLI: deployment status (poll until complete)
CLI->>FM: request layouts via Data API (using API key)
FM-->>CLI: layouts + metadata
CLI->>FS: write settings/env, generate files, update config
CLI->>FS: run codegen / install deps
CLI-->>User: show success and next steps
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (6)
packages/cli/tests/init-scaffold-contract.test.ts (1)
26-34: Module-level synchronous I/O may cause confusing failures.The
cliPackageJsonis read synchronously at module load time (line 33). Ifdist/index.jsorpackage.jsondoesn't exist (e.g., before a build), the test file will fail to load with a cryptic error rather than a clear test failure message.Consider moving this into
beforeAllor using a lazy initialization pattern for clearer error reporting during test discovery.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/tests/init-scaffold-contract.test.ts` around lines 26 - 34, The test reads cliPackageJson synchronously at module load (cliPackageJsonPath / cliPackageJson / cliVersion), which can cause noisy failures; move the readJsonFile call and cliVersion initialization into a beforeAll block (or implement lazy initialization) so the file I/O runs during test setup and can throw a clear test failure; update any references in the file to use the initialized variable from beforeAll (or a getter) and ensure the variable is declared at module scope (e.g., let cliPackageJson: PackageJsonShape | undefined and let cliVersion = "") so tests can access the values after setup.packages/cli/src/index.ts (2)
24-29: Duplicate helper function across files.The
getErrorMessagefunction is duplicated verbatim inpackages/cli/src/cli/init.ts(lines 144-149). Consider extracting it to a shared utility module (e.g.,~/utils/errorUtils.ts) to avoid duplication.♻️ Suggested refactor
Create a shared utility:
// packages/cli/src/utils/errorUtils.ts export function getErrorMessage(error: unknown): string { if (error instanceof Error) { return error.message; } return String(error); }Then import in both
index.tsandinit.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/index.ts` around lines 24 - 29, Extract the duplicated helper getErrorMessage into a single shared module (e.g., export function getErrorMessage from a new utils module) and replace the inline implementations in both index.ts and init.ts with imports of that function; specifically, create a new module that exports getErrorMessage, update the callers (the getErrorMessage references in index.ts and cli/init.ts) to import it, and remove the duplicate local implementations to avoid verbatim duplication.
87-87: Redundant type assertion.Inside the
err instanceof Errorbranch,erris already narrowed toError. The cast(err as Error & { cause?: unknown })is unnecessary sinceError.causeis a standard property in ES2022+.♻️ Simplified access
- const cause = (err as Error & { cause?: unknown }).cause; + const cause = err.cause;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/index.ts` at line 87, Inside the `err instanceof Error` branch, remove the redundant type assertion and read the cause directly from the narrowed error (replace `const cause = (err as Error & { cause?: unknown }).cause;` with `const cause = err.cause;`), ensuring you still handle the possibility that `cause` is undefined when using it elsewhere in the `index.ts` error-handling logic.packages/cli/src/cli/init.ts (2)
144-149: Duplicate helper function.This
getErrorMessagefunction is identical to the one inpackages/cli/src/index.ts(lines 24-29). As noted earlier, consider extracting to a shared utility.🤖 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 144 - 149, The duplicate getErrorMessage function should be consolidated: remove the local getErrorMessage in init.ts and import a single shared utility (e.g., error/getErrorMessage or a common utils module) used by packages/cli/src/index.ts; update references in init.ts to call the imported getErrorMessage, and ensure the shared module exports the function (preserve the same signature getErrorMessage(error: unknown): string) so both index.ts and init.ts use the common implementation.
371-384: The fallback to "browser" appears logically unnecessary but may be required for TypeScript type safety.At line 380,
state.appTypecannot be undefined from a control flow perspective because:
- The code only executes inside
if (shouldRunInitialCodegen)(line 374)shouldRunInitialCodegenrequiresstate.appType === "webviewer"to be true (line 372)- If that condition is true,
state.appTypeis definitely"webviewer", not undefinedHowever, since
state.appTypeis typed as"browser" | "webviewer" | undefined(state.ts line 10) andstateis a mutable global object, TypeScript's type narrowing may not automatically infer that the variable cannot be undefined at this point. The nullish coalescing operator may be necessary to satisfy the type checker, even if it's not logically required.If this fallback is purely for type safety, consider adding a comment explaining that. Alternatively, if type narrowing can be improved, the fallback could be removed.
🤖 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 371 - 384, The createPostInitGenerationError call uses state.appType ?? "browser" even though shouldRunInitialCodegen guarantees state.appType === "webviewer" at runtime; this fallback is only for TypeScript type-safety. Either add a short comment above that line stating the nullish coalescing is intentionally present to satisfy the compiler (e.g., "fallback only for TS type narrowing; runtime value is 'webviewer' because shouldRunInitialCodegen guarded it"), or remove the fallback and adjust type narrowing so the compiler knows state.appType cannot be undefined (e.g., refine the predicate that sets shouldRunInitialCodegen). Update the code around dataSource, shouldRunInitialCodegen, state.appType, runCodegenCommand, and createPostInitGenerationError accordingly.packages/cli/tests/init-non-interactive-failures.test.ts (1)
63-78: Consider clarifying null status handling.Line 69 checks
failure.status === null, which typically indicates the process was killed by a signal rather than exiting normally. This is being treated the same as a numeric exit code. If intentional (e.g., to handle SIGTERM), a comment would help clarify; otherwise, you may want to handle signal-killed processes differently.♻️ Suggested improvement
const runInitExpectFailure = (args: string[], cwd = testDir) => { try { runInitCommand(args, cwd); throw new Error(`Expected init to fail, but it succeeded: ${args.join(" ")}`); } catch (error) { const failure = error as ExecFailure; - if (typeof failure.status === "number" || failure.status === null) { + // status is number for normal exit, null if killed by signal + if (failure.status !== undefined) { return { status: failure.status, stdout: toText(failure.stdout), stderr: toText(failure.stderr), }; } throw error; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/tests/init-non-interactive-failures.test.ts` around lines 63 - 78, The helper runInitExpectFailure currently treats failure.status === null the same as a numeric exit code, which obscures signal-killed processes; update runInitExpectFailure to explicitly handle signal-terminated failures (failure.status === null) by checking for failure.signal and returning a distinct result (e.g., include signal name and set status to null) or rethrowing, and add a brief comment in the function explaining that null status means the child was terminated by a signal so future readers understand why it's handled separately; locate this logic in runInitExpectFailure and adjust the conditional that currently checks "typeof failure.status === 'number' || failure.status === null".
🤖 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/cli/init.ts`:
- Around line 144-149: The duplicate getErrorMessage function should be
consolidated: remove the local getErrorMessage in init.ts and import a single
shared utility (e.g., error/getErrorMessage or a common utils module) used by
packages/cli/src/index.ts; update references in init.ts to call the imported
getErrorMessage, and ensure the shared module exports the function (preserve the
same signature getErrorMessage(error: unknown): string) so both index.ts and
init.ts use the common implementation.
- Around line 371-384: The createPostInitGenerationError call uses state.appType
?? "browser" even though shouldRunInitialCodegen guarantees state.appType ===
"webviewer" at runtime; this fallback is only for TypeScript type-safety. Either
add a short comment above that line stating the nullish coalescing is
intentionally present to satisfy the compiler (e.g., "fallback only for TS type
narrowing; runtime value is 'webviewer' because shouldRunInitialCodegen guarded
it"), or remove the fallback and adjust type narrowing so the compiler knows
state.appType cannot be undefined (e.g., refine the predicate that sets
shouldRunInitialCodegen). Update the code around dataSource,
shouldRunInitialCodegen, state.appType, runCodegenCommand, and
createPostInitGenerationError accordingly.
In `@packages/cli/src/index.ts`:
- Around line 24-29: Extract the duplicated helper getErrorMessage into a single
shared module (e.g., export function getErrorMessage from a new utils module)
and replace the inline implementations in both index.ts and init.ts with imports
of that function; specifically, create a new module that exports
getErrorMessage, update the callers (the getErrorMessage references in index.ts
and cli/init.ts) to import it, and remove the duplicate local implementations to
avoid verbatim duplication.
- Line 87: Inside the `err instanceof Error` branch, remove the redundant type
assertion and read the cause directly from the narrowed error (replace `const
cause = (err as Error & { cause?: unknown }).cause;` with `const cause =
err.cause;`), ensuring you still handle the possibility that `cause` is
undefined when using it elsewhere in the `index.ts` error-handling logic.
In `@packages/cli/tests/init-non-interactive-failures.test.ts`:
- Around line 63-78: The helper runInitExpectFailure currently treats
failure.status === null the same as a numeric exit code, which obscures
signal-killed processes; update runInitExpectFailure to explicitly handle
signal-terminated failures (failure.status === null) by checking for
failure.signal and returning a distinct result (e.g., include signal name and
set status to null) or rethrowing, and add a brief comment in the function
explaining that null status means the child was terminated by a signal so future
readers understand why it's handled separately; locate this logic in
runInitExpectFailure and adjust the conditional that currently checks "typeof
failure.status === 'number' || failure.status === null".
In `@packages/cli/tests/init-scaffold-contract.test.ts`:
- Around line 26-34: The test reads cliPackageJson synchronously at module load
(cliPackageJsonPath / cliPackageJson / cliVersion), which can cause noisy
failures; move the readJsonFile call and cliVersion initialization into a
beforeAll block (or implement lazy initialization) so the file I/O runs during
test setup and can throw a clear test failure; update any references in the file
to use the initialized variable from beforeAll (or a getter) and ensure the
variable is declared at module scope (e.g., let cliPackageJson: PackageJsonShape
| undefined and let cliVersion = "") so tests can access the values after setup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d2cccd35-6b8f-4c4c-b966-b96285bc0be8
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
.github/workflows/continuous-release.yml.github/workflows/release.ymlpackages/cli/package.jsonpackages/cli/src/cli/init.tspackages/cli/src/index.tspackages/cli/tests/browser-apps.smoke.test.tspackages/cli/tests/init-non-interactive-failures.test.tspackages/cli/tests/init-post-init-generation-errors.test.tspackages/cli/tests/init-run-init-regression.test.tspackages/cli/tests/init-scaffold-contract.test.tspackages/cli/tests/test-utils.tspackages/cli/vitest.config.tspackages/cli/vitest.smoke.config.tspackages/create-proofkit/package.jsonpackages/create-proofkit/tests/index.test.jspackages/create-proofkit/vitest.config.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/cli/template/vite-wv/src/routes/query-demo.tsx (1)
4-12: Extract the timeout delay and query key into named constants for clarity and stronger typing.Line 5 uses a magic number
180without context, and line 11 can use a const assertion to preserve literal typing.♻️ Suggested refactor
+const CONNECTION_HINT_DELAY_MS = 180; +const STARTER_CONNECTION_HINT_QUERY_KEY = ["starter-connection-hint"] as const; + const getConnectionHint = async (): Promise<string> => { - await new Promise((resolve) => setTimeout(resolve, 180)); + await new Promise((resolve) => setTimeout(resolve, CONNECTION_HINT_DELAY_MS)); return "Use fmFetch or generated clients once your FileMaker file is ready."; }; export function QueryDemoPage() { const hintQuery = useQuery({ - queryKey: ["starter-connection-hint"], + queryKey: STARTER_CONNECTION_HINT_QUERY_KEY, queryFn: getConnectionHint, });Per coding guidelines: "Use meaningful variable names instead of magic numbers - extract constants with descriptive names" and "Use const assertions (
as const) for immutable values and literal types".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/template/vite-wv/src/routes/query-demo.tsx` around lines 4 - 12, Replace the magic number and inline query key by extracting them into named constants: define a numeric constant (e.g., CONNECTION_HINT_DELAY_MS) and use it in getConnectionHint instead of 180, and define the query key as a const assertion (e.g., CONNECTION_HINT_QUERY_KEY = ["starter-connection-hint"] as const) then pass CONNECTION_HINT_QUERY_KEY to useQuery inside QueryDemoPage; this improves readability and preserves the literal type for the query key.
🤖 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/new/src/utils/projectFiles.ts`:
- Around line 80-83: The current replaceTextInFiles implementation swallows all
fs.readdir errors by mapping them to undefined (childEntries) which hides real
IO/permission/race failures; wrap the fs.readdir(fullPath) call in a try-catch
inside replaceTextInFiles, and in the catch only suppress
directory-not-a-directory errors (check err.code === 'ENOTDIR' or similar) so
you treat that path as a non-directory, but rethrow any other errors
(permissions/IO) so they surface to the caller; keep references to childEntries,
fullPath and fs.readdir when making this change.
In `@packages/new/tests/integration.test.ts`:
- Line 204: The test currently asserts the install message by indexing
consoleTranscript.note[0]?.message which is brittle; change the assertion to
scan all notes for the message instead — use consoleTranscript.note.find(...) or
Array.prototype.some to check any note.message contains "npx
`@tanstack/intent`@latest install" and assert that is true; update the expect that
references consoleTranscript.note[0]?.message to assert existence via find/some
so ordering changes won't break the test.
---
Nitpick comments:
In `@packages/cli/template/vite-wv/src/routes/query-demo.tsx`:
- Around line 4-12: Replace the magic number and inline query key by extracting
them into named constants: define a numeric constant (e.g.,
CONNECTION_HINT_DELAY_MS) and use it in getConnectionHint instead of 180, and
define the query key as a const assertion (e.g., CONNECTION_HINT_QUERY_KEY =
["starter-connection-hint"] as const) then pass CONNECTION_HINT_QUERY_KEY to
useQuery inside QueryDemoPage; this improves readability and preserves the
literal type for the query key.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 984118f1-9ba6-4b04-8a2c-dc7fc013b023
📒 Files selected for processing (14)
packages/cli/template/vite-wv/.claude/launch.jsonpackages/cli/template/vite-wv/AGENTS.mdpackages/cli/template/vite-wv/CLAUDE.mdpackages/cli/template/vite-wv/package.jsonpackages/cli/template/vite-wv/src/App.tsxpackages/cli/template/vite-wv/src/main.tsxpackages/cli/template/vite-wv/src/router.tsxpackages/cli/template/vite-wv/src/routes/query-demo.tsxpackages/new/src/consts.tspackages/new/src/core/executeInitPlan.tspackages/new/src/core/planInit.tspackages/new/src/utils/projectFiles.tspackages/new/tests/init-fixtures.tspackages/new/tests/integration.test.ts
✅ Files skipped from review due to trivial changes (2)
- packages/cli/template/vite-wv/AGENTS.md
- packages/cli/template/vite-wv/CLAUDE.md
| expect(routerFile).toContain("createHashHistory"); | ||
| expect(mainFile).toContain("QueryClientProvider"); | ||
| expect(queryDemoFile).toContain("TanStack Query is preconfigured"); | ||
| expect(consoleTranscript.note[0]?.message).toContain("npx @tanstack/intent@latest install"); |
There was a problem hiding this comment.
Avoid index-coupled assertion for console notes.
Line 204 assumes the target message is always at index 0; that makes the test brittle if note ordering changes.
Suggested resilient assertion
- expect(consoleTranscript.note[0]?.message).toContain("npx `@tanstack/intent`@latest install");
+ expect(
+ consoleTranscript.note.some((entry) =>
+ entry.message.includes("npx `@tanstack/intent`@latest install"),
+ ),
+ ).toBe(true);📝 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
| expect(consoleTranscript.note[0]?.message).toContain("npx @tanstack/intent@latest install"); | |
| expect( | |
| consoleTranscript.note.some((entry) => | |
| entry.message.includes("npx `@tanstack/intent`@latest install"), | |
| ), | |
| ).toBe(true); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/new/tests/integration.test.ts` at line 204, The test currently
asserts the install message by indexing consoleTranscript.note[0]?.message which
is brittle; change the assertion to scan all notes for the message instead — use
consoleTranscript.note.find(...) or Array.prototype.some to check any
note.message contains "npx `@tanstack/intent`@latest install" and assert that is
true; update the expect that references consoleTranscript.note[0]?.message to
assert existence via find/some so ordering changes won't break the test.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/new/tests/integration.test.ts (1)
204-206:⚠️ Potential issue | 🟡 MinorAvoid order-coupled assertions for console notes.
Line 204–Line 206 assume the target message is always in
consoleTranscript.note[0], which can become flaky if note ordering changes.Suggested resilient assertion
- expect(consoleTranscript.note[0]?.message).toContain("Have your agent run this in the new project"); - expect(consoleTranscript.note[0]?.message).toContain("complete the interactive prompt"); - expect(consoleTranscript.note[0]?.message).toContain("npx `@tanstack/intent`@latest install"); + const nextStepsMessage = consoleTranscript.note.find((entry) => entry.title === "Next steps")?.message ?? ""; + expect(nextStepsMessage).toContain("Have your agent run this in the new project"); + expect(nextStepsMessage).toContain("complete the interactive prompt"); + expect(nextStepsMessage).toContain("npx `@tanstack/intent`@latest install");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/new/tests/integration.test.ts` around lines 204 - 206, The assertions assume the expected text appears at consoleTranscript.note[0], which is brittle; instead iterate/search the consoleTranscript.note array (using its note items' message property) to assert that at least one note contains each substring. Update the assertions that reference consoleTranscript.note[0]?.message to use a find/some on consoleTranscript.note (or a helper like hasNoteWithMessage) to check for "Have your agent run this in the new project", "complete the interactive prompt", and "npx `@tanstack/intent`@latest install" so the test no longer relies on note ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/new/tests/integration.test.ts`:
- Around line 204-206: The assertions assume the expected text appears at
consoleTranscript.note[0], which is brittle; instead iterate/search the
consoleTranscript.note array (using its note items' message property) to assert
that at least one note contains each substring. Update the assertions that
reference consoleTranscript.note[0]?.message to use a find/some on
consoleTranscript.note (or a helper like hasNoteWithMessage) to check for "Have
your agent run this in the new project", "complete the interactive prompt", and
"npx `@tanstack/intent`@latest install" so the test no longer relies on note
ordering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ba66c469-baeb-4df5-9725-87f5c34c51a0
📒 Files selected for processing (2)
packages/new/src/core/executeInitPlan.tspackages/new/tests/integration.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/new/src/index.ts (1)
215-229: Duplicated error rendering logic can be consolidated.Lines 218-224 have identical logic in both branches - extracting
Cause.squash(cause)and printing the message. This can be simplified since both thefailure && !isValidationError(failure)case and the!failurecase do the same thing.♻️ Proposed refactor to consolidate duplicate branches
function renderFailure(cause: Cause.Cause<unknown>, showDebugDetails: boolean) { const failure = getOrUndefined(Cause.failureOption(cause)); - if (failure && !isValidationError(failure)) { - const error = Cause.squash(cause); - console.error(error instanceof Error ? error.message : String(error)); - } else if (!failure) { + // ValidationError is already rendered by `@effect/cli`, so skip it + const shouldPrintError = !failure || !isValidationError(failure); + if (shouldPrintError) { const error = Cause.squash(cause); console.error(error instanceof Error ? error.message : String(error)); } if (showDebugDetails) { console.error(`\n[debug] ${Cause.pretty(cause)}`); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/new/src/index.ts` around lines 215 - 229, In renderFailure, the branches for "failure && !isValidationError(failure)" and "!failure" duplicate the same logic; simplify by computing failure once (using getOrUndefined(Cause.failureOption(cause))), then always call Cause.squash(cause) and log the message unless you need special handling for validation errors—i.e., replace the two duplicate branches with a single path that checks isValidationError(failure) and otherwise computes error = Cause.squash(cause) and console.error(error instanceof Error ? error.message : String(error)); preserve the final showDebugDetails block that logs Cause.pretty(cause).
🤖 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/new/src/index.ts`:
- Around line 215-229: In renderFailure, the branches for "failure &&
!isValidationError(failure)" and "!failure" duplicate the same logic; simplify
by computing failure once (using getOrUndefined(Cause.failureOption(cause))),
then always call Cause.squash(cause) and log the message unless you need special
handling for validation errors—i.e., replace the two duplicate branches with a
single path that checks isValidationError(failure) and otherwise computes error
= Cause.squash(cause) and console.error(error instanceof Error ? error.message :
String(error)); preserve the final showDebugDetails block that logs
Cause.pretty(cause).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 80e549d2-3646-4b41-a413-08efbb812027
📒 Files selected for processing (2)
packages/new/src/index.tspackages/new/tests/cli.test.ts
eluce2
force-pushed
the
review-new-tests
branch
from
5d6419e to
c971c02
Compare
eluce2
force-pushed
the
codex/proofkit-new-scaffold
branch
from
44dbbf5 to
4040eff
Compare
eluce2
force-pushed
the
review-new-tests
branch
from
5a8999c to
d0f3b97
Compare
@proofkit/better-auth
@proofkit/cli
create-proofkit
@proofkit/fmdapi
@proofkit/fmodata
@proofkit/typegen
@proofkit/webviewer
commit: |
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 159ab672-10b7-4d3f-8767-ea4f05ef36cc
⛔ Files ignored due to path filters (4)
packages/cli-old/template/fm-addon/ProofKitAuth/icon.pngis excluded by!**/*.pngpackages/cli-old/template/fm-addon/ProofKitAuth/icon@2x.pngis excluded by!**/*.pngpackages/cli-old/template/fm-addon/ProofKitAuth/preview.pngis excluded by!**/*.pngpackages/cli-old/template/fm-addon/ProofKitWV/icon.pngis excluded by!**/*.png
📒 Files selected for processing (291)
.changeset/proofkit-cli-major-migration.md.github/workflows/continuous-release.yml.github/workflows/release.ymlapps/docs/src/app/(home)/page.tsxapps/docs/src/components/AgentCommand.tsxbiome.jsonbtca.config.jsoncpackages/cli-old/.yarnrc.ymlpackages/cli-old/CHANGELOG.mdpackages/cli-old/README.mdpackages/cli-old/index.d.tspackages/cli-old/package.jsonpackages/cli-old/proofkit-cli-1.1.8.tgzpackages/cli-old/src/cli/add/auth.tspackages/cli-old/src/cli/add/data-source/deploy-demo-file.tspackages/cli-old/src/cli/add/data-source/filemaker.tspackages/cli-old/src/cli/add/data-source/index.tspackages/cli-old/src/cli/add/fmschema.tspackages/cli-old/src/cli/add/index.tspackages/cli-old/src/cli/add/page/index.tspackages/cli-old/src/cli/add/page/post-install/table-infinite.tspackages/cli-old/src/cli/add/page/post-install/table.tspackages/cli-old/src/cli/add/page/templates.tspackages/cli-old/src/cli/add/page/types.tspackages/cli-old/src/cli/add/registry/getOptions.tspackages/cli-old/src/cli/add/registry/http.tspackages/cli-old/src/cli/add/registry/install.tspackages/cli-old/src/cli/add/registry/listItems.tspackages/cli-old/src/cli/add/registry/postInstall/handlebars.tspackages/cli-old/src/cli/add/registry/postInstall/index.tspackages/cli-old/src/cli/add/registry/postInstall/package-script.tspackages/cli-old/src/cli/add/registry/postInstall/wrap-provider.tspackages/cli-old/src/cli/add/registry/preflight.tspackages/cli-old/src/cli/deploy/index.tspackages/cli-old/src/cli/fmdapi.tspackages/cli-old/src/cli/init.tspackages/cli-old/src/cli/menu.tspackages/cli-old/src/cli/ottofms.tspackages/cli-old/src/cli/prompts.tspackages/cli-old/src/cli/react-email.tspackages/cli-old/src/cli/remove/data-source.tspackages/cli-old/src/cli/remove/index.tspackages/cli-old/src/cli/remove/page.tspackages/cli-old/src/cli/remove/schema.tspackages/cli-old/src/cli/tanstack-query.tspackages/cli-old/src/cli/typegen/index.tspackages/cli-old/src/cli/update/index.tspackages/cli-old/src/cli/update/makeUpgradeCommand.tspackages/cli-old/src/cli/utils.tspackages/cli-old/src/consts.tspackages/cli-old/src/generators/auth.tspackages/cli-old/src/generators/fmdapi.tspackages/cli-old/src/generators/route.tspackages/cli-old/src/generators/tanstack-query.tspackages/cli-old/src/globalOptions.tspackages/cli-old/src/globals.d.tspackages/cli-old/src/helpers/createProject.tspackages/cli-old/src/helpers/fmHttp.tspackages/cli-old/src/helpers/git.tspackages/cli-old/src/helpers/installDependencies.tspackages/cli-old/src/helpers/installPackages.tspackages/cli-old/src/helpers/logNextSteps.tspackages/cli-old/src/helpers/replaceText.tspackages/cli-old/src/helpers/scaffoldProject.tspackages/cli-old/src/helpers/selectBoilerplate.tspackages/cli-old/src/helpers/setImportAlias.tspackages/cli-old/src/helpers/shadcn-cli.tspackages/cli-old/src/helpers/stealth-init.tspackages/cli-old/src/helpers/version-fetcher.tspackages/cli-old/src/index.tspackages/cli-old/src/installers/auth-shared.tspackages/cli-old/src/installers/better-auth.tspackages/cli-old/src/installers/clerk.tspackages/cli-old/src/installers/dependencyVersionMap.tspackages/cli-old/src/installers/envVars.tspackages/cli-old/src/installers/index.tspackages/cli-old/src/installers/install-fm-addon.tspackages/cli-old/src/installers/nextAuth.tspackages/cli-old/src/installers/proofkit-auth.tspackages/cli-old/src/installers/proofkit-webviewer.tspackages/cli-old/src/installers/react-email.tspackages/cli-old/src/state.tspackages/cli-old/src/upgrades/cursorRules.tspackages/cli-old/src/upgrades/index.tspackages/cli-old/src/upgrades/shadcn.tspackages/cli-old/src/utils/addPackageDependency.tspackages/cli-old/src/utils/addToEnvs.tspackages/cli-old/src/utils/formatting.tspackages/cli-old/src/utils/getProofKitVersion.tspackages/cli-old/src/utils/getUserPkgManager.tspackages/cli-old/src/utils/isTTYError.tspackages/cli-old/src/utils/logger.tspackages/cli-old/src/utils/parseNameAndPath.tspackages/cli-old/src/utils/parseSettings.tspackages/cli-old/src/utils/proofkitReleaseChannel.tspackages/cli-old/src/utils/removeTrailingSlash.tspackages/cli-old/src/utils/renderTitle.tspackages/cli-old/src/utils/renderVersionWarning.tspackages/cli-old/src/utils/ts-morph.tspackages/cli-old/src/utils/validateAppName.tspackages/cli-old/src/utils/validateImportAlias.tspackages/cli-old/template/extras/_cursor/conditional-rules/nextjs-framework.mdcpackages/cli-old/template/extras/_cursor/conditional-rules/npm.mdcpackages/cli-old/template/extras/_cursor/conditional-rules/pnpm.mdcpackages/cli-old/template/extras/_cursor/conditional-rules/yarn.mdcpackages/cli-old/template/extras/_cursor/rules/cursor-rules.mdcpackages/cli-old/template/extras/_cursor/rules/filemaker-api.mdcpackages/cli-old/template/extras/_cursor/rules/troubleshooting-patterns.mdcpackages/cli-old/template/extras/_cursor/rules/ui-components.mdcpackages/cli-old/template/extras/config/drizzle-config-mysql.tspackages/cli-old/template/extras/config/drizzle-config-postgres.tspackages/cli-old/template/extras/config/drizzle-config-sqlite.tspackages/cli-old/template/extras/config/fmschema.config.mjspackages/cli-old/template/extras/config/get-query-client.tspackages/cli-old/template/extras/config/postcss.config.cjspackages/cli-old/template/extras/config/query-provider-vite.tsxpackages/cli-old/template/extras/config/query-provider.tsxpackages/cli-old/template/extras/emailProviders/none/email.tsxpackages/cli-old/template/extras/emailProviders/plunk/email.tsxpackages/cli-old/template/extras/emailProviders/plunk/service.tspackages/cli-old/template/extras/emailProviders/resend/email.tsxpackages/cli-old/template/extras/emailProviders/resend/service.tspackages/cli-old/template/extras/emailTemplates/auth-code.tsxpackages/cli-old/template/extras/emailTemplates/generic.tsxpackages/cli-old/template/extras/fmaddon-auth/app/(main)/auth/profile/actions.tspackages/cli-old/template/extras/fmaddon-auth/app/(main)/auth/profile/page.tsxpackages/cli-old/template/extras/fmaddon-auth/app/(main)/auth/profile/profile-form.tsxpackages/cli-old/template/extras/fmaddon-auth/app/(main)/auth/profile/reset-password-form.tsxpackages/cli-old/template/extras/fmaddon-auth/app/(main)/auth/profile/schema.tspackages/cli-old/template/extras/fmaddon-auth/app/auth/forgot-password/actions.tspackages/cli-old/template/extras/fmaddon-auth/app/auth/forgot-password/forgot-form.tsxpackages/cli-old/template/extras/fmaddon-auth/app/auth/forgot-password/page.tsxpackages/cli-old/template/extras/fmaddon-auth/app/auth/forgot-password/schema.tspackages/cli-old/template/extras/fmaddon-auth/app/auth/login/actions.tspackages/cli-old/template/extras/fmaddon-auth/app/auth/login/login-form.tsxpackages/cli-old/template/extras/fmaddon-auth/app/auth/login/page.tsxpackages/cli-old/template/extras/fmaddon-auth/app/auth/login/schema.tspackages/cli-old/template/extras/fmaddon-auth/app/auth/reset-password/actions.tspackages/cli-old/template/extras/fmaddon-auth/app/auth/reset-password/page.tsxpackages/cli-old/template/extras/fmaddon-auth/app/auth/reset-password/reset-password-form.tsxpackages/cli-old/template/extras/fmaddon-auth/app/auth/reset-password/schema.tspackages/cli-old/template/extras/fmaddon-auth/app/auth/reset-password/verify-email/actions.tspackages/cli-old/template/extras/fmaddon-auth/app/auth/reset-password/verify-email/page.tsxpackages/cli-old/template/extras/fmaddon-auth/app/auth/reset-password/verify-email/schema.tspackages/cli-old/template/extras/fmaddon-auth/app/auth/reset-password/verify-email/verify-email-form.tsxpackages/cli-old/template/extras/fmaddon-auth/app/auth/signup/actions.tspackages/cli-old/template/extras/fmaddon-auth/app/auth/signup/page.tsxpackages/cli-old/template/extras/fmaddon-auth/app/auth/signup/schema.tspackages/cli-old/template/extras/fmaddon-auth/app/auth/signup/signup-form.tsxpackages/cli-old/template/extras/fmaddon-auth/app/auth/verify-email/actions.tspackages/cli-old/template/extras/fmaddon-auth/app/auth/verify-email/email-verification-form.tsxpackages/cli-old/template/extras/fmaddon-auth/app/auth/verify-email/page.tsxpackages/cli-old/template/extras/fmaddon-auth/app/auth/verify-email/resend-button.tsxpackages/cli-old/template/extras/fmaddon-auth/app/auth/verify-email/schema.tspackages/cli-old/template/extras/fmaddon-auth/components/auth/actions.tspackages/cli-old/template/extras/fmaddon-auth/components/auth/protect.tsxpackages/cli-old/template/extras/fmaddon-auth/components/auth/redirect.tsxpackages/cli-old/template/extras/fmaddon-auth/components/auth/use-user.tspackages/cli-old/template/extras/fmaddon-auth/components/auth/user-menu.tsxpackages/cli-old/template/extras/fmaddon-auth/emails/auth-code.tsxpackages/cli-old/template/extras/fmaddon-auth/middleware.tspackages/cli-old/template/extras/fmaddon-auth/server/auth/utils/email-verification.tspackages/cli-old/template/extras/fmaddon-auth/server/auth/utils/encryption.tspackages/cli-old/template/extras/fmaddon-auth/server/auth/utils/index.tspackages/cli-old/template/extras/fmaddon-auth/server/auth/utils/password-reset.tspackages/cli-old/template/extras/fmaddon-auth/server/auth/utils/password.tspackages/cli-old/template/extras/fmaddon-auth/server/auth/utils/redirect.tspackages/cli-old/template/extras/fmaddon-auth/server/auth/utils/session.tspackages/cli-old/template/extras/fmaddon-auth/server/auth/utils/user.tspackages/cli-old/template/extras/prisma/schema/base-planetscale.prismapackages/cli-old/template/extras/prisma/schema/base.prismapackages/cli-old/template/extras/prisma/schema/with-auth-planetscale.prismapackages/cli-old/template/extras/prisma/schema/with-auth.prismapackages/cli-old/template/extras/src/app/_components/post-tw.tsxpackages/cli-old/template/extras/src/app/_components/post.tsxpackages/cli-old/template/extras/src/app/api/auth/[...nextauth]/route.tspackages/cli-old/template/extras/src/app/api/trpc/[trpc]/route.tspackages/cli-old/template/extras/src/app/clerk-auth/layout.tsxpackages/cli-old/template/extras/src/app/clerk-auth/signin/[[...sign-in]]/page.tsxpackages/cli-old/template/extras/src/app/clerk-auth/signup/[[...sign-up]]/page.tsxpackages/cli-old/template/extras/src/app/layout/base.tsxpackages/cli-old/template/extras/src/app/layout/main-shell.tsxpackages/cli-old/template/extras/src/app/layout/with-trpc-tw.tsxpackages/cli-old/template/extras/src/app/layout/with-trpc.tsxpackages/cli-old/template/extras/src/app/layout/with-tw.tsxpackages/cli-old/template/extras/src/app/next-auth/layout.tsxpackages/cli-old/template/extras/src/app/next-auth/signin/page.tsxpackages/cli-old/template/extras/src/app/next-auth/signup/action.tspackages/cli-old/template/extras/src/app/next-auth/signup/page.tsxpackages/cli-old/template/extras/src/app/next-auth/signup/validation.tspackages/cli-old/template/extras/src/app/page/base.tsxpackages/cli-old/template/extras/src/app/page/with-auth-trpc-tw.tsxpackages/cli-old/template/extras/src/app/page/with-auth-trpc.tsxpackages/cli-old/template/extras/src/app/page/with-trpc-tw.tsxpackages/cli-old/template/extras/src/app/page/with-trpc.tsxpackages/cli-old/template/extras/src/app/page/with-tw.tsxpackages/cli-old/template/extras/src/components/clerk-auth/clerk-provider.tsxpackages/cli-old/template/extras/src/components/clerk-auth/user-menu-mobile.tsxpackages/cli-old/template/extras/src/components/clerk-auth/user-menu.tsxpackages/cli-old/template/extras/src/components/next-auth/next-auth-provider.tsxpackages/cli-old/template/extras/src/components/next-auth/user-menu-mobile.tsxpackages/cli-old/template/extras/src/components/next-auth/user-menu.tsxpackages/cli-old/template/extras/src/env/with-auth.tspackages/cli-old/template/extras/src/env/with-clerk.tspackages/cli-old/template/extras/src/index.module.csspackages/cli-old/template/extras/src/middleware/clerk.tspackages/cli-old/template/extras/src/middleware/next-auth.tspackages/cli-old/template/extras/src/pages/_app/base.tsxpackages/cli-old/template/extras/src/pages/_app/with-auth-trpc-tw.tsxpackages/cli-old/template/extras/src/pages/_app/with-auth-trpc.tsxpackages/cli-old/template/extras/src/pages/_app/with-auth-tw.tsxpackages/cli-old/template/extras/src/pages/_app/with-auth.tsxpackages/cli-old/template/extras/src/pages/_app/with-trpc-tw.tsxpackages/cli-old/template/extras/src/pages/_app/with-trpc.tsxpackages/cli-old/template/extras/src/pages/_app/with-tw.tsxpackages/cli-old/template/extras/src/pages/api/auth/[...nextauth].tspackages/cli-old/template/extras/src/pages/api/trpc/[trpc].tspackages/cli-old/template/extras/src/pages/index/base.tsxpackages/cli-old/template/extras/src/pages/index/with-auth-trpc-tw.tsxpackages/cli-old/template/extras/src/pages/index/with-auth-trpc.tsxpackages/cli-old/template/extras/src/pages/index/with-trpc-tw.tsxpackages/cli-old/template/extras/src/pages/index/with-trpc.tsxpackages/cli-old/template/extras/src/pages/index/with-tw.tsxpackages/cli-old/template/extras/src/server/api/root.tspackages/cli-old/template/extras/src/server/api/routers/post/base.tspackages/cli-old/template/extras/src/server/api/routers/post/with-auth-drizzle.tspackages/cli-old/template/extras/src/server/api/routers/post/with-auth-prisma.tspackages/cli-old/template/extras/src/server/api/routers/post/with-auth.tspackages/cli-old/template/extras/src/server/api/routers/post/with-drizzle.tspackages/cli-old/template/extras/src/server/api/routers/post/with-prisma.tspackages/cli-old/template/extras/src/server/api/trpc-app/base.tspackages/cli-old/template/extras/src/server/api/trpc-app/with-auth-db.tspackages/cli-old/template/extras/src/server/api/trpc-app/with-auth.tspackages/cli-old/template/extras/src/server/api/trpc-app/with-db.tspackages/cli-old/template/extras/src/server/api/trpc-pages/base.tspackages/cli-old/template/extras/src/server/api/trpc-pages/with-auth-db.tspackages/cli-old/template/extras/src/server/api/trpc-pages/with-auth.tspackages/cli-old/template/extras/src/server/api/trpc-pages/with-db.tspackages/cli-old/template/extras/src/server/data/users.tspackages/cli-old/template/extras/src/server/db/db-prisma-planetscale.tspackages/cli-old/template/extras/src/server/db/db-prisma.tspackages/cli-old/template/extras/src/server/db/index-drizzle/with-mysql.tspackages/cli-old/template/extras/src/server/db/index-drizzle/with-planetscale.tspackages/cli-old/template/extras/src/server/db/index-drizzle/with-postgres.tspackages/cli-old/template/extras/src/server/db/index-drizzle/with-sqlite.tspackages/cli-old/template/extras/src/server/db/schema-drizzle/base-mysql.tspackages/cli-old/template/extras/src/server/db/schema-drizzle/base-planetscale.tspackages/cli-old/template/extras/src/server/db/schema-drizzle/base-postgres.tspackages/cli-old/template/extras/src/server/db/schema-drizzle/base-sqlite.tspackages/cli-old/template/extras/src/server/db/schema-drizzle/with-auth-mysql.tspackages/cli-old/template/extras/src/server/db/schema-drizzle/with-auth-planetscale.tspackages/cli-old/template/extras/src/server/db/schema-drizzle/with-auth-postgres.tspackages/cli-old/template/extras/src/server/db/schema-drizzle/with-auth-sqlite.tspackages/cli-old/template/extras/src/server/next-auth/base.tspackages/cli-old/template/extras/src/server/next-auth/password.tspackages/cli-old/template/extras/src/server/next-auth/with-drizzle.tspackages/cli-old/template/extras/src/server/next-auth/with-prisma.tspackages/cli-old/template/extras/src/trpc/query-client.tspackages/cli-old/template/extras/src/trpc/react.tsxpackages/cli-old/template/extras/src/trpc/server.tspackages/cli-old/template/extras/src/utils/api.tspackages/cli-old/template/extras/start-database/mysql.shpackages/cli-old/template/extras/start-database/postgres.shpackages/cli-old/template/fm-addon/ProofKitAuth/de.xmlpackages/cli-old/template/fm-addon/ProofKitAuth/en.xmlpackages/cli-old/template/fm-addon/ProofKitAuth/es.xmlpackages/cli-old/template/fm-addon/ProofKitAuth/fr.xmlpackages/cli-old/template/fm-addon/ProofKitAuth/info.jsonpackages/cli-old/template/fm-addon/ProofKitAuth/info_de.jsonpackages/cli-old/template/fm-addon/ProofKitAuth/info_en.jsonpackages/cli-old/template/fm-addon/ProofKitAuth/info_es.jsonpackages/cli-old/template/fm-addon/ProofKitAuth/info_fr.jsonpackages/cli-old/template/fm-addon/ProofKitAuth/info_it.jsonpackages/cli-old/template/fm-addon/ProofKitAuth/info_ja.jsonpackages/cli-old/template/fm-addon/ProofKitAuth/info_ko.jsonpackages/cli-old/template/fm-addon/ProofKitAuth/info_nl.jsonpackages/cli-old/template/fm-addon/ProofKitAuth/info_pt.jsonpackages/cli-old/template/fm-addon/ProofKitAuth/info_sv.jsonpackages/cli-old/template/fm-addon/ProofKitAuth/info_zh.jsonpackages/cli-old/template/fm-addon/ProofKitAuth/it.xmlpackages/cli-old/template/fm-addon/ProofKitAuth/ja.xmlpackages/cli-old/template/fm-addon/ProofKitAuth/ko.xmlpackages/cli-old/template/fm-addon/ProofKitAuth/nl.xmlpackages/cli-old/template/fm-addon/ProofKitAuth/pt.xmlpackages/cli-old/template/fm-addon/ProofKitAuth/sv.xmlpackages/cli-old/template/fm-addon/ProofKitAuth/template.xmlpackages/cli-old/template/fm-addon/ProofKitAuth/zh.xmlpackages/cli-old/template/fm-addon/ProofKitWV/de.xmlpackages/cli-old/template/fm-addon/ProofKitWV/en.xmlpackages/cli-old/template/fm-addon/ProofKitWV/es.xmlpackages/cli-old/template/fm-addon/ProofKitWV/fr.xml
✅ Files skipped from review due to trivial changes (16)
- packages/cli-old/template/extras/src/index.module.css
- packages/cli-old/template/fm-addon/ProofKitAuth/fr.xml
- packages/cli-old/template/fm-addon/ProofKitAuth/info_it.json
- btca.config.jsonc
- packages/cli-old/template/fm-addon/ProofKitAuth/info_es.json
- packages/cli-old/README.md
- biome.json
- packages/cli-old/template/extras/config/postcss.config.cjs
- packages/cli-old/template/fm-addon/ProofKitAuth/info_en.json
- packages/cli-old/template/fm-addon/ProofKitAuth/info_sv.json
- packages/cli-old/template/fm-addon/ProofKitAuth/info_ja.json
- packages/cli-old/template/fm-addon/ProofKitAuth/info_pt.json
- packages/cli-old/template/extras/_cursor/conditional-rules/nextjs-framework.mdc
- packages/cli-old/template/fm-addon/ProofKitAuth/info_fr.json
- packages/cli-old/template/fm-addon/ProofKitAuth/info_ko.json
- apps/docs/src/app/(home)/page.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/release.yml
- .github/workflows/continuous-release.yml
Comment on lines
+1
to
+5
| packageExtensions: | ||
| chalk@5.0.1: | ||
| dependencies: | ||
| "#ansi-styles": npm:ansi-styles@6.1.0 | ||
| "#supports-color": npm:supports-color@9.2.2 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify packageExtensions syntax and chalk dependencies
# Check if ansi-styles@6.1.0 and supports-color@9.2.2 exist on npm
echo "=== Checking if specified versions exist on npm ==="
curl -s https://registry.npmjs.org/ansi-styles | jq -r '.versions | keys[] | select(. == "6.1.0")'
curl -s https://registry.npmjs.org/supports-color | jq -r '.versions | keys[] | select(. == "9.2.2")'
# Check chalk@5.0.1 actual dependencies
echo -e "\n=== Checking chalk@5.0.1 dependencies ==="
curl -s https://registry.npmjs.org/chalk/5.0.1 | jq '.dependencies, .peerDependencies'
# Check Yarn version in use
echo -e "\n=== Checking Yarn version ==="
fd -t f 'package.json' packages/cli-old --exec cat {} | jq -r '.packageManager // "not specified"'Repository: proofgeist/proofkit
Length of output: 266
🌐 Web query:
Yarn packageExtensions syntax with subpath imports
💡 Result:
packageExtensions keys are package descriptors (<packageName>@<semverRange>), not module-specifier strings, so you can’t target a subpath import like react-dom/client (or lodash/fp) in the key. Yarn applies the extension to the package whose version matches the range. [1]
Correct syntax
# .yarnrc.yml
packageExtensions:
react-dom@*:
dependencies:
scheduler: "^0.23.0"
"@scope/pkg@^1.2.0":
peerDependencies:
react: "*"
peerDependenciesMeta:
react:
optional: trueEach key is “a descriptor covering a semver range”; Yarn merges in dependencies, peerDependencies, and peerDependenciesMeta. [1][2]
How this relates to subpath imports
If some package is doing import "react-dom/client", the dependency it needs to list (and the one you can fix via packageExtensions) is still react-dom, because client is just a file/subpath inside that package (often governed by the package’s exports). packageExtensions won’t change whether a given subpath is exported; it only amends dependency metadata. [1]
Sources:
[1] Yarn .yarnrc.yml config docs (packageExtensions section) (documents.devdocs.io)
[2] Yarn configuration type for packageExtensions (yarnpkg.com)
Citations:
- 1: https://documents.devdocs.io/yarn~berry/configuration/yarnrc.html
- 2: https://yarnpkg.com/api/yarnpkg-core/interface/ConfigurationValueMap
Remove the "#" prefix from dependency keys in packageExtensions — this syntax is invalid.
The keys "#ansi-styles" and "#supports-color" violate Yarn's packageExtensions schema. According to Yarn documentation, packageExtensions keys must be package descriptors in the format <packageName>@<semverRange>, not subpath import names with # prefixes.
The correct syntax is:
Corrected packageExtensions syntax
packageExtensions:
ansi-styles@6.1.0:
# ...
supports-color@9.2.2:
# ...Or using semver ranges:
packageExtensions:
ansi-styles@*:
# ...
supports-color@*:
# ...Additionally, chalk@5.0.1 lists no dependencies on npm, so these overrides may be unnecessary. Verify whether these packageExtensions additions are required or if they can be removed entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-old/.yarnrc.yml` around lines 1 - 5, The packageExtensions block
uses invalid keys "#ansi-styles" and "#supports-color"; replace those entries
under packageExtensions -> chalk@5.0.1 with valid package descriptors such as
ansi-styles@6.1.0 and supports-color@9.2.2 (or use semver ranges like
ansi-styles@* and supports-color@*) or remove the packageExtensions for
chalk@5.0.1 entirely if those dependency overrides are not required; update the
keys and/or remove the overrides so the packageExtensions schema uses proper
"<packageName>@<semverRange>" descriptors.
Comment on lines
+378
to
+410
| while (fmsVersion === null) { | ||
| const serverUrl = | ||
| serverUrlToUse ?? | ||
| abortIfCancel( | ||
| await p.text({ | ||
| message: `What is the URL of your FileMaker Server?\n${chalk.cyan("TIP: You can copy any valid path on the server and paste it here.")}`, | ||
| validate: (value) => { | ||
| try { | ||
| // try to make sure the url is https | ||
| let normalizedValue = value; | ||
| if (!normalizedValue.startsWith("https://")) { | ||
| if (normalizedValue.startsWith("http://")) { | ||
| normalizedValue = normalizedValue.replace("http://", "https://"); | ||
| } else { | ||
| normalizedValue = `https://${normalizedValue}`; | ||
| } | ||
| } | ||
|
|
||
| // try to make sure the url is valid | ||
| new URL(normalizedValue); | ||
| return; | ||
| } catch { | ||
| return "Please enter a valid URL"; | ||
| } | ||
| }, | ||
| }), | ||
| ); | ||
|
|
||
| try { | ||
| url = new URL(serverUrl); | ||
| } catch { | ||
| p.log.error(`Invalid URL: ${serverUrl.toString()}`); | ||
| continue; |
There was a problem hiding this comment.
Normalize accepted server URLs before parsing them.
The prompt validator accepts example.com and http://... by rewriting them to HTTPS internally, but Line 407 still parses the original string. That makes accepted input fail on the next step, and if the bad value came from --server, the loop keeps retrying the same string forever.
Suggested fix
+function normalizeServerUrl(value: string): string {
+ if (value.startsWith("https://")) {
+ return value;
+ }
+ if (value.startsWith("http://")) {
+ return value.replace("http://", "https://");
+ }
+ return `https://${value}`;
+}
+
async function getValidFileMakerServerUrl(defaultServerUrl?: string | undefined): Promise<{
url: URL;
fmsVersion: SemVer;
@@
- const serverUrl =
+ const rawServerUrl =
serverUrlToUse ??
abortIfCancel(
await p.text({
@@
}),
);
+ const serverUrl = normalizeServerUrl(rawServerUrl);
try {
url = new URL(serverUrl);
} catch {
- p.log.error(`Invalid URL: ${serverUrl.toString()}`);
+ p.log.error(`Invalid URL: ${serverUrl}`);
+ serverUrlToUse = undefined;
continue;
}📝 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
| while (fmsVersion === null) { | |
| const serverUrl = | |
| serverUrlToUse ?? | |
| abortIfCancel( | |
| await p.text({ | |
| message: `What is the URL of your FileMaker Server?\n${chalk.cyan("TIP: You can copy any valid path on the server and paste it here.")}`, | |
| validate: (value) => { | |
| try { | |
| // try to make sure the url is https | |
| let normalizedValue = value; | |
| if (!normalizedValue.startsWith("https://")) { | |
| if (normalizedValue.startsWith("http://")) { | |
| normalizedValue = normalizedValue.replace("http://", "https://"); | |
| } else { | |
| normalizedValue = `https://${normalizedValue}`; | |
| } | |
| } | |
| // try to make sure the url is valid | |
| new URL(normalizedValue); | |
| return; | |
| } catch { | |
| return "Please enter a valid URL"; | |
| } | |
| }, | |
| }), | |
| ); | |
| try { | |
| url = new URL(serverUrl); | |
| } catch { | |
| p.log.error(`Invalid URL: ${serverUrl.toString()}`); | |
| continue; | |
| function normalizeServerUrl(value: string): string { | |
| if (value.startsWith("https://")) { | |
| return value; | |
| } | |
| if (value.startsWith("http://")) { | |
| return value.replace("http://", "https://"); | |
| } | |
| return `https://${value}`; | |
| } | |
| while (fmsVersion === null) { | |
| const rawServerUrl = | |
| serverUrlToUse ?? | |
| abortIfCancel( | |
| await p.text({ | |
| message: `What is the URL of your FileMaker Server?\n${chalk.cyan("TIP: You can copy any valid path on the server and paste it here.")}`, | |
| validate: (value) => { | |
| try { | |
| // try to make sure the url is https | |
| let normalizedValue = value; | |
| if (!normalizedValue.startsWith("https://")) { | |
| if (normalizedValue.startsWith("http://")) { | |
| normalizedValue = normalizedValue.replace("http://", "https://"); | |
| } else { | |
| normalizedValue = `https://${normalizedValue}`; | |
| } | |
| } | |
| // try to make sure the url is valid | |
| new URL(normalizedValue); | |
| return; | |
| } catch { | |
| return "Please enter a valid URL"; | |
| } | |
| }, | |
| }), | |
| ); | |
| const serverUrl = normalizeServerUrl(rawServerUrl); | |
| try { | |
| url = new URL(serverUrl); | |
| } catch { | |
| p.log.error(`Invalid URL: ${serverUrl}`); | |
| serverUrlToUse = undefined; | |
| continue; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-old/src/cli/add/data-source/filemaker.ts` around lines 378 -
410, The validator currently normalizes input to HTTPS (normalizedValue) but the
code still parses the original serverUrl, causing valid inputs like
"example.com" or "http://..." to fail; fix this by applying the same
normalization logic to serverUrl before calling new URL(serverUrl) (and when
using serverUrlToUse from flags), i.e., compute a normalizedServerUrl that
ensures "https://" and is a valid URL (same steps as in the validator) and then
do url = new URL(normalizedServerUrl); update references to ensure the loop and
any --server path use the normalized value so the validator and parser behave
consistently.
Comment on lines
+32
to
+45
| export const makeAddDataSourceCommand = () => { | ||
| const addDataSourceCommand = new Command("data"); | ||
| addDataSourceCommand.description("Add a new data source to your project"); | ||
| addDataSourceCommand.addOption(ciOption); | ||
| addDataSourceCommand.addOption(nonInteractiveOption); | ||
|
|
||
| addDataSourceCommand.hook("preAction", (_thisCommand, actionCommand) => { | ||
| initProgramState(actionCommand.opts()); | ||
| const settings = ensureProofKitProject({ commandName: "add" }); | ||
| actionCommand.setOptionValue("settings", settings); | ||
| }); | ||
|
|
||
| // addDataSourceCommand.action(); | ||
| return addDataSourceCommand; |
There was a problem hiding this comment.
Bind runAddDataSourceCommand before returning the subcommand.
makeAddDataSourceCommand() never registers an action, so proofkit add data will parse the command and then exit without running any workflow.
Minimal fix
export const makeAddDataSourceCommand = () => {
const addDataSourceCommand = new Command("data");
addDataSourceCommand.description("Add a new data source to your project");
addDataSourceCommand.addOption(ciOption);
addDataSourceCommand.addOption(nonInteractiveOption);
@@
- // addDataSourceCommand.action();
+ addDataSourceCommand.action(runAddDataSourceCommand);
return addDataSourceCommand;
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-old/src/cli/add/data-source/index.ts` around lines 32 - 45, The
subcommand created in makeAddDataSourceCommand never registers its action, so
addDataSourceCommand should call .action(...) to attach the handler; locate
makeAddDataSourceCommand and add a line like
addDataSourceCommand.action(runAddDataSourceCommand) (or pass a bound wrapper if
runAddDataSourceCommand is a method needing context) before returning
addDataSourceCommand so the runAddDataSourceCommand function is executed when
the "proofkit add data" command runs.
Comment on lines
+34
to
+65
| const data = await new Promise<WizardResponse>((resolve) => { | ||
| const pollingInterval = setInterval(() => { | ||
| axios | ||
| .get<{ response: WizardResponse }>(`${url.origin}/otto/api/cli/checkHash/${hash}`, { | ||
| headers: { | ||
| "Accept-Encoding": "deflate", | ||
| }, | ||
| }) | ||
| .then((result) => { | ||
| resolve(result.data.response); | ||
| clearTimeout(timeout); | ||
| clearInterval(pollingInterval); | ||
| axios | ||
| .delete(`${url.origin}/otto/api/cli/checkHash/${hash}`, { | ||
| headers: { | ||
| "Accept-Encoding": "deflate", | ||
| }, | ||
| }) | ||
| .catch(() => { | ||
| // Ignore cleanup errors | ||
| }); | ||
| }) | ||
| .catch(() => { | ||
| // noop - just try again | ||
| }); | ||
| }, 500); | ||
|
|
||
| const timeout = setTimeout(() => { | ||
| clearInterval(pollingInterval); | ||
| loginSpinner.stop("Login timed out. No worries - it happens to the best of us."); | ||
| }, 180_000); // 3 minutes | ||
| }); |
There was a problem hiding this comment.
Timeout path never settles the login promise
Line 61–64 stops polling, but the promise created at Line 34 is never resolved/rejected on timeout. This leaves await hanging indefinitely.
🐛 Proposed fix
- const data = await new Promise<WizardResponse>((resolve) => {
+ const data = await new Promise<WizardResponse>((resolve, reject) => {
@@
const timeout = setTimeout(() => {
clearInterval(pollingInterval);
loginSpinner.stop("Login timed out. No worries - it happens to the best of us.");
+ reject(new Error("Login timed out after 3 minutes."));
}, 180_000); // 3 minutes
});📝 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
| const data = await new Promise<WizardResponse>((resolve) => { | |
| const pollingInterval = setInterval(() => { | |
| axios | |
| .get<{ response: WizardResponse }>(`${url.origin}/otto/api/cli/checkHash/${hash}`, { | |
| headers: { | |
| "Accept-Encoding": "deflate", | |
| }, | |
| }) | |
| .then((result) => { | |
| resolve(result.data.response); | |
| clearTimeout(timeout); | |
| clearInterval(pollingInterval); | |
| axios | |
| .delete(`${url.origin}/otto/api/cli/checkHash/${hash}`, { | |
| headers: { | |
| "Accept-Encoding": "deflate", | |
| }, | |
| }) | |
| .catch(() => { | |
| // Ignore cleanup errors | |
| }); | |
| }) | |
| .catch(() => { | |
| // noop - just try again | |
| }); | |
| }, 500); | |
| const timeout = setTimeout(() => { | |
| clearInterval(pollingInterval); | |
| loginSpinner.stop("Login timed out. No worries - it happens to the best of us."); | |
| }, 180_000); // 3 minutes | |
| }); | |
| const data = await new Promise<WizardResponse>((resolve, reject) => { | |
| const pollingInterval = setInterval(() => { | |
| axios | |
| .get<{ response: WizardResponse }>(`${url.origin}/otto/api/cli/checkHash/${hash}`, { | |
| headers: { | |
| "Accept-Encoding": "deflate", | |
| }, | |
| }) | |
| .then((result) => { | |
| resolve(result.data.response); | |
| clearTimeout(timeout); | |
| clearInterval(pollingInterval); | |
| axios | |
| .delete(`${url.origin}/otto/api/cli/checkHash/${hash}`, { | |
| headers: { | |
| "Accept-Encoding": "deflate", | |
| }, | |
| }) | |
| .catch(() => { | |
| // Ignore cleanup errors | |
| }); | |
| }) | |
| .catch(() => { | |
| // noop - just try again | |
| }); | |
| }, 500); | |
| const timeout = setTimeout(() => { | |
| clearInterval(pollingInterval); | |
| loginSpinner.stop("Login timed out. No worries - it happens to the best of us."); | |
| reject(new Error("Login timed out after 3 minutes.")); | |
| }, 180_000); // 3 minutes | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-old/src/cli/ottofms.ts` around lines 34 - 65, The promise
created for polling (the const data = await new
Promise<WizardResponse>((resolve) => { ... })) never settles on the timeout
path; modify the timeout handler to both stop the spinner and settle the promise
(call the promise's reject or resolve) so awaiting code won't hang: capture both
resolve and reject in the Promise constructor, and in the timeout callback
clearInterval(pollingInterval), stop the loginSpinner, and call reject(new
Error("Login timed out")) (or resolve with an appropriate error/timeout
response) and also perform the same cleanup delete call if needed; update
references inside the block (pollingInterval, timeout, loginSpinner,
resolve/reject) accordingly.
Comment on lines
+102
to
+106
| if (dataSource?.type === "fm") { | ||
| // For FileMaker data sources, remove from fmschema.config.mjs | ||
| removeFromFmschemaConfig({ | ||
| dataSourceName, | ||
| }); |
There was a problem hiding this comment.
Await the fmschema update before regenerating types.
removeFromFmschemaConfig(...) is async. Without await, Line 122 can run typegen against stale config, and any write failure becomes an unhandled rejection.
Suggested fix
- removeFromFmschemaConfig({
+ await removeFromFmschemaConfig({
dataSourceName,
});📝 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
| if (dataSource?.type === "fm") { | |
| // For FileMaker data sources, remove from fmschema.config.mjs | |
| removeFromFmschemaConfig({ | |
| dataSourceName, | |
| }); | |
| if (dataSource?.type === "fm") { | |
| // For FileMaker data sources, remove from fmschema.config.mjs | |
| await removeFromFmschemaConfig({ | |
| dataSourceName, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-old/src/cli/remove/data-source.ts` around lines 102 - 106, The
call to removeFromFmschemaConfig is asynchronous but currently not awaited, so
await removeFromFmschemaConfig({ dataSourceName }) inside the block where
dataSource?.type === "fm" (and ensure the enclosing function is async), before
invoking the subsequent type generation step (e.g., the typegen/regenerate call)
to avoid running typegen against stale config and to prevent unhandled
rejections; optionally wrap the await in a try/catch to surface/log errors from
removeFromFmschemaConfig.
| @@ -0,0 +1,9 @@ | |||
| /** @type {import("@proofkit/fmdapi/dist/utils/typegen/types.d.ts").GenerateSchemaOptions} */ | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if GenerateSchemaOptions is exported from the main package entry or a public subpath
# Search for exports of GenerateSchemaOptions in the fmdapi package
fd -t f "package.json" --full-path ".*fmdapi.*" --exec cat {} \; 2>/dev/null | jq -r '.exports // empty'
# Check if the type is re-exported from the main index
ast-grep --pattern 'export { $$$GenerateSchemaOptions$$$ }'
# Alternative: search for the type export declarations
rg -n "GenerateSchemaOptions" --type ts -g '!dist' -g '!node_modules'Repository: proofgeist/proofkit
Length of output: 255
🏁 Script executed:
# Check the fmdapi package.json for exports field
cat packages/fmdapi/package.json | head -50Repository: proofgeist/proofkit
Length of output: 1456
🏁 Script executed:
# Verify the actual structure of type files in fmdapi
fd -t f "types\." packages/fmdapi --exclude dist --exclude node_modulesRepository: proofgeist/proofkit
Length of output: 187
🏁 Script executed:
# Check if dist/ actually exists and where the type file is located
find packages/fmdapi -name "types.js" -o -name "types.d.ts" 2>/dev/nullRepository: proofgeist/proofkit
Length of output: 45
🏁 Script executed:
# Find the typegen directory and files
find packages/fmdapi -path "*typegen*" -type f 2>/dev/nullRepository: proofgeist/proofkit
Length of output: 95
🏁 Script executed:
# Check the actual content of the typegen types file
find packages/fmdapi/src -path "*typegen*" -type f -exec cat {} \;Repository: proofgeist/proofkit
Length of output: 45
🏁 Script executed:
# Read the actual test file that's being imported
cat packages/fmdapi/tests/typegen.tsRepository: proofgeist/proofkit
Length of output: 964
🏁 Script executed:
# Search for where GenerateSchemaOptions is actually defined
rg -n "GenerateSchemaOptions" packages/fmdapi --type ts --type js -A 5Repository: proofgeist/proofkit
Length of output: 779
🏁 Script executed:
# Verify the actual dist structure for the built package
ls -la packages/fmdapi/dist/esm/ 2>/dev/null | head -20Repository: proofgeist/proofkit
Length of output: 45
🏁 Script executed:
# Check if there's a utils.d.ts file and what it contains
cat packages/fmdapi/dist/esm/utils.d.ts 2>/dev/null | head -50Repository: proofgeist/proofkit
Length of output: 45
🏁 Script executed:
# Search for where typegen is actually exported from in src
cat packages/fmdapi/src/utils/typegen/index.js 2>/dev/null || cat packages/fmdapi/src/utils/typegen/index.ts 2>/dev/nullRepository: proofgeist/proofkit
Length of output: 45
🏁 Script executed:
# Find all files in the typegen directory
find packages/fmdapi/src -path "*typegen*" -type fRepository: proofgeist/proofkit
Length of output: 45
🏁 Script executed:
# Check what's in the utils directory
ls -la packages/fmdapi/src/utils/Repository: proofgeist/proofkit
Length of output: 136
🏁 Script executed:
# Search for types.ts or types.js specifically
fd "types\.(ts|js)$" packages/fmdapi/srcRepository: proofgeist/proofkit
Length of output: 187
🏁 Script executed:
# List the actual structure of packages/fmdapi/src
ls -la packages/fmdapi/src/Repository: proofgeist/proofkit
Length of output: 540
🏁 Script executed:
# Look at all files in packages/fmdapi/src recursively to understand structure
find packages/fmdapi/src -type f -name "*.ts" -o -name "*.js" | head -30Repository: proofgeist/proofkit
Length of output: 632
🏁 Script executed:
# Check what the template file is trying to import - maybe the package doesn't have this type yet
cat packages/cli-old/template/extras/config/fmschema.config.mjsRepository: proofgeist/proofkit
Length of output: 338
🏁 Script executed:
# Check if the typegen type even exists anywhere in the codebase
rg -r "type GenerateSchemaOptions" --type ts --type jsRepository: proofgeist/proofkit
Length of output: 459
🏁 Script executed:
# Check the utils.ts file to see what it exports
cat packages/fmdapi/src/utils.tsRepository: proofgeist/proofkit
Length of output: 1309
🏁 Script executed:
# Check if there's a typegen feature at all
rg -r "typegen" --type ts --type js --type mjs packages/fmdapiRepository: proofgeist/proofkit
Length of output: 90
🏁 Script executed:
# Search for typegen in the codebase properly
rg "typegen" packages/fmdapi -A 2 -B 2Repository: proofgeist/proofkit
Length of output: 7831
🏁 Script executed:
# Check the cli-old directory to understand the template structure
find packages/cli-old -type f -name "*.ts" -o -name "*.js" -o -name "*.mjs" | head -20Repository: proofgeist/proofkit
Length of output: 1081
🏁 Script executed:
# Look for any typegen references in the entire repository
rg "GenerateSchemaOptions\|typegen" --type ts --type jsRepository: proofgeist/proofkit
Length of output: 45
🏁 Script executed:
# Check if typegen is a separate package in the monorepo
ls -la packages/ | grep -i typegenRepository: proofgeist/proofkit
Length of output: 119
🏁 Script executed:
# Look at the cli-old add fmschema command to understand what the template should be
cat packages/cli-old/src/cli/add/fmschema.tsRepository: proofgeist/proofkit
Length of output: 7212
🏁 Script executed:
# Check if there's a separate typegen package
find packages -maxdepth 1 -type d | sortRepository: proofgeist/proofkit
Length of output: 234
🏁 Script executed:
# Check the typegen package structure
ls -la packages/typegen/srcRepository: proofgeist/proofkit
Length of output: 941
🏁 Script executed:
# Look for the GenerateSchemaOptions type in the typegen package
rg "GenerateSchemaOptions" packages/typegen --type ts --type js -B 2 -A 5Repository: proofgeist/proofkit
Length of output: 45
🏁 Script executed:
# Check the typegen package.json for exports
cat packages/typegen/package.json | grep -A 30 '"exports"'Repository: proofgeist/proofkit
Length of output: 709
🏁 Script executed:
# Check what's exported from `@proofkit/typegen/config`
cat packages/typegen/src/types.ts | head -50Repository: proofgeist/proofkit
Length of output: 2070
🏁 Script executed:
# Verify if GenerateSchemaOptions is exported
rg "export.*GenerateSchemaOptions" packages/typegen/src/types.tsRepository: proofgeist/proofkit
Length of output: 45
🏁 Script executed:
# Read more of the types.ts file to find GenerateSchemaOptions
cat packages/typegen/src/types.ts | grep -A 10 "GenerateSchemaOptions\|export type\|export const"Repository: proofgeist/proofkit
Length of output: 2340
🏁 Script executed:
# Search for GenerateSchemaOptions in all of typegen
rg "GenerateSchemaOptions" packages/typegenRepository: proofgeist/proofkit
Length of output: 45
🏁 Script executed:
# Check the index.ts to see what's being exported
cat packages/typegen/src/index.tsRepository: proofgeist/proofkit
Length of output: 411
🏁 Script executed:
# Double-check: search for GenerateSchemaOptions type definition
rg "type GenerateSchemaOptions\|interface GenerateSchemaOptions" --type ts --type jsRepository: proofgeist/proofkit
Length of output: 45
🏁 Script executed:
# Check what the test file is actually trying to do and if that import is valid
ls -la packages/fmdapi/src/utils/ 2>/dev/null || echo "No utils directory in fmdapi/src"Repository: proofgeist/proofkit
Length of output: 95
🏁 Script executed:
# Verify the actual template file again to confirm what it's trying to import
cat -n packages/cli-old/template/extras/config/fmschema.config.mjsRepository: proofgeist/proofkit
Length of output: 401
🏁 Script executed:
# Check if there's a TypegenConfig type or similar that should be used
rg "export type.*Config" packages/typegen/src/types.tsRepository: proofgeist/proofkit
Length of output: 216
🏁 Script executed:
# Verify the cli-old add fmschema command to see what it's actually using
cat packages/cli-old/src/cli/add/fmschema.ts | grep -i "import.*typegen\|ValueListsOptions"Repository: proofgeist/proofkit
Length of output: 194
🏁 Script executed:
# Check if the test file is even in the repo or part of tests that should exist
git ls-files packages/fmdapi/tests/typegen.ts 2>/dev/null || echo "File may not be tracked"Repository: proofgeist/proofkit
Length of output: 95
Use correct type from @proofkit/typegen package.
The import path @proofkit/fmdapi/dist/utils/typegen/types.d.ts does not exist, and GenerateSchemaOptions is not exported from fmdapi. The correct type is TypegenConfig from @proofkit/typegen/config:
Corrected JSDoc annotation
/** `@type` {import("@proofkit/typegen/config").TypegenConfig} */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-old/template/extras/config/fmschema.config.mjs` at line 1,
Replace the incorrect JSDoc type import that references GenerateSchemaOptions
from a non-existent `@proofkit/fmdapi` path: change the annotation to use the
correct exported type TypegenConfig from `@proofkit/typegen/config` (i.e., update
the JSDoc to import("@proofkit/typegen/config").TypegenConfig) so the file uses
the proper TypegenConfig symbol instead of GenerateSchemaOptions.
| import { AuthCodeEmail } from "@/emails/auth-code"; | ||
| import { render } from "@react-email/render"; | ||
|
|
||
| import { plunk } from "../services/plunk"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the actual location of plunk service and verify import paths
# Find all plunk service files
echo "=== Plunk service file locations ==="
fd -t f "service.ts" --full-path ".*plunk.*"
# Check the directory structure under emailProviders
echo ""
echo "=== Directory structure under emailProviders ==="
fd . packages/cli-old/template/extras/emailProviders --type d
# Check if there's a services folder
echo ""
echo "=== Looking for services folder ==="
fd -t d "services" packages/cli-old/template/extras/Repository: proofgeist/proofkit
Length of output: 465
🏁 Script executed:
#!/bin/bash
# List all files in the plunk directory
echo "=== Files in plunk directory ==="
ls -la packages/cli-old/template/extras/emailProviders/plunk/
# Check the full file contents of email.tsx to see the imports and context
echo ""
echo "=== Content of email.tsx ==="
cat packages/cli-old/template/extras/emailProviders/plunk/email.tsxRepository: proofgeist/proofkit
Length of output: 935
🏁 Script executed:
#!/bin/bash
# Check what's exported from service.ts
echo "=== Content of service.ts ==="
cat packages/cli-old/template/extras/emailProviders/plunk/service.tsRepository: proofgeist/proofkit
Length of output: 213
Fix incorrect import path for plunk service.
The import import { plunk } from "../services/plunk" references a non-existent directory. The service.ts file is in the same directory as email.tsx. Change to:
import { plunk } from "./service";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-old/template/extras/emailProviders/plunk/email.tsx` at line 4,
The import at the top of email.tsx is pointing to a non-existent
../services/plunk; update the import to reference the local service file
(service.ts) instead. Replace the incorrect import statement that references
plunk with one that imports plunk from "./service" so symbols used in this file
(plunk) resolve to the service.ts implementation in the same directory.
| @@ -0,0 +1,24 @@ | |||
| import { AuthCodeEmail } from "@/emails/auth-code"; | |||
|
|
|||
| import { resend } from "../services/resend"; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the actual location of resend service
# Find all resend service files
echo "=== Resend service file locations ==="
fd -t f "service.ts" --full-path ".*resend.*"
# List all files under emailProviders to understand structure
echo ""
echo "=== All files under emailProviders ==="
fd . packages/cli-old/template/extras/emailProviders -t fRepository: proofgeist/proofkit
Length of output: 578
Fix incorrect import path for Resend service module.
The import import { resend } from "../services/resend"; resolves to a non-existent directory. The service file is located in the same directory as email.tsx, not in a services/ subdirectory. Change to import { resend } from "./service";.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-old/template/extras/emailProviders/resend/email.tsx` at line 3,
The import in email.tsx for the Resend service is pointing to a non-existent
../services/resend; update the import to the correct local module path where the
service actually lives (change the import for the resend symbol in email.tsx to
import from "./service") so that the resend export is resolved from the same
directory as email.tsx.
Comment on lines
+1
to
+2
| import { Session } from "@/server/auth/utils/session"; | ||
| import { User } from "@/server/auth/utils/user"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify value-vs-type imports in the hook and inspect referenced session/user modules.
set -euo pipefail
echo "==> use-user.ts imports"
sed -n '1,30p' packages/cli-old/template/extras/fmaddon-auth/components/auth/use-user.ts
echo
echo "==> Candidate session/user modules under fmaddon-auth"
fd '^(session|user)\.ts$' packages/cli-old/template/extras/fmaddon-auth | while read -r f; do
echo "--- $f"
sed -n '1,120p' "$f"
doneRepository: proofgeist/proofkit
Length of output: 7580
Use type-only imports for Session and User in this client component.
These are used only as types in the UseUserResult definition. Without import type, the value imports can pull server-side modules into the client bundle, breaking the client/server boundary.
✅ Suggested fix
-import { Session } from "@/server/auth/utils/session";
-import { User } from "@/server/auth/utils/user";
+import type { Session } from "@/server/auth/utils/session";
+import type { User } from "@/server/auth/utils/user";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-old/template/extras/fmaddon-auth/components/auth/use-user.ts`
around lines 1 - 2, Change the imports of Session and User to type-only imports
so the client component doesn't pull server code into the bundle: replace the
current value imports of Session and User with type imports (i.e., use `import
type { Session }` and `import type { User }`) where they are only used in the
UseUserResult type in use-user.ts.
Comment on lines
+36
to
+40
| export async function createEmailVerificationRequest( | ||
| id_user: string, | ||
| email: string | ||
| ): Promise<TemailVerification> { | ||
| deleteUserEmailVerificationRequest(id_user); |
There was a problem hiding this comment.
Await the old-request delete before creating the new one.
Right now deletion runs concurrently with insertion. If the delete resolves late, it can remove the freshly created request or leave duplicate rows behind.
Suggested fix
export async function createEmailVerificationRequest(
id_user: string,
email: string
): Promise<TemailVerification> {
- deleteUserEmailVerificationRequest(id_user);
+ await deleteUserEmailVerificationRequest(id_user);
const idBytes = new Uint8Array(20);
crypto.getRandomValues(idBytes);📝 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
| export async function createEmailVerificationRequest( | |
| id_user: string, | |
| email: string | |
| ): Promise<TemailVerification> { | |
| deleteUserEmailVerificationRequest(id_user); | |
| export async function createEmailVerificationRequest( | |
| id_user: string, | |
| email: string | |
| ): Promise<TemailVerification> { | |
| await deleteUserEmailVerificationRequest(id_user); | |
| const idBytes = new Uint8Array(20); | |
| crypto.getRandomValues(idBytes); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/cli-old/template/extras/fmaddon-auth/server/auth/utils/email-verification.ts`
around lines 36 - 40, The deletion of any previous verification request is
currently fired but not awaited in createEmailVerificationRequest, which can
race with the new insertion; update createEmailVerificationRequest to await the
call to deleteUserEmailVerificationRequest(id_user) before proceeding to
create/insert the new TemailVerification so the old request is removed
deterministically and duplicates cannot occur.
Collaborator
Author
eluce2
changed the base branch from
codex/proofkit-new-scaffold
to
graphite-base/165
Agent-Id: agent-b0866404-cae3-459d-a9c5-f749470d81b4
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Agent-Id: agent-b0866404-cae3-459d-a9c5-f749470d81b4
eluce2
force-pushed
the
review-new-tests
branch
from
6dc028b to
a5c9ea6
Compare
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.
Add direct wrapper tests for create-proofkit
Agent-Id: agent-b0866404-cae3-459d-a9c5-f749470d81b4
Update lockfile for create-proofkit vitest dependency
Add hermetic scaffold contract tests for init
Agent-Id: agent-649fd397-82c3-4dbb-8ac0-ad442d855700
Linked-Note-Id: ed886a0e-b3a0-4496-93fa-5e922ca39a15
Add non-interactive failure-path coverage for init
Agent-Id: agent-a6ac7194-02f8-4467-a849-f2a56b55bb9f
Add non-interactive failure-path coverage for init
Agent-Id: agent-a6ac7194-02f8-4467-a849-f2a56b55bb9f
Linked-Note-Id: 0ea41863-ced8-400f-8042-5317ba6bc86c
Handle init post-codegen typegen failures with actionable errors
Separate CLI external smoke tests from contract tests
Agent-Id: agent-a559dbb7-21fc-41bc-aa42-db55cfdbb200
Coordinator
Agent-Id: agent-d969e5ba-941c-421b-af74-e9c40ca728f3
Summary by CodeRabbit
New Features
Documentation
Tests
Chores