feat(mcp): add MCP app & macOS native Finder directory picker for setting working directory

[jqnatividad]

in this fallback order:
MCP app -> Native Directory Picker (macOS only) -> elicitation -> text prompt

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 8 comments.

Files not reviewed (1)

• .claude/skills/package-lock.json: Language not supported

[Copilot]

qsv_browse_directory bypasses the FilesystemResourceProvider's allowed-directory enforcement (it calls scanDirectory directly on the user-supplied path). This allows an App/LLM to enumerate arbitrary paths outside QSV_MCP_ALLOWED_DIRS. Resolve/validate targetDir via this.filesystemProvider.resolvePath(...) (or an equivalent provider API) before scanning, and ensure any returned child paths remain within allowed roots.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.

Files not reviewed (1)

• .claude/skills/package-lock.json: Language not supported

[Copilot]

The elicitation capability check doesn’t match the comment/intent: if (capabilities && !capabilities.elicitation) will still attempt elicitation when capabilities.elicitation is an empty object or when the client explicitly indicates form: false. Either tighten the condition to check the actual form capability (and treat false/empty as unsupported), or update the comment so it accurately reflects the logic.

Suggested change

	if (capabilities && !capabilities.elicitation) {
	// Client sent capabilities but elicitation is absent or explicitly opted out
	// Covers undefined, null, false, and empty object
	const hasFormElicitationCapability =
	!!(
	capabilities &&
	capabilities.elicitation &&
	typeof capabilities.elicitation === "object" &&
	(capabilities.elicitation as any).form === true
	);
	if (capabilities && !hasFormElicitationCapability) {
	// Client sent capabilities but does not support form elicitation
	// Covers: missing elicitation, empty object, or form explicitly false

[Copilot]

qsv_browse_directory is dispatched unconditionally here, even though the tool is intended to be App-only and is only listed when Apps are enabled/supported. As-is, clients can still invoke it directly when Apps are disabled, which makes the feature flag/tool visibility harder to reason about. Consider gating this handler behind config.enableMcpApps && this.clientSupportsApps() (or returning an explicit error) to align runtime behavior with how the tool is advertised.

Suggested change

	if (name === "qsv_browse_directory") {
	if (name === "qsv_browse_directory") {
	if (!(config.enableMcpApps && this.clientSupportsApps())) {
	return errorResult(
	"The qsv_browse_directory tool is only available when MCP Apps are enabled and supported by the client.",
	);
	}

[Copilot]

These tests mutate the shared config singleton (enableMcpApps) to assert tool metadata. Since the test runner (node --test) can run files concurrently, this can race with other test files that call createSetWorkingDirTool() and expect the default enableMcpApps=false, causing intermittent failures. Prefer avoiding global mutation (e.g., refactor createSetWorkingDirTool to accept an options/config override for tests, or use a module-mocking approach that doesn’t affect other files).

[Copilot]

This test doesn’t assert any observable behavior (it always passes). If the goal is to ensure _meta is optional, that’s already enforced by TypeScript at compile-time; in runtime tests this adds no coverage. Consider removing it, or replace it with a real assertion about _meta presence/absence on specific tool definitions.

Suggested change

	// Import and create a tool without _meta to verify the type allows it
	const { createGetWorkingDirTool } = await import("../src/mcp-tools.js");
	const tool = createGetWorkingDirTool();
	// _meta should be undefined for tools that don't declare it
	// (or it may be present — either way, it shouldn't break)
	assert.ok(true, "tool without _meta compiles and runs fine");
	// Import and create a tool that is expected NOT to declare _meta
	const { createGetWorkingDirTool } = await import("../src/mcp-tools.js");
	const tool = createGetWorkingDirTool();
	// At runtime, tools that don't declare _meta should not define the property
	const hasMeta = Object.prototype.hasOwnProperty.call(tool as object, "_meta");
	assert.strictEqual(hasMeta, false, "tool without _meta should not define _meta at runtime");

[Copilot]

This block only asserts that a locally-constructed Set contains the string "qsv_browse_directory"; it doesn’t validate the server’s actual ELICITATION_EXEMPT_TOOLS configuration. Consider testing the real source of truth (e.g., by exporting the set/adding an accessor, or by testing behavior that depends on the exemption) so the test will fail if the production exemption list regresses.

Suggested change

	test("qsv_browse_directory should be exempt from first-use elicitation", () => {
	// Mirror the server's exempt tools set
	const exemptTools = new Set([
	"qsv_config",
	"qsv_log",
	"qsv_search_tools",
	"qsv_set_working_dir",
	"qsv_get_working_dir",
	"qsv_browse_directory",
	]);
	assert.ok(exemptTools.has("qsv_browse_directory"), "browse_directory should be exempt");
	// Data tools should NOT be exempt
	assert.ok(!exemptTools.has("qsv_stats"), "data tools should not be exempt");
	test("qsv_browse_directory should be exempt from first-use elicitation", async () => {
	const { ELICITATION_EXEMPT_TOOLS } = await import("../src/server.js");
	// browse_directory should be exempt according to the server's configuration
	assert.ok(
	ELICITATION_EXEMPT_TOOLS.has("qsv_browse_directory"),
	"browse_directory should be exempt",
	);
	// Data tools should NOT be exempt
	assert.ok(
	!ELICITATION_EXEMPT_TOOLS.has("qsv_stats"),
	"data tools should not be exempt",
	);