Skip to content

Introduce Elicitation capability #520

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

Merged
merged 8 commits into from
Jun 16, 2025
Merged

Introduce Elicitation capability #520

merged 8 commits into from
Jun 16, 2025

Conversation

ihrpr
Copy link
Contributor

@ihrpr ihrpr commented May 20, 2025

Implementing spec:
modelcontextprotocol/modelcontextprotocol#382

npx tsx src/examples/server/simpleStreamableHttp.ts
npx tsx src/examples/client/simpleStreamableHttp.ts
Please provide the following information (attempt 1/3):
Rating (Rate your experience (1-5)) [1-5] *required*: 4
Comments (Additional comments (optional)): test
Would you recommend this? (Would you recommend this to others?) *required*: nope

✅ Collected data:
{
  "rating": 4,
  "comments": "test",
  "recommend": false
}

Submit this information? (yes/no/cancel): yes
Tool result:
  Thank you! Collected feedback information: {
  "rating": 4,
  "comments": "test",
  "recommend": false
}

> quit
Terminating session before exit...
Session terminated successfully

@bhosmer-ant
Copy link
Contributor

@ihrpr fyi rebased and fixed up the ajv imports and calls (needed due to the version downgrade)

FYI in addition to the stale resumption token issue (#530), I think there may be an issue when a request times out mid-elicitation. This was hard to repro and I'm not sure if it's due to the example code or deeper in the server logic.

@ihrpr
Copy link
Contributor Author

ihrpr commented May 22, 2025

I think there may be an issue when a request times out mid-elicitation.

We can set timeouts for requests and cover with tests

@ihrpr ihrpr marked this pull request as ready for review June 10, 2025 15:07
@ihrpr ihrpr changed the title elicitation example Introduce Elicitation capability Jun 10, 2025
@ihrpr ihrpr linked an issue Jun 10, 2025 that may be closed by this pull request
@gzuuus
Copy link

gzuuus commented Jun 12, 2025

I've implemented this feature in a demo https://github.com/gzuuus/elicitation-demo-mcp

@bhosmer-ant bhosmer-ant self-requested a review June 16, 2025 15:14
Copy link
Member

@dsp-ant dsp-ant left a comment

Choose a reason for hiding this comment

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

Accepting. Some comments that avoid writing our own parsers.

Comment on lines +215 to +405
} else {
// Parse the value based on type
let parsedValue: unknown;

if (field.type === 'boolean') {
parsedValue = answer.toLowerCase() === 'true' || answer.toLowerCase() === 'yes' || answer === '1';
} else if (field.type === 'number') {
parsedValue = parseFloat(answer);
if (isNaN(parsedValue as number)) {
throw new Error(`${fieldName} must be a valid number`);
}
} else if (field.type === 'integer') {
parsedValue = parseInt(answer, 10);
if (isNaN(parsedValue as number)) {
throw new Error(`${fieldName} must be a valid integer`);
}
} else if (field.enum) {
if (!field.enum.includes(answer)) {
throw new Error(`${fieldName} must be one of: ${field.enum.join(', ')}`);
}
parsedValue = answer;
} else {
parsedValue = answer;
}

content[fieldName] = parsedValue;
}
} catch (error) {
console.log(`❌ Error: ${error}`);
// Continue to next attempt
break;
}
}

if (inputCancelled) {
return { action: 'cancel' };
}

// If we didn't complete all fields due to an error, try again
if (Object.keys(content).length !== Object.keys(properties).filter(name =>
required.includes(name) || content[name] !== undefined
).length) {
if (attempts < maxAttempts) {
console.log('Please try again...');
continue;
} else {
console.log('Maximum attempts reached. Declining request.');
return { action: 'decline' };
}
}

// Validate the complete object against the schema
const isValid = validate(content);

if (!isValid) {
console.log('❌ Validation errors:');
validate.errors?.forEach(error => {
console.log(` - ${error.dataPath || 'root'}: ${error.message}`);
});

if (attempts < maxAttempts) {
console.log('Please correct the errors and try again...');
continue;
} else {
console.log('Maximum attempts reached. Declining request.');
return { action: 'decline' };
}
}

// Show the collected data and ask for confirmation
console.log('\n✅ Collected data:');
console.log(JSON.stringify(content, null, 2));

const confirmAnswer = await new Promise<string>((resolve) => {
readline.question('\nSubmit this information? (yes/no/cancel): ', (input) => {
resolve(input.trim().toLowerCase());
});
});


if (confirmAnswer === 'yes' || confirmAnswer === 'y') {
return {
action: 'accept',
content,
};
} else if (confirmAnswer === 'cancel' || confirmAnswer === 'c') {
return { action: 'cancel' };
} else if (confirmAnswer === 'no' || confirmAnswer === 'n') {
if (attempts < maxAttempts) {
console.log('Please re-enter the information...');
continue;
} else {
return { action: 'decline' };
}
}
}

console.log('Maximum attempts reached. Declining request.');
return { action: 'decline' };
});
Copy link
Member

Choose a reason for hiding this comment

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

This seems very complex. Should we use something like https://www.npmjs.com/package/@inquirer/prompts in order to make it easier?

Copy link
Contributor

Choose a reason for hiding this comment

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

Took a quick stab at this #637 but I don't think it's a good change - it's not significantly simpler, and too opinionated w.r.t. what clients should be doing IMO. As a super simple demo of the flow through the protocol I think this is fine

@ihrpr ihrpr merged commit bbc128c into main Jun 16, 2025
5 checks passed
@ihrpr ihrpr deleted the ihrpr/elicitation branch June 16, 2025 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce elicitation as new client capability
4 participants