Repository navigation
docs(sdk): guide package scaffolds to define builders - #344
Conversation
Close pm-3mph by aligning generated package scaffolds with the zero-cost define* SDK builder ADR. - document the defineCommand/defineAfterCommandHook upgrade path in generated package READMEs while preserving import-free entrypoints for install-before-dependencies - add scaffold regression coverage and update package authoring docs - regenerate package-owned CHANGELOG.md through pm-changelog Verification: node scripts/run-tests.mjs test -- tests/unit/extensions/extension-scaffold-define-guidance.spec.ts; pm test pm-3mph --run --progress; pnpm build; pnpm quality:static; node scripts/run-tests.mjs coverage (100/100/100/100, 245 files, 4853 tests); pnpm security:scan; pnpm contracts:check; pnpm changelog:pm && pnpm changelog:pm:check; pm package doctor --project --detail deep --json; pm health --check-only --summary --json; temp-dir package init/install/npm test smoke; sentry/telemetry gate.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
Warning Review limit reached
More reviews will be available in 37 minutes and 20 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds an "Authoring With define* Builders" section to the scaffold-generated package README in ChangesExtension scaffold define* guidance
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Get PR details
gh pr view 344 --repo unbraind/pm-cli --json title,body,additions,deletions,changedFiles,baseRefName,headRefName
echo "---FILES---"
gh pr diff 344 --repo unbraind/pm-cli --name-only
echo "---DIFF---"
gh pr diff 344 --repo unbraind/pm-cliLength of output: 30132
Review:
|
| Area | Finding | Severity |
|---|---|---|
scaffold.ts |
defineAfterCommandHook shown for all capabilities |
Minor |
| Test spec | package+commands combination not exercised |
Minor |
EXTENSIONS.md |
Long single paragraph style | Nit |
| Everything else | Correct and consistent | ✅ |
The changes are approved from a logic and correctness standpoint. The two minor items above are worth a quick pass before merging but won't break anything.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request closes the ADR pm-3mph by updating the extension documentation and package scaffolds to guide authors on upgrading to zero-cost define* SDK builders, while keeping the initial entrypoint import-free. A unit test was also added to verify the scaffolded output. The reviewer suggested expanding the README code example to include the activate and deactivate functions so that the example is fully functional and copy-pasteable.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| "```js", | ||
| 'import { defineCommand, defineAfterCommandHook } from "@unbrained/pm-cli/sdk";', | ||
| "", | ||
| "export const pingCommand = defineCommand({", | ||
| ` name: ${JSON.stringify(commandName)},`, | ||
| ' description: "Starter scaffold command. Replace with your own behavior.",', | ||
| " run: (context) => ({ ok: true, command: context.command }),", | ||
| "});", | ||
| "", | ||
| "export const afterCommandHook = defineAfterCommandHook((context) => {", | ||
| " if (!context.ok) return;", | ||
| " // React to context.affected here as your package grows.", | ||
| "});", | ||
| "```", |
There was a problem hiding this comment.
The code example showing how to use the define* builders is helpful, but it only defines the command and hook without showing how they should be registered inside the activate function. If a package author replaces their index.js with this example, the extension will fail to load because it lacks the activate export and registration calls. Showing the complete activate and deactivate exports makes the example fully copy-pasteable and functional.
"import { defineCommand, defineAfterCommandHook } from \"@unbrained/pm-cli/sdk\";",
"",
"export const pingCommand = defineCommand({",
" name: " + JSON.stringify(commandName) + ",",
" description: \"Starter scaffold command. Replace with your own behavior.\",",
" run: (context) => ({ ok: true, command: context.command }),",
"});",
"",
"export const afterCommandHook = defineAfterCommandHook((context) => {",
" if (!context.ok) return;",
" // React to context.affected here as your package grows.",
"});",
"",
"export function activate(api) {",
" api.registerCommand(pingCommand);",
" api.hooks.afterCommand(afterCommandHook);",
"}",
"",
"export function deactivate() {}",
"",
"export default {",
" activate,",
" deactivate,",
"};",![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@docs/EXTENSIONS.md`:
- Around line 424-425: The new documentation section introducing the "Prefer the
`define*` builders" guidance in docs/EXTENSIONS.md (lines 424-425) lacks a
tracker reference to the pm-3mph ADR decision that drove this change, which
violates the coding guideline for documentation files. Add an inline tracker
reference to pm-3mph near or within this section to improve traceability and
connect the documentation guidance to its underlying decision. The reference
should be formatted as a link following the pattern already established in the
codebase, placed where it provides clear context that this guidance originated
from the pm-3mph decision.
- Line 90: The expanded package init explanation on line 90 mentions upgrading
to SDK define* builders without referencing the pm-3mph ADR that introduced this
upgrade path. Locate the phrase "graduate exported command/hook definitions to
the SDK `define*` builders" in EXTENSIONS.md and convert the backtick-enclosed
"define* builders" text into a markdown link pointing to the pm-3mph ADR
decision document, following the inline reference pattern shown in the review
comment to provide consistency with coding guidelines and CHANGELOG patterns.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: d07697a3-5202-4ec0-9c62-266f516cf689
📒 Files selected for processing (6)
.agents/pm/decisions/pm-3mph.toon.agents/pm/history/pm-3mph.jsonlCHANGELOG.mddocs/EXTENSIONS.mdsrc/cli/commands/extension/scaffold.tstests/unit/extensions/extension-scaffold-define-guidance.spec.ts
Address PR #344 CodeRabbit feedback by marking the hook builder example as optional unless a package declares/registers hooks, and by covering package+commands scaffolds for the generated define* guidance. Verification: node scripts/run-tests.mjs test -- tests/unit/extensions/extension-scaffold-define-guidance.spec.ts; pnpm build; pnpm quality:static.
|
New head pushed after addressing CodeRabbit minor feedback: 38d546e. @codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr view 344 --repo unbraind/pm-cli --json title,body,additions,deletions,changedFiles,headRefOid
echo "---FILES---"
gh pr diff 344 --repo unbraind/pm-cli --name-only
echo "---DIFF---"
gh pr diff 344 --repo unbraind/pm-cliLength of output: 32218
What was fixed1. The generated README snippet now includes an inline comment that makes the optional nature explicit: export const afterCommandHook = defineAfterCommandHook((context) => {
// Only add this when your package declares and registers hooks.
if (!context.ok) return;
// React to context.affected here as your package grows.
});Clear, unambiguous, and doesn't require the reader to cross-reference the scaffold manifest. 2. it("documents the define* builder upgrade path for commands-only package scaffolds", () => {
const scaffold = buildStarterExtensionScaffoldFiles("tool-kit", "tool kit ping", "package", "commands");
// ...
expect(readme).toContain("## Authoring With define* Builders");
expect(readme).toContain("Only add this when your package declares and registers hooks.");
expect(entrypoint).not.toContain('from "`@unbrained/pm-cli/sdk`"');
});The three-test matrix now exhaustively covers the branching condition ( Remaining items
No new issues found in the follow-up diff. The PR is in good shape to merge. [approve] |
There was a problem hiding this comment.
Code Review
This pull request closes ADR pm-3mph by adding guidance on using zero-cost define* SDK builders to generated package READMEs and updating the extension authoring documentation. A new unit test suite has been added to verify this guidance while ensuring the entrypoint remains import-free. Feedback on the changes points out a potential runtime activation failure: the generated README always suggests using defineAfterCommandHook even if the scaffolded package lacks the hooks capability. The reviewer recommends dynamically tailoring the define* builder examples in the README to match the specific capability being scaffolded.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| "", | ||
| "## Authoring With define* Builders", | ||
| "`index.js` stays import-free so the package can be installed locally before", | ||
| "`npm install` materializes peer dependencies. After installing dependencies,", | ||
| "use the public SDK authoring builders for exported definitions you want", | ||
| "editor type-checking, contextual handler inference, and direct unit tests for:", | ||
| "```js", | ||
| 'import { defineCommand, defineAfterCommandHook } from "@unbrained/pm-cli/sdk";', | ||
| "", | ||
| "export const pingCommand = defineCommand({", | ||
| ` name: ${JSON.stringify(commandName)},`, | ||
| ' description: "Starter scaffold command. Replace with your own behavior.",', | ||
| " run: (context) => ({ ok: true, command: context.command }),", | ||
| "});", | ||
| "", | ||
| "export const afterCommandHook = defineAfterCommandHook((context) => {", | ||
| " // Only add this when your package declares and registers hooks.", | ||
| " if (!context.ok) return;", | ||
| " // React to context.affected here as your package grows.", | ||
| "});", | ||
| "```", | ||
| "The builders return their argument unchanged; runtime validation still lives", |
There was a problem hiding this comment.
The generated README currently always suggests using defineAfterCommandHook even if the scaffolded package does not have the hooks capability. If a package author copies this example and registers the hook without declaring the hooks capability in manifest.json, it will cause a blocking extension_capability_missing activation failure at runtime.
We should dynamically tailor the define* builders example in the README to match the specific capability being scaffolded (e.g., showing defineSearchProvider and defineVectorStoreAdapter for search packages, defineImporter and defineExporter for importers, etc.).
"",
"## Authoring With define* Builders",
"`index.js` stays import-free so the package can be installed locally before",
"`npm install` materializes peer dependencies. After installing dependencies,",
"use the public SDK authoring builders for exported definitions you want",
"editor type-checking, contextual handler inference, and direct unit tests for:",
"```js",
"import { " + (
capability === "hooks"
? "defineCommand, defineAfterCommandHook"
: capability === "search"
? "defineCommand, defineSearchProvider, defineVectorStoreAdapter"
: capability === "importers"
? "defineCommand, defineImporter, defineExporter"
: "defineCommand"
) + " } from \"@unbrained/pm-cli/sdk\";",
"",
"export const pingCommand = defineCommand({",
" name: " + JSON.stringify(commandName) + ",",
" description: \"Starter scaffold command. Replace with your own behavior.\",",
" run: (context) => ({ ok: true, command: context.command }),",
"});",
...(capability === "hooks"
? [
"",
"export const afterCommandHook = defineAfterCommandHook((context) => {",
" if (!context.ok) return;",
" // React to context.affected here as your package grows.",
"});",
]
: []),
...(capability === "search"
? [
"",
"export const searchProvider = defineSearchProvider({",
" name: " + JSON.stringify(extensionName + "-search") + ",",
" query: async (context) => {",
" const needle = context.query.toLowerCase();",
" const hits = context.documents",
" .filter((doc) => String(doc.metadata.title ?? '').toLowerCase().includes(needle))",
" .map((doc) => ({ id: doc.metadata.id, score: 1, matched_fields: ['title'] }));",
" return { hits };",
" },",
" embed: async (context) => [context.input.length],",
"});",
"",
"export const vectorStoreAdapter = defineVectorStoreAdapter({",
" name: " + JSON.stringify(extensionName + "-vector") + ",",
" query: async (context) => [{ id: 'starter-vector-hit', score: context.limit }],",
" upsert: async (context) => ({ upserted: context.points.length }),",
" delete: async (context) => ({ deleted: context.ids.length }),",
"});",
]
: []),
...(capability === "importers"
? [
"",
"export const importer = defineImporter(async (context) => ({",
" imported: 1,",
" source: context.options.source ?? 'starter',",
" args: context.args,",
"}));",
"",
"export const exporter = defineExporter(async (context) => ({",
" exported: true,",
" destination: context.options.destination ?? 'stdout',",
" args: context.args,",
"}));",
]
: []),
"```",
"The builders return their argument unchanged; runtime validation still lives",
"in `api.register*`, and behavior validation lives in `sdk/testing`."
There was a problem hiding this comment.
Code Review
This pull request closes the ADR for SDK define* builders by updating the package scaffold generator to document the define* builder upgrade path in the generated README, while keeping the initial entrypoint import-free. It also updates the extension documentation and adds regression tests. The reviewer suggested expanding the README code example to show the complete entrypoint structure, including the activate function and default export, to prevent users from breaking their extensions when applying the guidance.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| "```js", | ||
| 'import { defineCommand, defineAfterCommandHook } from "@unbrained/pm-cli/sdk";', | ||
| "", | ||
| "export const pingCommand = defineCommand({", | ||
| ` name: ${JSON.stringify(commandName)},`, | ||
| ' description: "Starter scaffold command. Replace with your own behavior.",', | ||
| " run: (context) => ({ ok: true, command: context.command }),", | ||
| "});", | ||
| "", | ||
| "export const afterCommandHook = defineAfterCommandHook((context) => {", | ||
| " // Only add this when your package declares and registers hooks.", | ||
| " if (!context.ok) return;", | ||
| " // React to context.affected here as your package grows.", | ||
| "});", | ||
| "```", |
There was a problem hiding this comment.
The code block in the generated README shows how to define the command and hook using the define* builders, but it does not show how to actually register them within the activate function or export the extension module. If a user replaces or updates their index.js following this example, they might inadvertently remove the activate export, causing the extension to fail to load.
Showing the complete entrypoint structure with the activate function and default export will make the guide fully actionable and prevent runtime loading issues.
"```js",
'import { defineCommand, defineAfterCommandHook } from "@unbrained/pm-cli/sdk";',
"",
"export const pingCommand = defineCommand({",
` name: ${JSON.stringify(commandName)},`,
' description: "Starter scaffold command. Replace with your own behavior.",',
" run: (context) => ({ ok: true, command: context.command }),",
"});",
"",
"export const afterCommandHook = defineAfterCommandHook((context) => {",
" // Only add this when your package declares and registers hooks.",
" if (!context.ok) return;",
" // React to context.affected here as your package grows.",
"});",
"",
"export function activate(api) {",
" api.registerCommand(pingCommand);",
" // api.hooks.afterCommand(afterCommandHook);",
"}",
"",
"export default {",
" activate,",
"};",
"```",Address PR #344 Gemini/CodeRabbit follow-up by making generated define* README examples complete and capability-aware, with activate/deactivate/default export shown and ADR links added to package-author docs. Verification: node scripts/run-tests.mjs test -- tests/unit/extensions/extension-scaffold-define-guidance.spec.ts; pnpm build; pnpm quality:static; temp scaffold smoke for commands/search/importers/hooks plus hook package install/invoke/npm test.
|
New head pushed after addressing Gemini and CodeRabbit follow-up: 338837f. @codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr view 344 --repo unbraind/pm-cli --json title,body,additions,deletions,changedFiles,headRefOid
echo "---FILES---"
gh pr diff 344 --repo unbraind/pm-cli --name-only
echo "---DIFF---"
gh pr diff 344 --repo unbraind/pm-cliLength of output: 38204
What changed
The
This is strictly better than the previous "mark it optional with a comment" approach — each scaffold now shows only what's relevant. The Test spec — exhaustive 4-test matrix ✅
Both new sentences now link to SummaryNo issues found. The PR is in excellent shape. [approve] |
There was a problem hiding this comment.
Code Review
This pull request closes the ADR pm-3mph by introducing guidance for the SDK define* builders in generated package scaffolds. It updates the scaffolding logic to generate capability-tailored README examples for defineCommand, defineAfterCommandHook, defineSearchProvider, defineVectorStoreAdapter, defineImporter, and defineExporter while keeping starter entrypoints import-free. Documentation and unit tests are also added. The reviewer pointed out that the generated snippet for the importers capability registers the importer and exporter without their required options object (which defines flags and actions), potentially causing runtime issues with undefined options. Corresponding test updates were also suggested to verify these options.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (capability === "importers") { | ||
| defineBuilderSnippet.push( | ||
| ` api.registerImporter(${JSON.stringify(adapterName)}, importer);`, | ||
| ` api.registerExporter(${JSON.stringify(adapterName)}, exporter);`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
The generated define* builder snippet for the importers capability registers the importer and exporter without their third argument (the options object containing action, description, and flags). Omitting these options means the CLI won't register the --source and --destination flags, causing context.options.source and context.options.destination to be undefined at runtime and breaking the starter's functionality. We should include the full registration options in the README snippet.
if (capability === "importers") {
defineBuilderSnippet.push(
" api.registerImporter(" + JSON.stringify(adapterName) + ", importer, {",
" action: " + JSON.stringify(adapterName + " import") + ",",
' description: "Import starter records into pm context.",',
' flags: [',
' {',
' long: "--source",',
' value_name: "name",',
' value_type: "string",',
' description: "Source name or path to import from.",',
' },',
' ],',
' });',
" api.registerExporter(" + JSON.stringify(adapterName) + ", exporter, {",
" action: " + JSON.stringify(adapterName + " export") + ",",
' description: "Export pm context into starter records.",',
' flags: [',
' {',
' long: "--destination",',
' value_name: "name",',
' value_type: "string",',
' description: "Destination name or path to export to.",',
' },',
' ],',
' });',
);
}| expect(importerReadme).toContain('api.registerImporter("sync kit items", importer);'); | ||
| expect(importerReadme).toContain('api.registerExporter("sync kit items", exporter);'); |
There was a problem hiding this comment.
Update the test assertions to verify that the generated importer and exporter registrations in the README include the options object.
| expect(importerReadme).toContain('api.registerImporter("sync kit items", importer);'); | |
| expect(importerReadme).toContain('api.registerExporter("sync kit items", exporter);'); | |
| expect(importerReadme).toContain('api.registerImporter("sync kit items", importer, {'); | |
| expect(importerReadme).toContain('api.registerExporter("sync kit items", exporter, {'); |
|
New head pushed after addressing Gemini importer options feedback: 22930e3. @codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr view 344 --repo unbraind/pm-cli --json title,body,additions,deletions,changedFiles,headRefOid
echo "---FILES---"
gh pr diff 344 --repo unbraind/pm-cli --name-only
echo "---DIFF---"
gh pr diff 344 --repo unbraind/pm-cliLength of output: 40379
What changed
The
The handler bodies also demonstrate Test spec — expanded importer assertions ✅ The third test now pins the full option shape for both sides of the importer pair, including action strings, flag long names, and adapter name derivation ( Summary
No new issues. The PR is in excellent shape to merge. [approve] |
There was a problem hiding this comment.
Code Review
This pull request closes out the ADR pm-3mph by updating the decision records, changelog, and extension documentation to align with the zero-cost define* SDK builders. Specifically, it enhances the package scaffolding logic in src/cli/commands/extension/scaffold.ts to include detailed define* builder guidance and copy-pasteable code snippets in the generated README.md based on the selected capabilities (commands, hooks, search, or importers), while ensuring the generated entrypoint remains import-free for local installations. Additionally, a comprehensive suite of unit tests has been added in tests/unit/extensions/extension-scaffold-define-guidance.spec.ts to verify this scaffolding behavior. There are no review comments provided, so we have no feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Summary
define*SDK builder ADR.npm install, while documenting thedefineCommand/defineAfterCommandHookupgrade path in generated package READMEs.CHANGELOG.mdthroughpm-changelog.Verification
node scripts/run-tests.mjs test -- tests/unit/extensions/extension-scaffold-define-guidance.spec.tspm test pm-3mph --run --progresspnpm buildpnpm quality:staticnode scripts/run-tests.mjs coverage= 100/100/100/100, 245 files, 4853 testspnpm security:scanpnpm contracts:checkpnpm changelog:pm && pnpm changelog:pm:checkpm package doctor --project --detail deep --jsonpm health --check-only --summary --jsonpm init,pm package init --capability hooks, local install before dependencies, command invocation, generated READMEdefine*check,npm install --no-savecurrent checkout,npm testnode scripts/release/sentry-telemetry-gate.mjs --json --sentry-window-days 14pm telemetry status --jsonGitHub / dependency checks
pnpm audit --audit-level moderatefound no known vulnerabilities.pnpm outdated --format jsononly reported Node-major-bound@types/node26 andcommander15 while current wanted versions remain pinned.npm view pm-changelog versionreturned2026.6.20, matching the installed managed extension version.