fix: update tool names to snake_case format for consistency#427
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR migrates MCP job and workflow tool identifiers from kebab-case naming (e.g., ChangesMCP Tool Naming Migration and Auto-Enablement
🎯 3 (Moderate) | ⏱️ ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant DirectClientImpl
participant MCPRegistry
participant ToolEntry
Client->>DirectClientImpl: listJobs / executeJob / getJobStatus (toolName: snake_case)
DirectClientImpl->>MCPRegistry: findTool(requestedName)
MCPRegistry->>ToolEntry: match candidate names (snake_case, legacy-kebab)
alt resolved via legacy alias
ToolEntry-->>MCPRegistry: matched (alias)
MCPRegistry->>DirectClientImpl: warn deprecation -> return tool
else direct match
ToolEntry-->>MCPRegistry: matched (canonical)
MCPRegistry->>DirectClientImpl: return tool
end
DirectClientImpl->>ToolEntry: callTool(...)
Possibly related issues
Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/sdk/src/scope/scope.instance.ts (1)
695-784: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftExtract jobs/workflows bootstrap logic out of
Scope.initialize().Line [695]-Line [784] adds a large feature-specific block inline; please move it into helper method(s) so
initialize()stays orchestration-focused and easier to maintain.♻️ Suggested extraction sketch
- // Initialize jobs and workflows (issue `#408`). - // ... - const jobsConfig = this.metadata.jobs as JobsConfig | undefined; - const metaRecord = this.metadata as unknown as Record<string, unknown>; - const jobsList = (metaRecord['jobTypes'] as JobType[] | undefined) ?? []; - const workflowsList = (metaRecord['workflowTypes'] as WorkflowType[] | undefined) ?? []; - // ... large inline block ... - if (effectivelyEnabled) { - // ... registration ... - } + const jobsBootstrap = this.resolveJobsBootstrapState(); + await this.initializeJobsAndWorkflows(scopeRef, jobsBootstrap);// example helper boundaries private resolveJobsBootstrapState(): { allJobs: JobType[]; allWorkflows: WorkflowType[]; explicitlyDisabled: boolean; autoEnabled: boolean; effectivelyEnabled: boolean; effectiveConfig: JobsConfig; } { /* ... */ } private async initializeJobsAndWorkflows( scopeRef: EntryOwnerRef, state: ReturnType<Scope['resolveJobsBootstrapState']>, ): Promise<void> { /* ... */ }As per coding guidelines,
libs/sdk/src/**/scope.instance.ts: “Keepscope.instance.tslean by using helper functions for feature-specific registration logic (e.g.,registerSkillCapabilitiesfrom skill module) instead of inlining 40+ lines”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/sdk/src/scope/scope.instance.ts` around lines 695 - 784, Extract the inlined jobs/workflows bootstrap block out of Scope.initialize() into two helpers: a synchronous resolveJobsBootstrapState() that computes allJobs, allWorkflows, hasJobDeclarations, explicitlyDisabled, explicitlyEnabled, autoEnabled, effectivelyEnabled and effectiveConfig (collecting app jobs/workflows and metadata like the current code uses jobsConfig, metaRecord, appJobs/appWorkflows), and an async initializeJobsAndWorkflows(scopeRef: EntryOwnerRef, state) that performs the side-effectful logic (logging warnings/info, constructing notifyFn, calling registerJobCapabilities, assigning this._scopeJobs/_scopeWorkflows/_jobExecutionManager/_jobStateStore/_jobDefinitionStore, iterating result.managementTools and creating ToolInstance via normalizeTool and registering with this.scopeTools.registerToolInstance, and final info log). Replace the original block in initialize() with calls to resolveJobsBootstrapState() and, if state.effectivelyEnabled, await initializeJobsAndWorkflows(scopeRef, state). Ensure all referenced symbols (registerJobCapabilities, normalizeTool, ToolInstance, scopeTools.registerToolInstance) are imported/accessible and preserve the same logging and error handling behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/sdk/src/tool/flows/call-tool.flow.ts`:
- Around line 295-314: The legacy-name aliasing is only applied when searching
activeTools, so the later remote-registry fallback still uses the original name
and can return TOOL_NOT_FOUND for legacy callers; modify the lookup logic in the
call-tool flow (where variables name, activeTools, tool, alt, altMatch, and
this.logger.warn are used) to build a single list of candidate names (e.g.,
[name, alt] when /[-_]/.test(name)) and use that same candidates array for both
the activeTools search and the remote-registry lookup; keep the one-time
deprecation warning when an alias matched (this.logger.warn) and ensure the
remote lookup iterates the same candidates instead of only the original name so
legacy hyphen/underscore aliases are honored everywhere.
In `@libs/skills/catalog/frontmcp-development/references/create-job.md`:
- Around line 421-424: The docs contradict themselves: the top section says
declaring `@App`({ jobs: [...] }) auto-enables the jobs subsystem, but later
checklist/troubleshooting still requires jobs.enabled: true; update the text so
the checklist and troubleshooting reflect the auto-enable semantics — remove or
mark as optional any requirement that jobs.enabled: true unless the user is
intentionally overriding the default for persistence, and instead document that
`@FrontMcp`({ jobs }) should only be used to configure persistent stores (Redis)
or change defaults; adjust references to management tools (execute_job,
list_jobs, get_job_status, register_job, remove_job) to note they are registered
automatically when `@App`({ jobs }) or workflows are declared; apply the same
corrections to the other occurrence around lines referenced (the 638-649 block)
to keep both sections consistent.
---
Outside diff comments:
In `@libs/sdk/src/scope/scope.instance.ts`:
- Around line 695-784: Extract the inlined jobs/workflows bootstrap block out of
Scope.initialize() into two helpers: a synchronous resolveJobsBootstrapState()
that computes allJobs, allWorkflows, hasJobDeclarations, explicitlyDisabled,
explicitlyEnabled, autoEnabled, effectivelyEnabled and effectiveConfig
(collecting app jobs/workflows and metadata like the current code uses
jobsConfig, metaRecord, appJobs/appWorkflows), and an async
initializeJobsAndWorkflows(scopeRef: EntryOwnerRef, state) that performs the
side-effectful logic (logging warnings/info, constructing notifyFn, calling
registerJobCapabilities, assigning
this._scopeJobs/_scopeWorkflows/_jobExecutionManager/_jobStateStore/_jobDefinitionStore,
iterating result.managementTools and creating ToolInstance via normalizeTool and
registering with this.scopeTools.registerToolInstance, and final info log).
Replace the original block in initialize() with calls to
resolveJobsBootstrapState() and, if state.effectivelyEnabled, await
initializeJobsAndWorkflows(scopeRef, state). Ensure all referenced symbols
(registerJobCapabilities, normalizeTool, ToolInstance,
scopeTools.registerToolInstance) are imported/accessible and preserve the same
logging and error handling behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 003b741b-4ef6-48bb-bb09-fbb003829343
⛔ Files ignored due to path filters (1)
libs/cli/src/commands/build/exec/cli-runtime/schema-extractor.tsis excluded by!**/build/**
📒 Files selected for processing (29)
docs/frontmcp/deployment/mcpb.mdxdocs/frontmcp/sdk-reference/decorators/job.mdxdocs/frontmcp/sdk-reference/decorators/workflow.mdxdocs/frontmcp/sdk-reference/registries/job-registry.mdxdocs/frontmcp/sdk-reference/registries/workflow-registry.mdxdocs/frontmcp/servers/jobs.mdxdocs/frontmcp/servers/workflows.mdxlibs/sdk/package.jsonlibs/sdk/src/__tests__/job-tools-exports.spec.tslibs/sdk/src/direct/__tests__/direct-client.spec.tslibs/sdk/src/direct/direct-client.tslibs/sdk/src/direct/direct-server.tslibs/sdk/src/index.tslibs/sdk/src/job/index.tslibs/sdk/src/job/tools/execute-job.tool.tslibs/sdk/src/job/tools/get-job-status.tool.tslibs/sdk/src/job/tools/list-jobs.tool.tslibs/sdk/src/job/tools/register-job.tool.tslibs/sdk/src/job/tools/remove-job.tool.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/sdk/src/workflow/index.tslibs/sdk/src/workflow/tools/execute-workflow.tool.tslibs/sdk/src/workflow/tools/get-workflow-status.tool.tslibs/sdk/src/workflow/tools/list-workflows.tool.tslibs/sdk/src/workflow/tools/register-workflow.tool.tslibs/sdk/src/workflow/tools/remove-workflow.tool.tslibs/skills/catalog/frontmcp-deployment/references/build-for-mcpb.mdlibs/skills/catalog/frontmcp-development/references/create-job.md
Contributor
Performance Test ResultsStatus: ✅ All tests passed Summary
Total: 101 tests across 21 projects 📊 View full report in workflow run Generated at: 2026-05-24T12:48:10.086Z |
…_case consistency
…_case consistency
… fix/408-jobs-auto-register
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/e2e/demo-e2e-jobs/e2e/jobs-client.e2e.spec.ts (2)
113-125: ⚡ Quick winConsider adding an e2e test for legacy alias backward compatibility.
The comment notes that hyphen forms remain callable through alias fallback, but there's no e2e test verifying this critical backward compatibility feature works across the wire.
E2E tests should validate that agents/callers using the old kebab-case names (e.g.,
execute-job) still function during the one-release migration period.🧪 Proposed test for legacy alias compatibility
}); + + it('should support legacy kebab-case tool names via alias fallback', async () => { + // Verify backward compatibility during migration period (Issue `#408`) + const result = await client.callTool('execute-job', { + name: 'greet', + input: { name: 'Alice' }, + background: false, + }); + const content = result.content?.find((c: { type: string }) => c.type === 'text'); + expect(content).toBeDefined(); + const parsed = JSON.parse(content.text); + expect(parsed.state).toBe('completed'); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/e2e/demo-e2e-jobs/e2e/jobs-client.e2e.spec.ts` around lines 113 - 125, Add an e2e test that verifies the legacy kebab-case tool aliases still work end-to-end: in the existing describe block that calls client.listTools() (referencing listTools and client), invoke the legacy names (e.g., "execute-job" and "get-job-status") through the same client call path used by callers (simulate an agent/caller call that resolves a tool by name) and assert the call succeeds and returns the same behavior/result as the canonical snake_case tools (execute_job, get_job_status); ensure the test triggers the alias fallback path used by call-tool.flow.ts and includes assertions that both the kebab-case call succeeds and that any returned status or output matches the snake_case equivalent.
122-123: ⚡ Quick winAssertions correctly updated to snake_case.
The test now expects
execute_jobandget_job_status, matching the renamed tool identifiers.Consider expanding coverage to verify all 10 renamed tools appear in the listing:
- Job tools:
list_jobs,register_job,remove_job,execute_job,get_job_status- Workflow tools:
list_workflows,register_workflow,remove_workflow,execute_workflow,get_workflow_statusThis would provide more comprehensive validation of the migration.
🧪 Proposed expansion for comprehensive coverage
it('should include job management tools in listTools', async () => { const tools = await client.listTools(); const toolNames = (tools as Array<{ name: string }>).map((t) => t.name); + // Job management tools + expect(toolNames).toContain('list_jobs'); + expect(toolNames).toContain('register_job'); + expect(toolNames).toContain('remove_job'); expect(toolNames).toContain('execute_job'); expect(toolNames).toContain('get_job_status'); + // Workflow management tools + expect(toolNames).toContain('list_workflows'); + expect(toolNames).toContain('register_workflow'); + expect(toolNames).toContain('remove_workflow'); + expect(toolNames).toContain('execute_workflow'); + expect(toolNames).toContain('get_workflow_status'); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/e2e/demo-e2e-jobs/e2e/jobs-client.e2e.spec.ts` around lines 122 - 123, The test currently asserts only two snake_case tool identifiers; update the assertions that examine the toolNames array to verify all ten renamed tools appear by adding expects for list_jobs, register_job, remove_job, execute_job, get_job_status, and list_workflows, register_workflow, remove_workflow, execute_workflow, get_workflow_status (use the existing toolNames variable and the same expect(...).toContain(...) pattern to add these checks so the spec fully validates both Job and Workflow tool migrations).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/e2e/demo-e2e-jobs/e2e/jobs-client.e2e.spec.ts`:
- Around line 113-125: Add an e2e test that verifies the legacy kebab-case tool
aliases still work end-to-end: in the existing describe block that calls
client.listTools() (referencing listTools and client), invoke the legacy names
(e.g., "execute-job" and "get-job-status") through the same client call path
used by callers (simulate an agent/caller call that resolves a tool by name) and
assert the call succeeds and returns the same behavior/result as the canonical
snake_case tools (execute_job, get_job_status); ensure the test triggers the
alias fallback path used by call-tool.flow.ts and includes assertions that both
the kebab-case call succeeds and that any returned status or output matches the
snake_case equivalent.
- Around line 122-123: The test currently asserts only two snake_case tool
identifiers; update the assertions that examine the toolNames array to verify
all ten renamed tools appear by adding expects for list_jobs, register_job,
remove_job, execute_job, get_job_status, and list_workflows, register_workflow,
remove_workflow, execute_workflow, get_workflow_status (use the existing
toolNames variable and the same expect(...).toContain(...) pattern to add these
checks so the spec fully validates both Job and Workflow tool migrations).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5170e290-3d54-47b0-9a44-a94ad1a48ddd
📒 Files selected for processing (1)
apps/e2e/demo-e2e-jobs/e2e/jobs-client.e2e.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
@App; opt-in exports/subpaths expose manual job/workflow management.Documentation
Tests