refactor(mcp): migrate mcp_service entry point from JS to TypeScript

[shayanshafii]

SUMMARY

Migrates superset/mcp_service/index.js to TypeScript (index.ts) as part of the ongoing "no JavaScript files" frontend modernization. The module is the Node entry point for the @superset/mcp-server package (it exports the SupersetMCPServer wrapper class around the Python MCP service).

Key decisions to keep the change behavior-preserving:

• CommonJS interop preserved. The original used module.exports = SupersetMCPServer, so index.ts uses export = SupersetMCPServer (compiles back to module.exports = SupersetMCPServer) rather than export default. This keeps require('@superset/mcp-server') returning the class directly. The options shape is exposed as SupersetMCPServer.Options via a class+namespace merge so the export = constraint (no other module-level exports) is satisfied.
• Typed options. Constructor options are typed via SupersetMCPServer.Options; the resolved/defaulted config is stored on a public readonly options: Required<Options>. This mirrors the original (in plain JS this.options was a public instance property) and avoids fabricating new behavior. Defaulting still uses || to exactly match the original runtime behavior.
• Dead imports dropped. The original imported child_process.spawn and path but never used them; they're removed. start() still performs the same side-effecting require('./bin/superset-mcp.js') (the bin script self-executes on import).
• Build setup. Added tsconfig.json (CommonJS, target ES2020, strict, declaration output) compiling in place so the relative ./bin/superset-mcp.js require keeps resolving. package.json gains a build (tsc) + prepublishOnly script, types field, and typescript/@types/node devDeps. The generated index.js/index.d.ts are gitignored build artifacts now (index.ts is the source of truth).

The compiled output is byte-for-byte equivalent in behavior to the previous index.js (verified: require() returns the class, default and overridden options resolve identically, start/stop present, stop() no-ops with a null process).

bin/superset-mcp.js is intentionally left as-is — this PR is scoped to index.js as requested.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

cd superset/mcp_service
npm install
npm run build          # compiles index.ts -> index.js + index.d.ts
node -e "const S = require('./index.js'); const s = new S({ port: 6000 }); console.log(s.options);"

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

Link to Devin session: https://app.devin.ai/sessions/c321172d25a6445fad70c37cfe5f4de3
Requested by: @shayanshafii

---

Devin Review

Status	Commit
⚪ Not started	—

Run Devin Review

---

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[devin-ai-integration]

Devin Review found 2 potential issues.

[devin-ai-integration]

🔴 start() never assigns this.process, making stop() a no-op

The start() method at superset/mcp_service/index.ts:46-49 calls require('./bin/superset-mcp.js') which executes main() in the bin script. That function spawns a Python child process as a local variable (pythonProcess at superset/mcp_service/bin/superset-mcp.js:250) and never exports it. As a result, this.process is only ever set to null (in the constructor at line 43 and in stop() at line 54), so the stop() method's if (this.process) guard at line 52 is always false and the spawned process can never be killed through this API. The class declares a private process: ChildProcess | null field and provides a stop() method, but they are entirely non-functional.

Prompt for agents

The start() method calls require('./bin/superset-mcp.js') which runs the bin script imperatively. The spawned Python child process (created by spawn() in bin/superset-mcp.js:250) is a local variable inside main() and is never exported or returned. This means the SupersetMCPServer.process field is never set, and stop() can never kill the spawned process.

To fix this, either:
1. Have bin/superset-mcp.js export the spawned ChildProcess (e.g., module.exports = pythonProcess) and capture it in start(): this.process = require('./bin/superset-mcp.js')
2. Refactor start() to directly use child_process.spawn() itself (importing it properly) and manage the subprocess lifecycle within the class, rather than delegating to the bin script.
3. If the class is not meant to manage the subprocess lifecycle (since the bin script already handles signals), remove the process field and stop() method entirely to avoid a misleading API.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Good catch, but this is preexisting behavior, not a regression introduced here. The original index.js had the exact same shape: the constructor set this.process = null, start() did const runner = require('./bin/superset-mcp.js') without ever assigning this.process, and stop() guarded on this.process (always null). The bin script spawns the Python process as a local pythonProcess and manages its own SIGINT/SIGTERM handlers, so the class-level lifecycle methods were already non-functional.

This PR is intentionally a 1:1 JS→TS migration, so I've preserved that behavior rather than changing the public API as part of a refactor. Wiring up the subprocess (your option 1/2) or dropping the dead stop()/process (option 3) is a reasonable follow-up, but it's a behavioral change that's out of scope for the migration. Happy to open a separate PR for it if desired.

[devin-ai-integration]

🚩 tsconfig enables noUnusedLocals and noUnusedParameters — verify build passes

The tsconfig.json enables both noUnusedLocals: true and noUnusedParameters: true. Since index.ts uses import type { ChildProcess } (type-only import) and ChildProcess is used in the process field declaration, this should be fine. However, if CI doesn't run npm run build for this sub-package, these strictness flags provide no actual enforcement. Worth confirming that the build is part of CI or at least that prepublishOnly is sufficient gating.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Confirmed — npm run build (i.e. tsc) compiles cleanly with both flags enabled (the import type { ChildProcess } is used in the process field type, and the resolved options are stored on the public options field, so nothing is flagged as unused).

You're right that the repo's CI doesn't currently build this sub-package, so today the enforcement comes from prepublishOnly (which runs build before publish) plus local/dev runs. I kept the strict flags on so the package is correctly gated whenever it is built. Adding a lightweight CI step for superset/mcp_service (e.g. npm ci && npm run build on changes to that path) would be a good follow-up to make the enforcement continuous, but it's outside the scope of this migration.