Enable expanded Biome lint rules#86
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
Summary by CodeRabbit
WalkthroughThe PR adds a PR quality workflow, pins the Node version, adds Biome configuration and package scripts, and updates workflow guidance. It refactors many CLI commands, controllers, libraries, presenters, and shell modules, with a few targeted behavior changes such as top-level await in the entrypoint, login paste-callback helper extraction, database connection removal serialization, masking updates, preview-build symlink materialization, auth workspace lookup wiring, and test helper default cwd handling. It also updates and extends tests across app deploy, preview build, env handling, auth, database, project, output, and command runner paths. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/cli/src/lib/project/resolution.ts (2)
255-265:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDon't hardcode
app deployin ambiguous-project recovery.
projectResolutionErrorToCliError()is also used byproject show,git connect, andgit disconnectinpackages/cli/src/controllers/project.ts, so this helper can surfacePROJECT_AMBIGUOUSoutside deploy flows. Suggestingprisma-cli app deploy --project ...here sends users to the wrong command. Either parameterize the retry step with the originating command or keep this helper command-agnostic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/lib/project/resolution.ts` around lines 255 - 265, The helper projectAmbiguousCliError currently hardcodes a retry suggestion with "prisma-cli app deploy --project ..." which is incorrect for callers like project show, git connect, and git disconnect; modify projectAmbiguousCliError to either accept an optional originatingCommand string parameter (e.g., originatingCommand?: string) and use it to build the nextSteps entry (`${originatingCommand} --project ${firstMatch.id}`) or remove the specific subcommand and instead push a command-agnostic hint such as `prisma-cli --project ${firstMatch.id}` or `prisma-cli <command> --project ${firstMatch.id}` so the suggestion is correct for all callers (update all call sites of projectAmbiguousCliError accordingly to pass the originating command when available).
524-539:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDon't treat arbitrary directory names as valid project-create defaults.
The package-name path is validated, but the directory fallback returns
path.basename(cwd)verbatim. In directories likemy app, downstream helpers buildprisma-cli project create 'my app', which then fails the CLI's own project-name validation. Only surface a suggested name/create command when the basename passes the same validity check, or sanitize/omit the suggestion first.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/lib/project/resolution.ts` around lines 524 - 539, The fallback that returns path.basename(cwd) verbatim can produce invalid project names (e.g., "my app"); update inferTargetName to only return a directory-derived suggestion when it passes isValidInferredTargetName or can be deterministically sanitized into a valid name: get dirName = path.basename(cwd), if isValidInferredTargetName(dirName) return it (source "directory-name"); otherwise produce a sanitized candidate (e.g., normalize whitespace to hyphens and strip invalid chars), validate it with isValidInferredTargetName and if valid return that with source "directory-name-sanitized"; if neither original nor sanitized pass validation, do not surface a usable suggestion (e.g., return a null/empty/none name or a source indicating no suggestion) so downstream callers like project-create are not given an invalid name. Ensure this logic sits in inferTargetName and relies on readPackageName/isValidInferredTargetName for consistency.packages/cli/src/lib/app/bun-project.ts (1)
21-31:⚠️ Potential issue | 🟡 MinorKeep abort handling consistent for filesystem operations.
readBunPackageJson(packages/cli/src/lib/app/bun-project.ts) wraps aborted reads in a genericFailed to read ...error instead of rethrowing the original abort error.LocalStateStore.write(packages/cli/src/adapters/local-state.ts) callswriteFilewithout passingthis.signal, so cancellation during the write isn’t propagated.🔧 Proposed fixes
--- a/packages/cli/src/lib/app/bun-project.ts +++ b/packages/cli/src/lib/app/bun-project.ts @@ } catch (error) { + if (signal?.aborted) throw error; if ((error as NodeJS.ErrnoException).code === "ENOENT") { return null; }--- a/packages/cli/src/adapters/local-state.ts +++ b/packages/cli/src/adapters/local-state.ts @@ await writeFile(this.stateFilePath, `${JSON.stringify(state, null, 2)}\n`, { encoding: "utf8", + signal: this.signal, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/lib/app/bun-project.ts` around lines 21 - 31, readBunPackageJson currently converts aborts into a generic "Failed to read..." error and LocalStateStore.write doesn't pass the instance signal into writeFile; update readBunPackageJson's catch to rethrow the original abort error when the read was cancelled (e.g. check error.name === "AbortError" or signal.aborted and rethrow) instead of wrapping it, and update LocalStateStore.write to pass this.signal into the writeFile options so cancellation is propagated (locate readBunPackageJson in packages/cli/src/lib/app/bun-project.ts and LocalStateStore.write in packages/cli/src/adapters/local-state.ts).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-quality.yml:
- Around line 22-23: The checkout steps currently use actions/checkout without
disabling persisted credentials; update each Checkout step (the steps with
"name: Checkout" and "uses: actions/checkout@...") to add the input
persist-credentials: false so the default token is not written into git config;
do this for all three occurrences (the existing Checkout steps at the start and
the other two occurrences referenced in the comment) by adding a
persist-credentials: false key under each checkout step.
In `@examples/next-smoke/app/globals.css`:
- Line 43: The font-family list containing "SFMono-Regular", ui-monospace,
"Cascadia Mono", "Segoe UI Mono", Menlo, should be updated to remove the
surrounding quotes for SFMono-Regular; find the font-family declaration that
includes "SFMono-Regular" and replace "SFMono-Regular" with SFMono-Regular so
the family name is unquoted and Stylelint stops flagging it.
In `@package.json`:
- Around line 11-12: The package.json scripts currently have "lint": "biome
check . --write" which mutates the workspace; change "lint" to a read-only
verification ("biome check ." or similar without --write) and add a separate
script like "lint:fix" (or "format") that runs "biome check . --write" (or
"biome format . --write") so CI can run yarn/pnpm run lint as a non-mutating
check while developers can run lint:fix to auto-apply fixes; update the scripts
entries "lint" and add "lint:fix"/"format" accordingly.
In `@packages/cli/src/controllers/project.ts`:
- Line 1: Remove the file-level directive "biome-ignore-all
lint/performance/noAwaitInLoops" and instead add a scoped biome-ignore comment
around only the sequential-await loop blocks used for GitHub App install polling
and repository pagination (the for/while/do-while loops that perform install
polling and paginated repo lookups in this controller); locate the loops that
perform the install polling and repository pagination in
packages/cli/src/controllers/project.ts and place a single-line "biome-ignore
lint/performance/noAwaitInLoops" immediately above each loop block so other
await-in-loop occurrences in this file remain linted.
In `@packages/cli/src/presenters/database.ts`:
- Around line 327-331: The serializer serializeDatabaseConnectionRemove
currently returns the raw DatabaseConnectionRemoveResult and can leak
verboseContext; update it to return stripVerboseContext(result) instead so the
output matches other database serializers and removes internal verboseContext
before emitting JSON (use the existing stripVerboseContext helper and keep the
function signature serializeDatabaseConnectionRemove(result:
DatabaseConnectionRemoveResult)).
In `@packages/cli/src/shell/ui.ts`:
- Around line 253-263: The isEmailLocalPartChar helper currently only allows
A-Z, a-z, 0-9 and the punctuation . _ % + - which causes characters like
apostrophe (') and ampersand (&) to be treated as delimiters and leak local-part
data; update isEmailLocalPartChar to include additional valid RFC-local-part
characters such as ' and & (and any other minimal set you deem safe for our use)
so maskValue will treat them as part of the local-part, then add unit tests
covering at least an apostrophe case (e.g., "o'hara@example.com") and an
ampersand case (e.g., "billing&alerts@example.com") to assert full masking
behavior.
---
Outside diff comments:
In `@packages/cli/src/lib/app/bun-project.ts`:
- Around line 21-31: readBunPackageJson currently converts aborts into a generic
"Failed to read..." error and LocalStateStore.write doesn't pass the instance
signal into writeFile; update readBunPackageJson's catch to rethrow the original
abort error when the read was cancelled (e.g. check error.name === "AbortError"
or signal.aborted and rethrow) instead of wrapping it, and update
LocalStateStore.write to pass this.signal into the writeFile options so
cancellation is propagated (locate readBunPackageJson in
packages/cli/src/lib/app/bun-project.ts and LocalStateStore.write in
packages/cli/src/adapters/local-state.ts).
In `@packages/cli/src/lib/project/resolution.ts`:
- Around line 255-265: The helper projectAmbiguousCliError currently hardcodes a
retry suggestion with "prisma-cli app deploy --project ..." which is incorrect
for callers like project show, git connect, and git disconnect; modify
projectAmbiguousCliError to either accept an optional originatingCommand string
parameter (e.g., originatingCommand?: string) and use it to build the nextSteps
entry (`${originatingCommand} --project ${firstMatch.id}`) or remove the
specific subcommand and instead push a command-agnostic hint such as `prisma-cli
--project ${firstMatch.id}` or `prisma-cli <command> --project ${firstMatch.id}`
so the suggestion is correct for all callers (update all call sites of
projectAmbiguousCliError accordingly to pass the originating command when
available).
- Around line 524-539: The fallback that returns path.basename(cwd) verbatim can
produce invalid project names (e.g., "my app"); update inferTargetName to only
return a directory-derived suggestion when it passes isValidInferredTargetName
or can be deterministically sanitized into a valid name: get dirName =
path.basename(cwd), if isValidInferredTargetName(dirName) return it (source
"directory-name"); otherwise produce a sanitized candidate (e.g., normalize
whitespace to hyphens and strip invalid chars), validate it with
isValidInferredTargetName and if valid return that with source
"directory-name-sanitized"; if neither original nor sanitized pass validation,
do not surface a usable suggestion (e.g., return a null/empty/none name or a
source indicating no suggestion) so downstream callers like project-create are
not given an invalid name. Ensure this logic sits in inferTargetName and relies
on readPackageName/isValidInferredTargetName for consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: efac4d70-d28c-446d-8327-9c7cf9d63bf1
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (136)
.github/workflows/AGENTS.md.github/workflows/pr-quality.yml.node-versionbiome.jsoncexamples/next-smoke/app/globals.cssexamples/next-smoke/app/page.tsxexamples/next-smoke/tsconfig.jsonpackage.jsonpackages/cli/src/adapters/git.tspackages/cli/src/adapters/local-state.tspackages/cli/src/adapters/mock-api.tspackages/cli/src/adapters/token-storage.tspackages/cli/src/bin.tspackages/cli/src/cli.tspackages/cli/src/commands/app/index.tspackages/cli/src/commands/auth/index.tspackages/cli/src/commands/branch/index.tspackages/cli/src/commands/database/index.tspackages/cli/src/commands/env.tspackages/cli/src/commands/git/index.tspackages/cli/src/commands/project/index.tspackages/cli/src/commands/version/index.tspackages/cli/src/controllers/app-env-api.tspackages/cli/src/controllers/app-env-file.tspackages/cli/src/controllers/app-env.tspackages/cli/src/controllers/app.tspackages/cli/src/controllers/auth.tspackages/cli/src/controllers/branch.tspackages/cli/src/controllers/database.tspackages/cli/src/controllers/project.tspackages/cli/src/controllers/select-prompt-port.tspackages/cli/src/controllers/version.tspackages/cli/src/lib/app/branch-database-deploy.tspackages/cli/src/lib/app/branch-database.tspackages/cli/src/lib/app/bun-project.tspackages/cli/src/lib/app/deploy-output.tspackages/cli/src/lib/app/domain-guidance.tspackages/cli/src/lib/app/env-config.tspackages/cli/src/lib/app/env-file.tspackages/cli/src/lib/app/env-vars.tspackages/cli/src/lib/app/local-dev.tspackages/cli/src/lib/app/preview-branch-database.tspackages/cli/src/lib/app/preview-build-settings.tspackages/cli/src/lib/app/preview-build.tspackages/cli/src/lib/app/preview-interaction.tspackages/cli/src/lib/app/preview-progress.tspackages/cli/src/lib/app/preview-provider.tspackages/cli/src/lib/app/production-deploy-gate.tspackages/cli/src/lib/auth/auth-ops.tspackages/cli/src/lib/auth/client.tspackages/cli/src/lib/auth/login.tspackages/cli/src/lib/database/provider.tspackages/cli/src/lib/diagnostics.tspackages/cli/src/lib/git/local-branch.tspackages/cli/src/lib/git/local-status.tspackages/cli/src/lib/project/interactive-setup.tspackages/cli/src/lib/project/local-pin.tspackages/cli/src/lib/project/resolution.tspackages/cli/src/lib/project/setup.tspackages/cli/src/lib/version.tspackages/cli/src/output/patterns.tspackages/cli/src/presenters/app-env.tspackages/cli/src/presenters/app.tspackages/cli/src/presenters/auth.tspackages/cli/src/presenters/branch.tspackages/cli/src/presenters/database.tspackages/cli/src/presenters/project.tspackages/cli/src/presenters/verbose-context.tspackages/cli/src/presenters/version.tspackages/cli/src/shell/command-arguments.tspackages/cli/src/shell/command-meta.tspackages/cli/src/shell/command-runner.tspackages/cli/src/shell/diagnostics-output.tspackages/cli/src/shell/errors.tspackages/cli/src/shell/global-flags.tspackages/cli/src/shell/help.tspackages/cli/src/shell/next-actions.tspackages/cli/src/shell/output.tspackages/cli/src/shell/prompt.tspackages/cli/src/shell/runtime.tspackages/cli/src/shell/ui.tspackages/cli/src/shell/update-check.tspackages/cli/src/types/app.tspackages/cli/src/types/project.tspackages/cli/src/use-cases/auth.tspackages/cli/src/use-cases/branch.tspackages/cli/src/use-cases/contracts.tspackages/cli/src/use-cases/create-cli-gateways.tspackages/cli/src/use-cases/project.tspackages/cli/tests/app-branch-database.test.tspackages/cli/tests/app-build.test.tspackages/cli/tests/app-bun-compat.test.tspackages/cli/tests/app-controller.test.tspackages/cli/tests/app-env-presenter.test.tspackages/cli/tests/app-env-vars.test.tspackages/cli/tests/app-env.test.tspackages/cli/tests/app-local-dev.test.tspackages/cli/tests/app-presenter.test.tspackages/cli/tests/app-provider.test.tspackages/cli/tests/app-state.test.tspackages/cli/tests/app.test.tspackages/cli/tests/auth-login.test.tspackages/cli/tests/auth-ops.test.tspackages/cli/tests/auth-real-mode.test.tspackages/cli/tests/auth.test.tspackages/cli/tests/branch-controller.test.tspackages/cli/tests/branch.test.tspackages/cli/tests/command-runner-auth.test.tspackages/cli/tests/command-runner.test.tspackages/cli/tests/database.test.tspackages/cli/tests/git-adapter.test.tspackages/cli/tests/helpers.tspackages/cli/tests/helpers/mock-factories.tspackages/cli/tests/output.test.tspackages/cli/tests/production-deploy-gate.test.tspackages/cli/tests/project-controller.test.tspackages/cli/tests/project-real-mode.test.tspackages/cli/tests/project-resolution.test.tspackages/cli/tests/project.test.tspackages/cli/tests/prompt.test.tspackages/cli/tests/publish-prep.test.tspackages/cli/tests/resolve-cli-version.test.tspackages/cli/tests/shell.test.tspackages/cli/tests/token-storage.test.tspackages/cli/tests/update-check.test.tspackages/cli/tests/use-case-helpers.tspackages/cli/tests/version.test.tspackages/compute/src/index.tspackages/compute/src/scale-to-zero-control.tspackages/compute/src/scale-to-zero.tspackages/compute/tests/scale-to-zero.test.tsscripts/prepare-cli-publish.mjsscripts/resolve-cli-version.mjsscripts/smoke-cli-nextjs-artifact.mjsscripts/validate-skills.mjsscripts/validate-skills.test.mjs
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/cli/src/presenters/database.ts (1)
327-330:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve the remove-result JSON shape here.
Returning only
{ connection }turns this serializer into a breaking payload reshaper instead of the sameverboseContextstripping used by the neighboring database serializers. Any consumer expecting the rest ofDatabaseConnectionRemoveResultwill lose fields in--jsonoutput.♻️ Proposed fix
export function serializeDatabaseConnectionRemove( result: DatabaseConnectionRemoveResult, ) { - return { connection: result.connection }; + return stripVerboseContext(result); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/presenters/database.ts` around lines 327 - 330, The serializer serializeDatabaseConnectionRemove currently returns only `{ connection }`, which reshapes the JSON output and drops fields from DatabaseConnectionRemoveResult; change it to preserve the original result shape by returning the whole result (or applying the same verboseContext helper used by the neighboring serializers) — e.g. return the full DatabaseConnectionRemoveResult (or `verboseContext(result)` if that helper is used elsewhere) instead of just ` { connection: result.connection }`, keeping the rest of the fields intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/cli/src/presenters/database.ts`:
- Around line 327-330: The serializer serializeDatabaseConnectionRemove
currently returns only `{ connection }`, which reshapes the JSON output and
drops fields from DatabaseConnectionRemoveResult; change it to preserve the
original result shape by returning the whole result (or applying the same
verboseContext helper used by the neighboring serializers) — e.g. return the
full DatabaseConnectionRemoveResult (or `verboseContext(result)` if that helper
is used elsewhere) instead of just ` { connection: result.connection }`, keeping
the rest of the fields intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: da324ee4-6edf-4a1e-a94a-b681b3a26019
📒 Files selected for processing (7)
.github/workflows/pr-quality.ymlexamples/next-smoke/app/globals.csspackage.jsonpackages/cli/src/controllers/project.tspackages/cli/src/presenters/database.tspackages/cli/src/shell/ui.tspackages/cli/tests/shell.test.ts
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
2 participants
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.
Enables the expanded Biome rule set for the CLI repo and brings the current codebase into compliance so future changes are checked consistently.
Changes
biome.jsonc, and removes the stale commented rule block once all rules are active.await, converts disposable guard tests tousing, and updates test assertions/mocks to satisfy the stricter rules.Why
The rules are enabled one at a time with isolated commits so reviewers can inspect each category independently. Some rules are intentionally suppressed for existing code where the lint recommendation would require a broader semantic audit, especially
noAwaitInLoopsanduseTopLevelRegex; this keeps the PR focused on establishing the guardrails without changing concurrency, polling, or parsing behavior opportunistically.Verification
pnpm run lintpnpm --filter @prisma/compute testpnpm --filter @prisma/compute buildpnpm --filter @prisma/cli buildpnpm --filter @prisma/cli exec vitest run tests/app-env-vars.test.ts tests/project-real-mode.test.tspnpm --filter @prisma/cli exec vitest run tests/update-check.test.tsFull
pnpm --filter @prisma/cli testwas also attempted and still fails on existing constructor-mock issues where mocked classes are arrow functions used withnew; those failures are unrelated to this lint-rule enablement.