Windsurf

[rachellerathbone]

What

Brief description of changes

Why

Why this change was needed

How

Brief technical approach

Testing

How to verify the changes

[github-actions]

multicorn-ops review

Persona	Role	Primary	Status	Summary
Jordan	Security Auditor	no	Concern	API key embedded in plaintext snippet written to stderr and config file; detection heuristic is fragile.
Priya	Open Source Contributor	yes	Concern	Two new Windsurf tests are nearly identical and share no helper, creating maintenance burden for a new contributor.
Marcus	Design-Conscious Developer	no	Passed	No UI changes in this PR; all output is CLI stderr text which is outside Marcus's scope.
Sarah	Non-Technical Decision-Maker	no	Passed	The new next-steps copy is clear, numbered, and free of developer jargon.
The Team	Acquisition Due Diligence	yes	Concern	Duplicate test boilerplate, a magic-number option index, and a substring-based config detection heuristic are mild but real tech-debt signals.
Alex	Accessibility Advocate	no	Passed	No HTML or interactive UI changed; all output is CLI text and outside accessibility scope.
Yuki	International User	yes	Concern	Most copy is clear, but one instruction uses an ambiguous idiom and path placeholders could be more explicit.

Concerns

Jordan (Security Auditor)

• src/proxy/config.ts:875 - The API key (apiKey) is interpolated directly into the JSON snippet printed to stderr (Bearer ${apiKey}). Terminal output may be captured in logs, CI artifacts, or bug reports, leaking the credential.
• src/proxy/config.ts:597 - isWindsurfConnected detects a Multicorn entry by checking whether serverUrl contains the string 'multicorn'. This substring match is too broad and could produce false positives for unrelated servers whose URL happens to include that word, or false negatives for legitimately renamed entries. A more precise ownership check (e.g. a known domain suffix) would be safer.

Priya (Open Source Contributor)

• src/proxy/__tests__/proxy.edge-cases.test.ts:851 - The two new Windsurf tests ('runInit completes Windsurf platform…' and 'runInit Windsurf Next steps includes download hint…') duplicate the entire setup block (captureStderr, mocks, mockPrompts, fetch mock). A shared helper or beforeEach would make these much easier to follow and extend.
• src/proxy/__tests__/proxy.edge-cases.test.ts:951 - The second Windsurf test calls await runInit(...) but discards the return value and only checks stderr strings. The first test already covers those same stderr assertions. It's unclear what unique behaviour the second test adds; this should be documented or the tests merged.
• src/proxy/config.ts:695 - PLATFORM_LABELS is a plain array and PLATFORM_BY_SELECTION is a separate object keyed by index. Adding a new platform requires edits in three places (array, map, DEFAULT_AGENT_NAMES). A single structured array of platform descriptors would be easier to understand and extend for a new contributor.

The Team (Acquisition Due Diligence)

• src/proxy/config.ts:1069 - The if (selection === 5) branch hard-codes the menu index for 'Local MCP / Other'. If another platform is inserted before it, this silently breaks. Consider comparing against the platform string ('other-mcp') derived from PLATFORM_BY_SELECTION instead.
• src/proxy/config.ts:597 - isWindsurfConnected uses url.includes('multicorn') as the ownership signal. This is fragile and undocumented; no test covers false-positive or false-negative cases. An explicit key name or a full domain match would be more robust.
• src/proxy/__tests__/proxy.edge-cases.test.ts:851 - ~80 lines of fetch/prompt/mock setup are copy-pasted verbatim across two Windsurf tests. This inflates file size and means any change to the mock contract requires multiple edits — a maintainability concern for an acquirer assessing code health.
• src/proxy/config.ts:898 - The else branch of the platform snippet printer still emits '~/.cursor/mcp.json' as a fallback label even though Windsurf now has its own branch. If a third platform is added and misses a branch, the error message will silently show Cursor instructions — misleading and hard to debug.

Yuki (International User)

• src/proxy/config.ts:1395 - 'or launch it for the first time' is a parenthetical idiom that may confuse non-native readers. Consider 'or start Windsurf if this is your first time opening it' for clarity.
• src/proxy/config.ts:1390 - The instruction says 'Open ~/.codeium/windsurf/mcp_config.json and paste the config snippet shown above'. On Windows the path is different; non-native users unfamiliar with tilde expansion may not know where this file is. Adding '(create it if it does not exist)' and a note about Windows equivalent would improve actionability.
• CHANGELOG.md:12 - The changelog uses the phrase 'gave no guidance to first-time users' — this is user-facing documentation and is fine, but the parenthetical '(Config file: ..., Restart Cursor to pick up MCP config changes)' in the same line is internal implementation detail mixed into a public changelog entry. Consider separating the old vs new behaviour more cleanly.

