refactor: move registry item installation to the core#184
refactor: move registry item installation to the core#184MihalyToth20 merged 71 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 1970010 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
WalkthroughMoves registry functionality from the CLI into core: adds an in-memory registry client with caching, new install-to-Xano flow, registry types and schema updates, CLI now prompts/selects components and delegates installation to core; includes tests, docs, and release changesets. Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI Command
participant Core as Core (Caly)
participant Reg as Registry Server
participant Xano as Xano API
CLI->>Core: getRegistryIndex(registryUrl)
Core->>Reg: GET /index.json
Reg-->>Core: index.json
Core-->>CLI: registry index (cached)
CLI->>CLI: Prompt user for components (multiselect)
CLI->>Core: getRegistryItem(componentName, registryUrl)
Core->>Reg: GET /{component}.json
Reg-->>Core: component JSON
Core-->>CLI: registry item
CLI->>Core: installRegistryItemToXano(item, context, registryUrl)
Core->>Core: sortFilesByType(item.files)
loop per file
Core->>Core: fetch content (inline or GET registry file)
Core->>Core: resolveInstallUrl(type, params)
Core->>Core: loadToken()
Core->>Xano: POST install content (Authorization)
Xano-->>Core: 200/4xx/5xx
Core->>Core: record installed/failed/skipped
end
Core-->>CLI: aggregated results
CLI->>CLI: print install summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/cli/src/commands/registry/index.ts (1)
18-41: Guard against undefinedcomponentswith optional variadic args.Optional variadic command arguments in Commander.js can be
undefinedwhen omitted. The code uses.argument('[components...]', ...)which passes this potentially undefined value directly toaddToXano. IfaddToXanoexpects an array, this causes runtime errors. Normalize to an empty array before passing:Suggested fix
.action( withErrorHandler(async (components, options) => { + const componentNames = components ?? []; if (options.registry) { console.log('command registry option: ', options.registry); process.env.CALY_REGISTRY_URL = options.registry; } await addToXano({ - componentNames: components, + componentNames, context: { instance: options.instance, workspace: options.workspace, branch: options.branch, }, core, }); }) );packages/cli/src/utils/feature-focused/registry/general.ts (1)
79-96: Missing return statement inelsebranch.When
selectedGroupis not found, a new API group is created viametaApiPostbut the result is not returned. This will cause the function to returnundefinedinstead of the newly created group.🐛 Proposed fix
} else { - selectedGroup = await metaApiPost({ + return await metaApiPost({ baseUrl: instanceConfig.url, token: await core.loadToken(instanceConfig.name), path: `/workspace/${workspaceConfig.id}/apigroup`, body: { name: groupName, branch: branchConfig.label, swagger: false, docs: '', }, }); } }
🤖 Fix all issues with AI agents
In `@packages/cli/src/commands/registry/implementation/registry.ts`:
- Around line 55-64: When iterating registryItem.files before calling
getApiGroupByName, validate that file.apiGroupName is defined and non-empty; if
it's missing, throw or surface a clear error (e.g., include registryItem.id or
file.name for context) instead of calling getApiGroupByName with undefined.
Update the loop that inspects registryItem.files and the branch where file.type
=== 'registry:query' to guard on file.apiGroupName and set file.apiGroupId only
after a successful lookup via getApiGroupByName.
In `@packages/core/src/features/registry/install-to-xano.ts`:
- Around line 71-88: The loop over filesToInstall (computed via
sortFilesByType(item.files || [])) skips items that have no item.files but do
have item.content; change the logic that builds filesToInstall to handle
content-only registry items by falling back to a synthetic file entry: if
sortFilesByType returns an empty array and item.content exists, create a
single-file array e.g. [{ name: item.name || 'inline', content: item.content }]
so the subsequent for (const file of filesToInstall) block (and its fetch logic)
will process inline content; update references to filesToInstall and ensure the
synthetic file provides a path/name and content so existing code paths work
unchanged.
In `@packages/types/src/index.ts`:
- Around line 314-342: The RegistryItemFile interface currently requires path
and lacks the runtime apiGroupId field, preventing valid inline (content-only)
files; update the RegistryItemFile type so path is optional (path?: string) to
allow embedded files that only use content, and add apiGroupId?: string to
capture the installer/CLI-set field; keep type and content as-is and ensure any
downstream code that assumed path is present can handle undefined.
🧹 Nitpick comments (8)
packages/core/src/features/registry/general.ts (1)
1-27: Add explicit typings to avoid implicit-anyin strict TS.If
noImplicitAnyis enabled,filesandtypePriorityindexing can fail compilation. Consider explicit typing to keep the API clear and robust.♻️ Suggested typing tweak
-const typePriority = { +type FileLike = { type?: string }; + +const typePriority: Record<string, number> = { 'registry:table': 0, 'registry:addon': 1, 'registry:function': 2, 'registry:apigroup': 3, 'registry:query': 4, 'registry:middleware': 5, 'registry:task': 6, 'registry:tool': 7, 'registry:mcp': 8, 'registry:agent': 9, 'registry:realtime': 10, 'registry:workspace/trigger': 11, 'registry:table/trigger': 12, 'registry:mcp/trigger': 13, 'registry:agent/trigger': 14, 'registry:realtime/trigger': 15, 'registry:test': 16, }; -function sortFilesByType(files) { +function sortFilesByType<T extends FileLike>(files: readonly T[]): T[] { return files.slice().sort((a, b) => { const aPriority = typePriority[a.type] ?? 99; const bPriority = typePriority[b.type] ?? 99; return aPriority - bPriority; }); }Please verify against your TypeScript compiler settings.
packages/cli/src/utils/feature-focused/registry/registry-url-map.ts (1)
37-47: Inconsistent indentation style.The new entries use 4-space indentation while existing entries use 3-space indentation. Consider aligning with the existing style for consistency.
♻️ Suggested fix
- 'registry:test': ({ workspaceConfig, branchConfig }) => - `/workspace/${workspaceConfig.id}/workflow_test?branch=${branchConfig.label}`, - 'registry:snippet': () => { - throw new Error('registry:snippet items are not directly installable'); - }, - 'registry:file': () => { - throw new Error('registry:file items are not directly installable'); - }, - 'registry:item': () => { - throw new Error('registry:item items are not directly installable'); - }, + 'registry:test': ({ workspaceConfig, branchConfig }) => + `/workspace/${workspaceConfig.id}/workflow_test?branch=${branchConfig.label}`, + 'registry:snippet': () => { + throw new Error('registry:snippet items are not directly installable'); + }, + 'registry:file': () => { + throw new Error('registry:file items are not directly installable'); + }, + 'registry:item': () => { + throw new Error('registry:item items are not directly installable'); + },packages/core/src/features/registry/__tests__/api.spec.ts (1)
65-98: Consider adding error handling test forfetchRegistryFileContent.The test suite covers the happy path (inline content, fetched content, slash normalization) but doesn't test the error case when fetch fails for file content. Consider adding a test similar to the
getRegistryIndexerror test for completeness.💡 Suggested test case
it('should throw error on fetch failure when fetching file content', async () => { const item = {}; (global.fetch as jest.Mock).mockResolvedValueOnce({ ok: false, status: 500, }); await expect(fetchRegistryFileContent(item, 'path/file.js', mockRegistryUrl)) .rejects.toThrow(); });packages/core/src/index.ts (2)
538-541: Consider efficiency ofgetRegistryItemimplementation.This method fetches the entire registry index to find a single item. For large registries, this could be inefficient. The imported
getRegistryItemfrom./features/registry/api(per the code snippet) appears to fetch items directly. Consider using it instead of re-fetching the index.♻️ Suggested improvement
+import { getRegistryIndex, getRegistryItem as getRegistryItemFromApi } from './features/registry/api'; ... async getRegistryItem(componentName: string, registryUrl: string) { - const index = await this.getRegistryIndex(registryUrl); - return index.find(item => item.name === componentName); + return getRegistryItemFromApi(componentName, registryUrl); }
543-550: Consider adding proper type definitions.The
itemandresolvedContextparameters useanytypes, reducing type safety. Consider using the proper types from@repo/types(e.g.,RegistryItem,InstanceConfig,WorkspaceConfig,BranchConfig).♻️ Suggested type improvement
- async installRegistryItemToXano( - item: any, - resolvedContext: { instanceConfig: any; workspaceConfig: any; branchConfig: any }, - registryUrl: string, - ) { + async installRegistryItemToXano( + item: RegistryItem, + resolvedContext: { instanceConfig: InstanceConfig; workspaceConfig: WorkspaceConfig; branchConfig: BranchConfig }, + registryUrl: string, + ) {packages/cli/src/utils/feature-focused/registry/general.ts (1)
35-38: Remove debug console.log statements.The
console.logstatements appear to be debug output that duplicates information already shown by the multiselect prompt. Consider removing them for cleaner UX.♻️ Suggested fix
async function promptForComponents(core, registryUrl) { try { const registry = await core.getRegistryIndex(registryUrl); - console.log('Available components:'); - registry.items.forEach((item, index) => { - console.log(`${index + 1}. ${item.name} - ${item.description}`); - }); const options = registry.items.map((item) => ({packages/core/src/features/registry/api.ts (1)
6-27: Optional: fetch registry paths in parallelIf multiple paths are requested, you can reduce latency by resolving them concurrently while keeping the cache logic.
♻️ Suggested refactor
async function fetchRegistry(paths, registryUrl) { - const results = []; - for (const path of paths) { - const cacheKey = `${registryUrl}/${path}`; - if (registryCache.has(cacheKey)) { - results.push(await registryCache.get(cacheKey)); - continue; - } - const promise = fetch(`${registryUrl}/${path}`) - .then(async (res) => { - if (!res.ok) throw new Error(`Failed to fetch ${path}: ${res.status}`); - return res.json(); - }) - .catch((err) => { - registryCache.delete(cacheKey); - throw err; - }); - registryCache.set(cacheKey, promise); - const resolvedPromise = await promise; - results.push(resolvedPromise); - } - return results; + const promises = paths.map((path) => { + const cacheKey = `${registryUrl}/${path}`; + if (registryCache.has(cacheKey)) return registryCache.get(cacheKey); + const promise = fetch(`${registryUrl}/${path}`) + .then(async (res) => { + if (!res.ok) throw new Error(`Failed to fetch ${path}: ${res.status}`); + return res.json(); + }) + .catch((err) => { + registryCache.delete(cacheKey); + throw err; + }); + registryCache.set(cacheKey, promise); + return promise; + }); + return Promise.all(promises); }packages/core/src/features/registry/install-to-xano.ts (1)
102-120: Reuse token (and avoid URL double-slash)Loading the token per file adds avoidable I/O. Also,
installUrlalready starts with/workspace, so the current join yields//workspace. Load token once and concatenate without the extra/.♻️ Suggested refactor
- for (const file of filesToInstall) { + const xanoToken = await core.loadToken(instanceConfig.name); + const xanoApiUrl = `${instanceConfig.url}/api:meta`; + + for (const file of filesToInstall) { try { // Get content: use inline content if present, else fetch from file path let content; if (file.content) { content = file.content; } else { const normalized = file.path.replace(/^\/+/, ''); const url = `${registryUrl}/${normalized}`; const res = await fetch(url); if (!res.ok) throw new Error(`Failed to fetch file content: ${file.path} (${res.status})`); content = await res.text(); } @@ - const xanoToken = await core.loadToken(instanceConfig.name); - const xanoApiUrl = `${instanceConfig.url}/api:meta`; const installUrl = resolveInstallUrl(file.type, { instanceConfig, workspaceConfig, branchConfig, file, apiGroupId, }); - const response = await fetch(`${xanoApiUrl}/${installUrl}`, { + const response = await fetch(`${xanoApiUrl}${installUrl}`, { method: 'POST', headers: { Authorization: `Bearer ${xanoToken}`, 'Content-Type': 'text/x-xanoscript', }, body: content, });
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@packages/cli/src/commands/registry/implementation/registry.ts`:
- Around line 46-51: The current pushes to results.failed (e.g., where
registryItem is missing in the registry and other spots around lines 99-100)
produce entries that may lack the file field and assume error is an Error with
message; update all places that push into results.failed (look for uses of
results.failed.push in this file, including the branch referencing registryItem
and the other referenced block) to always include a file property (use
componentName or an empty string when unknown) and normalize the error to a
string (if the caught value is an Error use err.message, otherwise stringify
safely via String(err) or JSON.stringify fallback) so each failed entry matches
the declared return shape and downstream code can rely on file:string and
error:string.
- Around line 54-67: The code assumes getApiGroupByName returns a non-null group
and directly accesses apiGroup.id (in the registry item loop in registry.ts);
change this by either updating getApiGroupByName (in
packages/cli/src/utils/feature-focused/registry/general.ts) to always return the
created or found ApiGroup, or add a defensive check after the await: verify
apiGroup is defined and throw a clear error (or create the group) before
assigning file.apiGroupId; reference getApiGroupByName and the local variable
apiGroup and file.apiGroupId when making the fix.
In `@packages/core/src/features/registry/api.ts`:
- Around line 49-58: In fetchRegistryFileContent, don't treat empty-string
inline content as missing by using a truthy check on item.content; replace the
condition that currently checks "if (item.content)" with an explicit
null/undefined check (e.g., item.content !== null && item.content !== undefined
or item.content == null guard) so that empty strings are returned directly and
only null/undefined trigger the remote fetch; keep the rest of the logic
(normalizing filePath, building url, fetching, and error handling) unchanged.
In `@packages/core/src/features/registry/general.ts`:
- Around line 26-31: The comparator in sortFilesByType can receive undefined
priorities for unknown types (e.g., 'registry:unknown'), causing NaN and
unstable ordering; update sortFilesByType to coerce typePriority[a.type] and
typePriority[b.type] to numeric values by providing a default low-priority
number (e.g., a large integer constant) when the lookup is undefined, then use
those numeric priorities in the subtraction; reference the sortFilesByType
function and the typePriority lookup to locate where to apply the default value.
In `@packages/core/src/features/registry/install-to-xano.ts`:
- Around line 124-132: The pushed objects into results.installed and
results.failed do not match the InstallResults shape: include a component field
(e.g., component: file.component || file.path || '<inline>') when pushing to
results.installed and results.failed so each entry satisfies the InstallResults
type, keep response: body for success and error info for failures; also handle
the catch parameter as unknown and extract the message safely (use error
instanceof Error ? error.message : String(error)) before pushing to
results.failed so TypeScript strict mode compiles cleanly.
- Around line 77-91: The loop over filesToInstall can call
file.path.replace(...) when file.path is undefined; update the logic in the
install-to-xano.ts loop (inside the for (const file of filesToInstall) block
around the normalized variable) to first validate that file.path exists when
file.content is falsy and handle the missing path explicitly: either throw a
clear, descriptive error referencing the file (e.g., include file.name or index)
or skip the file, and only call file.path.replace(...) after confirming
file.path is a non-empty string; ensure the error/skip is logged or surfaced so
missing path issues are detectable at runtime.
In `@packages/types/src/index.ts`:
- Around line 317-323: The RegistryItemFile interface's apiGroupId is typed as
string but must match InstallParams.apiGroupId and ApiGroupConfig.id which are
string | number; update RegistryItemFile (the apiGroupId property) to use type
string | number so all three uses (RegistryItemFile.apiGroupId,
InstallParams.apiGroupId, ApiGroupConfig.id) are consistent.
🧹 Nitpick comments (3)
packages/cli/src/commands/registry/implementation/registry.ts (1)
32-40: Avoid double-fetching registry index
getRegistryIndexis called here and again insidepromptForComponents, which can trigger an extra network call. Consider allowingpromptForComponentsto accept a pre-fetched index.♻️ Suggested refactor (registry.ts + utils)
- const registryIndex = await core.getRegistryIndex(registryUrl); - - if (!componentNames?.length) - componentNames = (await promptForComponents(core, registryUrl)) as string[]; + const registryIndex = await core.getRegistryIndex(registryUrl); + + if (!componentNames?.length) + componentNames = (await promptForComponents(core, registryUrl, registryIndex)) as string[];-async function promptForComponents(core, registryUrl) { +async function promptForComponents(core, registryUrl, registryIndex?) { try { - const registry = await core.getRegistryIndex(registryUrl); + const registry = registryIndex ?? await core.getRegistryIndex(registryUrl); const options = registry.items.map((item) => ({ value: item.name, label: `${item.name} - ${item.description}`, }));packages/core/src/features/registry/install-to-xano.ts (2)
1-11: Consider using stricter types from@repo/typesThe
itemparameter (line 58) andfileproperty are typed asany. Import and useRegistryItemandRegistryItemFilefor better type safety.♻️ Proposed import changes
-import { BranchConfig, InstanceConfig, WorkspaceConfig } from '@repo/types'; +import { BranchConfig, InstanceConfig, WorkspaceConfig, RegistryItem, RegistryItemFile } from '@repo/types'; import type { Caly } from '../..'; import { sortFilesByType } from './general'; interface InstallParams { instanceConfig: InstanceConfig | null; workspaceConfig: WorkspaceConfig; branchConfig: BranchConfig; apiGroupId?: string | number; - file?: any; + file?: RegistryItemFile; }Then update line 58:
async function installRegistryItemToXano( - item: any, + item: RegistryItem, resolvedContext: { instanceConfig: any; workspaceConfig: any; branchConfig: any },
64-64: Add explicit type annotation for resultsThe
resultsobject should be typed asInstallResultsto ensure the structure remains consistent and catch mismatches at compile time.♻️ Proposed fix
+import { BranchConfig, InstanceConfig, WorkspaceConfig, InstallResults } from '@repo/types'; // ... - const results = { installed: [], failed: [], skipped: [] }; + const results: InstallResults = { installed: [], failed: [], skipped: [] };
| if (!registryItem) { | ||
| results.failed.push({ | ||
| component: componentName, | ||
| error: `Component '${componentName}' not found in registry`, | ||
| }); | ||
| continue; |
There was a problem hiding this comment.
Keep failure records consistent with the declared return shape
failed entries can be pushed without a file, and error.message assumes Error. Consider normalizing both to avoid downstream surprises.
🛠️ Suggested fix
if (!registryItem) {
results.failed.push({
component: componentName,
+ file: '<registry>',
error: `Component '${componentName}' not found in registry`,
});
continue;
}
...
} catch (error) {
- results.failed.push({ component: componentName, error: error.message });
+ const message = error instanceof Error ? error.message : String(error);
+ results.failed.push({ component: componentName, file: '<unknown>', error: message });
}Also applies to: 99-100
🤖 Prompt for AI Agents
In `@packages/cli/src/commands/registry/implementation/registry.ts` around lines
46 - 51, The current pushes to results.failed (e.g., where registryItem is
missing in the registry and other spots around lines 99-100) produce entries
that may lack the file field and assume error is an Error with message; update
all places that push into results.failed (look for uses of results.failed.push
in this file, including the branch referencing registryItem and the other
referenced block) to always include a file property (use componentName or an
empty string when unknown) and normalize the error to a string (if the caught
value is an Error use err.message, otherwise stringify safely via String(err) or
JSON.stringify fallback) so each failed entry matches the declared return shape
and downstream code can rely on file:string and error:string.
| // Resolve apiGroupIds for query files | ||
| if (registryItem.files) { | ||
| for (const file of registryItem.files) { | ||
| if (file.type === 'registry:query') { | ||
| if (!file.apiGroupName) { | ||
| throw new Error(`Missing apiGroupName for file ${file.path || 'unnamed'} in registry item ${registryItem.name || registryItem.id}`); | ||
| } | ||
| const apiGroup = await getApiGroupByName( | ||
| file.apiGroupName, | ||
| { instanceConfig, workspaceConfig, branchConfig }, | ||
| core, | ||
| ); | ||
| file.apiGroupId = apiGroup.id; | ||
| } |
There was a problem hiding this comment.
Ensure apiGroup resolution can’t return undefined
If the API group doesn’t exist, getApiGroupByName currently doesn’t return the created group, so apiGroup.id can throw at runtime. Please make the helper return the created group (or add a defensive check here).
🛠️ Suggested fix (packages/cli/src/utils/feature-focused/registry/general.ts)
if (selectedGroup) {
return selectedGroup;
} else {
selectedGroup = await metaApiPost({
baseUrl: instanceConfig.url,
token: await core.loadToken(instanceConfig.name),
path: `/workspace/${workspaceConfig.id}/apigroup`,
body: {
name: groupName,
branch: branchConfig.label,
swagger: false,
docs: '',
},
});
}
+ return selectedGroup;
}📝 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.
| // Resolve apiGroupIds for query files | |
| if (registryItem.files) { | |
| for (const file of registryItem.files) { | |
| if (file.type === 'registry:query') { | |
| if (!file.apiGroupName) { | |
| throw new Error(`Missing apiGroupName for file ${file.path || 'unnamed'} in registry item ${registryItem.name || registryItem.id}`); | |
| } | |
| const apiGroup = await getApiGroupByName( | |
| file.apiGroupName, | |
| { instanceConfig, workspaceConfig, branchConfig }, | |
| core, | |
| ); | |
| file.apiGroupId = apiGroup.id; | |
| } | |
| if (selectedGroup) { | |
| return selectedGroup; | |
| } else { | |
| selectedGroup = await metaApiPost({ | |
| baseUrl: instanceConfig.url, | |
| token: await core.loadToken(instanceConfig.name), | |
| path: `/workspace/${workspaceConfig.id}/apigroup`, | |
| body: { | |
| name: groupName, | |
| branch: branchConfig.label, | |
| swagger: false, | |
| docs: '', | |
| }, | |
| }); | |
| } | |
| return selectedGroup; | |
| } |
🤖 Prompt for AI Agents
In `@packages/cli/src/commands/registry/implementation/registry.ts` around lines
54 - 67, The code assumes getApiGroupByName returns a non-null group and
directly accesses apiGroup.id (in the registry item loop in registry.ts); change
this by either updating getApiGroupByName (in
packages/cli/src/utils/feature-focused/registry/general.ts) to always return the
created or found ApiGroup, or add a defensive check after the await: verify
apiGroup is defined and throw a clear error (or create the group) before
assigning file.apiGroupId; reference getApiGroupByName and the local variable
apiGroup and file.apiGroupId when making the fix.
There was a problem hiding this comment.
TODO: add deffensive check.
There was a problem hiding this comment.
@MihalyToth20 Sounds good! Adding a defensive check will prevent the runtime error. You have two options:
- Update
getApiGroupByNameinpackages/cli/src/utils/feature-focused/registry/general.tsto always return the created group (addreturn selectedGroup;at the end) - Add a defensive check after calling
getApiGroupByNamein the registry.ts file to verifyapiGroupis defined before accessingapiGroup.id
Feel free to ping me if you need help implementing either approach! 👍
| async function fetchRegistryFileContent(item, filePath, registryUrl) { | ||
| if (item.content) { | ||
| return item.content; | ||
| } | ||
| // Remove leading slash if present | ||
| const normalized = filePath.replace(/^\/+/, ''); | ||
| const url = `${registryUrl}/${normalized}`; | ||
| const res = await fetch(url); | ||
| if (!res.ok) throw new Error(`Failed to fetch file content: ${filePath} (${res.status})`); | ||
| return await res.text(); |
There was a problem hiding this comment.
Treat empty inline content as valid
Using a truthy check will skip empty-string content and unnecessarily fetch remote content. Consider a null/undefined check instead.
🛠️ Suggested fix
- if (item.content) {
+ if (item.content != null) {
return item.content;
}📝 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.
| async function fetchRegistryFileContent(item, filePath, registryUrl) { | |
| if (item.content) { | |
| return item.content; | |
| } | |
| // Remove leading slash if present | |
| const normalized = filePath.replace(/^\/+/, ''); | |
| const url = `${registryUrl}/${normalized}`; | |
| const res = await fetch(url); | |
| if (!res.ok) throw new Error(`Failed to fetch file content: ${filePath} (${res.status})`); | |
| return await res.text(); | |
| async function fetchRegistryFileContent(item, filePath, registryUrl) { | |
| if (item.content != null) { | |
| return item.content; | |
| } | |
| // Remove leading slash if present | |
| const normalized = filePath.replace(/^\/+/, ''); | |
| const url = `${registryUrl}/${normalized}`; | |
| const res = await fetch(url); | |
| if (!res.ok) throw new Error(`Failed to fetch file content: ${filePath} (${res.status})`); | |
| return await res.text(); | |
| } |
🤖 Prompt for AI Agents
In `@packages/core/src/features/registry/api.ts` around lines 49 - 58, In
fetchRegistryFileContent, don't treat empty-string inline content as missing by
using a truthy check on item.content; replace the condition that currently
checks "if (item.content)" with an explicit null/undefined check (e.g.,
item.content !== null && item.content !== undefined or item.content == null
guard) so that empty strings are returned directly and only null/undefined
trigger the remote fetch; keep the rest of the logic (normalizing filePath,
building url, fetching, and error handling) unchanged.
There was a problem hiding this comment.
TODO: ensure we only fetch remote content if it's absolutely needed.
There was a problem hiding this comment.
@MihalyToth20, CodeRabbit's suggestion above directly addresses your TODO. The current truthy check if (item.content) treats empty strings as falsy, which means even when inline content is provided (just empty), the function will unnecessarily fetch from remote.
By changing to if (item.content != null), the function will:
- Return empty strings directly (avoiding the fetch)
- Only fetch remote when
item.contentis truly absent (nullorundefined)
This ensures remote content is fetched only when absolutely needed—exactly what your TODO calls for. Implementing the suggested fix would resolve this concern and make the TODO unnecessary.
| for (const file of filesToInstall) { | ||
| try { | ||
| // Get content: use inline content if present, else fetch from file path | ||
| let content; | ||
| if (file.content) { | ||
| content = file.content; | ||
| } else { | ||
| const normalized = file.path.replace(/^\/+/, ''); | ||
| const url = `${registryUrl}/${normalized}`; | ||
| const res = await fetch(url); | ||
| if (!res.ok) | ||
| throw new Error(`Failed to fetch file content: ${file.path} (${res.status})`); | ||
| content = await res.text(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Potential runtime error: file.path may be undefined
At line 84, file.path.replace() is called, but file.path is optional. When file.content is falsy but file.path is also undefined, this will throw.
🛠️ Proposed fix
try {
// Get content: use inline content if present, else fetch from file path
let content;
if (file.content) {
content = file.content;
+ } else if (!file.path) {
+ throw new Error(`File entry has neither content nor path`);
} else {
const normalized = file.path.replace(/^\/+/, '');
const url = `${registryUrl}/${normalized}`;🤖 Prompt for AI Agents
In `@packages/core/src/features/registry/install-to-xano.ts` around lines 77 - 91,
The loop over filesToInstall can call file.path.replace(...) when file.path is
undefined; update the logic in the install-to-xano.ts loop (inside the for
(const file of filesToInstall) block around the normalized variable) to first
validate that file.path exists when file.content is falsy and handle the missing
path explicitly: either throw a clear, descriptive error referencing the file
(e.g., include file.name or index) or skip the file, and only call
file.path.replace(...) after confirming file.path is a non-empty string; ensure
the error/skip is logged or surfaced so missing path issues are detectable at
runtime.
There was a problem hiding this comment.
TODO: ensure that at least one of the two is required. Either the file path or the inline content.
There was a problem hiding this comment.
@MihalyToth20, thanks for noting this! You're right that the runtime validation is needed here.
While the schema in schemas/registry/registry-item.json already enforces the oneOf constraint (either path or content must be present), the runtime code in install-to-xano.ts should also validate this to provide a clear error message if invalid data somehow gets through.
The fix suggested in the review comment above is a good approach—checking !file.path when file.content is falsy and throwing a descriptive error. This would catch the issue early in the loop and provide better debugging information.
Would you like me to help implement this validation? I can create a code change that adds the runtime check with a clear error message.
| if (response.ok) { | ||
| const body = await response.json(); | ||
| results.installed.push({ file: file.path || '<inline>', response: body }); | ||
| } else { | ||
| results.failed.push({ file: file.path || '<inline>', error: `HTTP ${response.status}` }); | ||
| } | ||
| } catch (error) { | ||
| results.failed.push({ file: file.path || '<inline>', error: error.message }); | ||
| } |
There was a problem hiding this comment.
Results structure doesn't match InstallResults type
InstallResults (from @repo/types) expects objects with a component field, but the code only pushes { file, response } and { file, error }. This will cause type mismatches if consumers rely on InstallResults.
🛠️ Proposed fix
if (response.ok) {
const body = await response.json();
- results.installed.push({ file: file.path || '<inline>', response: body });
+ results.installed.push({ component: item.name, file: file.path || '<inline>', response: body });
} else {
- results.failed.push({ file: file.path || '<inline>', error: `HTTP ${response.status}` });
+ results.failed.push({ component: item.name, file: file.path || '<inline>', error: `HTTP ${response.status}` });
}
} catch (error) {
- results.failed.push({ file: file.path || '<inline>', error: error.message });
+ const message = error instanceof Error ? error.message : String(error);
+ results.failed.push({ component: item.name, file: file.path || '<inline>', error: message });
}Also note: error.message on line 131 will fail in TypeScript strict mode since error is unknown. Use error instanceof Error ? error.message : String(error).
📝 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.
| if (response.ok) { | |
| const body = await response.json(); | |
| results.installed.push({ file: file.path || '<inline>', response: body }); | |
| } else { | |
| results.failed.push({ file: file.path || '<inline>', error: `HTTP ${response.status}` }); | |
| } | |
| } catch (error) { | |
| results.failed.push({ file: file.path || '<inline>', error: error.message }); | |
| } | |
| if (response.ok) { | |
| const body = await response.json(); | |
| results.installed.push({ component: item.name, file: file.path || '<inline>', response: body }); | |
| } else { | |
| results.failed.push({ component: item.name, file: file.path || '<inline>', error: `HTTP ${response.status}` }); | |
| } | |
| } catch (error) { | |
| const message = error instanceof Error ? error.message : String(error); | |
| results.failed.push({ component: item.name, file: file.path || '<inline>', error: message }); | |
| } |
🤖 Prompt for AI Agents
In `@packages/core/src/features/registry/install-to-xano.ts` around lines 124 -
132, The pushed objects into results.installed and results.failed do not match
the InstallResults shape: include a component field (e.g., component:
file.component || file.path || '<inline>') when pushing to results.installed and
results.failed so each entry satisfies the InstallResults type, keep response:
body for success and error info for failures; also handle the catch parameter as
unknown and extract the message safely (use error instanceof Error ?
error.message : String(error)) before pushing to results.failed so TypeScript
strict mode compiles cleanly.
There was a problem hiding this comment.
TODO: ensure consistent and meaningful error messages
There was a problem hiding this comment.
@MihalyToth20, good call on noting this TODO. For consistency in this module, consider:
Error message structure:
- Include the component/item name for context:
${item.name} - Specify the operation that failed: "installation", "fetch", "validation"
- Add the affected file:
${file.path || '<inline>'} - Include actionable details where relevant (HTTP status, missing fields)
TypeScript safety:
In TypeScript strict mode with useUnknownInCatchVariables, caught errors are unknown, so check error instanceof Error before accessing error.message.
Example pattern for this code:
// Fetch error
throw new Error(`[${item.name}] Failed to fetch file '${file.path}': HTTP ${res.status}`);
// Validation error
throw new Error(`[${item.name}] Missing apiGroupId for query file '${file.path || '<inline>'}'`);
// Safe catch handling
const message = error instanceof Error ? error.message : String(error);
results.failed.push({
component: item.name,
file: file.path || '<inline>',
error: `Installation failed: ${message}`
});Would you like me to create an issue to track this TODO for a follow-up PR?
| export interface RegistryItemFile { | ||
| path?: string; | ||
| content?: string; | ||
| type: RegistryItemType; | ||
| apiGroupName?: string; | ||
| apiGroupId?: string; | ||
| meta?: Record<string, any>; |
There was a problem hiding this comment.
Type inconsistency: apiGroupId should be string | number
The apiGroupId field is typed as string, but InstallParams.apiGroupId in install-to-xano.ts and ApiGroupConfig.id (line 163) use string | number. Align these for consistency.
🛠️ Proposed fix
export interface RegistryItemFile {
path?: string;
content?: string;
type: RegistryItemType;
apiGroupName?: string;
- apiGroupId?: string;
+ apiGroupId?: string | number;
meta?: Record<string, any>;
}🤖 Prompt for AI Agents
In `@packages/types/src/index.ts` around lines 317 - 323, The RegistryItemFile
interface's apiGroupId is typed as string but must match
InstallParams.apiGroupId and ApiGroupConfig.id which are string | number; update
RegistryItemFile (the apiGroupId property) to use type string | number so all
three uses (RegistryItemFile.apiGroupId, InstallParams.apiGroupId,
ApiGroupConfig.id) are consistent.
There was a problem hiding this comment.
TODO: fix types, apiGroupId should be passed as number to the install-to-xano methods, as the Xano metadata API expects integer numbers.
There was a problem hiding this comment.
@MihalyToth20, thank you for clarifying! I've noted that apiGroupId should be typed as number specifically since the Xano metadata API expects integer values. This is an important constraint to ensure compatibility with the API.
Would you like me to create a GitHub issue to track this type refinement? The fix would involve:
- Updating
RegistryItemFile.apiGroupIdtonumber(notstring | number) - Ensuring
InstallParams.apiGroupIdininstall-to-xano.tsis alsonumber - Reviewing and updating
ApiGroupConfig.idfor consistency if needed
✏️ Learnings added
Learnt from: MihalyToth20
Repo: calycode/xano-tools PR: 184
File: packages/types/src/index.ts:489-495
Timestamp: 2026-02-07T16:24:34.001Z
Learning: In the codebase, `apiGroupId` should be typed as `number` (not `string` or `string | number`) because the Xano metadata API expects integer numbers for API group identifiers.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
schemas/registry/registry-item.json (1)
51-71: Includeregistry:fileinfiles[].typeenum.
RegistryItemTypeincludesregistry:file, but the files enum omits it, so valid items would fail validation.🛠️ Suggested fix
"registry:test", "registry:snippet", + "registry:file", "registry:item"packages/cli/src/commands/registry/implementation/registry.ts (1)
99-101: Normalize error handling and add missingfilefield.The catch block accesses
error.messagewithout a type guard and omits thefilefield. This was noted in a previous review.Suggested fix
} catch (error) { - results.failed.push({ component: componentName, error: error.message }); + const message = error instanceof Error ? error.message : String(error); + results.failed.push({ component: componentName, file: '<unknown>', error: message }); }
🤖 Fix all issues with AI agents
In `@AGENTS.md`:
- Around line 136-140: Update the "Test files" bullet in AGENTS.md to remove
"currently no test files exist" and reflect that test files have been added
(e.g., rename the extension example to `.spec.ts` or mention existing files like
`api.spec.ts` and `general.spec.ts`); modify the "Test files" line so it
accurately describes the current test files and extensions instead of stating
none exist.
In `@packages/cli/jest.config.js`:
- Around line 2-5: The current Jest config override replaces
config.testPathIgnorePatterns instead of merging it; update the export so
testPathIgnorePatterns merges the base array with the new pattern (use
config.testPathIgnorePatterns || [] and append
'src/commands/test/implementation/'), ensuring you reference the existing config
symbol and the testPathIgnorePatterns key so existing ignore patterns are
preserved.
In `@packages/cli/src/utils/feature-focused/registry/general.ts`:
- Around line 32-55: In promptForComponents, guard against missing or empty
registry.items returned by core.getRegistryIndex: after obtaining registry from
getRegistryIndex, check that registry && Array.isArray(registry.items) &&
registry.items.length > 0 and if not return an empty array (optionally log an
informational message) before calling registry.items.map or invoking
multiselect; this prevents mapping an undefined value and avoids prompting with
zero options and noisy error logs from the catch block.
In `@packages/core/src/features/registry/install-to-xano.ts`:
- Around line 31-35: The route builders for 'registry:table/trigger',
'registry:mcp/trigger', 'registry:agent/trigger' and 'registry:realtime/trigger'
currently use optional chaining (p.file?.tableId etc.) and can produce URLs with
"undefined"; update each arrow function to explicitly validate the required id
(tableId, mcpId, agentId, realtimeId) on p.file and throw a clear Error
mentioning which ID is missing (e.g. "Missing tableId for
registry:table/trigger") before building the URL; this ensures any call to those
mapping entries fails loudly rather than returning an invalid path.
♻️ Duplicate comments (6)
packages/core/src/features/registry/general.ts (1)
26-30: Guard against unknown registry types to avoid NaN ordering.
typePriority[a.type]can be undefined for unknown types, causing NaN in the comparator and unstable ordering.🛠️ Suggested fix
- const aPriority = typePriority[a.type]; - const bPriority = typePriority[b.type]; + const aPriority = typePriority[a.type] ?? 99; + const bPriority = typePriority[b.type] ?? 99; return aPriority - bPriority;packages/cli/src/commands/registry/implementation/registry.ts (1)
46-51: Keep failure records consistent with declared return shape.The
failedentry pushed here is missing thefilefield that the return type declares (file: string). This was noted in a previous review.Suggested fix
results.failed.push({ component: componentName, + file: '<registry>', error: `Component '${componentName}' not found in registry`, });packages/types/src/index.ts (1)
317-323: AlignRegistryItemFile.apiGroupIdwith other ID types (string | number).This remains inconsistent with
ApiGroupConfig.idandInstallParams.apiGroupId, which arestring | number. Aligning prevents type friction.🛠️ Proposed fix
export interface RegistryItemFile { path?: string; content?: string; type: RegistryItemType; apiGroupName?: string; - apiGroupId?: string; + apiGroupId?: string | number; meta?: Record<string, any>; }packages/core/src/features/registry/api.ts (1)
49-52: Treat empty inline content as valid.Truthy check skips empty-string content and triggers an unnecessary fetch. Use a null/undefined check instead.
🛠️ Suggested fix
- if (item.content) { + if (item.content != null) { return item.content; }packages/core/src/features/registry/install-to-xano.ts (2)
79-85: Guard against missingfile.pathwhen content is absent.
file.path.replace(...)will throw iffile.pathis undefined. Validate before use and fail fast with a clear error.🐛 Suggested fix
let content; if (file.content) { content = file.content; + } else if (!file.path) { + throw new Error('File entry has neither content nor path'); } else { const normalized = file.path.replace(/^\/+/, ''); const url = `${registryUrl}/${normalized}`;
124-132: Align result entries withInstallResultsand handle unknown errors safely.
InstallResultsexpects acomponentfield, anderror.messageis unsafe forunknown. This will break typing and can throw at runtime.🛠️ Suggested fix
if (response.ok) { const body = await response.json(); - results.installed.push({ file: file.path || '<inline>', response: body }); + results.installed.push({ component: item.name, file: file.path || '<inline>', response: body }); } else { - results.failed.push({ file: file.path || '<inline>', error: `HTTP ${response.status}` }); + results.failed.push({ component: item.name, file: file.path || '<inline>', error: `HTTP ${response.status}` }); } } catch (error) { - results.failed.push({ file: file.path || '<inline>', error: error.message }); + const message = error instanceof Error ? error.message : String(error); + results.failed.push({ component: item.name, file: file.path || '<inline>', error: message }); }
🧹 Nitpick comments (6)
packages/core/src/features/registry/__tests__/api.spec.ts (2)
38-63: Missing error handling test forgetRegistryItem.The
getRegistryIndextests include an error case (lines 28-35), butgetRegistryItemlacks a similar test for fetch failures. Consider adding a test for consistency.Suggested test case
+ it('should throw error on fetch failure', async () => { + (global.fetch as jest.Mock).mockResolvedValueOnce({ + ok: false, + status: 404, + }); + + await expect(getRegistryItem('test-item', mockRegistryUrl)).rejects.toThrow(); + });
9-12: Consider testing cache hit behavior.
clearRegistryCacheis called inbeforeEach, but there's no test verifying that repeated calls togetRegistryIndexorgetRegistryItemuse cached results (i.e.,fetchis called only once). This would validate the caching optimization works as intended.packages/cli/src/utils/feature-focused/registry/registry-url-map.ts (1)
3-48: Inconsistent leading slash in URL paths.Some URL mappings omit the leading slash (lines 4-8:
workspace/...), while others include it (lines 13-36:/workspace/...). This inconsistency could cause issues depending on how the base URL is joined. Standardize to one approach.Example fix (remove leading slashes for consistency with first entries)
'registry:addon': ({ workspaceConfig, branchConfig }) => - `/workspace/${workspaceConfig.id}/addon?branch=${branchConfig.label}`, + `workspace/${workspaceConfig.id}/addon?branch=${branchConfig.label}`,Apply similar changes to lines 15-36 and 37-38.
packages/cli/src/commands/registry/implementation/registry.ts (1)
92-98: Consider renamingerrortoreasonfor skipped entries.Skipped entries typically represent intentional non-processing (e.g., already exists), not failures. Using
errorfor the skip reason might be misleading. Considerreasonormessagefor clarity.Suggested change
for (const skipped of installResults.skipped) { results.skipped.push({ component: componentName, file: skipped.file, - error: skipped.error, + reason: skipped.reason ?? skipped.error, }); }packages/core/src/index.ts (1)
529-551: Add complete JSDoc documentation for new public API methods.Per the coding guidelines (AGENTS.md line 123), JSDoc comments are required for public APIs. The new registry methods have minimal or missing documentation. Add parameter descriptions, return types, and example usage.
Example JSDoc for installRegistryItemToXano
+ /** + * Installs a registry item to a Xano instance. + * + * `@param` item - The registry item to install + * `@param` resolvedContext - Resolved configuration context + * `@param` resolvedContext.instanceConfig - Instance configuration + * `@param` resolvedContext.workspaceConfig - Workspace configuration + * `@param` resolvedContext.branchConfig - Branch configuration + * `@param` registryUrl - Base URL of the registry + * `@returns` Promise resolving to installation results with installed, failed, and skipped arrays + * + * `@example` + * ```typescript + * const results = await calyInstance.installRegistryItemToXano( + * registryItem, + * { instanceConfig, workspaceConfig, branchConfig }, + * 'https://registry.example.com' + * ); + * ``` + */ async installRegistryItemToXano(packages/cli/src/utils/feature-focused/registry/general.ts (1)
4-21: Add priorities for the new registry item types to keep ordering deterministic.The type map doesn’t include the newly added
registry:snippet,registry:file, andregistry:item, so they fall back to99and may interleave unexpectedly with other types. Consider extending the map.♻️ Suggested update
const typePriority = { 'registry:table': 1, 'registry:addon': 2, 'registry:function': 3, 'registry:apigroup': 4, 'registry:query': 5, 'registry:middleware': 6, 'registry:task': 7, 'registry:tool': 8, 'registry:mcp': 9, 'registry:agent': 10, 'registry:realtime': 11, 'registry:workspace/trigger': 12, 'registry:table/trigger': 13, 'registry:mcp/trigger': 14, 'registry:agent/trigger': 15, 'registry:realtime/trigger': 16, 'registry:test': 17, + 'registry:snippet': 18, + 'registry:file': 19, + 'registry:item': 20, };
| - **Test framework**: Jest with ts-jest transformer | ||
| - **Test files**: `.test.ts` extension (currently no test files exist) | ||
| - **Test configuration**: JSON-based config files for API testing | ||
| - **Test environment**: Node.js environment | ||
| - **Coverage**: Use `jest-html-reporter` for HTML coverage reports |
There was a problem hiding this comment.
Update outdated statement about test files.
Line 137 states "currently no test files exist", but this PR adds test files (e.g., api.spec.ts, general.spec.ts). Update this statement to reflect the current state.
Suggested fix
-- **Test files**: `.test.ts` extension (currently no test files exist)
+- **Test files**: `.test.ts` or `.spec.ts` extension📝 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.
| - **Test framework**: Jest with ts-jest transformer | |
| - **Test files**: `.test.ts` extension (currently no test files exist) | |
| - **Test configuration**: JSON-based config files for API testing | |
| - **Test environment**: Node.js environment | |
| - **Coverage**: Use `jest-html-reporter` for HTML coverage reports | |
| - **Test framework**: Jest with ts-jest transformer | |
| - **Test files**: `.test.ts` or `.spec.ts` extension | |
| - **Test configuration**: JSON-based config files for API testing | |
| - **Test environment**: Node.js environment | |
| - **Coverage**: Use `jest-html-reporter` for HTML coverage reports |
🤖 Prompt for AI Agents
In `@AGENTS.md` around lines 136 - 140, Update the "Test files" bullet in
AGENTS.md to remove "currently no test files exist" and reflect that test files
have been added (e.g., rename the extension example to `.spec.ts` or mention
existing files like `api.spec.ts` and `general.spec.ts`); modify the "Test
files" line so it accurately describes the current test files and extensions
instead of stating none exist.
There was a problem hiding this comment.
TODO: update test-related comments / documentation
There was a problem hiding this comment.
@MihalyToth20, sounds good! Since you're planning to update the test-related documentation later, would you like me to create an issue to track this TODO? It would help ensure the documentation update doesn't get forgotten.
🧠 Learnings used
Learnt from: CR
Repo: calycode/xano-tools PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T05:59:33.267Z
Learning: Applies to **/*.test.ts?(x) : Use Jest for testing across all packages
| export default { | ||
| ...config, | ||
| testPathIgnorePatterns: ['src/commands/test/implementation/'], | ||
| }; |
There was a problem hiding this comment.
Preserve existing Jest ignore patterns instead of overwriting.
Overriding testPathIgnorePatterns can drop base config ignores and unexpectedly run tests. Merge with the base config list.
🛠️ Suggested fix
export default {
...config,
- testPathIgnorePatterns: ['src/commands/test/implementation/'],
+ testPathIgnorePatterns: [
+ ...(config.testPathIgnorePatterns ?? []),
+ 'src/commands/test/implementation/',
+ ],
};🤖 Prompt for AI Agents
In `@packages/cli/jest.config.js` around lines 2 - 5, The current Jest config
override replaces config.testPathIgnorePatterns instead of merging it; update
the export so testPathIgnorePatterns merges the base array with the new pattern
(use config.testPathIgnorePatterns || [] and append
'src/commands/test/implementation/'), ensuring you reference the existing config
symbol and the testPathIgnorePatterns key so existing ignore patterns are
preserved.
| async function promptForComponents(core, registryUrl) { | ||
| try { | ||
| const registry = await core.getRegistryIndex(registryUrl); | ||
|
|
||
| const options = registry.items.map((item) => ({ | ||
| value: item.name, | ||
| label: `${item.name} - ${item.description}`, | ||
| })); | ||
|
|
||
| const selected = await multiselect({ | ||
| message: 'Select components to add:', | ||
| options, | ||
| required: true, | ||
| }); | ||
|
|
||
| if (isCancel(selected)) { | ||
| return []; | ||
| } | ||
|
|
||
| return selected; | ||
| } catch (error) { | ||
| console.error('Failed to fetch available components:', error); | ||
| return []; | ||
| } |
There was a problem hiding this comment.
Guard against missing/empty registry items before prompting.
If registry.items is missing or empty, the current code either throws (then gets caught) or prompts with no options. Handle this explicitly to avoid confusing UX and noisy error logs.
🔧 Suggested fix
async function promptForComponents(core, registryUrl) {
try {
const registry = await core.getRegistryIndex(registryUrl);
- const options = registry.items.map((item) => ({
+ const items = registry?.items ?? [];
+ if (!items.length) {
+ console.error('No components available in registry index.');
+ return [];
+ }
+
+ const options = items.map((item) => ({
value: item.name,
- label: `${item.name} - ${item.description}`,
+ label: item.description ? `${item.name} - ${item.description}` : item.name,
}));📝 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.
| async function promptForComponents(core, registryUrl) { | |
| try { | |
| const registry = await core.getRegistryIndex(registryUrl); | |
| const options = registry.items.map((item) => ({ | |
| value: item.name, | |
| label: `${item.name} - ${item.description}`, | |
| })); | |
| const selected = await multiselect({ | |
| message: 'Select components to add:', | |
| options, | |
| required: true, | |
| }); | |
| if (isCancel(selected)) { | |
| return []; | |
| } | |
| return selected; | |
| } catch (error) { | |
| console.error('Failed to fetch available components:', error); | |
| return []; | |
| } | |
| async function promptForComponents(core, registryUrl) { | |
| try { | |
| const registry = await core.getRegistryIndex(registryUrl); | |
| const items = registry?.items ?? []; | |
| if (!items.length) { | |
| console.error('No components available in registry index.'); | |
| return []; | |
| } | |
| const options = items.map((item) => ({ | |
| value: item.name, | |
| label: item.description ? `${item.name} - ${item.description}` : item.name, | |
| })); | |
| const selected = await multiselect({ | |
| message: 'Select components to add:', | |
| options, | |
| required: true, | |
| }); | |
| if (isCancel(selected)) { | |
| return []; | |
| } | |
| return selected; | |
| } catch (error) { | |
| console.error('Failed to fetch available components:', error); | |
| return []; | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@packages/cli/src/utils/feature-focused/registry/general.ts` around lines 32 -
55, In promptForComponents, guard against missing or empty registry.items
returned by core.getRegistryIndex: after obtaining registry from
getRegistryIndex, check that registry && Array.isArray(registry.items) &&
registry.items.length > 0 and if not return an empty array (optionally log an
informational message) before calling registry.items.map or invoking
multiselect; this prevents mapping an undefined value and avoids prompting with
zero options and noisy error logs from the catch block.
| 'registry:query': (p) => `apigroup/${p.apiGroupId}/query`, | ||
| 'registry:table/trigger': (p) => `table/${p.file?.tableId}/trigger`, | ||
| 'registry:mcp/trigger': (p) => `mcp/${p.file?.mcpId}/trigger`, | ||
| 'registry:agent/trigger': (p) => `agent/${p.file?.agentId}/trigger`, | ||
| 'registry:realtime/trigger': (p) => `realtime/${p.file?.realtimeId}/trigger`, |
There was a problem hiding this comment.
Validate required IDs for trigger install URLs.
tableId/mcpId/agentId/realtimeId are optional on file, so the URL can become .../undefined/trigger and fail silently. Throw a clear error when required IDs are missing.
🛠️ Suggested fix
- 'registry:table/trigger': (p) => `table/${p.file?.tableId}/trigger`,
- 'registry:mcp/trigger': (p) => `mcp/${p.file?.mcpId}/trigger`,
- 'registry:agent/trigger': (p) => `agent/${p.file?.agentId}/trigger`,
- 'registry:realtime/trigger': (p) => `realtime/${p.file?.realtimeId}/trigger`,
+ 'registry:table/trigger': (p) => {
+ if (!p.file?.tableId) throw new Error('tableId required for table trigger installation');
+ return `table/${p.file.tableId}/trigger`;
+ },
+ 'registry:mcp/trigger': (p) => {
+ if (!p.file?.mcpId) throw new Error('mcpId required for MCP trigger installation');
+ return `mcp/${p.file.mcpId}/trigger`;
+ },
+ 'registry:agent/trigger': (p) => {
+ if (!p.file?.agentId) throw new Error('agentId required for agent trigger installation');
+ return `agent/${p.file.agentId}/trigger`;
+ },
+ 'registry:realtime/trigger': (p) => {
+ if (!p.file?.realtimeId) throw new Error('realtimeId required for realtime trigger installation');
+ return `realtime/${p.file.realtimeId}/trigger`;
+ },🤖 Prompt for AI Agents
In `@packages/core/src/features/registry/install-to-xano.ts` around lines 31 - 35,
The route builders for 'registry:table/trigger', 'registry:mcp/trigger',
'registry:agent/trigger' and 'registry:realtime/trigger' currently use optional
chaining (p.file?.tableId etc.) and can produce URLs with "undefined"; update
each arrow function to explicitly validate the required id (tableId, mcpId,
agentId, realtimeId) on p.file and throw a clear Error mentioning which ID is
missing (e.g. "Missing tableId for registry:table/trigger") before building the
URL; this ensures any call to those mapping entries fails loudly rather than
returning an invalid path.
feat: bundle into binary to have a native host to call from chrome extension
…r xano oc command
… oc setup to include them
|
@copilot Review this pull request, collect all identified issues, and rank them according to priority. When ready write them into a new markdown file into the repo /plans directory and commit it. |
|
@MihalyToth20 I've opened a new pull request, #185, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: MihalyToth20 <34799518+MihalyToth20@users.noreply.github.com>
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 145 out of 160 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const REGISTRY_MAP: Record<string, string | UrlResolver> = { | ||
| // Simple static-like paths | ||
| 'registry:function': (p) => `function`, | ||
| 'registry:table': (p) => `table`, | ||
| 'registry:addon': (p) => `addon`, | ||
| 'registry:apigroup': (p) => `apigroup`, | ||
| 'registry:middleware': (p) => `middleware`, | ||
| 'registry:task': (p) => `task`, | ||
| 'registry:tool': (p) => `tool`, | ||
| 'registry:mcp': (p) => `mcp`, | ||
| 'registry:agent': (p) => `agent`, | ||
| 'registry:realtime': (p) => `realtime`, | ||
| 'registry:test': (p) => `test`, | ||
| 'registry:workspace/trigger': (p) => `trigger`, | ||
|
|
||
| // Complex/Nested paths | ||
| 'registry:query': (p) => `apigroup/${p.apiGroupId}/query`, | ||
| 'registry:table/trigger': (p) => `table/${p.file?.tableId}/trigger`, | ||
| 'registry:mcp/trigger': (p) => `mcp/${p.file?.mcpId}/trigger`, | ||
| 'registry:agent/trigger': (p) => `agent/${p.file?.agentId}/trigger`, | ||
| 'registry:realtime/trigger': (p) => `realtime/${p.file?.realtimeId}/trigger`, | ||
| }; |
There was a problem hiding this comment.
The meta API path segments in REGISTRY_MAP don’t match the CLI’s existing registry-url-map for at least registry:test and registry:realtime* (CLI uses workflow_test and realtime/channel...). This will cause core-based installations to POST to the wrong endpoints for those types. Align these mappings with the known-good CLI URL map (ideally by moving the URL map into core and sharing it) so install-to-xano uses the same endpoints everywhere.
There was a problem hiding this comment.
- Consider the CLI url maps the correct one and implement thise into the Core. If needed move the path resolver into the
@repo/utilswhere the rest of the reusable methods are.
| // Get content: use inline content if present, else fetch from file path | ||
| let content; | ||
| if (file.content) { | ||
| content = file.content; | ||
| } else { | ||
| const normalized = file.path.replace(/^\/+/, ''); | ||
| const url = `${registryUrl}/${normalized}`; | ||
| const res = await fetch(url); | ||
| if (!res.ok) | ||
| throw new Error(`Failed to fetch file content: ${file.path} (${res.status})`); | ||
| content = await res.text(); | ||
| } |
There was a problem hiding this comment.
This fetch path normalization only strips leading slashes and does not prevent path traversal (e.g. ../...) or encoded traversal; additionally file.path is optional in the new RegistryItemFile type and will throw if absent. Reuse the core validateRegistryPath() / fetchRegistryFileContent() utilities from features/registry/api.ts (or equivalent) and handle the path-missing case explicitly to avoid traversal and runtime crashes.
There was a problem hiding this comment.
- ensure the flow doesn't throw fake errors
- For path traversal check if we can use a library or more reusable pattern.
| if (!file.apiGroupName) { | ||
| throw new Error(`Missing apiGroupName for file ${file.path || 'unnamed'} in registry item ${registryItem.name || registryItem.id}`); | ||
| } | ||
| const apiGroup = await getApiGroupByName( | ||
| file.apiGroupName, | ||
| { instanceConfig, workspaceConfig, branchConfig }, | ||
| core, | ||
| ); | ||
| file.apiGroupId = apiGroup.id; |
There was a problem hiding this comment.
registryItem.id is referenced but RegistryItem (as defined in @repo/types) doesn’t include an id field, so this will fail type-checking (and may be undefined at runtime). Use only registryItem.name (and/or componentName) in the error message, or extend the RegistryItem type if an id is genuinely present in registry payloads.
There was a problem hiding this comment.
- Remove the registry id reference in the errors, comply to the existing type
| @@ -140,6 +149,8 @@ async function loadTestConfig(testConfigPath) { | |||
| const content = await readFile(testConfigPath, 'utf8'); | |||
| return JSON.parse(content); | |||
| } else if (ext === '.js') { | |||
| // SECURITY: require() executes the JS file. This is intentional for dynamic configs. | |||
| // Users must only load trusted config files. | |||
| const config = require(path.resolve(testConfigPath)); | |||
| return config.default || config; | |||
| } else { | |||
There was a problem hiding this comment.
The JSDoc states .ts configs are supported, but the implementation only handles .json and .js and will throw for .ts. Either add actual .ts support (e.g., via tsx/ts-node loader or dynamic import in ESM) or update the docstring and error text to match the real supported extensions.
There was a problem hiding this comment.
- Remove the reference to .ts
- Create a specification or plan to add .ts test running support (low priority)
| import { testRunner } from '../features/testing'; | ||
| import type { Caly } from '..'; | ||
| import { ApiGroupConfig, CoreContext, PrepareRequestArgs, AssertDefinition } from '@repo/types'; | ||
| import { ApiGroupConfig, CoreContext, AssertDefinition, TestConfigEntry, TestResult, TestGroupResult } from '@repo/types'; |
There was a problem hiding this comment.
AssertDefinition and TestResult are imported but not used anywhere in this file (the signature now uses TestConfigEntry and returns TestGroupResult[]). Removing unused imports will avoid lint/typecheck noise and keep this module minimal.
| import { ApiGroupConfig, CoreContext, AssertDefinition, TestConfigEntry, TestResult, TestGroupResult } from '@repo/types'; | |
| import { ApiGroupConfig, CoreContext, TestConfigEntry, TestGroupResult } from '@repo/types'; |
There was a problem hiding this comment.
- Remove unused references
| function validateRegistryPath(inputPath: string): string { | ||
| // Remove leading slashes | ||
| let normalized = inputPath.replace(/^\/+/, ''); | ||
|
|
||
| // Block path traversal attempts | ||
| // Check for ".." in path components (handles both / and \ separators) | ||
| if (/(^|\/)\.\.($|\/|\\)|(^|\\)\.\.($|\/|\\)/.test(normalized)) { | ||
| throw new Error(`Invalid registry path: "${inputPath}" contains path traversal`); | ||
| } | ||
|
|
||
| // Also block encoded traversal attempts | ||
| if (normalized.includes('%2e%2e') || normalized.includes('%2E%2E')) { | ||
| throw new Error(`Invalid registry path: "${inputPath}" contains encoded path traversal`); | ||
| } |
There was a problem hiding this comment.
The new path traversal protections aren’t covered by unit tests (current tests cover leading slash normalization but not traversal rejection). Add test cases in packages/core/src/features/registry/__tests__/api.spec.ts for inputs like "../secrets", "..\\secrets", and "%2e%2e/evil" to ensure these validations are enforced.
There was a problem hiding this comment.
- After refactoring the path traversal checks rewrite the tests to cover for the new implementation
| // ----- REGISTRY METHODS ----- // | ||
| /** | ||
| * Get the main registry index. | ||
| */ | ||
| async getRegistryIndex(registryUrl: string) { | ||
| return getRegistryIndex(registryUrl); | ||
| } | ||
|
|
||
| /** | ||
| * Get a specific registry item by name. | ||
| */ | ||
| async getRegistryItem(componentName: string, registryUrl: string) { | ||
| return getRegistryItem(componentName, registryUrl); | ||
| } | ||
|
|
||
| async installRegistryItemToXano( | ||
| item: RegistryItem, | ||
| resolvedContext: { instanceConfig: InstanceConfig; workspaceConfig: WorkspaceConfig; branchConfig: BranchConfig }, | ||
| registryUrl: string, | ||
| ) { | ||
| const results = await installRegistryItemToXano(item, resolvedContext, registryUrl, this); | ||
| return results; | ||
| } |
There was a problem hiding this comment.
The new registry methods have inconsistent indentation relative to the rest of the class, which makes the public API additions harder to scan and maintain. It’d also help readability to avoid the imported-function vs method name collision for installRegistryItemToXano (e.g., alias the import), and optionally add explicit return types for these new public methods.
There was a problem hiding this comment.
- Prettify the file, fix indentation
There was a problem hiding this comment.
Actionable comments posted: 10
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
packages/cli/src/utils/feature-focused/registry/general.ts (1)
59-92:⚠️ Potential issue | 🟠 MajorMissing return statement for newly created API group.
The
getApiGroupByNamefunction creates a new group viametaApiPostwhen the group doesn't exist, but doesn't return the result. The caller will receiveundefinedinstead of the newly created group.🐛 Proposed fix
if (selectedGroup) { return selectedGroup; } else { selectedGroup = await metaApiPost({ baseUrl: instanceConfig.url, token: await core.loadToken(instanceConfig.name), path: `/workspace/${workspaceConfig.id}/apigroup`, body: { name: groupName, branch: branchConfig.label, swagger: false, docs: '', }, }); + return selectedGroup; } }docs/commands/generate.md (1)
3-3:⚠️ Potential issue | 🟡 MinorFix typos in the description.
“Transformative” and “your Xano” are misspelled.✏️ Suggested edit
-> #### Transforamtive operations that allow you to view you Xano through a fresh set of eyes. +> #### Transformative operations that allow you to view your Xano through a fresh set of eyes.Also applies to: 12-12
packages/cli/src/utils/index.ts (1)
2-3:⚠️ Potential issue | 🟡 MinorDuplicate export statement.
Lines 2 and 3 both export from
'./commands/option-sets'. This is likely unintentional and should be removed to avoid confusion.Suggested fix
export * from './commands/context-resolution'; export * from './commands/option-sets'; -export * from './commands/option-sets'; export * from './commands/project-root-finder';packages/cli/README.md (1)
130-130:⚠️ Potential issue | 🟡 MinorOutdated command reference in GitHub Actions example.
Line 130 still references the old command name
generate-oas --all, but the rest of the README uses the new command structure (generate spec --all). This should be updated for consistency.Suggested fix
run: | - generate-oas --all + generate spec --allpackages/cli/src/utils/feature-focused/registry/registry-url-map.ts (1)
4-14:⚠️ Potential issue | 🔴 CriticalFix inconsistent leading slashes in URL paths—currently causes malformed URLs for first four registry types.
URLs are concatenated directly with
${xanoApiUrl}${installUrl}at install-to-xano.ts line 124. SincexanoApiUrlends with/api:meta(no trailing slash), URLs without leading slashes produce malformed paths like/api:metaworkspace/...instead of/api:meta/workspace/....Lines 4–12 (
registry:table,registry:function,registry:apigroup,registry:query) are missing the leading/. All other registry types (lines 13–38) correctly include it. Add the leading/to the first four entries to ensure consistent URL formation.packages/core/src/features/testing/index.ts (1)
227-238:⚠️ Potential issue | 🟡 MinorAdd type guard for error in catch block.
Accessing
error.stackanderror.messageassumes the caught value is anErrorinstance. If a non-Error is thrown, this could produce unexpected results.🛠️ Proposed fix
} catch (error) { const testEnd = Date.now(); const testDuration = testEnd - testStart; + const errorMessage = error instanceof Error + ? (error.stack || error.message) + : String(error); results.push({ path, method, success: false, - errors: error.stack || error.message, + errors: errorMessage, warnings: null, duration: testDuration, }); }
🤖 Fix all issues with AI agents
In @.changeset/loose-pandas-hide.md:
- Line 7: Update the changeset description to reflect the actual PR intent:
replace the generic "chore: updates to testing, to have robust env support, and
better documentation" with a summary that states the refactor moves registry
item installation from the CLI into the core, introduces new registry APIs (core
registry installation and registry-related functions) and updates the CLI to
delegate installation to core; mention any breaking behavior or migration notes
for users of the CLI/registry APIs and keep the tone as a concise changelog
entry matching the PR title "refactor: move registry item installation to the
core".
In `@packages/browser-consumer/esbuild.config.ts`:
- Around line 11-32: The build uses relative paths for entryPoints and outdir
causing CWD-dependent output; update both build invocations in main() to use
absolute paths via resolve(__dirname, ...): set entryPoints to
[resolve(__dirname, 'src/index.ts')] and outdir to resolve(__dirname, 'dist')
(or to the existing distDir if that's already resolve(__dirname, 'dist')), for
both the esmResult and cjsResult builds so the metafile writes (writeFile using
distDir) align with the actual build outputs.
In `@packages/cli/scripts/installer/install.bat`:
- Around line 1-48: The batch installer (install.bat) currently has Unix (LF)
line endings which will break Windows batch parsing; convert this file to
Windows CRLF line endings before merging (ensure the checked-in file uses CRLF),
and add a repo-level rule so future changes stay CRLF (e.g., enable
core.autocrlf or add a .gitattributes entry forcing *.bat eol=crlf); verify the
committed install.bat (the wrapper that checks powershell, uses %~dp0 as
SCRIPT_DIR and conditionally runs install.ps1) contains CRLF line endings and
re-run the static check to confirm the issue is resolved.
In `@packages/cli/src/commands/opencode/implementation.ts`:
- Around line 499-571: The code currently calls killProcessOnPort from
restartServer and the stop branch in handleMessage which can kill unrelated
services; change the logic to only terminate processes we started by tracking
the child PID/handle returned by startServer (use the existing serverProc
variable and a persisted PID from startServer) and avoid calling
killProcessOnPort unless the PID matches our tracked PID; if no tracked PID
exists for the requested port, return/send an error asking the user to pick a
different port instead of force-killing. Update restartServer, the 'stop' branch
in handleMessage, and any startServer result handling to set/clear the tracked
PID/serverProc consistently and only kill that PID when performing stop/restart.
Also ensure restartServer waits for the tracked process to exit before starting
a new one and includes user-facing error messages when refusing to kill unknown
owners.
In `@packages/cli/src/commands/serve/implementation.ts`:
- Around line 5-20: The validatePathArg function currently rejects paths
containing ':' which breaks Windows drive-letter paths like "C:\..."; update
validatePathArg to allow a single colon immediately after a leading drive letter
(e.g., /^[A-Za-z]:/) or strip a leading drive prefix before running the existing
metacharacter regex. Target the validatePathArg function and ensure the logic
only permits that single leading drive colon (not arbitrary colons elsewhere)
before applying the existing unsafe-character test and throwing the same Error
on failure.
In `@packages/cli/src/utils/github-content-fetcher.ts`:
- Around line 131-133: Normalize and sanitize the incoming subpath before
building cacheSubDir: replace backslashes with forward slashes (or use
path.normalize), remove or collapse any '..' segments, then use
path.resolve/path.join to construct cacheSubDir from cacheDir (or
this.defaultCacheDir) and verify with path.relative or startsWith that the
resolved cacheSubDir is inside the base cacheDir; if the check fails, reject the
input or throw an error. Ensure you update the code around cacheDir, cacheSubDir
and subpath in github-content-fetcher.ts to perform these normalization and
validation steps so Windows backslashes or path-traversal attempts cannot escape
the base cache directory.
In `@packages/core/src/features/registry/api.ts`:
- Around line 20-22: The encoded-traversal check in the registry API currently
checks normalized for '%2e%2e' and '%2E%2E' and can miss mixed-case encodings;
update the logic around the normalized variable (used alongside inputPath) to
convert it to a single case (e.g., normalized = normalized.toLowerCase()) before
performing the includes() check for '%2e%2e', or replace the check with a
case-insensitive regex; ensure the thrown Error still references inputPath for
context.
In `@packages/opencode-templates/agents/xano-ai-builder.md`:
- Line 4: Update the provider parameter names and constraints in
xano-ai-builder.md: remove or restrict reasoning_effort where used with gpt-4o
(only allow it for reasoning models like o1 or delete the param for gpt-4o
references), replace any thinking_tokens occurrences with thinkingBudget for
Gemini 2.5 or thinkingLevel for Gemini 3+ and adjust documented ranges to the
provider spec, remove search_grounding and instead document adding google_search
to the tools list to enable grounding, and keep include_thoughts as-is; ensure
model identifiers (e.g., anthropic/claude-sonnet-4-20250514) remain unchanged.
In `@packages/opencode-templates/agents/xano-api-writer.md`:
- Around line 110-116: The db.query calls use quoted table names which
XanoScript disallows; locate the db.query "product" occurrences (the blocks that
assign to $products) and remove the quotes so they read db.query product,
ensuring the where, sort, and return clauses remain unchanged; update both
occurrences mentioned (the blocks producing $products around the given diff
ranges) to use the bare identifier product.
In `@packages/xano-skills/skills/xano-monitoring/SKILL.md`:
- Around line 98-113: Replace invalid XanoScript usages across the file: change
any now() calls to the "now" timestamp literal; update db.add usage such as
db.add performance_log { ... } to include the required wrapper db.add
performance_log { data = { endpoint: $request.path, method = $request.method,
response_time_ms = $duration_ms, query_count = $query_count } } as $result;
convert try/catch blocks to the Xano try_catch form try_catch { try { ... }
catch { /* catch receives $error with message, code, stack */ } }; replace
db.raw calls with db.direct_query { sql = "...", arg = [...] } and replace
http.post usages with the External API Request equivalent from Xano’s visual
builder; and change any log(...) calls to debug.log { value = ... } so all
examples use correct Xano function names and argument shapes.
🟡 Minor comments (37)
packages/cli/LICENSES/opencode-ai.txt-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorConsider hyphenating the compound adjective.
“open-source” is the conventional compound adjective form in “open-source AI coding agent.”
✏️ Proposed tweak
-OpenCode - The open source AI coding agent +OpenCode - The open-source AI coding agentpackages/opencode-templates/agents/xano-task-writer.md-31-32 (1)
31-32:⚠️ Potential issue | 🟡 MinorUse future/placeholder dates in examples to avoid “past date” confusion.
As of Feb 7, 2026, several example
starts_onvalues are already in the past, which could be misread as required values rather than placeholders. Consider replacing with a generic placeholder or a clearly future date.Proposed doc tweak
- schedule = [{starts_on: 2026-01-01 09:00:00+0000, freq: 86400}] + schedule = [{starts_on: YYYY-MM-DD 09:00:00+0000, freq: 86400}] - starts_on: 2026-01-01 09:00:00+0000, + starts_on: YYYY-MM-DD 09:00:00+0000, - schedule = [{starts_on: 2026-01-01 00:00:00+0000, freq: 21600}] + schedule = [{starts_on: YYYY-MM-DD 00:00:00+0000, freq: 21600}] - schedule = [{starts_on: 2026-05-01 23:00:00+0000, freq: 86400}] + schedule = [{starts_on: YYYY-MM-DD 23:00:00+0000, freq: 86400}] - schedule = [{starts_on: 2026-01-01 23:00:00+0000, freq: 86400}] + schedule = [{starts_on: YYYY-MM-DD 23:00:00+0000, freq: 86400}] - schedule = [{starts_on: 2026-01-01 09:00:00+0000, freq: 86400}] + schedule = [{starts_on: YYYY-MM-DD 09:00:00+0000, freq: 86400}]Also applies to: 41-44, 149-150, 193-194, 232-233, 279-280
packages/xano-skills/skills/xano-database-best-practices/SKILL.md-110-111 (1)
110-111:⚠️ Potential issue | 🟡 MinorUse consistent spelling for “Add-ons”.
“Addons” reads like a typo; use “Add-ons” (or “addons”) for consistency and readability.
✏️ Suggested edit
-- Use Addons for efficient joins +- Use Add-ons for efficient joinspackages/xano-skills/skills/xano-database-best-practices/SKILL.md-26-27 (1)
26-27:⚠️ Potential issue | 🟡 MinorClarify or soften the time-based default claim.
“Default since late 2025” can become stale or disputed. Consider either removing the time qualifier or replacing it with a precise, verifiable date or “as of” phrasing.
✏️ Suggested wording tweak
-**Standard SQL Format (Default since late 2025):** +**Standard SQL Format (Default as of 2025-12):**packages/opencode-templates/agents/xano-api-writer.md-158-171 (1)
158-171:⚠️ Potential issue | 🟡 MinorAlign operator syntax in examples and operator table.
The operator table lists
includes, but the CRUD example usesincludes?. Please standardize to the correct operator name so readers don’t copy an invalid variant.Also applies to: 216-220
packages/xano-skills/skills/caly-xano-cli/SKILL.md-477-477 (1)
477-477:⚠️ Potential issue | 🟡 MinorFix typo in comment.
The word "genreted" should be "generated."
📝 Proposed fix
-# Optionally here do logic that would move your genreted library as an (internal) npm-package for reuse in FE projects +# Optionally here do logic that would move your generated library as an (internal) npm-package for reuse in FE projectspackages/xano-skills/skills/caly-xano-cli/SKILL.md-232-232 (1)
232-232:⚠️ Potential issue | 🟡 MinorFix grammatical error in warning message.
The phrase "might result with errors" is grammatically incorrect. It should be "might result in errors."
📝 Proposed fix
-**WARNING**: This is destructive! Overwrites all business logic and restores v1 branch. Data is also restored. Error-prone, this action might result with errors, that cannot be resolved without Support from Xano Team. +**WARNING**: This is destructive! Overwrites all business logic and restores v1 branch. Data is also restored. Error-prone, this action might result in errors that cannot be resolved without support from the Xano Team..changeset/loose-pandas-hide.md-2-4 (1)
2-4:⚠️ Potential issue | 🟡 MinorEnsure new packages have changeset entries.
This changeset covers patch-level chore work (testing, environment, documentation) for the three listed packages, which is appropriate. However, several new packages added in this commit lack changesets:
@calycode/browser-consumer(0.1.0)@calycode/opencode-templates(0.1.0)@calycode/utilsAdd changesets for these packages to document their inclusion in the release, or clarify if they should not be versioned separately.
packages/xano-skills/skills/xano-monitoring/SKILL.md-14-16 (1)
14-16:⚠️ Potential issue | 🟡 MinorAdd language specifiers to fenced code blocks.
Eight code blocks are missing language identifiers, which affects consistent rendering and violates markdown best practices.
📝 Proposed fix: add language specifiers
Line 14:
-``` +```text Dashboard → Analytics → Query Performance**Line 30:** ```diff -``` +```text Sort by:Line 58:
-``` +```text Seq Scan on orders (cost=0.00..1000.00 rows=100 width=200)Line 72:
-``` +```text Index Scan using idx_orders_user_id on orders (cost=0.00..10.00 rows=100 width=200)Line 336:
-``` +```text 1. Check Query Analytics for slow endpointsLine 344:
-``` +```text 1. Get specific request parametersLine 363:
-``` +```text 1. Apply fix (index, code change, etc.)Line 412:
-``` +```text Webhook → External monitoring:Also applies to: 30-35, 58-62, 72-75, 336-340, 344-348, 363-368, 412-422
packages/xano-skills/README.md-47-63 (1)
47-63:⚠️ Potential issue | 🟡 MinorAdd language identifier to fenced code block.
The directory structure code block should specify a language identifier for proper rendering and accessibility.
📝 Proposed fix
-``` +```text skills/ ├── caly-xano-cli/docs/commands/oc-init.md-2-7 (1)
2-7:⚠️ Potential issue | 🟡 MinorFix markdownlint heading increment + command prompt without output.
MD001 triggers because the
####jumps from h1 to h4; MD014 flags$commands without output. Align heading level and remove the shell prompt (or add output).🔧 Proposed fix
-> #### Initialize OpenCode native host integration and configuration for use with the CalyCode extension. +> ## Initialize OpenCode native host integration and configuration for use with the CalyCode extension. ```term -$ xano oc init [options] +xano oc init [options]</details> </blockquote></details> <details> <summary>docs/commands/oc-run.md-2-7 (1)</summary><blockquote> `2-7`: _⚠️ Potential issue_ | _🟡 Minor_ **Fix markdownlint heading increment + command prompt without output.** MD001 triggers because the `####` jumps from h1 to h4; MD014 flags `$` commands without output. Consider using a lower heading level or plain bold text and remove the `$` prompt (or add output). <details> <summary>🔧 Proposed fix</summary> ```diff -> #### Run any OpenCode CLI command (default) +> ## Run any OpenCode CLI command (default) ```term -$ xano oc run [options] +xano oc run [options]</details> </blockquote></details> <details> <summary>docs/commands/oc-templates-install.md-2-7 (1)</summary><blockquote> `2-7`: _⚠️ Potential issue_ | _🟡 Minor_ **Fix markdownlint heading increment + command prompt without output.** MD001 triggers because the `####` jumps from h1 to h4; MD014 flags `$` commands without output. Align heading level and remove the shell prompt (or add output). <details> <summary>🔧 Proposed fix</summary> ```diff -> #### Install or reinstall OpenCode configuration templates. +> ## Install or reinstall OpenCode configuration templates. ```term -$ xano oc templates install [options] +xano oc templates install [options]</details> </blockquote></details> <details> <summary>docs/commands/oc-templates-status.md-2-7 (1)</summary><blockquote> `2-7`: _⚠️ Potential issue_ | _🟡 Minor_ **Fix markdownlint heading increment + command prompt without output.** MD001 triggers because the `####` jumps from h1 to h4; MD014 flags `$` commands without output. Align heading level and remove the shell prompt (or add output). <details> <summary>🔧 Proposed fix</summary> ```diff -> #### Show the status of installed OpenCode templates. +> ## Show the status of installed OpenCode templates. ```term -$ xano oc templates status [options] +xano oc templates status [options]</details> </blockquote></details> <details> <summary>docs/commands/oc-templates-update.md-2-7 (1)</summary><blockquote> `2-7`: _⚠️ Potential issue_ | _🟡 Minor_ **Fix markdownlint heading increment + command prompt without output.** MD001 triggers because the `####` jumps from h1 to h4; MD014 flags `$` commands without output. Align heading level and remove the shell prompt (or add output). <details> <summary>🔧 Proposed fix</summary> ```diff -> #### Update templates by fetching the latest versions from GitHub. +> ## Update templates by fetching the latest versions from GitHub. ```term -$ xano oc templates update [options] +xano oc templates update [options]</details> </blockquote></details> <details> <summary>docs/_sidebar.md-37-44 (1)</summary><blockquote> `37-44`: _⚠️ Potential issue_ | _🟡 Minor_ **Fix list indentation so “templates” nests under “oc”.** Current indentation breaks the list hierarchy and triggers MD005. <details> <summary>🛠️ Suggested fix</summary> ```diff - - **templates** + - **templates**docs/commands/oc-templates-clear-cache.md-1-12 (1)
1-12:⚠️ Potential issue | 🟡 MinorAddress markdownlint MD001/MD014 warnings.
Heading level jumps from H1 to H4, and
$prompts in code blocks lack output.🛠️ Suggested fix
-> #### Clear the template cache (templates will be re-downloaded on next install). +> ## Clear the template cache (templates will be re-downloaded on next install). -$ xano oc templates clear-cache [options] + xano oc templates clear-cache [options] -$ xano oc templates clear-cache --help + xano oc templates clear-cache --helpdocs/commands/oc-templates.md-1-12 (1)
1-12:⚠️ Potential issue | 🟡 MinorAddress markdownlint MD001/MD014 warnings.
Heading level skips H2, and
$prompts are used without output.🛠️ Suggested fix
-> #### Manage OpenCode configuration templates (agents, commands, instructions). +> ## Manage OpenCode configuration templates (agents, commands, instructions). -$ xano oc templates [options] + xano oc templates [options] -$ xano oc templates --help + xano oc templates --helppackages/opencode-templates/AGENTS.md-105-119 (1)
105-119:⚠️ Potential issue | 🟡 MinorAdd a language to the fenced block (MD040).
Line 107 opens a code fence without a language tag. Add
text(or similar) to satisfy markdown linting.🔧 Proposed fix
-``` +```text project/ ├── tables/ # Database schemas ├── functions/ # Reusable logic │ └── namespace/ # Organized by purpose ├── apis/ # REST endpoints │ └── group/ # Organized by API group ├── addons/ # Data fetchers ├── tasks/ # Scheduled jobs ├── agents/ # AI agents ├── tools/ # AI tools └── mcp_servers/ # MCP servers</details> </blockquote></details> <details> <summary>packages/cli/scripts/dev/install-win.bat-1-97 (1)</summary><blockquote> `1-97`: _⚠️ Potential issue_ | _🟡 Minor_ **Ensure CRLF line endings for Windows batch compatibility.** Static analysis reports LF-only; CMD parsing can break with LF-only files. </blockquote></details> <details> <summary>docs/guides/xanoscript.md-137-161 (1)</summary><blockquote> `137-161`: _⚠️ Potential issue_ | _🟡 Minor_ **Add a language to the fenced block for markdownlint compliance.** <details> <summary>Suggested fix</summary> ```diff -``` +```text {output-dir}/ ├── addons/ │ ├── utility-addon.xs │ └── helper-addon.xs ├── functions/ │ ├── auth/ │ │ ├── login.xs │ │ └── verify-token.xs │ └── utils/ │ └── format-date.xs ├── apis/ │ ├── user-api/ │ │ ├── _group.xs │ │ ├── get-users.xs │ │ └── create-user.xs │ └── product-api/ │ └── ... ├── tables/ │ ├── user.xs │ └── product.xs ├── tasks/ │ └── daily-cleanup.xs └── ... -``` +```packages/xano-skills/skills/xano-data-access/SKILL.md-279-284 (1)
279-284:⚠️ Potential issue | 🟡 MinorAdd a language tag to the fenced block for lint compliance.
Line 279 starts a fenced code block without a language specifier; markdownlint flags this.
Suggested fix
-``` +```text API Settings → Caching: - Cache Duration: 60 seconds - Cache Key: Based on inputs - Invalidation: Manual or time-based -``` +```packages/xano-skills/skills/xano-security/SKILL.md-307-314 (1)
307-314:⚠️ Potential issue | 🟡 MinorAdd a language tag to the fenced block for lint compliance.
Line 307 starts a fenced code block without a language specifier; markdownlint flags this.
Suggested fix
-``` +```text API Settings: 1. Select API group 2. Set Rate Limiting: - Requests per minute: 60 - Per: IP address or Auth token 3. Save -``` +```docs/guides/testing.md-500-514 (1)
500-514:⚠️ Potential issue | 🟡 MinorSyntax error in example code.
The "Bad" example has a syntax issue with a semicolon inside the object literal, and the "Good" example has the same issue. This will cause a parsing error if users copy the code.
📝 Proposed fix
// ❌ Bad { requestBody: { - password: 'mysecret123'; + password: 'mysecret123' } } // ✅ Good { requestBody: { - password: '{{ENVIRONMENT.TEST_PASSWORD}}'; + password: '{{ENVIRONMENT.TEST_PASSWORD}}' } }packages/cli/scripts/dev/install-unix.sh-78-79 (1)
78-79:⚠️ Potential issue | 🟡 MinorArch Linux: avoid partial upgrades with
pacman -SyUsing
pacman -Sywithout-ucan lead to partial upgrades, which Arch explicitly warns against as it may break dependencies. Use-Syufor a full system upgrade or just-Sif the user has recently synced.♻️ Suggested fix
- elif [ -f /etc/arch-release ]; then - sudo pacman -Sy --noconfirm nodejs npm + elif [ -f /etc/arch-release ]; then + sudo pacman -S --noconfirm nodejs npmpackages/cli/src/utils/feature-focused/registry/api.ts-20-23 (1)
20-23:⚠️ Potential issue | 🟡 MinorIncomplete encoded traversal check: mixed-case variants not blocked
The check only catches
%2e%2eand%2E%2E, but misses mixed-case variants like%2E%2eor%2e%2E. Attackers could bypass this check.🛡️ Suggested fix
// Also block encoded traversal attempts - if (normalized.includes('%2e%2e') || normalized.includes('%2E%2E')) { + if (normalized.toLowerCase().includes('%2e%2e')) { throw new Error(`Invalid registry path: "${inputPath}" contains encoded path traversal`); }packages/cli/scripts/installer/install.bat-30-36 (1)
30-36:⚠️ Potential issue | 🟡 MinorArguments not forwarded in remote download path
Line 32 passes
%*to the local PowerShell script, but line 35 usingirm|iexdoesn't forward any arguments. Users expecting-Versionor-Uninstallflags to work in the remote case will be surprised.🛠️ Suggested fix
if exist "%SCRIPT_DIR%install.ps1" ( echo [INFO] Running local PowerShell installer... powershell -NoProfile -ExecutionPolicy Bypass -File "%SCRIPT_DIR%install.ps1" %* ) else ( echo [INFO] Downloading and running PowerShell installer... - powershell -NoProfile -ExecutionPolicy Bypass -Command "irm https://get.calycode.com/install.ps1 | iex" + powershell -NoProfile -ExecutionPolicy Bypass -Command "& { $script = irm https://get.calycode.com/install.ps1; Invoke-Expression $script } %*" )Note: Passing arguments through
iexrequires additional handling. Consider downloading the script to a temp file first, then executing with arguments.packages/cli/scripts/README.md-7-17 (1)
7-17:⚠️ Potential issue | 🟡 MinorAdd a language for the directory tree fence.
markdownlint flags MD040 here. Use
text(orbash) to satisfy the rule.✅ Suggested fix
-``` +```text scripts/ ├── installer/ # Production installers (for end-users) │ ├── install.sh # Unix (macOS, Linux) installer │ ├── install.ps1 # Windows PowerShell installer │ └── install.bat # Windows batch wrapper (launches PowerShell) ├── dev/ # Development scripts (for CLI developers) │ ├── install-unix.sh # Unix development setup │ └── install-win.bat # Windows development setup └── README.md # This file</details> </blockquote></details> <details> <summary>packages/cli/scripts/installer/install.sh-37-63 (1)</summary><blockquote> `37-63`: _⚠️ Potential issue_ | _🟡 Minor_ **Validate `--version` has a value before shifting.** With `set -u`, calling `--version` without a value will abort on an unbound `$2`. Add a simple guard to keep the error message clean. <details> <summary>✅ Suggested fix</summary> ```diff --version|-v) - VERSION="$2" - shift 2 + if [[ -z "${2:-}" || "$2" == -* ]]; then + fatal "Missing value for --version" + fi + VERSION="$2" + shift 2 ;;packages/browser-consumer/src/__tests__/browser-config-storage.test.ts-100-106 (1)
100-106:⚠️ Potential issue | 🟡 MinorTest assertion will fail:
readFilereturnsUint8Array, notstring.The
readFileimplementation (line 147-156 in browser-config-storage.ts) always returnsUint8Array. The test expects a string comparison which will fail.🐛 Proposed fix
describe('writeFile and readFile', () => { it('should write and read file', async () => { const content = 'test content'; await storage.writeFile('test.txt', content); const readContent = await storage.readFile('test.txt'); - expect(readContent).toBe(content); + // readFile returns Uint8Array, decode to compare + const decoded = new TextDecoder().decode(readContent as Uint8Array); + expect(decoded).toBe(content); });docs/commands/oc.md-4-4 (1)
4-4:⚠️ Potential issue | 🟡 MinorHyphenate “open-source” in the description.
Compound adjective should be hyphenated in both occurrences.✏️ Suggested edit
- Powered by OpenCode - The open source AI coding agent. + Powered by OpenCode - The open-source AI coding agent.Also applies to: 16-16
docs/guides/scaffolding.md-350-363 (1)
350-363:⚠️ Potential issue | 🟡 MinorAdd a language tag to the fenced block (MD040).
Usetextfor inline format examples.✏️ Suggested edit
-``` +```text category/component-name Good: - auth/jwt-verify - utils/string-helpers - tables/user - api/user-management Bad: - myFunction - StringHelpers - USER_TABLE</details> </blockquote></details> <details> <summary>docs/guides/scaffolding.md-260-288 (1)</summary><blockquote> `260-288`: _⚠️ Potential issue_ | _🟡 Minor_ **Add a language tag to the fenced block (MD040).** Use `text` for directory trees. <details> <summary>✏️ Suggested edit</summary> ```diff -``` +```text my-registry/ ├── registry.json ├── items/ │ ├── auth/ │ │ ├── login/ │ │ │ ├── definition.json │ │ │ └── login.xs │ │ ├── register/ │ │ │ ├── definition.json │ │ │ └── register.xs │ │ └── middleware/ │ │ ├── definition.json │ │ └── auth-middleware.xs │ ├── utils/ │ │ ├── string-helpers/ │ │ │ ├── definition.json │ │ │ └── helpers.xs │ │ └── date-utils/ │ │ ├── definition.json │ │ └── date.xs │ └── tables/ │ ├── user/ │ │ ├── definition.json │ │ └── user.xs │ └── session/ │ ├── definition.json │ └── session.xs</details> </blockquote></details> <details> <summary>docs/guides/scaffolding.md-71-79 (1)</summary><blockquote> `71-79`: _⚠️ Potential issue_ | _🟡 Minor_ **Add a language tag to the fenced block (MD040).** Use `text` for directory trees. <details> <summary>✏️ Suggested edit</summary> ```diff -``` +```text my-components/ ├── registry.json # Registry configuration ├── items/ │ └── sample-function/ │ ├── definition.json # Component metadata │ └── function.xs # XanoScript code └── README.md # Documentation</details> </blockquote></details> <details> <summary>plans/opencode-native-host-analysis.md-180-186 (1)</summary><blockquote> `180-186`: _⚠️ Potential issue_ | _🟡 Minor_ **Malformed markdown table.** The table rows contain unescaped pipe characters inside cells, causing rendering issues. The version check column values like `^v1[8-9] ^v2` and `^v(18 | 19 | 2[0-9])` break the table structure. <details> <summary>📝 Suggested fix</summary> ```diff -| Script | Node Version Check | -| ------------------------- | -------------------------- | --- | --------- | -| `dev/install-win.bat` | Yes (regex `^v1[8-9] ^v2`) | -| `dev/install-unix.sh` | Yes (numeric comparison) | -| `installer/install.bat` | No (just checks existence) | -| `installer/install.sh` | Yes (regex `^v(18 | 19 | 2[0-9])`) | -| `scripts/install-win.bat` | No | +| Script | Node Version Check | +| ------------------------- | --------------------------------------- | +| `dev/install-win.bat` | Yes (regex `^v1[8-9] ^v2`) | +| `dev/install-unix.sh` | Yes (numeric comparison) | +| `installer/install.bat` | No (just checks existence) | +| `installer/install.sh` | Yes (regex `^v(18\|19\|2[0-9])`) | +| `scripts/install-win.bat` | No |packages/cli/src/utils/github-content-fetcher.ts-332-349 (1)
332-349:⚠️ Potential issue | 🟡 MinorRequire both
ownerandrepofor targeted cache clears.With only one provided, the computed directory includes
undefinedand could remove an unintended path.🧹 Suggested guard
const { owner, repo, subpath } = options; + if (!owner || !repo) { + throw new Error('clearCache requires both owner and repo for targeted deletion'); + } const targetDir = path.join( cacheDir, `${owner}-${repo}`, subpath?.replace(/\//g, '-') || '', );packages/cli/src/commands/opencode/index.ts-164-173 (1)
164-173:⚠️ Potential issue | 🟡 MinorValidate
--portbefore passing toserveOpencode.
parseIntcan yieldNaNor out‑of‑range values; reject invalid ports early.✅ Suggested guard
.action(async (options) => { - await serveOpencode({ - port: options.port ? parseInt(options.port, 10) : undefined, - detach: options.detach, - }); + const port = options.port ? Number(options.port) : undefined; + if (port != null && (!Number.isInteger(port) || port < 1 || port > 65535)) { + throw new Error('--port must be an integer between 1 and 65535'); + } + await serveOpencode({ + port: port ?? undefined, + detach: options.detach, + }); });
🧹 Nitpick comments (38)
packages/xano-skills/skills/xano-query-performance/SKILL.md (2)
12-15: Add languages to fenced code blocks to satisfy Markdown linting.
Missing language identifiers trigger MD040; please annotate the fences.Proposed fix
-``` +```text Standard SQL format → Full optimization available JSONB format → Limited indexing (GIN only) -``` +``` -``` +```text Steps: 1. Database → [table name] → Indexes tab 2. Click "Create Index" 3. Select field(s) to index 4. Choose index type: - Index: Standard B-tree (most common) - Unique: Enforces uniqueness (email, slug) - Spatial: Geographic queries - Search: Full-text search (GIN) 5. Save -``` +``` -``` +```text Index fields: user_id (ASC), status (ASC), created_at (DESC) -``` +``` -``` +```text Standard SQL: B-tree indexes on any column JSONB: GIN index on entire 'data' column only JSONB supports: data @> '{"status": "active"}' JSONB does NOT support: Efficient range queries, partial indexes -``` +``` -``` +```text Dashboard → Analytics → Query Performance -``` +```Also applies to: 95-106, 121-123, 127-133, 302-305
330-334: Prefer Markdown links over raw URLs in the Resources section.
Raw URLs hurt readability; link text also survives formatting changes.Proposed fix
-## Resources - -- Xano Indexing Guide: https://docs.xano.com/the-database/database-performance-and-maintenance/indexing -- Database Performance: https://docs.xano.com/the-database/database-performance-and-maintenance -- Query Optimization Tutorial: https://www.xano.com/learn/database-index-basics-speed-up-queries/ +## Resources + +- [Xano Indexing Guide](https://docs.xano.com/the-database/database-performance-and-maintenance/indexing) +- [Database Performance](https://docs.xano.com/the-database/database-performance-and-maintenance) +- [Query Optimization Tutorial](https://www.xano.com/learn/database-index-basics-speed-up-queries/)packages/opencode-templates/agents/xano-task-writer.md (1)
55-64: Clarify that “Monthly = 2,592,000 seconds” is a 30‑day approximation.Months aren’t fixed-length; the table can mislead users into drift. Consider noting it’s a 30‑day approximation and recommend date-based logic for true month‑end scheduling.
Proposed doc tweak
-| Monthly | `2592000` | Monthly billing | +| Monthly (approx. 30 days) | `2592000` | Monthly billing (use date-based logic for true month-end) |packages/cli/esbuild.config.ts (1)
28-28: ExplicitkeepNames: falseis redundant but harmless.Since
minify: trueis already set, esbuild defaultskeepNamestofalseanyway. Adding it explicitly documents the intent, which is fine.One consideration: with both
keepNames: falseandsourcemap: false, stack traces in production errors will show mangled function names with no way to map them back. For a CLI tool where users may report errors, preserving names (keepNames: true) can aid debugging at a small bundle size cost.packages/xano-skills/skills/caly-xano-cli/SKILL.md (1)
382-391: Document the optional component selection flow.According to the PR changes, the
registry addcommand now accepts optional variadic components. Consider adding an example showing the interactive selection flow when no component names are provided.📝 Suggested addition
Add before line 385:
# Interactive component selection xano registry addThis would help users discover the interactive mode introduced by the registry refactor.
packages/opencode-templates/agents/api-designer.md (1)
73-95: Optional: Specify language for the code block.Adding a language identifier to the code block (e.g.,
textormarkdown) would improve rendering consistency and satisfy markdown linters.📝 Proposed improvement
-``` +```text Endpoint: [METHOD] /path/to/resource Description: What this endpoint does.changeset/forty-phones-rhyme.md (1)
1-7: Changeset correctly reflects the core refactor, but verify PR scope.This changeset aligns with the PR title and appropriately uses patch-level bumps for the internal refactoring of moving registry installation logic to core.
However, this PR includes 5 changesets describing distinct changes (styling revamp, OpenCode integration, registry refactor, agent loading, graceful exit fix). While technically valid, consider whether this PR's scope is broader than a single focused change.
packages/xano-skills/README.md (1)
69-69: Optional: Consider hyphenating compound adjective.When "Open source" modifies "AI coding agent", it functions as a compound adjective and style guides recommend hyphenation: "Open-source AI coding agent".
packages/xano-skills/skills/xano-schema-design/SKILL.md (1)
43-79: Consider adding language hints to fenced code blocks.Several code blocks showing table schemas and UI steps lack language specifiers. While these aren't executable code, adding hints like
textorplaintextimproves accessibility and prevents linter warnings.📝 Example fix for Lines 43-46
-``` +```text orders table: | id | customer_name | customer_email | product_name | product_price |</details> </blockquote></details> <details> <summary>packages/cli/scripts/dev/install-unix.sh (1)</summary><blockquote> `61-65`: **Security consideration: piping remote scripts to bash** Lines 63 and 69/72 download and execute scripts directly via `curl | bash`. While common for installers, this pattern bypasses integrity verification. Consider adding checksum verification or at minimum documenting the trust assumption in the script header. For the Homebrew install specifically, you could verify the script before execution or link to Homebrew's official verification guidance. </blockquote></details> <details> <summary>packages/cli/src/utils/feature-focused/registry/api.ts (1)</summary><blockquote> `46-46`: **Consider normalizing trailing slashes on `REGISTRY_URL`** If `CALY_REGISTRY_URL` ends with a trailing slash, the constructed URL will have a double slash (e.g., `http://example.com/registry//file.json`). While most servers handle this, some may not. <details> <summary>♻️ Optional fix</summary> ```diff async function fetchRegistry(paths: string[]) { - const REGISTRY_URL = process.env.CALY_REGISTRY_URL || 'http://localhost:5500/registry'; + const REGISTRY_URL = (process.env.CALY_REGISTRY_URL || 'http://localhost:5500/registry').replace(/\/+$/, '');packages/cli/scripts/installer/install.ps1 (1)
273-318: Uninstall error handling could be more verboseIn
Invoke-Uninstall, errors are suppressed with2>$nullon line 280. Consider logging when uninstall operations fail rather than silently continuing, to help users diagnose partial uninstall states.♻️ Optional improvement
# Remove npm package if (Get-Command xano -ErrorAction SilentlyContinue) { Write-Log "Removing $PackageName package..." "INFO" - npm uninstall -g $PackageName 2>$null + $uninstallResult = npm uninstall -g $PackageName 2>&1 + if ($LASTEXITCODE -ne 0) { + Write-Log "npm uninstall returned: $uninstallResult" "WARN" + } }packages/core/src/features/registry/install-to-xano.ts (1)
57-62: Consider usingRegistryItemtype instead ofanyThe
itemparameter is typed asany, which bypasses TypeScript's type checking. SinceRegistryItemis imported in related files and available from@repo/types, using it here would improve type safety.- item: any, + item: RegistryItem,examples/test-config.example.js (1)
84-88: Clarify the relationship betweenstorekeys andENVIRONMENTprefixThe
storearray uses keys likeAUTH_TOKEN(line 86), but subsequent tests reference{{ENVIRONMENT.AUTH_TOKEN}}(line 116). Consider adding a brief comment explaining that stored values are accessed via theENVIRONMENTprefix to help users understand the convention.📝 Suggested documentation addition
// Extract auth token for subsequent requests + // Note: Stored values are accessed as {{ENVIRONMENT.<key>}} in subsequent requests store: [ { key: 'AUTH_TOKEN', path: '$.authToken' },packages/core/src/implementations/run-tests.ts (1)
3-3: Unused import:AssertDefinition.
AssertDefinitionis imported but not used in this file.🧹 Proposed fix
-import { ApiGroupConfig, CoreContext, AssertDefinition, TestConfigEntry, TestResult, TestGroupResult } from '@repo/types'; +import { ApiGroupConfig, CoreContext, TestConfigEntry, TestGroupResult } from '@repo/types';packages/browser-consumer/src/indexeddb-utils.ts (1)
192-197: Placeholder migration function should be documented or removed.The
migrateDBfunction is a no-op placeholder that only logs. Consider adding a// TODO:comment or removing it until needed to avoid confusion.📝 Proposed improvement
// Migration support (for future versions) +// TODO: Implement migration logic when DB_VERSION is incremented export async function migrateDB(fromVersion: number, toVersion: number): Promise<void> { - // Placeholder for migration logic - console.log(`Migrating DB from ${fromVersion} to ${toVersion}`); - // Implement migration steps here + // Placeholder for future migration logic + console.warn(`DB migration from ${fromVersion} to ${toVersion} not yet implemented`); }packages/browser-consumer/src/browser-config-storage.ts (3)
16-19: DuplicatedGlobalConfiginterface.This interface is identical to the one exported from
./indexeddb-utils. Import it instead of redefining to maintain DRY.♻️ Proposed fix
-import type { ConfigStorage, InstanceConfig, CoreContext, WorkspaceConfig, BranchConfig } from '@repo/types'; +import type { ConfigStorage, InstanceConfig, WorkspaceConfig, BranchConfig } from '@repo/types'; import { initDB, getGlobalConfig, setGlobalConfig, getInstanceConfig, setInstanceConfig, getToken, setToken, getFile, setFile, deleteFile, listFiles, + GlobalConfig, } from './indexeddb-utils'; - -interface GlobalConfig { - currentContext: CoreContext; - instances: string[]; -}
103-123: Potential stale cache in synchronousloadMergedConfig.This method is synchronous and relies on
cachedInstanceConfigs, but the cache is only populated duringensureDirs()orloadInstanceConfig(). If a caller hasn't called these methods first, the cache may be stale or empty. Consider documenting this requirement or making the method async.
196-211: Unexpected side effect:tarExtractpersists all files to IndexedDB.The method both returns extracted files and writes them to IndexedDB (line 207). This implicit persistence may be unexpected. Consider making this behavior explicit via a parameter or documenting it clearly.
📝 Proposed improvement
- async tarExtract(tarGzBuffer: Uint8Array): Promise<{ [filename: string]: Uint8Array | string }> { + /** + * Extracts a tar archive and optionally persists files to IndexedDB. + * `@param` tarGzBuffer - The tar archive buffer + * `@param` persist - Whether to persist extracted files to IndexedDB (default: true) + */ + async tarExtract(tarGzBuffer: Uint8Array, persist = true): Promise<{ [filename: string]: Uint8Array | string }> { // Use js-untar for browser-compatible tar extraction const { untar } = await import('js-untar'); const files = await untar(tarGzBuffer); const result: { [filename: string]: Uint8Array | string } = {}; for (const file of files) { if (file.isFile) { // Convert buffer to Uint8Array if needed const content = file.buffer instanceof Uint8Array ? file.buffer : new Uint8Array(file.buffer); result[file.name] = content; - // Optionally store in IndexedDB - await setFile(file.name, content); + if (persist) { + await setFile(file.name, content); + } } } return result; }packages/cli/src/commands/registry/index.ts (1)
29-31: Consider removing or gating the debug log.The
console.log('command registry option: ', options.registry)will output to stdout in production usage. For a cleaner CLI experience, consider removing this or gating it behind a--verboseflag.♻️ Proposed fix
withErrorHandler(async (components, options) => { if (options.registry) { - console.log('command registry option: ', options.registry); process.env.CALY_REGISTRY_URL = options.registry; }packages/cli/src/index.ts (1)
4-12: Consider more precise argument detection.The current check
args.includes('opencode') && args.includes('native-host')could theoretically match unintended argument combinations (e.g., if these strings appear as option values). For robustness, consider checking for consecutive arguments or the specific subcommand pattern.♻️ Proposed refinement for more precise matching
const args = process.argv; -if (args.includes('opencode') && args.includes('native-host')) { +const opcodeIdx = args.indexOf('opencode'); +if (opcodeIdx !== -1 && args[opcodeIdx + 1] === 'native-host') { startNativeHost(); } else { program.parseAsync(); }That said, the current implementation is pragmatic and unlikely to cause issues given the CLI's command structure.
plans/browser-consumer-plan.md (1)
13-23: Add language specifier to the fenced code block.The package structure code block should specify a language for proper rendering and linting compliance.
📝 Suggested fix
-``` +```text packages/browser-consumer/ ├── src/ │ ├── index.ts # Main exportspackages/core/src/features/registry/__tests__/api.spec.ts (1)
14-36: Consider adding a test for caching behavior.The
getRegistryIndexfunction includes in-memory caching per the AI summary, but there's no test verifying that subsequent calls return the cached value without making additional fetch requests.🧪 Suggested test case
it('should return cached index on subsequent calls', async () => { const mockIndex = { items: ['item1', 'item2'] }; (global.fetch as jest.Mock).mockResolvedValueOnce({ ok: true, json: () => Promise.resolve(mockIndex), }); const result1 = await getRegistryIndex(mockRegistryUrl); const result2 = await getRegistryIndex(mockRegistryUrl); expect(global.fetch).toHaveBeenCalledTimes(1); expect(result1).toEqual(mockIndex); expect(result2).toEqual(mockIndex); });plans/opencode-native-host-analysis.md (1)
31-65: Add language specifier for ASCII diagram.The architecture diagram code block should specify a language (e.g.,
textorplaintext) for proper rendering.📝 Suggested fix
-``` +```text Chrome Extension │packages/core/src/features/registry/__tests__/install-to-xano.spec.ts (1)
131-147: Consider adding tests for nullworkspaceConfigandbranchConfig.The test covers
instanceConfig: null, but the AI summary indicates the function validates required context including workspace and branch configs. Adding similar tests for these scenarios would improve coverage.🧪 Suggested additional test cases
it('should throw when workspaceConfig is null', async () => { const item = { name: 'test', type: 'registry:function', content: 'test', files: [], }; await expect( installRegistryItemToXano( item, { ...mockContext, workspaceConfig: null } as any, 'https://registry.example.com', createMockCore(), ), ).rejects.toThrow('workspaceConfig is required'); }); it('should throw when branchConfig is null', async () => { const item = { name: 'test', type: 'registry:function', content: 'test', files: [], }; await expect( installRegistryItemToXano( item, { ...mockContext, branchConfig: null } as any, 'https://registry.example.com', createMockCore(), ), ).rejects.toThrow('branchConfig is required'); });docs/guides/git-workflow.md (3)
87-103: Add language specifier for directory structure.📝 Suggested fix
-``` +```text project/ ├── schema/
120-131: Add language specifier for branching diagram.📝 Suggested fix
-``` +```text main (production) │
349-365: Add language specifier for commit message format.📝 Suggested fix
-``` +```text type(scope): description Types:docs/guides/patterns.md (2)
84-98: Add language specifiers for pseudocode blocks.These code blocks contain pseudocode/examples that should have language specifiers for proper rendering.
📝 Suggested fixes
-``` +```text // Pseudocode for logging function function log_to_axiom(level, message, data) {-``` +```text // In your Xano functions call: log_to_axiom("info", "User logged in", { user_id: $user.id })
201-209: Specify JSON language for error response example.This code block contains a JSON structure and should use the
jsonlanguage specifier.📝 Suggested fix
-``` +```json // Error response structure { "error": true,Note: If keeping the comment, use
jsonc(JSON with comments) or move the comment outside the code block.docs/guides/registry-authoring.md (1)
50-58: Consider adding language specifier to fenced code blocks.The static analysis tool flagged several code blocks without language specifiers (lines 50, 123, 358, 543, 592). For directory tree structures like this one, you can use
textorplaintextto suppress the warning while preserving readability.Example fix
-``` +```text my-registry/ ├── registry.json # Registry metadatapackages/cli/build-sea.ts (2)
2-2: Remove unused imports.
rmSyncandchmodSyncare imported but never used in this file.Suggested fix
-import { copyFileSync, existsSync, mkdirSync, rmSync, chmodSync } from 'fs'; +import { copyFileSync, existsSync, mkdirSync } from 'fs';
50-59: Consider capturing error details for debugging.When blob injection fails, the error is logged but the message may not be informative since
errorcould be anErrorobject. Consider extracting the message for clearer output.Suggested improvement
} catch (error) { - console.error('Failed to inject blob:', error); + console.error('Failed to inject blob:', error instanceof Error ? error.message : error); process.exit(1); }packages/cli/src/utils/feature-focused/registry/registry-url-map.ts (1)
37-48: Indentation differs from the rest of the file.Lines 37-48 use 4-space indentation while the rest of the file uses 3-space. Consider aligning for consistency.
plans/create-native-bundle.md (1)
20-35: Consider adding language specifiers to fenced code blocks.Static analysis flagged two code blocks without language specifiers (lines 20 and 30). For ASCII diagrams or plain text flows, use
textorplaintextto satisfy markdown linting rules.📝 Proposed fix
-``` +```text Extension → JSON message → Native Host → `npx opencode-ai@latest serve --port 4096`-``` +```text 1. Download calycode-x64.exe / calycode-macos (one binary)packages/core/src/index.ts (2)
529-551: Inconsistent indentation in new registry methods.The new registry methods use 4-space indentation, while the rest of the class uses 3-space indentation. This creates visual inconsistency.
🔧 Proposed fix for consistent indentation
// ----- REGISTRY METHODS ----- // - /** - * Get the main registry index. - */ - async getRegistryIndex(registryUrl: string) { - return getRegistryIndex(registryUrl); - } + /** + * Get the main registry index. + */ + async getRegistryIndex(registryUrl: string) { + return getRegistryIndex(registryUrl); + } - /** - * Get a specific registry item by name. - */ - async getRegistryItem(componentName: string, registryUrl: string) { - return getRegistryItem(componentName, registryUrl); - } + /** + * Get a specific registry item by name. + */ + async getRegistryItem(componentName: string, registryUrl: string) { + return getRegistryItem(componentName, registryUrl); + } - async installRegistryItemToXano( - item: RegistryItem, - resolvedContext: { instanceConfig: InstanceConfig; workspaceConfig: WorkspaceConfig; branchConfig: BranchConfig }, - registryUrl: string, - ) { - const results = await installRegistryItemToXano(item, resolvedContext, registryUrl, this); - return results; - } + async installRegistryItemToXano( + item: RegistryItem, + resolvedContext: { instanceConfig: InstanceConfig; workspaceConfig: WorkspaceConfig; branchConfig: BranchConfig }, + registryUrl: string, + ) { + const results = await installRegistryItemToXano(item, resolvedContext, registryUrl, this); + return results; + }
544-551: Consider adding JSDoc forinstallRegistryItemToXano.Other methods in this class have comprehensive JSDoc with
@paramand@returnsannotations. Adding documentation here would maintain consistency and help consumers understand the method's contract.packages/cli/src/commands/test/implementation/test.ts (1)
283-303: Inconsistent indentation in CI mode block.This block uses 4-space indentation while the rest of the file uses 3-space indentation.
🔧 Proposed fix
- // CI mode: determine exit code - if (ciMode) { - const hasFailures = totalFailed > 0; - const hasWarnings = failOnWarnings && totalWarnings > 0; - - if (hasFailures || hasWarnings) { - const reasons = []; - if (hasFailures) reasons.push(`${totalFailed} test(s) failed`); - if (hasWarnings) reasons.push(`${totalWarnings} warning(s)`); - - outro(`Tests failed: ${reasons.join(', ')}`); - log.error(`\nCI mode: Exiting with code 1 due to ${reasons.join(' and ')}`); - // Return exit code instead of calling process.exit() directly - // Let the CLI commander action handle the actual process exit - return 1; - } else { - outro(`All ${totalPassed} tests passed!`); - } - } - - return totalFailed > 0 ? 1 : 0; + // CI mode: determine exit code + if (ciMode) { + const hasFailures = totalFailed > 0; + const hasWarnings = failOnWarnings && totalWarnings > 0; + + if (hasFailures || hasWarnings) { + const reasons = []; + if (hasFailures) reasons.push(`${totalFailed} test(s) failed`); + if (hasWarnings) reasons.push(`${totalWarnings} warning(s)`); + + outro(`Tests failed: ${reasons.join(', ')}`); + log.error(`\nCI mode: Exiting with code 1 due to ${reasons.join(' and ')}`); + // Return exit code instead of calling process.exit() directly + // Let the CLI commander action handle the actual process exit + return 1; + } else { + outro(`All ${totalPassed} tests passed!`); + } + } + + return totalFailed > 0 ? 1 : 0;
| "@calycode/cli": patch | ||
| --- | ||
|
|
||
| chore: updates to testing, to have robust env support, and better documentation |
There was a problem hiding this comment.
Changeset description doesn't match PR objectives.
The description states "chore: updates to testing, to have robust env support, and better documentation", but the PR title is "refactor: move registry item installation to the core". The description should accurately reflect the primary changes: moving registry installation logic from CLI to core, introducing new registry APIs, and refactoring the CLI to delegate to core.
📝 Suggested description rewrite
-chore: updates to testing, to have robust env support, and better documentation
+refactor: move registry item installation logic to core package
+
+Relocates registry installation functionality from CLI to core, adding new APIs for registry index fetching, item retrieval, and Xano installation. The CLI now delegates to core for registry operations. Also includes enhancements to testing, environment support, and documentation.📝 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.
| chore: updates to testing, to have robust env support, and better documentation | |
| refactor: move registry item installation logic to core package | |
| Relocates registry installation functionality from CLI to core, adding new APIs for registry index fetching, item retrieval, and Xano installation. The CLI now delegates to core for registry operations. Also includes enhancements to testing, environment support, and documentation. |
🤖 Prompt for AI Agents
In @.changeset/loose-pandas-hide.md at line 7, Update the changeset description
to reflect the actual PR intent: replace the generic "chore: updates to testing,
to have robust env support, and better documentation" with a summary that states
the refactor moves registry item installation from the CLI into the core,
introduces new registry APIs (core registry installation and registry-related
functions) and updates the CLI to delegate installation to core; mention any
breaking behavior or migration notes for users of the CLI/registry APIs and keep
the tone as a concise changelog entry matching the PR title "refactor: move
registry item installation to the core".
There was a problem hiding this comment.
- Create updated chabgeset files according to the available commit messages and changed files (we forgot to properly update the changesets along the way)
There was a problem hiding this comment.
🧩 Analysis chain
@MihalyToth20 Good plan! I can help you review the commit messages to identify what changesets should be created. Let me gather the commit history from this PR.
🏁 Script executed:
#!/bin/bash
# Get the list of commits in the PR branch that are not in main
git log --oneline main..dev
echo ""
echo "=== Detailed commit messages ==="
git log --format="%H%n%s%n%b%n---" main..dev | head -100
Length of output: 413
🏁 Script executed:
#!/bin/bash
# Get commits from the PR using GitHub CLI
gh pr view 184 --json commits --jq '.commits[] | "[\(.oid[0:7])] \(.messageHeadline)\n\(.messageBody)"'
echo ""
echo "=== Files changed in PR ==="
gh pr view 184 --json files --jq '.files[] | .path' | sort
Length of output: 7658
🏁 Script executed:
#!/bin/bash
# Read all changeset files to see what's already documented
echo "=== Existing Changeset Files ==="
for file in .changeset/*.md; do
if [[ "$file" != *"README"* ]] && [[ -f "$file" ]]; then
echo ""
echo "--- $(basename "$file") ---"
cat "$file"
fi
done
Length of output: 1207
@MihalyToth20 Great! I've analyzed the commits and existing changesets. Here's what I found:
📊 Current Coverage Analysis
Covered by existing changesets:
- ✅ Registry refactor →
forty-phones-rhyme.md(correctly describes the PR title) - ✅ OpenCode/native host features →
dull-facts-lose.md - ✅ Skills features →
large-bugs-drop.md+ten-regions-switch.md - ✅ Help command styling →
light-crabs-march.md - ✅ Registry graceful exit →
plain-ways-fall.md - ✅ Remote Xano templates →
spicy-bags-guess.md ⚠️ Test/env/docs improvements →loose-pandas-hide.md(needs better description)
❌ Missing changeset:
- Browser consumer package (
@calycode/browser-consumer) - This is a new package with significant changes but no changeset!
🔧 Recommended Actions
1. Create new changeset for browser-consumer
The new packages/browser-consumer/ package (IndexedDB config storage for browser extensions) needs its own changeset:
---
"@calycode/browser-consumer": minor
---
feat: add browser-consumer package for IndexedDB-based config storage
Introduces a new package to support browser extensions with IndexedDB utilities and config storage capabilities.2. Update loose-pandas-hide.md description
Replace the generic description with something more specific:
chore: enhance test runner with environment handling, add test examples and improve documentation
Improves test configuration with robust environment variable support, adds test config examples, and updates documentation guides for testing, git workflow, patterns, scaffolding, and xanoscript.3. Consider consolidating skills changesets
You have two skills-related changesets (large-bugs-drop.md and ten-regions-switch.md). Consider whether these should be one changeset or if they represent distinct features.
Would you like me to help draft the complete updated changeset files?
| async function main() { | ||
| const esmResult = await build({ | ||
| ...baseConfig, | ||
| entryPoints: ['src/index.ts'], | ||
| outdir: 'dist', | ||
| format: 'esm', | ||
| outExtension: { '.js': '.js' }, | ||
| sourcemap: false, | ||
| metafile: true, | ||
| }); | ||
| const cjsResult = await build({ | ||
| ...baseConfig, | ||
| entryPoints: ['src/index.ts'], | ||
| outdir: 'dist', | ||
| format: 'cjs', | ||
| outExtension: { '.js': '.cjs' }, | ||
| sourcemap: false, | ||
| metafile: true, | ||
| }); | ||
|
|
||
| await writeFile(resolve(distDir, 'esm-meta.json'), JSON.stringify(esmResult.metafile, null, 2)); | ||
| await writeFile(resolve(distDir, 'cjs-meta.json'), JSON.stringify(cjsResult.metafile, null, 2)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/browser-consumer/esbuild.config.ts | head -50Repository: calycode/xano-tools
Length of output: 1437
🏁 Script executed:
cat packages/browser-consumer/package.jsonRepository: calycode/xano-tools
Length of output: 1958
🏁 Script executed:
git ls-files packages/browser-consumer/ | head -20Repository: calycode/xano-tools
Length of output: 520
🏁 Script executed:
cat packages/browser-consumer/tsconfig.jsonRepository: calycode/xano-tools
Length of output: 328
🏁 Script executed:
cat esbuild.base.tsRepository: calycode/xano-tools
Length of output: 466
🏁 Script executed:
cat -n pnpm-workspace.yaml 2>/dev/null || echo "No pnpm-workspace.yaml found"Repository: calycode/xano-tools
Length of output: 102
🏁 Script executed:
find packages -name "esbuild.config.ts" -type fRepository: calycode/xano-tools
Length of output: 202
🏁 Script executed:
for file in $(find packages -name "esbuild.config.ts" -type f); do
echo "=== $file ==="
head -20 "$file" | grep -E "(entryPoints|outdir|resolve\(__dirname)"
doneRepository: calycode/xano-tools
Length of output: 681
🏁 Script executed:
cat packages/cli/esbuild.config.ts | head -35Repository: calycode/xano-tools
Length of output: 1059
Make build paths absolute to avoid CWD-dependent output.
entryPoints: ['src/index.ts'] and outdir: 'dist' are relative to the process CWD, while the metafile write uses distDir derived from this file's directory. When the script runs from the repo root (standard in monorepos), the build outputs and metafiles will land in different folders. Use resolve(__dirname, ...) for both entryPoints and outdir to match the pattern already used in packages/cli/esbuild.config.ts.
🛠️ Proposed fix
+const rootDir = resolve(__dirname); const esmResult = await build({
...baseConfig,
- entryPoints: ['src/index.ts'],
+ entryPoints: [resolve(rootDir, 'src/index.ts')],
- outdir: 'dist',
+ outdir: distDir, const cjsResult = await build({
...baseConfig,
- entryPoints: ['src/index.ts'],
+ entryPoints: [resolve(rootDir, 'src/index.ts')],
- outdir: 'dist',
+ outdir: distDir,📝 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.
| async function main() { | |
| const esmResult = await build({ | |
| ...baseConfig, | |
| entryPoints: ['src/index.ts'], | |
| outdir: 'dist', | |
| format: 'esm', | |
| outExtension: { '.js': '.js' }, | |
| sourcemap: false, | |
| metafile: true, | |
| }); | |
| const cjsResult = await build({ | |
| ...baseConfig, | |
| entryPoints: ['src/index.ts'], | |
| outdir: 'dist', | |
| format: 'cjs', | |
| outExtension: { '.js': '.cjs' }, | |
| sourcemap: false, | |
| metafile: true, | |
| }); | |
| await writeFile(resolve(distDir, 'esm-meta.json'), JSON.stringify(esmResult.metafile, null, 2)); | |
| await writeFile(resolve(distDir, 'cjs-meta.json'), JSON.stringify(cjsResult.metafile, null, 2)); | |
| async function main() { | |
| const rootDir = resolve(__dirname); | |
| const esmResult = await build({ | |
| ...baseConfig, | |
| entryPoints: [resolve(rootDir, 'src/index.ts')], | |
| outdir: distDir, | |
| format: 'esm', | |
| outExtension: { '.js': '.js' }, | |
| sourcemap: false, | |
| metafile: true, | |
| }); | |
| const cjsResult = await build({ | |
| ...baseConfig, | |
| entryPoints: [resolve(rootDir, 'src/index.ts')], | |
| outdir: distDir, | |
| format: 'cjs', | |
| outExtension: { '.js': '.cjs' }, | |
| sourcemap: false, | |
| metafile: true, | |
| }); | |
| await writeFile(resolve(distDir, 'esm-meta.json'), JSON.stringify(esmResult.metafile, null, 2)); | |
| await writeFile(resolve(distDir, 'cjs-meta.json'), JSON.stringify(cjsResult.metafile, null, 2)); |
🤖 Prompt for AI Agents
In `@packages/browser-consumer/esbuild.config.ts` around lines 11 - 32, The build
uses relative paths for entryPoints and outdir causing CWD-dependent output;
update both build invocations in main() to use absolute paths via
resolve(__dirname, ...): set entryPoints to [resolve(__dirname, 'src/index.ts')]
and outdir to resolve(__dirname, 'dist') (or to the existing distDir if that's
already resolve(__dirname, 'dist')), for both the esmResult and cjsResult builds
so the metafile writes (writeFile using distDir) align with the actual build
outputs.
There was a problem hiding this comment.
- Fix file path resolution issue, pick examples from the cli and the core packages
There was a problem hiding this comment.
🧩 Analysis chain
@MihalyToth20, I'll gather the correct patterns from the cli and core packages for you!
🏁 Script executed:
#!/bin/bash
# Get the full esbuild config from cli package
echo "=== packages/cli/esbuild.config.ts ==="
cat packages/cli/esbuild.config.ts
echo ""
echo ""
# Get the full esbuild config from core package
echo "=== packages/core/esbuild.config.ts ==="
cat packages/core/esbuild.config.ts
Length of output: 2602
@MihalyToth20, here's what I found:
✅ Correct Pattern (from packages/cli/esbuild.config.ts):
const __dirname = dirname(fileURLToPath(import.meta.url));
const rootDir = resolve(__dirname);
const distDir = resolve(__dirname, 'dist');
const result = await build({
entryPoints: [resolve(rootDir, 'src/index.ts')], // ✅ Absolute path
outfile: resolve(distDir, 'index.cjs'), // ✅ Absolute path
// ... other options
});
await writeFile(resolve(distDir, 'meta.json'), ...); // ✅ Absolute path❌ Issue Found (both packages/core/esbuild.config.ts AND packages/browser-consumer/esbuild.config.ts):
const distDir = resolve(__dirname, 'dist');
await build({
entryPoints: ['src/index.ts'], // ❌ Relative to CWD
outdir: 'dist', // ❌ Relative to CWD
// ...
});
await writeFile(resolve(distDir, 'esm-meta.json'), ...); // ✅ Uses absolute distDirThe problem: When the build runs from the repo root, entryPoints and outdir resolve relative to the repo root, but the metafile writes use distDir which points to the package directory. They end up in different locations.
🔧 Fix for packages/browser-consumer/esbuild.config.ts:
const __dirname = dirname(fileURLToPath(import.meta.url));
+const rootDir = resolve(__dirname);
const distDir = resolve(__dirname, 'dist');
async function main() {
const esmResult = await build({
...baseConfig,
- entryPoints: ['src/index.ts'],
+ entryPoints: [resolve(rootDir, 'src/index.ts')],
- outdir: 'dist',
+ outdir: distDir,
format: 'esm',
// ...
});
const cjsResult = await build({
...baseConfig,
- entryPoints: ['src/index.ts'],
+ entryPoints: [resolve(rootDir, 'src/index.ts')],
- outdir: 'dist',
+ outdir: distDir,
format: 'cjs',
// ...
});Note: The packages/core/esbuild.config.ts file has the exact same issue and should be fixed the same way! 🔍
🧠 Learnings used
Learnt from: CR
Repo: calycode/xano-tools PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T05:59:33.267Z
Learning: Use Esbuild or Rollup for bundling distribution packages
Learnt from: CR
Repo: calycode/xano-tools PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T05:59:33.267Z
Learning: Applies to packages/cli/src/index.ts : CLI entry point is packages/cli/src/index.ts (source) and dist/index.cjs (compiled distribution)
Learnt from: CR
Repo: calycode/xano-tools PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T05:59:33.267Z
Learning: Applies to packages/*/src/**/*.ts?(x) : Configure and use Eslint and Prettier for linting and formatting with default configs
Learnt from: CR
Repo: calycode/xano-tools PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T05:59:33.267Z
Learning: Applies to packages/core/src/index.ts : Core logic entry point is packages/core/src/index.ts
Learnt from: CR
Repo: calycode/xano-tools PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T05:59:33.267Z
Learning: Applies to packages/*/src/**/*.ts?(x) : Use TypeScript for all source code in the monorepo
| @echo off | ||
| REM ============================================================================ | ||
| REM CalyCode CLI Installer (Windows) | ||
| REM ============================================================================ | ||
| REM This is a wrapper script that launches the PowerShell installer. | ||
| REM For direct PowerShell usage, run: | ||
| REM irm https://get.calycode.com/install.ps1 | iex | ||
| REM ============================================================================ | ||
|
|
||
| setlocal | ||
|
|
||
| echo. | ||
| echo CalyCode CLI Installer | ||
| echo ====================== | ||
| echo. | ||
|
|
||
| REM Check if PowerShell is available | ||
| where powershell >nul 2>nul | ||
| if %ERRORLEVEL% NEQ 0 ( | ||
| echo [ERROR] PowerShell is required but not found in PATH. | ||
| echo Please install PowerShell or run the installer manually. | ||
| pause | ||
| exit /b 1 | ||
| ) | ||
|
|
||
| REM Get the directory where this script is located | ||
| set "SCRIPT_DIR=%~dp0" | ||
|
|
||
| REM Check if the PowerShell script exists locally | ||
| if exist "%SCRIPT_DIR%install.ps1" ( | ||
| echo [INFO] Running local PowerShell installer... | ||
| powershell -NoProfile -ExecutionPolicy Bypass -File "%SCRIPT_DIR%install.ps1" %* | ||
| ) else ( | ||
| echo [INFO] Downloading and running PowerShell installer... | ||
| powershell -NoProfile -ExecutionPolicy Bypass -Command "irm https://get.calycode.com/install.ps1 | iex" | ||
| ) | ||
|
|
||
| if %ERRORLEVEL% NEQ 0 ( | ||
| echo. | ||
| echo [ERROR] Installation failed with error code %ERRORLEVEL% | ||
| echo. | ||
| pause | ||
| exit /b %ERRORLEVEL% | ||
| ) | ||
|
|
||
| echo. | ||
| echo Press any key to exit... | ||
| pause >nul |
There was a problem hiding this comment.
Critical: Unix line endings will break batch file execution on Windows
Static analysis detected Unix line endings (LF-only) in this batch file. Windows batch parser has known issues with LF-only line endings, particularly around GOTO/CALL label parsing and 512-byte boundary bugs. This can cause silent failures or unexpected behavior.
Convert to Windows line endings (CRLF) before merging. You can configure git with:
git config core.autocrlf true
Or add to .gitattributes:
*.bat text eol=crlf
🧰 Tools
🪛 Blinter (1.0.112)
[error] 1-1: Unix line endings detected. Explanation: Batch file uses Unix line endings (LF-only) which can cause GOTO/CALL label parsing failures and script malfunction due to Windows batch parser 512-byte boundary bugs. Recommendation: Convert file to Windows line endings (CRLF). Use tools like dos2unix, notepad++, or configure git with 'git config core.autocrlf true'. Context: File uses Unix line endings (LF-only) - 48 LF sequences found
(E018)
[error] 32-32: PowerShell execution policy bypass. Explanation: Bypassing PowerShell execution policy can allow malicious scripts to run. Recommendation: Avoid using -ExecutionPolicy Bypass unless absolutely necessary. Context: PowerShell execution policy bypass detected
(SEC009)
🤖 Prompt for AI Agents
In `@packages/cli/scripts/installer/install.bat` around lines 1 - 48, The batch
installer (install.bat) currently has Unix (LF) line endings which will break
Windows batch parsing; convert this file to Windows CRLF line endings before
merging (ensure the checked-in file uses CRLF), and add a repo-level rule so
future changes stay CRLF (e.g., enable core.autocrlf or add a .gitattributes
entry forcing *.bat eol=crlf); verify the committed install.bat (the wrapper
that checks powershell, uses %~dp0 as SCRIPT_DIR and conditionally runs
install.ps1) contains CRLF line endings and re-run the static check to confirm
the issue is resolved.
There was a problem hiding this comment.
I haven't experienced failures with these on my windows machine, ignore for now.
There was a problem hiding this comment.
@MihalyToth20, understood! I'll skip this issue for now since it's working fine on your Windows setup. The line ending concern is environment-specific and may not affect all configurations.
| const restartServer = async (port: number = 4096, extraOrigins: string[] = []) => { | ||
| logger.log('Restart requested', { port, extraOrigins }); | ||
|
|
||
| // Kill existing server process if we have a reference | ||
| if (serverProc) { | ||
| logger.log('Killing existing server process for restart...'); | ||
| serverProc.kill(); | ||
| serverProc = null; | ||
| // Give it a moment to release the port | ||
| await new Promise((resolve) => setTimeout(resolve, 500)); | ||
| } | ||
|
|
||
| // Kill any orphan process on the port (handles lost references) | ||
| logger.log('Checking for orphan processes on port...'); | ||
| const killed = killProcessOnPort(port, logger); | ||
| if (killed) { | ||
| logger.log('Killed orphan process(es) on port, waiting for port release...'); | ||
| // Give more time for port to be released after force kill | ||
| await new Promise((resolve) => setTimeout(resolve, 1000)); | ||
| } | ||
|
|
||
| // Verify port is actually free now | ||
| const serverUrl = `http://localhost:${port}`; | ||
| try { | ||
| await fetch(serverUrl); | ||
| // If we get here, something is still running on the port | ||
| logger.error('Port still in use after kill attempts'); | ||
| sendMessage({ | ||
| status: 'error', | ||
| message: `Port ${port} still in use after cleanup attempts. Please try again or use a different port.` | ||
| }); | ||
| return; | ||
| } catch (e) { | ||
| // Good, nothing running - port is free | ||
| logger.log('Port is now free, starting server...'); | ||
| } | ||
|
|
||
| // Start fresh with new config | ||
| await startServer(port, extraOrigins); | ||
| }; | ||
|
|
||
| const handleMessage = (msg: any) => { | ||
| logger.log('Received message', msg); | ||
|
|
||
| try { | ||
| if (msg.type === 'ping') { | ||
| sendMessage({ type: 'pong', timestamp: Date.now() }); | ||
| } else if (msg.type === 'start') { | ||
| const port = msg.port ? parseInt(msg.port, 10) : 4096; | ||
| const origins = Array.isArray(msg.origins) ? msg.origins : []; | ||
| startServer(port, origins); | ||
| } else if (msg.type === 'restart') { | ||
| // Restart the server with new origins - used when CORS configuration needs updating | ||
| const port = msg.port ? parseInt(msg.port, 10) : 4096; | ||
| const origins = Array.isArray(msg.origins) ? msg.origins : []; | ||
| restartServer(port, origins); | ||
| } else if (msg.type === 'stop') { | ||
| const port = msg.port ? parseInt(msg.port, 10) : 4096; | ||
| logger.log('Stop requested', { port, hasServerProc: !!serverProc }); | ||
|
|
||
| // Kill by process reference if we have it | ||
| if (serverProc) { | ||
| logger.log('Killing server process by reference...'); | ||
| serverProc.kill(); | ||
| serverProc = null; | ||
| } | ||
|
|
||
| // Also kill any orphan process on the port (handles lost references) | ||
| const killed = killProcessOnPort(port, logger); | ||
| if (killed) { | ||
| logger.log('Killed orphan process(es) on port'); | ||
| } | ||
|
|
There was a problem hiding this comment.
Avoid killing arbitrary processes bound to the port.
killProcessOnPort is invoked on restart/stop and will terminate any listener on that port, even if it’s unrelated to OpenCode. That’s risky for users who already have services on 4096. Prefer killing only a process you started (PID tracking) or return a clear error asking for a different port.
✅ Suggested direction (guard against unknown owners)
- // Kill any orphan process on the port (handles lost references)
- logger.log('Checking for orphan processes on port...');
- const killed = killProcessOnPort(port, logger);
- if (killed) {
- logger.log('Killed orphan process(es) on port, waiting for port release...');
- // Give more time for port to be released after force kill
- await new Promise((resolve) => setTimeout(resolve, 1000));
- }
+ // Only try a port-level kill if we previously started the server.
+ const hadServerProc = !!serverProc;
+ if (!hadServerProc) {
+ sendMessage({
+ status: 'error',
+ message: `Port ${port} is in use by another process. Please stop it or choose a different port.`,
+ });
+ return;
+ }
+ logger.log('Checking for orphan processes on port...');
+ const killed = killProcessOnPort(port, logger);
+ if (killed) {
+ logger.log('Killed orphan process(es) on port, waiting for port release...');
+ await new Promise((resolve) => setTimeout(resolve, 1000));
+ }📝 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.
| const restartServer = async (port: number = 4096, extraOrigins: string[] = []) => { | |
| logger.log('Restart requested', { port, extraOrigins }); | |
| // Kill existing server process if we have a reference | |
| if (serverProc) { | |
| logger.log('Killing existing server process for restart...'); | |
| serverProc.kill(); | |
| serverProc = null; | |
| // Give it a moment to release the port | |
| await new Promise((resolve) => setTimeout(resolve, 500)); | |
| } | |
| // Kill any orphan process on the port (handles lost references) | |
| logger.log('Checking for orphan processes on port...'); | |
| const killed = killProcessOnPort(port, logger); | |
| if (killed) { | |
| logger.log('Killed orphan process(es) on port, waiting for port release...'); | |
| // Give more time for port to be released after force kill | |
| await new Promise((resolve) => setTimeout(resolve, 1000)); | |
| } | |
| // Verify port is actually free now | |
| const serverUrl = `http://localhost:${port}`; | |
| try { | |
| await fetch(serverUrl); | |
| // If we get here, something is still running on the port | |
| logger.error('Port still in use after kill attempts'); | |
| sendMessage({ | |
| status: 'error', | |
| message: `Port ${port} still in use after cleanup attempts. Please try again or use a different port.` | |
| }); | |
| return; | |
| } catch (e) { | |
| // Good, nothing running - port is free | |
| logger.log('Port is now free, starting server...'); | |
| } | |
| // Start fresh with new config | |
| await startServer(port, extraOrigins); | |
| }; | |
| const handleMessage = (msg: any) => { | |
| logger.log('Received message', msg); | |
| try { | |
| if (msg.type === 'ping') { | |
| sendMessage({ type: 'pong', timestamp: Date.now() }); | |
| } else if (msg.type === 'start') { | |
| const port = msg.port ? parseInt(msg.port, 10) : 4096; | |
| const origins = Array.isArray(msg.origins) ? msg.origins : []; | |
| startServer(port, origins); | |
| } else if (msg.type === 'restart') { | |
| // Restart the server with new origins - used when CORS configuration needs updating | |
| const port = msg.port ? parseInt(msg.port, 10) : 4096; | |
| const origins = Array.isArray(msg.origins) ? msg.origins : []; | |
| restartServer(port, origins); | |
| } else if (msg.type === 'stop') { | |
| const port = msg.port ? parseInt(msg.port, 10) : 4096; | |
| logger.log('Stop requested', { port, hasServerProc: !!serverProc }); | |
| // Kill by process reference if we have it | |
| if (serverProc) { | |
| logger.log('Killing server process by reference...'); | |
| serverProc.kill(); | |
| serverProc = null; | |
| } | |
| // Also kill any orphan process on the port (handles lost references) | |
| const killed = killProcessOnPort(port, logger); | |
| if (killed) { | |
| logger.log('Killed orphan process(es) on port'); | |
| } | |
| const restartServer = async (port: number = 4096, extraOrigins: string[] = []) => { | |
| logger.log('Restart requested', { port, extraOrigins }); | |
| // Track if we had a server process before killing it | |
| const hadServerProc = !!serverProc; | |
| // Kill existing server process if we have a reference | |
| if (serverProc) { | |
| logger.log('Killing existing server process for restart...'); | |
| serverProc.kill(); | |
| serverProc = null; | |
| // Give it a moment to release the port | |
| await new Promise((resolve) => setTimeout(resolve, 500)); | |
| } | |
| // Only try a port-level kill if we previously started the server. | |
| if (!hadServerProc) { | |
| sendMessage({ | |
| status: 'error', | |
| message: `Port ${port} is in use by another process. Please stop it or choose a different port.`, | |
| }); | |
| return; | |
| } | |
| // Kill any orphan process on the port (handles lost references) | |
| logger.log('Checking for orphan processes on port...'); | |
| const killed = killProcessOnPort(port, logger); | |
| if (killed) { | |
| logger.log('Killed orphan process(es) on port, waiting for port release...'); | |
| // Give more time for port to be released after force kill | |
| await new Promise((resolve) => setTimeout(resolve, 1000)); | |
| } | |
| // Verify port is actually free now | |
| const serverUrl = `http://localhost:${port}`; | |
| try { | |
| await fetch(serverUrl); | |
| // If we get here, something is still running on the port | |
| logger.error('Port still in use after kill attempts'); | |
| sendMessage({ | |
| status: 'error', | |
| message: `Port ${port} still in use after cleanup attempts. Please try again or use a different port.` | |
| }); | |
| return; | |
| } catch (e) { | |
| // Good, nothing running - port is free | |
| logger.log('Port is now free, starting server...'); | |
| } | |
| // Start fresh with new config | |
| await startServer(port, extraOrigins); | |
| }; |
🤖 Prompt for AI Agents
In `@packages/cli/src/commands/opencode/implementation.ts` around lines 499 - 571,
The code currently calls killProcessOnPort from restartServer and the stop
branch in handleMessage which can kill unrelated services; change the logic to
only terminate processes we started by tracking the child PID/handle returned by
startServer (use the existing serverProc variable and a persisted PID from
startServer) and avoid calling killProcessOnPort unless the PID matches our
tracked PID; if no tracked PID exists for the requested port, return/send an
error asking the user to pick a different port instead of force-killing. Update
restartServer, the 'stop' branch in handleMessage, and any startServer result
handling to set/clear the tracked PID/serverProc consistently and only kill that
PID when performing stop/restart. Also ensure restartServer waits for the
tracked process to exit before starting a new one and includes user-facing error
messages when refusing to kill unknown owners.
There was a problem hiding this comment.
Valid concern. We need to ensure that a chrome extension that calls the native host can control the creation and recreation of the process.
So far we've seen that the extension can drift from the running process (maybe it was a mistake in the setup on my machine), and as a result restarting the connection could result in the native host respondibg with status running.
I guess the sokution would then just be to force the killing again from the extension side.
- Analyze the issue and write the analysis into the "plans" directory
There was a problem hiding this comment.
🧩 Analysis chain
@MihalyToth20, I'll analyze the native host process management issue and create a comprehensive document in the plans directory. Let me first check the existing structure.
🏁 Script executed:
#!/bin/bash
# Check the plans directory structure to understand naming conventions
ls -la plans/ 2>/dev/null || echo "Plans directory not found"
# Also check if there's any existing documentation about the native host
fd -t f "native" plans/ 2>/dev/null || echo "No native host related plans found"
Length of output: 985
🏁 Script executed:
#!/bin/bash
# Read the existing native host analysis to see if it covers the state drift issue
cat plans/opencode-native-host-analysis.md
Length of output: 12816
Perfect! I can see there's already an architecture analysis. Now I'll create a focused analysis document specifically about the state synchronization and process management issue you've identified.
Task: Create Native Host State Management Analysis
I'll create a comprehensive analysis document in the plans directory that covers:
- Problem Definition: State drift between Chrome extension and native host
- Root Causes: Process lifecycle tracking limitations
- Current Risks: Indiscriminate port-based process killing
- Proposed Solutions: PID tracking, state recovery, graceful handling
- Implementation Strategy: Changes needed in both native host and extension
I've created a comprehensive analysis document that covers:
- Problem Definition - Clear description of state drift and its impacts
- Current Architecture - Process hierarchy and state variables
- State Drift Scenarios - Four concrete scenarios showing how things go wrong
- Root Causes - Five fundamental issues including stateless host and port-based identification
- Current Risks - Categorized by severity (critical arbitrary process termination!)
- Proposed Solutions - Five detailed solutions:
- Solution 1 (Recommended): PID tracking with persistence
- Solution 2: Lock file with metadata
- Solution 3: Health check protocol
- Solution 4: State recovery on reconnect
- Solution 5: Graceful port conflict handling
- Implementation Plan - 5 phases with concrete tasks and checkboxes
- Testing Strategy - Unit tests, integration tests, manual test scenarios
- Security Considerations - Vulnerabilities and mitigations
- Success Metrics - KPIs to measure improvement
The document emphasizes that Solution 1 (PID Tracking) should be implemented immediately to address the critical security risk of killing arbitrary processes.
✏️ Learnings added
Learnt from: MihalyToth20
Repo: calycode/xano-tools PR: 184
File: packages/cli/src/commands/opencode/implementation.ts:499-571
Timestamp: 2026-02-07T20:53:06.194Z
Learning: In the OpenCode native host implementation at `packages/cli/src/commands/opencode/implementation.ts`, the state synchronization between the Chrome extension and the spawned OpenCode server process can drift because the native host's `serverProc` variable is in-memory only and lost on restart, and the current `killProcessOnPort()` function kills ANY process on the port without verifying ownership. The recommended solution is to persist the server's PID to a file (`~/.calycode/opencode/server.pid`) and only kill processes with tracked PIDs, refusing to kill unknown processes with a clear error message asking the user to choose a different port.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
💡 For code changes across multiple files, use @coderabbitai plan in an issue comment
| /** | ||
| * Validates a path argument to prevent command injection. | ||
| * Ensures the path doesn't contain shell metacharacters. | ||
| * @param value - Path value to validate | ||
| * @param name - Name of the parameter for error messages | ||
| * @throws {Error} if path contains potentially dangerous characters | ||
| */ | ||
| function validatePathArg(value: string, name: string): void { | ||
| // Block shell metacharacters that could be used for command injection | ||
| // Allow alphanumeric, path separators, dots, hyphens, underscores, and spaces | ||
| if (/[;&|`$(){}[\]<>!#*?]/.test(value)) { | ||
| throw new Error( | ||
| `Invalid ${name}: "${value}". Path contains potentially unsafe characters.` | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix Windows drive-letter paths in validatePathArg.
: is required in paths like C:\..., so current validation will throw on Windows. Consider stripping a leading drive prefix before validation (or allowing a single colon after the drive letter).
Suggested fix
function validatePathArg(value: string, name: string): void {
- // Block shell metacharacters that could be used for command injection
- // Allow alphanumeric, path separators, dots, hyphens, underscores, and spaces
- if (/[;&|`$(){}[\]<>!#*?]/.test(value)) {
+ // Block shell metacharacters that could be used for command injection
+ // Allow alphanumeric, path separators, dots, hyphens, underscores, and spaces
+ const isWindows = process.platform === 'win32';
+ const normalized = isWindows ? value.replace(/^[a-zA-Z]:/, '') : value;
+ if (/[;&|`$(){}[\]<>!#*?]/.test(normalized)) {
throw new Error(
`Invalid ${name}: "${value}". Path contains potentially unsafe characters.`
);
}
}🤖 Prompt for AI Agents
In `@packages/cli/src/commands/serve/implementation.ts` around lines 5 - 20, The
validatePathArg function currently rejects paths containing ':' which breaks
Windows drive-letter paths like "C:\..."; update validatePathArg to allow a
single colon immediately after a leading drive letter (e.g., /^[A-Za-z]:/) or
strip a leading drive prefix before running the existing metacharacter regex.
Target the validatePathArg function and ensure the logic only permits that
single leading drive colon (not arbitrary colons elsewhere) before applying the
existing unsafe-character test and throwing the same Error on failure.
There was a problem hiding this comment.
- Reqrite path validation logic as requested in other comments as well.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for other instances of spawn usage that might need path validation
rg -n "spawn\(" --type=ts -C 3
echo "---"
# Search for other path validation or security validation functions
rg -n "validate.*[Pp]ath|[Pp]ath.*[Vv]alidat" --type=ts -C 2
echo "---"
# Search for other instances where user input might be passed to shell commands
rg -n "getSpawnOptions|shell.*true" --type=ts -C 3
Length of output: 21470
@MihalyToth20 Thank you for confirming! I've identified the path validation functions in the codebase that may need attention:
Shell Injection Prevention (Windows path issue)
packages/cli/src/commands/serve/implementation.ts(current file)validatePathArg()- Used forspecHtmlPathandrootparameters before passing tospawn- Issue: Blocks
:which breaks Windows drive letters likeC:\...
Path Traversal Prevention (different security concern)
These handle URL/filesystem traversal attacks and have different requirements:
-
packages/cli/src/utils/github-content-fetcher.tsvalidatePathWithinBase()- Ensures filesystem paths stay within base directory
-
packages/cli/src/utils/feature-focused/registry/api.tsvalidateRegistryPath()- Prevents..in registry URL paths
-
packages/core/src/features/registry/api.tsvalidateRegistryPath()- Same as above, core package version
Other spawn Usage (may need validation)
packages/cli/src/features/code-gen/open-api-generator.ts(line 60)- Uses
spawnwithshell: trueandcliBin+args- check if user-provided paths need validation
- Uses
The primary fix for the Windows drive letter issue should focus on validatePathArg() in the serve/implementation.ts file. Let me know if you'd like guidance on the specific regex pattern to use! 🐰
🧠 Learnings used
Learnt from: CR
Repo: calycode/xano-tools PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T05:59:33.267Z
Learning: Applies to packages/cli/src/index.ts : CLI entry point is packages/cli/src/index.ts (source) and dist/index.cjs (compiled distribution)
| const cacheDir = options.cacheDir || this.defaultCacheDir; | ||
| const cacheSubDir = path.join(cacheDir, `${owner}-${repo}`, subpath.replace(/\//g, '-')); | ||
|
|
There was a problem hiding this comment.
Prevent cache directory traversal via subpath (Windows backslashes).
Only / is replaced, so subpath containing \ or .. can escape cacheDir on Windows. Normalize separators and validate the final subdir is within the base cache directory.
🛡️ Proposed fix
- const cacheSubDir = path.join(cacheDir, `${owner}-${repo}`, subpath.replace(/\//g, '-'));
+ const safeSubpath = subpath.replace(/[\\/]/g, '-');
+ const cacheSubDir = validatePathWithinBase(
+ cacheDir,
+ path.join(`${owner}-${repo}`, safeSubpath),
+ );🤖 Prompt for AI Agents
In `@packages/cli/src/utils/github-content-fetcher.ts` around lines 131 - 133,
Normalize and sanitize the incoming subpath before building cacheSubDir: replace
backslashes with forward slashes (or use path.normalize), remove or collapse any
'..' segments, then use path.resolve/path.join to construct cacheSubDir from
cacheDir (or this.defaultCacheDir) and verify with path.relative or startsWith
that the resolved cacheSubDir is inside the base cacheDir; if the check fails,
reject the input or throw an error. Ensure you update the code around cacheDir,
cacheSubDir and subpath in github-content-fetcher.ts to perform these
normalization and validation steps so Windows backslashes or path-traversal
attempts cannot escape the base cache directory.
There was a problem hiding this comment.
- Ensure this windows path traversal is handled
There was a problem hiding this comment.
@MihalyToth20, thanks for tracking this! The Windows path traversal issue is an important security concern that should be addressed.
When you're ready to implement the fix, the solution is straightforward:
- Replace both
/and\with dashes in thesubpath - Use the existing
validatePathWithinBasehelper to ensure the resulting path stays within the cache directory
Would you like me to generate the code changes for this fix, or do you prefer to implement it yourself?
| // Also block encoded traversal attempts | ||
| if (normalized.includes('%2e%2e') || normalized.includes('%2E%2E')) { | ||
| throw new Error(`Invalid registry path: "${inputPath}" contains encoded path traversal`); |
There was a problem hiding this comment.
Harden encoded traversal checks against mixed‑case encodings.
Percent‑encoding is case‑insensitive, so %2e%2E can bypass the current check. Normalize to lowercase before checking.
🛡️ Proposed fix
- if (normalized.includes('%2e%2e') || normalized.includes('%2E%2E')) {
+ const normalizedLower = normalized.toLowerCase();
+ if (normalizedLower.includes('%2e%2e')) {
throw new Error(`Invalid registry path: "${inputPath}" contains encoded path traversal`);
}📝 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.
| // Also block encoded traversal attempts | |
| if (normalized.includes('%2e%2e') || normalized.includes('%2E%2E')) { | |
| throw new Error(`Invalid registry path: "${inputPath}" contains encoded path traversal`); | |
| // Also block encoded traversal attempts | |
| const normalizedLower = normalized.toLowerCase(); | |
| if (normalizedLower.includes('%2e%2e')) { | |
| throw new Error(`Invalid registry path: "${inputPath}" contains encoded path traversal`); | |
| } |
🤖 Prompt for AI Agents
In `@packages/core/src/features/registry/api.ts` around lines 20 - 22, The
encoded-traversal check in the registry API currently checks normalized for
'%2e%2e' and '%2E%2E' and can miss mixed-case encodings; update the logic around
the normalized variable (used alongside inputPath) to convert it to a single
case (e.g., normalized = normalized.toLowerCase()) before performing the
includes() check for '%2e%2e', or replace the check with a case-insensitive
regex; ensure the thrown Error still references inputPath for context.
There was a problem hiding this comment.
- See the other tasks about traversal checks and refactor
There was a problem hiding this comment.
@MihalyToth20, sounds good! When you're ready to tackle the traversal check refactor across the codebase, I can help identify all the locations that need attention. Just let me know if you'd like me to search for other path validation or traversal check patterns, or if you'd prefer to create a tracking issue for this work.
| --- | ||
| description: Build XanoScript AI agents, MCP servers, and tools for AI-powered automation and intelligent features. | ||
| mode: subagent | ||
| model: anthropic/claude-sonnet-4-20250514 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/opencode-templates/agents/xano-ai-builder.mdRepository: calycode/xano-tools
Length of output: 12265
🌐 Web query:
Anthropic Claude Sonnet 4 current model IDs 2025
💡 Result:
Claude Sonnet 4 (2025) model IDs
- Anthropic API (snapshot / versioned ID):
claude-sonnet-4-20250514[1][2] - Anthropic API (alias):
claude-sonnet-4-0→ points toclaude-sonnet-4-20250514[2] - AWS Bedrock model ID:
anthropic.claude-sonnet-4-20250514-v1:0[1][2] - Google Vertex AI model ID:
claude-sonnet-4@20250514[1][2]
How to confirm what’s currently available in your account: call GET /v1/models (it returns entries like id: "claude-sonnet-4-20250514" for “Claude Sonnet 4”). [3]
Sources
[1] Anthropic docs — Models overview / model names
[2] Anthropic docs — All models / aliases table
[3] Anthropic docs — List Models (GET /v1/models)
🌐 Web query:
OpenAI GPT-4o model ID reasoning_effort parameter 2025
💡 Result:
GPT‑4o model ID (API, 2025)
- Use the stable alias:
gpt-4o(recommended in OpenAI’s “which model should I use” FAQ). [1] - Snapshot IDs you’ll also see (esp. in fine-tuning / historical references) include:
gpt-4o-2024-05-13[2]gpt-4o-2024-08-06[2][3]
reasoning_effort parameter (and whether it applies to GPT‑4o)
reasoning_effortis a parameter intended for reasoning models; supported values arenone,minimal,low,medium,high,xhigh(defaults tomediumin that API reference). [4]- GPT‑4o is a non‑reasoning (“legacy/frontier”) model; OpenAI’s guidance indicates the “reasoning effort” control is not for GPT‑4o (it’s called out as a GPT‑5-specific control as
reasoning.effort, and “legacy models do not support” those controls). [5]
Practical takeaway: for GPT‑4o, set model: "gpt-4o" (or a snapshot like gpt-4o-2024-08-06) and control output with token limits (e.g., max_output_tokens / max_completion_tokens) rather than reasoning_effort. [1][5]
Sources
[1] OpenAI API FAQ – “Which model should I use?” (gpt-4o) (platform.openai.com)
[2] GPT‑4o & GPT‑4 fine-tuning page (mentions gpt-4o-2024-05-13, gpt-4o-2024-08-06) (openai.com)
[3] “Fine-tuning now available for GPT‑4o” (select gpt-4o-2024-08-06) (openai.com)
[4] API reference showing reasoning_effort values/default (platform.openai.com)
[5] Help Center: “reasoning.effort” controls only on GPT‑5; legacy models don’t support (help.openai.com)
Citations:
- 1: https://platform.openai.com/docs/faq/which-model-should-i-use?utm_source=openai
- 2: https://openai.com/gpt-4o-and-gpt-4-fine-tuning-experimental-access/?utm_source=openai
- 3: https://openai.com/index/gpt-4o-fine-tuning/?utm_source=openai
- 4: https://platform.openai.com/docs/api-reference/evals/createRun?utm_source=openai
- 5: https://help.openai.com/en/articles/5072518-controlling-the-length-of-openai-model-responses?utm_source=openai
🌐 Web query:
Google Gemini thinking_tokens include_thoughts search_grounding parameters 2025
💡 Result:
“thinking_tokens” / thinking_tokens
There isn’t a request parameter literally named thinking_tokens in the Gemini API. The count of thinking tokens used is reported in the response as usageMetadata.thoughtsTokenCount (often what people mean by “thinking tokens”). [1]
include_thoughts / includeThoughts
To return thought summaries (not raw chain-of-thought), enable:
- SDK (snake_case):
thinking_config=ThinkingConfig(include_thoughts=True)[2] - JSON / JS (camelCase):
"thinkingConfig": {"includeThoughts": true}[2]
The thought summary then appears as a normal content part with a boolean like part.thought == true. [2]
Thinking controls (2025+)
- Gemini 2.5: use
thinkingBudget(and you can set0to disable for models that support it;-1enables “dynamic thinking”). [3] - Gemini 3: use
thinkingLevel(recommended for Gemini 3+). [4]
“search_grounding” / grounding with Google Search
There isn’t a request parameter literally named search_grounding. To ground with Google Search you enable the Google Search tool:
- REST JSON:
"tools": [{"google_search": {}}][5] - SDK:
tools=[Tool(google_search=GoogleSearch())][5]
Grounded responses include groundingMetadata (queries used + source chunks/supports for citations). [6]
Sources:
[1] Vertex AI “Thinking” docs (usageMetadata.thoughtsTokenCount) (docs.cloud.google.com)
[2] Gemini API “Thinking” docs (includeThoughts / thought parts) (ai.google.dev)
[3] Gemini API “Thinking” docs (thinkingBudget, ranges, 0 / -1 behavior) (ai.google.dev)
[4] Gemini API “Thinking” docs (thinkingLevel for Gemini 3) (ai.google.dev)
[5] Gemini API “Grounding with Google Search” docs (tools: google_search) (ai.google.dev)
[6] Gemini API “Grounding with Google Search” docs (groundingMetadata) (ai.google.dev)
Citations:
- 1: https://docs.cloud.google.com/vertex-ai/generative-ai/docs/thinking
- 2: https://ai.google.dev/gemini-api/docs/thinking
- 3: https://ai.google.dev/gemini-api/docs/thinking
- 4: https://ai.google.dev/gemini-api/docs/thinking
- 5: https://ai.google.dev/gemini-api/docs/grounding/
- 6: https://ai.google.dev/gemini-api/docs/grounding/
🌐 Web query:
Anthropic Claude Sonnet 4.5 model ID claude-sonnet-4-5-20250929
💡 Result:
claude-sonnet-4-5-20250929 is the version-pinned Anthropic API model ID for Claude Sonnet 4.5, corresponding to the Sep 29, 2025 Sonnet 4.5 release date. [1][2]
If you want the rolling “latest Sonnet 4.5” instead of a pinned version, Anthropic’s public guidance is to use the alias claude-sonnet-4-5. [1]
This pinned ID also appears as the model identifier in third-party distributions (e.g., Microsoft Foundry’s catalog/examples). [3]
Sources
[1] Anthropic model page (Sonnet 4.5)
[2] Anthropic announcement (Sep 29, 2025)
[3] Microsoft Foundry model catalog page for Claude Sonnet 4.5
Fix provider parameter names and configurations—several are unsupported or incorrect.
The documentation includes invalid parameter names and configurations that contradict official provider APIs:
-
OpenAI (lines 101–102):
reasoning_effortis not supported forgpt-4o. This parameter is only for reasoning models (e.g., o1). Remove it or clarify that it applies only to reasoning-capable models. -
Gemini (lines 78, 86):
thinking_tokensis not a valid parameter name. UsethinkingBudget(for Gemini 2.5) orthinkingLevel(for Gemini 3+). The documented range(0-24576, -1 for dynamic)does not match the API spec. -
Gemini (line 80, 63):
search_groundingis not a valid parameter. Enable grounding by addinggoogle_searchto the tools list instead. -
Gemini (line 79):
include_thoughtsis correct; keep it as-is.
The Anthropic and core model identifiers are correct.
Also applies to: 69–81, 84–89, 92–104, 106–110, 113–123, 126–128
🤖 Prompt for AI Agents
In `@packages/opencode-templates/agents/xano-ai-builder.md` at line 4, Update the
provider parameter names and constraints in xano-ai-builder.md: remove or
restrict reasoning_effort where used with gpt-4o (only allow it for reasoning
models like o1 or delete the param for gpt-4o references), replace any
thinking_tokens occurrences with thinkingBudget for Gemini 2.5 or thinkingLevel
for Gemini 3+ and adjust documented ranges to the provider spec, remove
search_grounding and instead document adding google_search to the tools list to
enable grounding, and keep include_thoughts as-is; ensure model identifiers
(e.g., anthropic/claude-sonnet-4-20250514) remain unchanged.
There was a problem hiding this comment.
- Remove models from the agebts and commands markdown files. Let's allow the user what model they want to use.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all markdown files in the agents and commands directories
echo "=== Markdown files in agents/ ==="
fd -e md . packages/opencode-templates/agents/
echo -e "\n=== Markdown files in commands/ ==="
fd -e md . packages/opencode-templates/commands/
echo -e "\n=== Searching for 'model:' in YAML frontmatter ==="
# Search for model specifications in YAML frontmatter
rg -n "^model:" packages/opencode-templates/agents/ packages/opencode-templates/commands/ --type md -C 2
Length of output: 6127
🏁 Script executed:
#!/bin/bash
# Double-check commands directory for any model specifications
echo "=== Checking commands directory for any 'model' occurrences ==="
rg -i "model" packages/opencode-templates/commands/ --type md -C 1
echo -e "\n=== Also checking for any provider/model config patterns ==="
rg -i "(anthropic|openai|claude|gpt-)" packages/opencode-templates/commands/ --type md -C 1
Length of output: 125
@MihalyToth20, I'll remove the hardcoded model specifications from all agent markdown files so users can choose their preferred models.
Task: Remove hardcoded model specifications from agent markdown files
- Remove
model:line from YAML frontmatter in all 10 agent files - Commands directory is clean (no model specifications found)
- This allows users to configure their preferred model at runtime
💡 For code changes across multiple files, use @coderabbitai plan in an issue comment
| ```xanoscript | ||
| // At start of endpoint | ||
| now() as $start_time | ||
|
|
||
| // ... endpoint logic ... | ||
|
|
||
| // At end of endpoint | ||
| now() - $start_time as $duration_ms | ||
|
|
||
| db.add performance_log { | ||
| endpoint = $request.path, | ||
| method = $request.method, | ||
| response_time_ms = $duration_ms, | ||
| query_count = $query_count | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
XanoScript syntax documentation Xano functions now() db.add variable assignment
💡 Result:
Variable assignment (Create Variable) in XanoScript
Use the var statement and assign with value = ... (variables typically start with $). (docs.xano.com)
var $total {
value = $input.quantity * $input.price_per_item
}
db.add (insert a record)
db.add <table_name> { data = { ... } } as <var> inserts a row and stores the inserted record in a variable. (docs.xano.com)
db.add user {
data = {
created_at: "now",
name: $input.name,
email: $input.email
}
} as $newUser
“now()” / current time in XanoScript
In XanoScript examples, the “current time” is represented as the literal "now" (commonly used for timestamp fields), and you can pipe it through timestamp filters (example shown subtracting months). (docs.xano.com)
var $cutoff {
value = ("now"|timestamp_subtract_months:1)
}
(Also useful context: Xano stores timestamps as Unix epoch milliseconds.) (docs.xano.com)
Citations:
- 1: https://docs.xano.com/xano-features/metadata-api/instance_api/create_a_new_function_using_xanoscript?utm_source=openai
- 2: https://docs.xano.com/xanoscript/function-reference/database-operations?utm_source=openai
- 3: https://docs.xano.com/xanoscript/function-reference/database-operations?utm_source=openai
- 4: https://docs.xano.com/the-function-stack/data-types/timestamp?utm_source=openai
🌐 Web query:
Xano database operations db.add db.raw http.post json_encode
💡 Result:
In Xano, the equivalents of the things you listed are:
db.add (insert a row)
Use Database Operations → Add Record / XanoScript db.add to insert a new record by passing a data object. (docs.xano.com)
db.add user {
data = {
created_at: "now",
name: $input.name,
email: $input.email
}
} as $new_user
db.raw (raw SQL)
Xano doesn’t expose a db.raw primitive like many code frameworks do; the “raw SQL” feature in Xano is Direct Database Query (visual builder) / XanoScript db.direct_query, where you provide SQL and (optionally) ? placeholders with statement arguments. (docs.xano.com)
db.direct_query {
sql = "SELECT * FROM x52_245 WHERE id = ?"
response_type = "single"
arg = [$input.id]
} as $row
http.post (HTTP POST)
In Xano, outbound HTTP calls are done with External API Request (method = POST, plus headers + params/body). (docs.xano.com)
json_encode
Xano has a json_encode conversion filter that turns a value (object/array) into JSON text (string). (docs.xano.com)
When to use json_encode with an HTTP POST
- If the API request builder is sending a structured JSON body already, you often just provide an object in params/body.
- If the API specifically requires a raw JSON string body, encode your object with
json_encodeand setContent-Type: application/json. (Xano’s External API Request supports configuring headers and body/params.) (docs.xano.com)
If you tell me whether you’re using XanoScript or the visual function stack, and what the target API expects (example request body + headers), I can show the exact function stack steps / code.
Citations:
- 1: https://docs.xano.com/xanoscript/function-reference/database-operations?utm_source=openai
- 2: https://docs.xano.com/the-function-stack/functions/database-requests/direct-database-query?utm_source=openai
- 3: https://docs.xano.com/the-function-stack/functions/apis-and-lambdas/external-api-request?utm_source=openai
- 4: https://docs.xano.com/the-function-stack/filters/conversion?utm_source=openai
- 5: https://docs.xano.com/the-function-stack/functions/apis-and-lambdas/external-api-request?utm_source=openai
🌐 Web query:
XanoScript try catch error handling log function
💡 Result:
In XanoScript, wrap anything that might fail inside try_catch, and use debug.log inside catch to record the error without stopping execution (unless you re-throw).
try_catch {
try {
function.run risky_function { input = { foo: "bar" } } as $result
}
catch {
debug.log { value = $error } // log the error object
// optionally re-throw a cleaner error to the caller:
// throw { name = "inputerror" value = "Something went wrong" }
}
finally {
debug.log { value = "cleanup" }
}
}
debug.log { value = ... }writes to the Debug Logs tab when using Run & Debug (it doesn’t affect execution). [1]try_catchrunscatchonly iftryerrors;finallyalways runs (optional). [1]- In the visual Function Stack Try/Catch, the catch block exposes error details via
code,message, andresult. [2] - To intentionally fail (and optionally be caught by an outer
try_catch), usethrow { name = "...", value = "..." }. [1]
Sources: XanoScript Utility Functions (Try/Catch, Debug Log, Throw) [1]; Function Stack Utility Functions (Try/Catch error vars) [2].
🏁 Script executed:
cat -n packages/xano-skills/skills/xano-monitoring/SKILL.md | sed -n '98,113p'Repository: calycode/xano-tools
Length of output: 473
🏁 Script executed:
cat -n packages/xano-skills/skills/xano-monitoring/SKILL.md | sed -n '145,167p'Repository: calycode/xano-tools
Length of output: 725
🏁 Script executed:
cat -n packages/xano-skills/skills/xano-monitoring/SKILL.md | sed -n '298,310p'Repository: calycode/xano-tools
Length of output: 623
🏁 Script executed:
cat -n packages/xano-skills/skills/xano-monitoring/SKILL.md | sed -n '314,328p'Repository: calycode/xano-tools
Length of output: 503
🏁 Script executed:
cat -n packages/xano-skills/skills/xano-monitoring/SKILL.md | sed -n '352,359p'Repository: calycode/xano-tools
Length of output: 279
Correct multiple XanoScript syntax errors across all code examples.
The examples contain several syntax errors that conflict with official Xano documentation:
- Lines 98–113 & other blocks:
now()is not a function—use the string literal"now"for timestamps.db.addrequiresdata = { ... }wrapper:db.add performance_log { data = { endpoint: $request.path, ... } } as $result - Line 149:
try/catchmust wrap insidetry_catch { try { ... } catch { ... } }. The catch block receives$errorwithmessage,code, andstackproperties. - Lines 298–310:
db.rawdoes not exist; usedb.direct_query { sql = "...", arg = [...] }for raw SQL.http.postdoes not exist; use the External API Request function (visual builder) or equivalent XanoScript. - Lines 352–359:
log()function does not exist; usedebug.log { value = ... }for debug output.
All code blocks need corrections to match official Xano function signatures and XanoScript syntax.
🤖 Prompt for AI Agents
In `@packages/xano-skills/skills/xano-monitoring/SKILL.md` around lines 98 - 113,
Replace invalid XanoScript usages across the file: change any now() calls to the
"now" timestamp literal; update db.add usage such as db.add performance_log {
... } to include the required wrapper db.add performance_log { data = {
endpoint: $request.path, method = $request.method, response_time_ms =
$duration_ms, query_count = $query_count } } as $result; convert try/catch
blocks to the Xano try_catch form try_catch { try { ... } catch { /* catch
receives $error with message, code, stack */ } }; replace db.raw calls with
db.direct_query { sql = "...", arg = [...] } and replace http.post usages with
the External API Request equivalent from Xano’s visual builder; and change any
log(...) calls to debug.log { value = ... } so all examples use correct Xano
function names and argument shapes.
There was a problem hiding this comment.
- Fix the xanoscript syntax errors
There was a problem hiding this comment.
@MihalyToth20 Thank you for acknowledging this! If you need any clarification on the correct XanoScript syntax while making the fixes, feel free to ask. The key corrections needed are:
now()→"now"— timestamps use string literalsdb.addrequiresdatawrapper —db.add table_name { data = { ... } }try/catch→try_catch { try { ... } catch { ... } }— use the structured formdb.raw→db.direct_query— withsqlandargparametershttp.post→ External API Request — use Xano's visual builder equivalentlog()→debug.log { value = ... }— correct function name
Let me know if you'd like me to provide corrected code examples for any specific section!
* Initial plan * fix: address critical and major PR review comments - Fix path traversal by using fetchRegistryFileContent from api.ts - Fix file.path undefined error with explicit validation - Fix results structure to match InstallResults type with component field - Fix error handling to properly detect error instances - Validate required IDs for trigger install URLs - Fix apiGroupId type to string | number for consistency - Fix getApiGroupByName to return created group - Guard against empty registry items in promptForComponents - Fix Jest config to preserve base ignore patterns - Update AGENTS.md to reflect test files exist - Fix treat empty inline content as valid with null check - Fix registryItem.id reference error Co-authored-by: MihalyToth20 <34799518+MihalyToth20@users.noreply.github.com> * fix: align registry URL mappings with CLI and fix test - Update registry URL map to match CLI paths (workflow_test, mcp_server, realtime/channel, branch params) - Fix test to use 'error' field instead of 'reason' for skipped results - Add Jest config to ignore dist directory in core tests Co-authored-by: MihalyToth20 <34799518+MihalyToth20@users.noreply.github.com> * fix: codegen, name sanitization --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MihalyToth20 <34799518+MihalyToth20@users.noreply.github.com> Co-authored-by: Mihály Tóth <t.mihaly@icloud.com>
Summary by CodeRabbit