From 9dff322d635fe4b65985f2bd6c7642094a3c6a0a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 14:55:15 +0000 Subject: [PATCH 1/2] fix(cli): doctor prescribes no codemod command that does not exist (#10680) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After `--scan-deprecations` reported its hits, `os doctor` printed ``Run `objectstack codemod v2-to-v3` to auto-fix``. No `codemod` command is registered: oclif resolves this CLI by globbing `dist/commands/**`, nothing under `src/commands/` compiles to `codemod`, and neither bundled plugin supplies one — the prescription exits 2, `command codemod:v2-to-v3 not found`, only after the operator has spent time on it. The docs already recorded the automated codemod as "not yet available"; the tool was the last place still claiming otherwise. The hint is deliberately NOT repointed at `os migrate meta`. That command's subject is an authored stack config replayed across protocol majors, and its own header declines the rewrite this hint promised — "does not silently rewrite TS config source (that AST rewrite is unsafe and lossy)" — with both writes guarded behind `--out` JSON snapshots. It cannot touch the `src/**` TypeScript these patterns are found in, and three of the eight DEPRECATED_PATTERNS are not metadata at all. `os lint --fix` is print-only by declaration. Nothing registered does the job, so the line says so and points at the per-finding replacement it already computes, printed under `--verbose`. The new pin closes the class rather than the instance: every `os …` / `objectstack …` hint in `doctor.ts` must resolve to a command file under `src/commands/`, and the whole rendered report is held to the same rule. The scanner is untouched — file:line attribution, `→ replacement` detail and the advisory exit 0 are pinned here so this edit cannot regress them. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r --- ...tor-deprecation-hint-no-phantom-codemod.md | 5 + .../doctor-deprecation-hint-commands.test.ts | 316 ++++++++++++++++++ packages/cli/src/commands/doctor.ts | 32 +- 3 files changed, 352 insertions(+), 1 deletion(-) create mode 100644 .changeset/doctor-deprecation-hint-no-phantom-codemod.md create mode 100644 packages/cli/src/commands/doctor-deprecation-hint-commands.test.ts diff --git a/.changeset/doctor-deprecation-hint-no-phantom-codemod.md b/.changeset/doctor-deprecation-hint-no-phantom-codemod.md new file mode 100644 index 0000000000..138af00aa4 --- /dev/null +++ b/.changeset/doctor-deprecation-hint-no-phantom-codemod.md @@ -0,0 +1,5 @@ +--- +'@objectstack/cli': patch +--- + +`os doctor --scan-deprecations` no longer prescribes a command that does not exist. After listing its hits the report used to print ``Run `objectstack codemod v2-to-v3` to auto-fix``, but no `codemod` command has ever been registered — following the advice returned oclif's exit 2, `command codemod:v2-to-v3 not found`, after the operator had already spent time on it. The hint is not repointed at `os migrate meta` either: that command replays the protocol migration chain over an authored stack config and declines the source rewrite by design ("does not silently rewrite TS config source"), writing only an `--out` JSON snapshot, so it cannot fix the `src/**` TypeScript the scan reports on. It now names the count and the remedy that really exists — the per-finding replacement, printed under `--verbose`. The scanner is unchanged: same file:line attribution, same `→ replacement` detail, still advisory with exit 0 either way. diff --git a/packages/cli/src/commands/doctor-deprecation-hint-commands.test.ts b/packages/cli/src/commands/doctor-deprecation-hint-commands.test.ts new file mode 100644 index 0000000000..439c0fa8d0 --- /dev/null +++ b/packages/cli/src/commands/doctor-deprecation-hint-commands.test.ts @@ -0,0 +1,316 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os doctor` never prescribes a command `os` does not register (#10680). + * + * ── The defect ─────────────────────────────────────────────────────────── + * + * After `--scan-deprecations` reported its hits, doctor printed: + * + * ℹ Found N deprecated pattern(s). Run `objectstack codemod v2-to-v3` to auto-fix. + * + * There is no `codemod` command. oclif resolves this CLI's commands by + * globbing `dist/commands/**` (`package.json` → `oclif.commands`, pattern + * strategy); nothing under `src/commands/` compiles to `codemod`, and neither + * bundled plugin (`@oclif/plugin-help`, `@oclif/plugin-plugins`) supplies one. + * An operator who followed the prescription got oclif's exit 2, + * `command codemod:v2-to-v3 not found` — after spending their time on it. + * `content/docs/protocol/backward-compatibility.mdx` already recorded the + * automated codemod as "not yet available"; the tool was the last place still + * claiming otherwise. + * + * This is the same class as the two defects fixed next door: `os doctor` + * printing `✓` over a tree it never walked (#10679) and `os datasource + * validate` passing over drift it never read. A tool that speaks with + * confidence about work it cannot do costs the reader their time before they + * can find out — and a WRONG prescription costs more than no prescription. + * + * ── Why the hint was removed rather than repointed ─────────────────────── + * + * The obvious repair — point it at `os migrate meta`, the registered metadata + * migrator — would have been the same defect respelled, so it is deliberately + * NOT what landed: + * + * · `os migrate meta`'s subject is an authored stack CONFIG replayed across + * protocol majors. Its own header declines the rewrite this hint promises: + * it "does not silently rewrite TS config source (that AST rewrite is + * unsafe and lossy)". Both of its writes are `--out` JSON snapshots, and + * `--from` is required, so a bare `os migrate meta` fails too. It cannot + * touch the `src/**` TypeScript the scan reports on, and three of the + * eight DEPRECATED_PATTERNS (`EnhancedObjectKernel` and the two + * deep-import paths) are not metadata at all. + * · `os lint --fix` is print-only by declaration: "Show what would be fixed + * (dry-run)". + * + * Nothing registered does the job, so the line says nothing about a codemod + * and points at the replacement it already computed for each finding instead. + * + * ── What this file pins ────────────────────────────────────────────────── + * + * The first describe closes the CLASS, not the instance: EVERY `os …` / + * `objectstack …` hint doctor can print must name a command that resolves + * under `src/commands/`. It fails on the shipped defect (that hint was the + * only unresolvable one of the six in the file) and on any future one. The + * rest drive the real command and pin both the corrected line and the scan + * clauses that were already correct and must stay that way. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import Doctor from './doctor.js'; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +/** `packages/cli/src/commands` — where oclif's pattern strategy finds commands. */ +const COMMANDS_DIR = HERE; +/** `packages/cli` — the oclif root the real command is loaded against below. */ +const CLI_ROOT = path.resolve(HERE, '..', '..'); +const DOCTOR_SRC = path.join(COMMANDS_DIR, 'doctor.ts'); + +/** + * chalk may or may not emit SGR codes depending on TTY detection. + * + * The escape is written as `\x1b`, never as the byte itself: one raw control + * character makes `grep` treat the whole file as binary, and a test file no + * `git grep` can find stops being maintained. + */ +const SGR = /\x1b\[[0-9;]*m/g; +const plain = (s: string) => s.replace(SGR, ''); + +/** + * Every backticked `os …` / `objectstack …` hint in a blob of text. + * + * Reads the raw source as well as rendered output, so the escaped backticks a + * template literal carries (`` \` ``) are tolerated on both ends. + */ +function commandHintsIn(text: string): string[] { + const hints: string[] = []; + const re = /\\?`(os|objectstack)\s+([^`\n]*?)\\?`/g; + for (const m of text.matchAll(re)) hints.push(`${m[1]} ${m[2]}`.trim()); + return hints; +} + +/** + * The command words of a hint: everything up to the first flag or placeholder. + * + * `os migrate meta --stored` addresses the command `migrate meta`; the flags + * are that command's business, not the registry's. + */ +function commandWords(hint: string): string[] { + const words = hint.split(/\s+/).slice(1); + const out: string[] = []; + for (const w of words) { + if (!/^[a-z][a-z0-9-]*$/.test(w)) break; + out.push(w); + } + return out; +} + +/** + * The file oclif would load for a hint, or `null` when nothing registers it. + * + * Longest prefix wins, mirroring oclif's topic resolution: `os migrate plan` + * is `migrate/plan.ts`, `os serve` is `serve.ts`. + */ +function resolveCommand(hint: string): string | null { + const words = commandWords(hint); + for (let k = words.length; k > 0; k--) { + const stem = path.join(COMMANDS_DIR, ...words.slice(0, k)); + for (const candidate of [`${stem}.ts`, path.join(stem, 'index.ts')]) { + if (fs.existsSync(candidate)) return candidate; + } + } + return null; +} + +function unregisteredHintsIn(text: string): string[] { + return [...new Set(commandHintsIn(text))].filter((h) => resolveCommand(h) === null); +} + +describe('resolveCommand — the registry probe these cases are judged with', () => { + // A probe that cannot fail proves nothing about the source it clears, so it + // is exercised on both verdicts before being trusted below. + it('resolves a real command, including a topic command', () => { + expect(resolveCommand('os serve')).toBe(path.join(COMMANDS_DIR, 'serve.ts')); + expect(resolveCommand('os migrate meta')).toBe(path.join(COMMANDS_DIR, 'migrate', 'meta.ts')); + }); + + it('refuses the exact prescription this issue is about', () => { + expect(resolveCommand('objectstack codemod v2-to-v3')).toBeNull(); + // …and is not merely rejecting the version suffix. + expect(resolveCommand('os codemod')).toBeNull(); + }); + + it('addresses the command, not its flags', () => { + expect(resolveCommand('os migrate meta --stored --apply')).toBe( + path.join(COMMANDS_DIR, 'migrate', 'meta.ts'), + ); + expect(resolveCommand('os doctor --scan-deprecations')).toBe(path.join(COMMANDS_DIR, 'doctor.ts')); + }); +}); + +describe('doctor.ts source — no phantom prescriptions anywhere in the file', () => { + const source = fs.readFileSync(DOCTOR_SRC, 'utf8'); + + it('names only commands `os` registers', () => { + // The class-closing assertion. Before the fix this listed exactly one + // entry, `objectstack codemod v2-to-v3`; it must stay empty. + expect(unregisteredHintsIn(source)).toEqual([]); + }); + + it('finds enough hints for the sweep above to mean something', () => { + // Guards the assertion against a regex that silently stops matching: an + // empty haystack would make the case above pass over anything. + expect(commandHintsIn(source).length).toBeGreaterThan(5); + }); + + it('has dropped the dead codemod prescription outright', () => { + expect(source).not.toContain('codemod v2-to-v3'); + // Not repointed at `os migrate meta` either: that command declines the TS + // rewrite this hint promised, so naming it here would restate the defect. + // (The reasoning lives in a comment at the print site; the ban is on the + // hint text, so the comment is stripped before this looks.) + const withoutComments = source.replace(/\/\/[^\n]*/g, '').replace(/\/\*[\s\S]*?\*\//g, ''); + expect(withoutComments).not.toContain('migrate meta'); + }); +}); + +/** + * `node_modules/` exists in every temp cwd on purpose — without it doctor's + * `Dependencies` check is itself an `error` and exits 1 on its own, which + * would make an assertion pass for a reason having nothing to do with this + * change. + */ +function makeTempCwd(tag: string): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), `os-doctor-10680-${tag}-`)); + fs.mkdirSync(path.join(dir, 'node_modules')); + fs.mkdirSync(path.join(dir, 'src')); + return dir; +} + +describe('os doctor --scan-deprecations, end to end', () => { + let tmp: string; + let cwdSpy: ReturnType; + + beforeEach(() => { + tmp = makeTempCwd('scan'); + cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(tmp); + }); + + afterEach(() => { + cwdSpy.mockRestore(); + fs.rmSync(tmp, { recursive: true, force: true }); + }); + + function writeDeprecatedSource(): void { + fs.writeFileSync( + path.join(tmp, 'src', 'legacy.ts'), + [ + "import { EnhancedObjectKernel } from '@objectstack/core/enhanced';", + 'export const field = { max_length: 40 };', + '', + ].join('\n'), + 'utf8', + ); + } + + async function runDoctor(argv: string[]): Promise<{ out: string; exitCode: number | undefined }> { + const logs: string[] = []; + const logSpy = vi.spyOn(console, 'log').mockImplementation((...a: unknown[]) => { + logs.push(a.join(' ')); + }); + let exitCode: number | undefined; + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => { + exitCode = code; + throw new Error(`__PROCESS_EXIT__:${code}`); + }) as never); + + try { + await Doctor.run(argv, { root: CLI_ROOT }); + } catch (err) { + if (!(err instanceof Error) || !err.message.startsWith('__PROCESS_EXIT__')) throw err; + } finally { + logSpy.mockRestore(); + exitSpy.mockRestore(); + } + return { out: plain(logs.join('\n')), exitCode }; + } + + // A real `Doctor.run()` takes seconds (it shells out to `git --version`, + // walks the workspace and reads the ledger), so every case carries an + // explicit timeout instead of racing vitest's 5s default. + const E2E_TIMEOUT = 60_000; + + it('prescribes no codemod, and nothing else `os` cannot run', async () => { + writeDeprecatedSource(); + const run = await runDoctor(['--scan-deprecations']); + + // ① THE assertion of this issue. + expect(run.out).not.toContain('codemod v2-to-v3'); + expect(run.out).not.toContain('to auto-fix'); + + // ② …and the whole report is held to the same rule, not just this line. + expect(unregisteredHintsIn(run.out)).toEqual([]); + }, E2E_TIMEOUT); + + it('still reports the count, and says plainly that no codemod exists', async () => { + writeDeprecatedSource(); + const run = await runDoctor(['--scan-deprecations']); + + // Removing a false prescription must not remove the finding's summary — + // silence about the count would trade one defect for a smaller one. + // 3, not 2: line 1 trips both `EnhancedObjectKernel` and the + // `@objectstack/core/enhanced` import pattern. Pinning the real number + // keeps this honest about what the scanner reports. + expect(run.out).toContain('Found 3 deprecated pattern(s).'); + expect(run.out).toContain('No automated codemod ships with the CLI'); + // The remedy it points at is one that exists: the per-finding replacement, + // reachable by a flag this command really declares. + expect(run.out).toContain('re-run with --verbose to print them'); + }, E2E_TIMEOUT); + + it('drops the re-run advice once --verbose is already on', async () => { + writeDeprecatedSource(); + const run = await runDoctor(['--scan-deprecations', '--verbose']); + + expect(run.out).toContain('No automated codemod ships with the CLI'); + expect(run.out).not.toContain('re-run with --verbose'); + // The thing it now points at is on screen: the → replacement per finding. + expect(run.out).toContain('Use maxLength (camelCase)'); + expect(run.out).toContain("Use import from '@objectstack/core'"); + }, E2E_TIMEOUT); + + it('keeps file:line attribution for every hit', async () => { + // A passing clause of the QA item — pinned here so this edit cannot be the + // thing that regresses it. + writeDeprecatedSource(); + const run = await runDoctor(['--scan-deprecations']); + + expect(run.out).toContain('legacy.ts:1'); + expect(run.out).toContain('legacy.ts:2'); + expect(run.out).toContain('snake_case config key: max_length'); + }, E2E_TIMEOUT); + + it('still warns rather than gates — hits do not change the exit code', async () => { + // The other passing clause: the scan is advisory. Measured as a DELTA + // against the same cwd with a clean `src/`, so an unrelated finding in the + // temp cwd cannot make this pass or fail for the wrong reason. + const clean = await runDoctor(['--scan-deprecations']); + writeDeprecatedSource(); + const dirty = await runDoctor(['--scan-deprecations']); + + expect(clean.out).toContain('No deprecated patterns found'); + expect(dirty.exitCode).toBe(clean.exitCode); + expect(dirty.exitCode).not.toBe(1); + }, E2E_TIMEOUT); + + it('says nothing about codemods when the tree is clean', async () => { + const run = await runDoctor(['--scan-deprecations']); + + expect(run.out).toContain('No deprecated patterns found'); + expect(run.out).not.toContain('No automated codemod ships with the CLI'); + }, E2E_TIMEOUT); +}); diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index f16540ff48..7026b08032 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -2255,7 +2255,37 @@ export default class Doctor extends Command { } } console.log(''); - printInfo(`Found ${deprecations.length} deprecated pattern(s). Run \`objectstack codemod v2-to-v3\` to auto-fix.`); + // #10680 — this line used to prescribe `objectstack codemod v2-to-v3`, + // a command `os` has never registered. oclif resolves commands by + // globbing `dist/commands/**/*.js` (package.json `oclif.commands`); + // there is no `src/commands/codemod*`, and neither bundled plugin + // (`@oclif/plugin-help`, `@oclif/plugin-plugins`) supplies one — so the + // prescription exited 2, `command codemod:v2-to-v3 not found`, for every + // operator who followed it. `content/docs/protocol/backward-compatibility.mdx` + // already records the automated codemod as "not yet available". + // + // Re-pointing it at `os migrate meta` — the one registered command in + // this neighborhood — would be the same defect respelled. That command's + // subject is an authored stack CONFIG replayed across protocol majors, + // and its own header states it "does not silently rewrite TS config + // source (that AST rewrite is unsafe and lossy)": both of its writes are + // `--out` JSON snapshots. It cannot touch the `src/**` TypeScript these + // patterns are found in, and three of the eight (`EnhancedObjectKernel` + // and the two deep-import paths) are not metadata at all. `os lint --fix` + // is likewise print-only — "Show what would be fixed (dry-run)". + // + // So nothing registered auto-fixes these, and the line now says so. The + // replacements are real and already computed per finding; the loop above + // prints them under `--verbose`. A hint that prescribes nothing beats one + // that sends the operator to a command they will spend their time on + // before discovering it was never built — the same false-confidence class + // as a `✓` printed over a tree that was never walked. + printInfo( + `Found ${deprecations.length} deprecated pattern(s). No automated codemod ships with the CLI — ` + + (flags.verbose + ? 'apply the → replacement shown under each finding above.' + : 'apply each finding’s replacement by hand (re-run with --verbose to print them).'), + ); } else { printSuccess('Deprecation scan No deprecated patterns found'); } From af5c56435295b8ce9141ef0f6b281a81fe7bbf12 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 15:07:35 +0000 Subject: [PATCH 2/2] test(cli): scope the hint sweep to what doctor can print MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The class-closing sweep read the raw file, so the print site's own comment — which quotes the dead prescription verbatim, on purpose, as the record of why the line reads as it does — tripped it. The rule is about the report an operator sees, so the sweep now reads printable source: whole comment lines dropped, never a trailing `//`, because `http://localhost:3000` lives inside these strings and a greedier strip would delete real hints and leave the sweep passing over less than it claims. The haystack size is asserted for the same reason. The ban is on the prescription, not the word: the corrected line has to be able to say there is no codemod. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r --- .../doctor-deprecation-hint-commands.test.ts | 44 ++++++++++++++----- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/commands/doctor-deprecation-hint-commands.test.ts b/packages/cli/src/commands/doctor-deprecation-hint-commands.test.ts index 439c0fa8d0..5222890032 100644 --- a/packages/cli/src/commands/doctor-deprecation-hint-commands.test.ts +++ b/packages/cli/src/commands/doctor-deprecation-hint-commands.test.ts @@ -130,6 +130,24 @@ function unregisteredHintsIn(text: string): string[] { return [...new Set(commandHintsIn(text))].filter((h) => resolveCommand(h) === null); } +/** + * The source with its comment lines removed — what doctor can actually PRINT. + * + * The rule this file enforces is about the report an operator reads, so a + * command named in a comment (the print site documents the dead prescription + * it replaced, verbatim) is prose, not a prescription. Only whole comment + * lines are dropped, never a trailing `//`: `http://localhost:3000` lives + * inside these strings, and a regex greedy enough to strip after it would + * silently delete real hints and leave the sweep below passing over less than + * it claims. + */ +function printableSource(src: string): string { + return src + .split('\n') + .filter((line) => !/^\s*(\/\/|\/\*|\*)/.test(line)) + .join('\n'); +} + describe('resolveCommand — the registry probe these cases are judged with', () => { // A probe that cannot fail proves nothing about the source it clears, so it // is exercised on both verdicts before being trusted below. @@ -153,28 +171,32 @@ describe('resolveCommand — the registry probe these cases are judged with', () }); describe('doctor.ts source — no phantom prescriptions anywhere in the file', () => { - const source = fs.readFileSync(DOCTOR_SRC, 'utf8'); + const printable = printableSource(fs.readFileSync(DOCTOR_SRC, 'utf8')); it('names only commands `os` registers', () => { // The class-closing assertion. Before the fix this listed exactly one // entry, `objectstack codemod v2-to-v3`; it must stay empty. - expect(unregisteredHintsIn(source)).toEqual([]); + expect(unregisteredHintsIn(printable)).toEqual([]); }); it('finds enough hints for the sweep above to mean something', () => { - // Guards the assertion against a regex that silently stops matching: an - // empty haystack would make the case above pass over anything. - expect(commandHintsIn(source).length).toBeGreaterThan(5); + // Guards the assertion against a haystack that quietly emptied — a regex + // that stops matching, or a comment filter that ate the code with the + // comments. Either would make the case above pass over nothing at all. + expect(commandHintsIn(printable).length).toBeGreaterThan(10); }); it('has dropped the dead codemod prescription outright', () => { - expect(source).not.toContain('codemod v2-to-v3'); + // The print site still documents the dead command in a comment, on + // purpose — that is the record of why this line reads as it does. What + // must not survive is the prescription reaching an operator's screen. + // (The word itself does survive, in the sentence that reports there is no + // codemod; banning the word would ban saying so.) + expect(printable).not.toContain('codemod v2-to-v3'); + expect(printable).not.toContain('os codemod'); // Not repointed at `os migrate meta` either: that command declines the TS - // rewrite this hint promised, so naming it here would restate the defect. - // (The reasoning lives in a comment at the print site; the ban is on the - // hint text, so the comment is stripped before this looks.) - const withoutComments = source.replace(/\/\/[^\n]*/g, '').replace(/\/\*[\s\S]*?\*\//g, ''); - expect(withoutComments).not.toContain('migrate meta'); + // rewrite this hint promised, so prescribing it would restate the defect. + expect(printable).not.toContain('migrate meta'); }); });