Open-Source Readiness Checklist

Code Quality

• All functions have clear, descriptive names — New functions getWindsurfConfigPath and isWindsurfConnected are clearly named and purposeful.
• No hardcoded secrets, API keys, internal URLs, or employee names in code or comments — URLs present (multicorn.ai, windsurf.com, cursor.com) are public product URLs, not secrets or internal addresses.
• No // TODO without a public issue reference — No TODO comments introduced in the diff.
• No commented-out code blocks — No commented-out code found in the diff.
• No debug logging (console.log, println) left in — All output uses process.stderr.write for intentional CLI messaging, no debug logs added.
• All any types eliminated (TypeScript) — In isWindsurfConnected, cast obj uses Record<string, unknown> which is fine, but entry as Record<string, unknown> after a non-null check is a forced cast that bypasses type safety; acceptable but worth noting. No explicit any introduced.
• Error handling is complete — no swallowed exceptions, no empty catch blocks — The catch block in isWindsurfConnected silently returns false without logging, swallowing unexpected errors (e.g. permission denied vs. file not found). Same pattern exists for isCursorConnected, so it's consistent, but it is a swallowed exception.
• No Atlassian-internal references, no proprietary patterns or terminology — No Atlassian or other internal references found.

Testing

• All new code has tests — Two new integration tests cover the Windsurf platform path, next-steps copy, and snippet content.
• [~] Coverage meets or exceeds repo minimum — Coverage metrics are not visible in the diff; CI results needed to confirm.
• [~] Tests pass locally and in CI — Cannot verify from diff alone; CI results needed.
• Edge cases and error paths are tested — The new tests cover the happy path and copy assertions but do not test isWindsurfConnected with a malformed JSON file, missing mcpServers key, or permission-denied scenario.
• No flaky tests — Tests use deterministic mocks; no obvious sources of flakiness introduced.

Security

• No secrets in code, comments, config files, or git history — No secrets detected.
• All user input is validated — Platform selection input is validated (parseInt + range check 1–5); no new unvalidated inputs introduced.
• [~] Dependencies audited — no known vulnerabilities — No dependency changes in this diff; audit must be checked separately.
• HTTPS enforced for all external communication — All URLs in the diff use HTTPS.
• API keys/tokens never logged — API key appears only in the intentional user-facing config snippet output, consistent with existing Cursor behaviour.

Documentation

• [~] README.md is accurate and up to date — README.md not modified in this diff; unclear if it needs updating for the new Windsurf platform.
• [~] CONTRIBUTING.md is accurate and up to date — CONTRIBUTING.md not touched; cannot assess from diff.
• CHANGELOG.md updated with this change — CHANGELOG.md updated with a 0.8.0 entry describing the Windsurf addition and Next Steps rewrite.
• New public APIs have JSDoc/KDoc with examples — getWindsurfConfigPath and isWindsurfConnected both have JSDoc comments with @returns descriptions.
• [~] Any new config options are documented — New windsurf platform option is surfaced only via the CLI wizard; no separate config-option documentation file is visible in the diff.
• [~] Architecture decisions documented in ADR if significant — No ADR directory changes; whether this warrants an ADR depends on project conventions not visible here.

Open Source Hygiene

• [~] Licence header present in source files (if required by licence) — No licence header policy visible in the diff; existing files do not show headers so it may not be required.
• [~] CODE_OF_CONDUCT.md present — Not modified in this diff; presence in repo cannot be confirmed from diff alone.
• [~] Issue templates are current — Not modified in this diff.
• [~] PR template is current — Not modified in this diff.
• No internal company references or links — Only public URLs (multicorn.ai, windsurf.com, cursor.com, codeium) present.
• [~] Package name and description are correct in package.json — package.json not modified in this diff.
• [~] Repository topics/tags are set on GitHub — Cannot be assessed from a code diff.

---

Advisory only. Does not block merge. Actions logged to Shield as pr_review and oss_check.