Skip to content

Test plugin setup and MCP config writing - #49

Merged
nrodd merged 1 commit into
tests/harness-and-cifrom
tests/plugin-setup
Sep 28, 2026
Merged

nrodd merged 1 commit into
tests/harness-and-cifrom
tests/plugin-setup

Conversation

@nrodd

@nrodd nrodd commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Chunk 7 of the test plan. src/pluginSetup.test.ts, 47 tests. No source changes.

Based on #42 (the harness). GitHub will retarget this to main when that merges. Independent of #43–#48.

Why this one got the most care

This is the only code in the wizard that edits files in the user's home directory, and it runs after the install has already succeeded. Three rules follow, and each has tests:

  1. Never clobber a config it cannot parse
  2. Never remove an entry it did not write
  3. Never throw — a plugin problem must not turn a finished install into a reported failure

Everything is driven through offerPluginSetup against a temp HOME rather than through the private writers, so the decision tree and the file I/O are exercised together. That's also why this chunk needed no exports.

Unparseable means untouched

Asserted on the file's bytes, not just the return value — a writer that returns 'unparseable' after already having written is still a bug:

  • .mcp.json with a JSONC comment
  • a JSON root that isn't an object
  • an mcpServers that isn't an object
  • a Codex [mcp_servers.subtext] table with no url line

Plus: a commented-out TOML header doesn't count as the real table, and a re-run that would change nothing reports "already configured" instead of rewriting.

Never remove what we didn't write

removeOurs only deletes a subtext entry whose URL is one of our two realm endpoints. A user who pointed subtext at their own proxy keeps it, and neighbouring servers are never touched.

The EU branch

The packaged plugin ships the NA endpoint only, so an EU org must never take that path — it would talk to the wrong realm. Covered: no plugin install attempted, the EU URL written directly, a warning when the NA plugin is already installed alongside, and the openskills pointer for the review skills Claude Code would otherwise lose.

Two cancels that mean opposite things

By this point the install has succeeded, so Ctrl+C here skips the step — exit 130 and "nothing was changed" would both be lies. But guidePluginSetup runs before the GUI handoff, where the answer decides telemetry ownership, so there Ctrl+C aborts. Both are pinned, so the asymmetry is on the record rather than looking like an oversight. (Chunk 4's picker is the third case — it aborts too.)

Also: approving a plugin install is not approving a config-file edit, so there's a test that the raw-entry fallback asks again.

One thing for review

removeSupersededRawEntry opens with if (region === 'eu') return;, but that's unreachable — the function is only called from the packaged-plugin path, which EU never takes. Harmless defensive code and I left it alone, but the comment above it implies it's load-bearing and it isn't. The EU entry is kept safe by the branch in offerPluginSetup, which is what the test actually covers. Flagging rather than changing, since deleting it is a judgement call.

Verification

47 tests passing (100 on this branch: chunk 0 plus this one), typecheck clean.

Mutation testing found a real gap here, which is worth mentioning: my first pass had three mutations and only one failed. Removing the non-object-mcpServers guard went undetected because I'd only tested a non-object root. That test now exists. (The third mutation exposed the unreachable guard above.) Final pass: both live guards caught, then reverted.


Note

Low Risk
Test-only addition; no runtime or config-writing logic is modified.

Overview
Adds src/pluginSetup.test.ts (~47 Vitest cases) with no production code changes. Coverage exercises offerPluginSetup and guidePluginSetup end-to-end against a temp HOME and project dir, with mocked @clack/prompts, homedir, and runTerminalAgent.

The suite pins the wizard’s post-install plugin/MCP behavior: realm-aware US/EU endpoints; manual instructions vs Codex TOML, Claude Code plugin CLI + .mcp.json fallback, and Gemini extension/settings; safe file edits (skip unparseable configs, preserve user entries, only remove Subtext URLs the tool wrote); EU orgs bypassing the NA-only packaged plugin; consent/--yes/double-confirm for config writes; and Ctrl+C semantics (skip post-install vs abort in guidePluginSetup).

Reviewed by Cursor Bugbot for commit bcf3bc2. Bugbot is set up for automated code reviews on this repo. Configure here.

Chunk 7. Adds src/pluginSetup.test.ts, 47 tests. No source changes.

This is the only code in the wizard that edits files in the user's home
directory, and it runs after the install has already succeeded, so the
rules under test are: never clobber a config it cannot parse, never
remove an entry it did not write, and never throw.

Everything is driven through offerPluginSetup against a temp HOME rather
than through the private writers, so the decision tree and the file I/O
are exercised together and no exports were needed.

Unparseable-means-untouched is asserted on the file's bytes, not just on
the return value: JSONC with comments, a non-object root, a non-object
mcpServers, and a Codex table with no url line all leave the file
exactly as found. removeOurs only deletes an entry pointing at one of
our two realm endpoints, so a user's own subtext server survives.

The EU branch gets its own section — the packaged plugin is NA-only, so
an EU org must never take that path.

Ctrl+C at these prompts skips the step without throwing, which is the
opposite of the same gesture in chunk 4, and guidePluginSetup (pre-
handoff, where the answer decides telemetry ownership) still aborts.
There are tests for both so the difference is deliberate on the record.

One note for review: removeSupersededRawEntry's `region === 'eu'` early
return is unreachable — it is only called from the packaged-plugin path,
which EU never takes. Harmless defensive code, left alone, but it is not
what keeps the EU entry safe.
@nrodd nrodd mentioned this pull request Sep 25, 2026
@nrodd
nrodd merged commit d1bb707 into tests/harness-and-ci Sep 28, 2026
5 checks passed
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.

1 participant