Conversation
…ectMessage Add POST /agents/:identifier/welcome-message endpoint that sends a proactive DM to the user who installed the agent on their chat platform. Uses chat SDK's openDM() for platform-agnostic direct messaging. The ChatSdkService.sendDirectMessage primitive is reusable for any future proactive messaging needs beyond the onboarding welcome flow. Co-authored-by: Cursor <cursoragent@cursor.com>
…nnected follow-up fixes NV-7450 Implements a two-step onboarding welcome message flow for generated agents: 1. On Slack OAuth success, the API sends a welcome DM to the installer and returns the resulting conversationId to the dashboard. 2. When the user connects their bridge URL, a second "setup complete" message is posted to the same Slack conversation using the stored serialized thread. Also adds a richer "no bridge URL" fallback reply — a card element with a "Continue setup" link button pointing to the agent's dashboard page. Additional changes: - Normalise Slack Assistant Thread IDs (trailing-colon format) inside sendDirectMessage so callers receive a consistent platformThreadId. - Add assistant:write and im:write OAuth scopes required for DM posting. - Type buildNoBridgeReply return as CardElement/CardChild, removing the double-cast at the call site. - Stabilise handleBridgeConnected via a render-time ref so searchParams is not in the callback's dependency array. - Add default case to getWelcomeText to avoid a silent undefined return when new platforms are added. Co-authored-by: Cursor <cursoragent@cursor.com>
- Merge next's Slack quick-setup mode (feature-flagged) with our onboarding welcome message flow in slack-setup-guide.tsx - Switch slackInstallConnectControl to use handleSlackOAuthSuccess so the welcome DM is sent on workspace install in both quick and manual setup modes - Keep our dynamic assistant_description and suggested_prompts in the manifest Co-authored-by: Cursor <cursoragent@cursor.com>
- Extract SendAgentWelcomeMessageRequestDto and add @apioperation / @ApiNotFoundResponse decorators to the welcome-message endpoint - Guard environmentRepository.findOne with try/catch so a DB error during no-bridge fallback renders a text-only card rather than dropping the reply - Replace bridgeMessageSentRef (global boolean) with lastSentConversationIdRef to scope dedup per conversation ID, allowing subsequent onboarding flows in the same mount to proceed - Add analytics tracking to sendBridgeConnectedMessage for parity with the welcome-message path Co-authored-by: Cursor <cursoragent@cursor.com>
…undHandler spec Adding EnvironmentRepository to the constructor shifted the positional arguments in the manually-constructed test stub, causing analytics.track and attachmentStorage.storeInbound to land in the wrong slots. Co-authored-by: Cursor <cursoragent@cursor.com>
…s NV-7450 Add --run flag to `novu dev` that spawns the user's app server as a child process before setting up the tunnel. The scaffolded agent template now generates `dev:novu` script that combines Next.js + tunnel in one command. - CLI: add --run <cmd> option with proper signal forwarding and exit handling - Template: generate dev:novu script, resolve CLI npm tag from package version - Init: prefill project name with --agent-identifier value - Init: clickable terminal link (OSC 8) to agent file in post-scaffold output - Init: visual box layout for the get-started command - Dashboard: update onboarding step to show `npm run dev:novu` Co-authored-by: Cursor <cursoragent@cursor.com>
✅ Deploy Preview for dashboard-v2-novu-staging canceled.
|
Contributor
📝 WalkthroughWalkthroughAdds a local agent development flow: a new ChangesAgent Local Development Workflow
Sequence Diagram(s)sequenceDiagram
participant CLI as "Novu CLI"
participant DevCmd as "dev command"
participant AppProc as "Spawned App Server"
participant Tunnel as "Dev Tunnel"
participant Dashboard as "Dashboard UI"
CLI->>DevCmd: parse `--run <command>`
DevCmd->>AppProc: spawnAppServer(command) (cross-platform shell)
AppProc-->>DevCmd: started / PID
DevCmd->>Tunnel: open dev tunnel
Tunnel-->>Dashboard: tunnel URL
Dashboard->>DevCmd: user registers bridge URL (UX)
AppProc-->>Dashboard: local agent serves (dev:novu)
DevCmd->>AppProc: on SIGINT/SIGTERM -> terminate AppProc
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/novu/src/commands/dev/dev.ts`:
- Around line 17-25: The cleanup routine currently only sends SIGTERM to the
immediate child (appProcess) and exits immediately, which leaves orphaned
processes on Windows and POSIX; update cleanup() and spawnAppServer() so that
when starting the child (spawnAppServer) you use detached:true on POSIX and
capture appProcess.pid, and on shutdown perform a platform-specific teardown: on
Windows run taskkill /PID <pid> /T /F (or equivalent child process termination)
and on POSIX send the signal to the process group (e.g., kill(-pid)) to
terminate the whole tree; ensure cleanup waits for child termination (or a short
timeout) before calling process.exit(), and use appProcess, appProcess.pid and
spawnAppServer as the referenced symbols when implementing these changes.
In `@packages/novu/src/commands/init/templates/index.ts`:
- Around line 35-43: resolveCliTag currently collapses all prerelease versions
to "rc", causing alpha/next installs to resolve to the wrong dist-tag; update
resolveCliTag to parse the prerelease identifier from pkg.version (e.g., const
prerelease = pkg.version.split('-')[1]?.split('.')[0]) and return that
identifier when it matches known tags ("alpha", "next", "rc"), otherwise return
that identifier for unknown prereleases or fall back to "latest"; also update
the generated dev:novu script template (the template that injects the tag for
the dev script) to use the new resolveCliTag value so the scaffolded script
references the correct npm dist-tag instead of always using "rc" or "latest".
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 23b6e2ca-e3af-4bdf-ac20-72bb2a3933a2
📒 Files selected for processing (7)
apps/dashboard/src/components/agents/agent-code-setup-section.tsxpackages/novu/src/commands/dev/dev.tspackages/novu/src/commands/dev/types.tspackages/novu/src/commands/init/create-app.tspackages/novu/src/commands/init/index.tspackages/novu/src/commands/init/templates/index.tspackages/novu/src/index.ts
Comment on lines
+35
to
+43
| function resolveCliTag(): string { | ||
| const pkg = resolveCliPackageJson(); | ||
| if (!pkg?.version) return 'latest'; | ||
|
|
||
| if (pkg.version.includes('-rc')) return 'rc'; | ||
| if (pkg.version.includes('-alpha')) return 'rc'; | ||
|
|
||
| return 'latest'; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
npm view novu dist-tags
💡 Result:
The command npm view novu dist-tags displays the distribution tags for the novu npm package. Distribution tags are human-readable labels like 'latest' that point to specific versions [1][2]. For the novu package (CLI tool), the latest version is 2.8.0 (published Mar 27, 2026), with previous versions including RC tags like 2.8.0-rc.1 to 2.8.0-rc.8, 2.7.0, etc. [3][4]. No explicit list of current dist-tags (e.g., latest: 2.8.0) was found in search results, but npm pages typically show only the latest version prominently, implying 'latest' points to 2.8.0 [3]. Other packages like @novu/node show canary tags in history (e.g., 2.0.0-canary.0) [5]. To get the exact current output, run the command directly, as it queries the npm registry in real-time [2].
Citations:
- 1: https://docs.npmjs.com/adding-dist-tags-to-packages/
- 2: https://docs.npmjs.com/cli/v8/commands/npm-dist-tag/
- 3: https://www.npmjs.com/package/novu
- 4: https://registry.npmjs.org/novu
- 5: https://www.npmjs.com/package/@novu/node
🏁 Script executed:
# Get the exact dist-tags for novu from npm registry
npm view novu dist-tags --json 2>/dev/null || npm view novu dist-tagsRepository: novuhq/novu
Length of output: 157
🏁 Script executed:
# Check the file to see the referenced lines and context
wc -l packages/novu/src/commands/init/templates/index.tsRepository: novuhq/novu
Length of output: 110
🏁 Script executed:
# Read the relevant sections of the file
sed -n '35,43p' packages/novu/src/commands/init/templates/index.ts
sed -n '255,258p' packages/novu/src/commands/init/templates/index.tsRepository: novuhq/novu
Length of output: 455
🏁 Script executed:
# Search for release/publish configuration in the repo
fd -type f \( -name "release*.yml" -o -name "release*.yaml" -o -name ".npmrc" -o -name "lerna.json" -o -name "release.config.*" \) | head -20Repository: novuhq/novu
Length of output: 226
🏁 Script executed:
# Check package.json scripts for publish/release commands
rg -A 5 '"(publish|release|npm)"' packages/novu/package.json || cat packages/novu/package.jsonRepository: novuhq/novu
Length of output: 3435
🏁 Script executed:
# Check the other location mentioned at lines 255-258
sed -n '250,260p' packages/novu/src/commands/init/templates/index.tsRepository: novuhq/novu
Length of output: 330
Fix the dist-tag mapping to align with actual npm tags published for novu.
The current code maps both -rc and -alpha versions to the rc dist-tag, but the npm registry shows distinct tags: alpha (0.3.1-alpha.8), next (0.10.0-alpha.0), latest (2.8.0), and rc (2.8.1-rc.6). If a user scaffolds a project with novu@alpha, the generated dev:novu script will resolve to novu@rc (version 2.8.1-rc.6), creating major version skew (0.3.x → 2.8.x). The same issue affects novu@next and any unrecognized pre-release suffixes, which fall through to latest.
Update the mapping to handle alpha, next, and other pre-release identifiers correctly, or align it with your actual release pipeline's dist-tag strategy:
Example: Extract pre-release id from version string
function resolveCliTag(): string {
const pkg = resolveCliPackageJson();
if (!pkg?.version) return 'latest';
- if (pkg.version.includes('-rc')) return 'rc';
- if (pkg.version.includes('-alpha')) return 'rc';
-
- return 'latest';
+ // Extract pre-release identifier (part after '-' and before '.' or end of string)
+ const match = /-([0-9A-Za-z]+)/.exec(pkg.version);
+ const preId = match?.[1]?.toLowerCase();
+ if (!preId) return 'latest';
+
+ // Map pre-release IDs to dist-tags available on npm registry
+ const TAG_BY_PRE_ID: Record<string, string> = {
+ rc: 'rc',
+ alpha: 'alpha',
+ next: 'next',
+ beta: 'latest', // adjust if beta has its own tag
+ canary: 'latest', // adjust if canary has its own tag
+ };
+
+ return TAG_BY_PRE_ID[preId] ?? 'latest';
}This also affects the generated script template at lines 255–258.
📝 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
| function resolveCliTag(): string { | |
| const pkg = resolveCliPackageJson(); | |
| if (!pkg?.version) return 'latest'; | |
| if (pkg.version.includes('-rc')) return 'rc'; | |
| if (pkg.version.includes('-alpha')) return 'rc'; | |
| return 'latest'; | |
| } | |
| function resolveCliTag(): string { | |
| const pkg = resolveCliPackageJson(); | |
| if (!pkg?.version) return 'latest'; | |
| // Extract pre-release identifier (part after '-' and before '.' or end of string) | |
| const match = /-([0-9A-Za-z]+)/.exec(pkg.version); | |
| const preId = match?.[1]?.toLowerCase(); | |
| if (!preId) return 'latest'; | |
| // Map pre-release IDs to dist-tags available on npm registry | |
| const TAG_BY_PRE_ID: Record<string, string> = { | |
| rc: 'rc', | |
| alpha: 'alpha', | |
| next: 'next', | |
| beta: 'latest', // adjust if beta has its own tag | |
| canary: 'latest', // adjust if canary has its own tag | |
| }; | |
| return TAG_BY_PRE_ID[preId] ?? 'latest'; | |
| } |
🤖 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/novu/src/commands/init/templates/index.ts` around lines 35 - 43,
resolveCliTag currently collapses all prerelease versions to "rc", causing
alpha/next installs to resolve to the wrong dist-tag; update resolveCliTag to
parse the prerelease identifier from pkg.version (e.g., const prerelease =
pkg.version.split('-')[1]?.split('.')[0]) and return that identifier when it
matches known tags ("alpha", "next", "rc"), otherwise return that identifier for
unknown prereleases or fall back to "latest"; also update the generated dev:novu
script template (the template that injects the tag for the dev script) to use
the new resolveCliTag value so the scaffolded script references the correct npm
dist-tag instead of always using "rc" or "latest".
Use detached process group on POSIX with process.kill(-pid) and taskkill /T /F on Windows to reliably terminate grandchild processes (e.g. next dev) when the CLI exits. Add short delay before exit to allow signal delivery. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
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/novu/src/commands/dev/dev.ts`:
- Around line 287-298: The exit handler is logging a failure when the child was
intentionally terminated; introduce a module-scoped boolean flag (e.g.,
isShuttingDown or intentionalShutdown) that cleanup() sets to true before
calling taskkill/child.kill, then update the child.on('exit', ...) handler to
check this flag and treat exits during intentional shutdown as normal (skip the
red error log and exit with 0 or no-op) while preserving the existing behavior
for unexpected exits; also ensure child.on('error', ...) behavior is unchanged
but can consult the same flag if you want to suppress error noise during
shutdown.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8784ce01-4240-434d-a7f6-601ad109977f
📒 Files selected for processing (1)
packages/novu/src/commands/dev/dev.ts
Comment on lines
+287
to
+298
| child.on('error', (err) => { | ||
| console.error(chalk.red(`\n ✗ Failed to start app server: ${err.message}`)); | ||
| }); | ||
|
|
||
| child.on('exit', (code, signal) => { | ||
| if (signal === 'SIGINT' || signal === 'SIGTERM') { | ||
| process.exit(0); | ||
| } | ||
|
|
||
| console.error(chalk.red(`\n ✗ App server exited with code ${code ?? 1}`)); | ||
| process.exit(code ?? 1); | ||
| }); |
Contributor
There was a problem hiding this comment.
Spurious "App server exited with code 1" on intentional shutdown (Windows).
When cleanup() runs taskkill /T /F on Windows, the child's exit event fires with code=1, signal=null — Windows has no real POSIX signals, so the signal === 'SIGINT' || signal === 'SIGTERM' guard does not match and the handler falls through to the red error log and process.exit(1). The same can happen on POSIX if the fallback child.kill('SIGTERM') path is taken after a failed process.kill(-pid). Track the shutdown intent explicitly so the handler can distinguish "we asked it to die" from a real crash.
🛡️ Proposed fix
let appProcess: ChildProcess | null = null;
+let isShuttingDown = false; function cleanup() {
+ isShuttingDown = true;
if (appProcess && !appProcess.killed) {
killProcessTree(appProcess);
}
setTimeout(() => process.exit(), 200).unref();
} child.on('exit', (code, signal) => {
- if (signal === 'SIGINT' || signal === 'SIGTERM') {
+ if (isShuttingDown || signal === 'SIGINT' || signal === 'SIGTERM') {
process.exit(0);
}
console.error(chalk.red(`\n ✗ App server exited with code ${code ?? 1}`));
process.exit(code ?? 1);
});🤖 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/novu/src/commands/dev/dev.ts` around lines 287 - 298, The exit
handler is logging a failure when the child was intentionally terminated;
introduce a module-scoped boolean flag (e.g., isShuttingDown or
intentionalShutdown) that cleanup() sets to true before calling
taskkill/child.kill, then update the child.on('exit', ...) handler to check this
flag and treat exits during intentional shutdown as normal (skip the red error
log and exit with 0 or no-op) while preserving the existing behavior for
unexpected exits; also ensure child.on('error', ...) behavior is unchanged but
can consult the same flag if you want to suppress error noise during shutdown.
ChmaraX
deleted the
nv-7450-reduce-cli-setup-friction-for-generated-agents-1
branch
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.
Summary
--run <cmd>flag tonovu devthat spawns the user's app server as a child process before setting up the tunnel, enabling single-command startup for agent projectsdev(app only) anddev:novu(app + tunnel + bridge) scripts —npm run dev:novuis the one-command onboarding experience@rcvs@latest) in the generated script is auto-resolved from the CLI's own package version--agent-identifiervalue so users just press Enternpm run dev:novuinstead of the previous two-terminal instructionTest plan
novu dev --run "next dev --port=4000"spawns Next.js, connects tunnel, health-checks, discovers agentsnovu devwithout--runworks identically to before (backward compatible)novu init -t agent --agent-identifier fooprefills project name asfoopackage.jsoncontains bothdevanddev:novuscriptscd && npm run dev:novucommandnpm run dev:novuwith correct copy behaviorMade with Cursor
What changed
Added a
--run <command>flag to thenovu devcommand that spawns a user's app server as a child process before tunnel setup, enabling single-command startup for scaffolded agent projects. The agent template now generates bothdev(app only) anddev:novu(app + tunnel + bridge) npm scripts, with the latter serving as the one-command onboarding experience. Project name prompts now pre-fill with the--agent-identifiervalue, and post-scaffold output was redesigned to show a boxed command, clickable agent file link, and relevant command reference.Affected areas
packages/novu): Added--runoption to the dev command with cross-platform child process spawning and robust signal/exit handling. Enhanced the init command to pre-fill project names and improved agent template generation to resolve CLI npm tags (@latestvs@rc) from the CLI's package version. Post-scaffold output now renders a boxed combinedcd && npm run dev:novucommand, terminal hyperlink to the agent file, and a commands table.apps/dashboard): Updated the agent code setup flow's second step from "Start the dev tunnel" to "Start your agent locally" with instructions to runnpm run dev:novuinstead of the previous two-terminal approach.Key technical decisions
--runare tracked and cleaned up on SIGINT or app exit; app crashes propagate their exit code to the CLI.dev:novuscript in generated agent templates.Testing
Manual verification completed for core flows: spawning Next.js via
--run, signal handling, backward compatibility without--run, project name pre-fill, generated npm scripts, and post-scaffold output formatting. Dashboard onboarding copy update verified visually but not yet tested in the full dashboard context.