Skip to content

refactor(mcp): project doctor family output schema from its owning module - #3040

Merged
thymikee merged 2 commits into
refactor/mcp-output-schemas-2819-management-prepare-schemafrom
refactor/mcp-output-schemas-2819-management-doctor-schema
Sep 29, 2026
Merged

thymikee merged 2 commits into
refactor/mcp-output-schemas-2819-management-prepare-schemafrom
refactor/mcp-output-schemas-2819-management-doctor-schema

Conversation

@thymikee

@thymikee thymikee commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

Summary

Projects the MCP output schema for the doctor command family out of the central
command-output-schemas.ts registry and into doctor.ts, its owning module, following the
same pattern applied to the other command families in this stacked series. No behavior change:
the published MCP outputSchema for doctor is unchanged, only its declaration site moves.

Part of #2819. Touches 4 files. Stacked on
refactor/mcp-output-schemas-2819-management-prepare-schema.

Validation

Tested commit: ad91cc368ec88b8d7cc05767cd20cd2ad51f3cfe

  • pnpm check:affected --run: 165 test files, 1371 tests passed, 0 failed.
  • No device needed (pure MCP schema move). Verified the published outputSchema for doctor
    is byte-identical before/after: built a temporary detached worktree of origin/main
    (with node_modules symlinked from this branch to resolve workspace packages), then ran
    node -e "import('.../src/mcp/command-output-schemas.ts').then(m => console.log(JSON.stringify(m.COMMAND_OUTPUT_SCHEMAS.doctor)))"
    on both origin/main and this branch's HEAD. diff reported no difference, and a
    structural-equality check on the parsed JSON also returned true. The verification worktree
    was removed afterward.

No unresolved risk: this is a pure move with no schema-shape or Swift changes.

Review in cubic

@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.88 MB 4.88 MB +18 B
Package (unpacked) 4.88 MB 4.88 MB +18 B
Package (download) 1.46 MB 1.46 MB +20 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 29.0 ms 27.9 ms -1.1 ms
CLI --help 85.4 ms 86.6 ms +1.2 ms

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 4 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread src/commands/management/doctor.ts
@thymikee
thymikee added this pull request to stack #3043 September 28, 2026 13:44
@thymikee

Copy link
Copy Markdown
Member Author

Reviewed at ad91cc3. This looks good: the doctor output schema moves to its owning module with the same text. The one new edge, doctor.ts to kernel/device, was already in the CLI import closure.

The Coverage failure is the same eager-closure budget check as on the rest of this stack, through src/commands/system/index.ts and src/commands/recording/index.ts, which this diff does not touch. Smoke Tests was still running at review time.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 28, 2026
…dule

Moves the single hand-authored MCP outputSchema entry for doctor out of
command-output-schemas.ts into a frozen DOCTOR_COMMAND_OUTPUT_SCHEMAS map
exported from src/commands/management/doctor.ts, following the #2810
projection seam already used by the other projected families on this stack.
Registers the family in the PROJECTED_FAMILIES disjointness check and adds
command-tools-management-doctor-schemas.test.ts with a reference-equality
proof against the module object and an owner-file check, matching the
pattern from the other projected families.

The moved code is byte-identical to what command-output-schemas.ts held:
same objectSchema/enumSchema/stringSchema/numberSchema/looseObjectSchema
calls and field order. Verified the published outputSchema for doctor is
byte-identical to origin/main by dumping COMMAND_OUTPUT_SCHEMAS.doctor as
JSON in a temporary detached worktree of origin/main and diffing against
this branch's head; both dumps matched byte-for-byte.

Part of #2819.
@thymikee

Copy link
Copy Markdown
Member Author

The code at ad91cc3 is unchanged since the last review, and that review stays clean. The branch now conflicts with its base, refactor/mcp-output-schemas-2819-management-prepare-schema, after the prepare PR was rebased. Please rebase this PR onto the updated base. I removed ready-for-human until the conflict is resolved.

@thymikee thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 29, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Rebased onto the updated prepare-schema base; new head is 8cb0ff5.

  • Duplicate import in doctor.ts (cubic): merged into one named import from command-input.ts, so the namespace import is gone. Fixed in 8cb0ff5.
  • Duplicate helpers: none in this link. It uses the shared constSchema from command-input.ts; grep of src/ and packages/ shows a single definition.
  • Coverage eager-closure budget: same failure as the rest of the stack, through system/index.ts and recording/index.ts, which this diff does not touch.
  • Checks: check:affected passes locally on this head. Smoke Tests, if it fails, is the known iOS fixture flake and not related to this diff.

@thymikee
thymikee force-pushed the refactor/mcp-output-schemas-2819-management-doctor-schema branch from ad91cc3 to 8cb0ff5 Compare September 29, 2026 06:30
@thymikee

Copy link
Copy Markdown
Member Author

Thanks for the update. The conflict from the earlier review (ad91cc3) is fixed, and I found no problems in 8cb0ff5. The doctor family output schema now comes from its owning module, and the checks are green. I did not run the tests locally and relied on the passing checks. I also did not check whether the remaining schema imports in command-output-schemas.ts are all still used, but lint would catch an unused import. Nothing else is needed before merge.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 29, 2026
@thymikee
thymikee merged commit 7c235bf into main Sep 29, 2026
19 checks passed
@thymikee
thymikee deleted the refactor/mcp-output-schemas-2819-management-doctor-schema branch September 29, 2026 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant