Skip to content

fix: improve prompts #90

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/shared/params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ export const baseAbsolutePathParam = z
.refine(sanitizePath, 'Invalid path: Must be an absolute path and cannot contain path traversal sequences');

export const directoryParam = baseAbsolutePathParam.describe(`The directory to run this tool from.
AGENT INSTRUCTIONS:
We need to know where the user wants to run this tool from.
Look at your current Workspace Context to determine this filepath.
ALWAYS USE A FULL PATH TO THE DIRECTORY.
Unless the user explicitly asks for a different directory, or a new directory is created from the action of a tool, use this same directory for future tool calls.
Use the full absolute path to the root directory of the active project in the current IDE workspace context.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we want to reference IDE in this context. The mcp server can be used in different applications which would not be recognized as IDE by the agent. For example openai/codex or google-gemini/gemini-cli. This could confuse the agent.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call.
While MCPs could be used in any app that supports it, at least for this server we are making some assumptions about the kind of MCP client users are in (IDEs, CLI agent tools like you mentioned), I'll take a look at some open source client prompts to see if there's something else that can improve this param description.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw, MCP roots could help remove the inference step when figuring out the open project directory but support in clients it's not good enough yet:
https://modelcontextprotocol.io/clients


This directory must be an absolute path to ensure consistent tool behavior.

Unless the user explicitly specifies a different directory, or the action of another tool creates a new working directory, use the same project directory for subsequent tool calls.
`);
16 changes: 10 additions & 6 deletions src/tools/metadata/sf-deploy-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,13 @@ import { getConnection } from '../../shared/auth.js';
import { SfMcpServer } from '../../sf-mcp-server.js';

const deployMetadataParams = z.object({
sourceDir: z
filePaths: z
.array(z.string())
.describe('Path to the local source files to deploy. Leave this unset if the user is vague about what to deploy.')
.describe(
`Path to the local source files to deploy. Leave this unset if the user is vague about what to deploy.
All file paths should be relative to the current directory.
`
)
.optional(),
manifest: z.string().describe('Full file path for manifest (XML file) of components to deploy.').optional(),
// `RunSpecifiedTests` is excluded on purpose because the tool sets this level when Apex tests to run are passed in.
Expand Down Expand Up @@ -102,12 +106,12 @@ Deploy X to my org and run A,B and C apex tests.
destructiveHint: true,
openWorldHint: false,
},
async ({ sourceDir, usernameOrAlias, apexTests, apexTestLevel, directory, manifest }) => {
async ({ filePaths, usernameOrAlias, apexTests, apexTestLevel, directory, manifest }) => {
if (apexTests && apexTestLevel) {
return textResponse("You can't specify both `apexTests` and `apexTestLevel` parameters.", true);
}

if (sourceDir && manifest) {
if (filePaths && manifest) {
return textResponse("You can't specify both `sourceDir` and `manifest` parameters.", true);
}

Expand All @@ -125,7 +129,7 @@ Deploy X to my org and run A,B and C apex tests.

const org = await Org.create({ connection });

if (!sourceDir && !manifest && !(await org.tracksSource())) {
if (!filePaths && !manifest && !(await org.tracksSource())) {
return textResponse(
'This org does not have source-tracking enabled or does not support source-tracking. You should specify the files or a manifest to deploy.',
true
Expand All @@ -140,7 +144,7 @@ Deploy X to my org and run A,B and C apex tests.
subscribeSDREvents: true,
});

const componentSet = await buildDeployComponentSet(connection, project, stl, sourceDir, manifest);
const componentSet = await buildDeployComponentSet(connection, project, stl, filePaths, manifest);

if (componentSet.size === 0) {
// STL found no changes
Expand Down
Loading