feat(cli): add agent skills setup flow#100
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds the 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
8dd0cd5 to
ec1d0ce
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@packages/cli/src/controllers/agent.ts`:
- Around line 186-198: The child process error handler in the install flow is
treating aborts as failures, so update the `child.on("error")` path to detect an
`AbortError` from `spawn` and re-throw or reject with that abort instead of
wrapping it in `skillsInstallFailed(...)`. Keep `child.on("close")` for real
exit codes, but let cancellations from the `context.runtime.signal` propagate
cleanly so the runtime handles them as user aborts rather than install errors.
In `@packages/cli/src/controllers/app.ts`:
- Around line 4246-4258: The optional agent-setup prompt in app.ts currently
aborts when confirmPrompt is canceled instead of treating cancel like a declined
install. Update the flow around confirmPrompt so Esc/Ctrl+C is handled the same
as a false response, using the existing shouldInstall branch to record
setAgentSetupPromptDismissedAt and continue without throwing, rather than
exiting before the install step.
In `@packages/cli/src/lib/agent/package-manager.ts`:
- Around line 135-150: The Windows handling in packageRunner currently routes
every binary through binary(), which incorrectly turns bunx into a .cmd shim.
Update packageRunner to special-case the "bun" branch so it uses bunx.exe on
win32 (or plain bunx when appropriate), while keeping binary() and the existing
.cmd behavior for pnpm, yarn, and npm shims.
In `@packages/cli/src/lib/agent/setup-status.ts`:
- Around line 25-41: The setup status logic in setup-status.ts is duplicating
the skills lock filename between the computed skillsLockPath used by
hasPrismaSkillsLock and the returned skillsLockPath field in the status object.
Update the setup-status flow around readPrismaAgentProjectFileStatus,
hasPrismaSkillsLock, and the returned object to derive both from the same
source, such as a shared constant or path.basename(skillsLockPath), so the value
stays consistent if the filename changes.
🪄 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: 6a6d0fa7-6817-4cd3-a20b-faf56031ffa5
📒 Files selected for processing (19)
docs/product/command-spec.mdpackages/cli/src/adapters/local-state.tspackages/cli/src/cli.tspackages/cli/src/commands/agent/index.tspackages/cli/src/controllers/agent.tspackages/cli/src/controllers/app.tspackages/cli/src/controllers/auth.tspackages/cli/src/lib/agent/constants.tspackages/cli/src/lib/agent/package-manager.tspackages/cli/src/lib/agent/project-files.tspackages/cli/src/lib/agent/setup-status.tspackages/cli/src/presenters/agent.tspackages/cli/src/presenters/auth.tspackages/cli/src/shell/command-meta.tspackages/cli/src/types/agent.tspackages/cli/src/types/auth.tspackages/cli/tests/agent.test.tspackages/cli/tests/app-controller.test.tspackages/cli/tests/auth.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/cli/src/controllers/agent.ts`:
- Around line 165-178: runChildProcess currently spawns the executable directly,
which fails on Windows when resolveSkillsPackageRunner() returns .cmd/.bat
runners like npx.cmd or pnpm.cmd. Update runChildProcess to detect win32 and
launch these runners through a Windows-compatible path (for example using shell
execution or cmd.exe /c) while preserving args, cwd, env, and signal handling;
use the runChildProcess and resolveSkillsPackageRunner symbols to locate the
fix.
🪄 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: 10e171f8-8039-4f05-b73b-edebb47bf814
📒 Files selected for processing (19)
docs/product/command-spec.mdpackages/cli/src/adapters/local-state.tspackages/cli/src/cli.tspackages/cli/src/commands/agent/index.tspackages/cli/src/controllers/agent.tspackages/cli/src/controllers/app.tspackages/cli/src/controllers/auth.tspackages/cli/src/lib/agent/constants.tspackages/cli/src/lib/agent/package-manager.tspackages/cli/src/lib/agent/project-files.tspackages/cli/src/lib/agent/setup-status.tspackages/cli/src/presenters/agent.tspackages/cli/src/presenters/auth.tspackages/cli/src/shell/command-meta.tspackages/cli/src/types/agent.tspackages/cli/src/types/auth.tspackages/cli/tests/agent.test.tspackages/cli/tests/app-controller.test.tspackages/cli/tests/auth.test.ts
ec1d0ce to
bd6a348
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/cli/src/controllers/app.ts (1)
4248-4260: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winCancel still aborts the deploy (previously flagged as addressed).
confirmPromptthrowsusageErroron Esc/Ctrl+C, and thetry/catchhere only wrapsrunAgentInstall(Line 4262+), not theconfirmPromptcall. So canceling this optional prompt aborts the whole deploy before any deploy work begins. The earlier review marked this resolved inec1d0ce, but the current code does not handle cancellation — please confirm whether the fix was reverted.Treat cancel the same as a decline: record the dismissal and continue.
🛡️ Proposed handling
- const shouldInstall = await confirmPrompt({ - input: context.runtime.stdin, - output: context.runtime.stderr, - message: "Install Prisma agent guidance for this project?", - initialValue: true, - }); - - if (!shouldInstall) { + let shouldInstall: boolean; + try { + shouldInstall = await confirmPrompt({ + input: context.runtime.stdin, + output: context.runtime.stderr, + message: "Install Prisma agent guidance for this project?", + initialValue: true, + }); + } catch { + shouldInstall = false; + } + + if (!shouldInstall) { await context.stateStore.setAgentSetupPromptDismissedAt( new Date().toISOString(), ); return []; }#!/bin/bash # Confirm confirmPrompt still throws on cancel and isn't wrapped at the call site. fd -t f 'prompt.ts' packages/cli/src/shell --exec sed -n '60,95p' {} printf '\n--- call site ---\n' rg -nP -C3 'await confirmPrompt' packages/cli/src/controllers/app.ts🤖 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/controllers/app.ts` around lines 4248 - 4260, The optional install prompt in app.ts is not handling cancel, so Esc/Ctrl+C from confirmPrompt still aborts the deploy. Update the confirmPrompt call site around shouldInstall to catch usageError cancellation and treat it the same as a false response: setAgentSetupPromptDismissedAt, then continue by returning [] only for a real decline and otherwise proceed without failing the deploy. Use the existing runAgentInstall try/catch location as a reference, but move the cancel handling to the prompt step itself.
🤖 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 `@packages/cli/tests/app-controller.test.ts`:
- Around line 2872-3126: Add a deploy test for the cancel path of the agent
guidance prompt, since the current cases only cover accept and decline. In the
`runAppDeploy` flow in `app-controller.test.ts`, make `confirmPrompt` reject to
simulate cancellation and assert it is handled like a decline: `deployApp` still
runs, `runAgentInstall` is not called, and `readAgentSetupPromptDismissedAt` is
recorded. Use the existing interactive deploy test setup and the
`confirmPrompt`, `runAppDeploy`, and `context.stateStore` symbols to place the
new case alongside the current prompt behavior tests.
---
Duplicate comments:
In `@packages/cli/src/controllers/app.ts`:
- Around line 4248-4260: The optional install prompt in app.ts is not handling
cancel, so Esc/Ctrl+C from confirmPrompt still aborts the deploy. Update the
confirmPrompt call site around shouldInstall to catch usageError cancellation
and treat it the same as a false response: setAgentSetupPromptDismissedAt, then
continue by returning [] only for a real decline and otherwise proceed without
failing the deploy. Use the existing runAgentInstall try/catch location as a
reference, but move the cancel handling to the prompt step itself.
🪄 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: d828c377-27dd-4fe7-9674-9565d8830d77
📒 Files selected for processing (19)
docs/product/command-spec.mdpackages/cli/src/adapters/local-state.tspackages/cli/src/cli.tspackages/cli/src/commands/agent/index.tspackages/cli/src/controllers/agent.tspackages/cli/src/controllers/app.tspackages/cli/src/controllers/auth.tspackages/cli/src/lib/agent/constants.tspackages/cli/src/lib/agent/package-manager.tspackages/cli/src/lib/agent/project-files.tspackages/cli/src/lib/agent/setup-status.tspackages/cli/src/presenters/agent.tspackages/cli/src/presenters/auth.tspackages/cli/src/shell/command-meta.tspackages/cli/src/types/agent.tspackages/cli/src/types/auth.tspackages/cli/tests/agent.test.tspackages/cli/tests/app-controller.test.tspackages/cli/tests/auth.test.ts
c181b4c to
1eb31e9
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/cli/src/lib/agent/package-manager.ts`:
- Around line 147-164: packageRunnerForPackageManager currently mixes the spawn
runner with the display formatter by always applying binary(), which makes
help/examples emit .cmd on Windows. Split the logic used by
resolvePackageRunner* so resolveSkillsPackageRunner keeps the platform-specific
spawn binary behavior for agent.ts, while
resolvePrismaCliPackageCommandFormatter* returns plain package commands; update
the packageRunnerForPackageManager helper and any related formatter functions to
use the correct output for each path.
In `@packages/cli/tests/agent.test.ts`:
- Around line 54-62: The agent help and managed-file command strings are being
formatted with the Windows .cmd suffix from resolvePackageRunner(), which leaks
npx.cmd/pnpm.cmd into displayed output. Update the display-path in
formatPrismaCliCommand() and the agent command generation used by agent
help/status/install/update so .cmd is stripped for user-facing text, while
keeping the executable name with .cmd only for spawning.
🪄 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: 7894b920-baa6-41f8-a079-ae5770d48413
📒 Files selected for processing (22)
docs/product/command-spec.mdpackages/cli/src/adapters/local-state.tspackages/cli/src/cli.tspackages/cli/src/commands/agent/index.tspackages/cli/src/controllers/agent.tspackages/cli/src/controllers/app.tspackages/cli/src/controllers/auth.tspackages/cli/src/lib/agent/cli-command.tspackages/cli/src/lib/agent/constants.tspackages/cli/src/lib/agent/package-manager.tspackages/cli/src/lib/agent/project-files.tspackages/cli/src/lib/agent/setup-status.tspackages/cli/src/presenters/agent.tspackages/cli/src/presenters/auth.tspackages/cli/src/shell/cli-command.tspackages/cli/src/shell/command-meta.tspackages/cli/src/shell/help.tspackages/cli/src/types/agent.tspackages/cli/src/types/auth.tspackages/cli/tests/agent.test.tspackages/cli/tests/app-controller.test.tspackages/cli/tests/auth.test.ts
1eb31e9 to
416804c
Compare
cea395c to
452762b
Compare
luanvdw
left a comment
There was a problem hiding this comment.
Nice work on this!
Could we make these minor docs changes?
agentis now listed incommand-spec.mdas part of the beta command surface, butcommand-principles.mdstill says the preview only includesauth,project,git,branch,database, andapp.- The implementation adds a new structured error code,
AGENT_SKILLS_INSTALL_FAILED, buterror-conventions.mddoesn’t list or define it yet.
Both are small, but since these docs are the contract for future CLI work and agent-facing behavior, I’d update them in this PR so we don’t leave the model slightly split.
076e420 to
bfd2d37
Compare
Summary
agent install,agent update, andagent statusto install/check Prisma skills fromprisma/skills.bunx,pnpm dlx,yarn dlx, ornpx -y.skills-lock.jsonstructurally for status/prompt decisions.app deploycan prompt once to install onlyprisma-compute.--skill '*'.Examples
Without package-manager detection:
Deploy prompt output:
Testing
pnpm --filter @prisma/cli exec vitest run tests/prompt.test.ts tests/app-controller.test.ts tests/agent.test.tspnpm --filter @prisma/cli testpnpm --recursive exec tsc --noEmitpnpm lintagent install --skill prisma-computesmoke from/Users/aman/dev/test/prisma-cli-agent-install-quiet/Users/aman/dev/test/prisma-cli-deploy-quiet