Skip to content

Discover project profiles instead of enumerating client brands - #11

Merged
zhoupengjie merged 1 commit into
mainfrom
fix/client-neutral-profile-discovery
Aug 30, 2026
Merged

zhoupengjie merged 1 commit into
mainfrom
fix/client-neutral-profile-discovery

Conversation

@zhoupengjie

Copy link
Copy Markdown
Collaborator

Why CI was red

Merge 29d4382 combined a branch that deleted .codex/config.toml with a main
that had independently grown a host-profile model still referencing it. Doctor
kept reading a file that no longer existed:

not ok 16 - doctor validates both project-scoped host profiles on request
  + 'warning'   (actual)
  - 'ok'        (expected)
# tests 247 / pass 182 / fail 1

The file was gone, but every reference to it came back from main —
README.md:261, README_CN.md:239, docs/hosts/codex.md:10,
docs/quickstart.md:69, docs/quickstart_cn.md:67, and scripts/doctor.mjs:433.

The deeper cause

client-neutral had no testable definition. Two contributors implemented it
27 minutes apart on the same day, in incompatible ways:

  • ede9e89 (18:01) — strict: ship no client config at all
  • 588c03f (18:28) — permissive: ship project profiles, add --host codex|claude

Both wrote their reading into AGENTS.md as prose. Prose does not fail in CI, so
the merge silently kept the deletion from one and the references from the other.

The rule this PR introduces

A client brand may appear as data — a discovered profile's file name, prose in
docs/ telling a user how to register the server. It must never appear as
control flow.

// before — a new client means editing this file
if (selectedHost === "codex") { ... }
supportedHosts = new Set(["core", "codex", "claude", "all"])

// after — a new client means adding a config file
for (const profile of await discoverProjectProfiles()) {
  record(`project-profile:${profile.id}`, profileLaunchesCanonicalEntry(profile) ...)
}

That judgement also settles the original dispute: shipping .codex/config.toml was
never the violation; the --host codex branch in Doctor was. The strict branch
deleted the innocent file and left the real coupling in place — and dropped
.codex/config.toml while keeping .mcp.json, an asymmetry with no principle
behind it. Discovery removes it: both are just profiles.

Changes

  • scripts/doctor.mjs — discovers .mcp.json and any <dot-dir>/config.toml
    or <dot-dir>/mcp.json, validates only the launch contract
    (synthv-agent-bridge started as node dist/src/cli.js).
    --host core|codex|claude|all → --host core|profiles|all.
  • tests/client-neutrality.test.ts (new) — fails if a brand token appears in
    src/, synthv/, scripts/, or .github/.
  • .codex/config.toml — restored; still referenced by both READMEs, both
    quickstarts, and docs/hosts/codex.md.
  • AGENTS.md — the prose replaced with the rule above.
  • Docs — --host codex|claude|all → --host profiles.

Verification

Onboarding a new client with zero code changes, and catching a bad one:

ok       project-profile:.someclient/config.toml     # auto-discovered
warning  project-profile:.brokenclient/config.toml   # wrong entry point
ok       project-profile:.codex/config.toml
ok       project-profile:.mcp.json

The guard test goes red when coupling is introduced (verified by injecting
if (process.env.HOST === "codex") into scripts/clean.mjs):

✖ the Runtime and its tooling contain no MCP client brand coupling
  scripts/clean.mjs:6: if (process.env.HOST === "codex") { ... }

Full suite:

npm run check                    → 249 tests, 249 pass, 0 fail   (was 247 / 1 fail)
node --check scripts/*.mjs       → OK
luac -p synthv/*.lua (3 files)   → OK
doctor default (core)            → ok: true, profile checks: 0
doctor --host profiles           → 2 profiles, all OK

One deliberate exception

tests/client-neutrality.test.ts itself contains nine brand names — the denylist
it matches against. A test that checks for brands has to know their names. It is
data, not control flow, and lives outside the four enforced directories.

cursor is deliberately not on that list: it collides with pageCursor,
cursorToken, and the parsing cursors throughout score-import.ts, where the
false-positive rate would drown the signal. Noted in the test.

Known gap

The negative path (a misconfigured profile → warning) is verified by hand above
but has no regression test; that needs a profile-root override so a test can point
at a fixture. Not included here to keep the change scoped.

CI broke because merge 29d4382 combined a branch that deleted
.codex/config.toml with a main that had grown a host-profile model still
referencing it. Doctor read a file that no longer existed and reported a
warning, failing the profile test.

The deeper cause was that "client-neutral" had no testable definition, so
two contributors implemented incompatible versions 27 minutes apart and
the merge silently kept the wrong half of each.

Define the boundary as: a client brand may appear as data (a discovered
profile's file name, prose in docs) but never as control flow. Doctor now
discovers .mcp.json and any dot-directory config.toml or mcp.json, then
checks only the launch contract, so onboarding a client needs no code
change. --host codex/claude collapse into --host profiles.

Add tests/client-neutrality.test.ts to enforce zero brand tokens in src/,
synthv/, scripts/, and .github/, and restore .codex/config.toml, which the
READMEs, quickstarts, and host docs still reference.

Skip locally modified profiles in the doctor test. Emptying these files to
stop a stale build from auto-launching once made this test look like a real
regression (docs/issue-8-fix-and-verification-2026-08-11.zh-CN.md); CI
checks out clean, so it stays strict there.
@zhoupengjie
zhoupengjie merged commit 7ab093f into main Aug 30, 2026
3 checks passed
@zhoupengjie
zhoupengjie deleted the fix/client-neutral-profile-discovery branch August 30, 2026 07:29
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