Merge pull request #708 from campfirein/feat/add-migration-hint

feat: add VC-sync hint after successful migrate

Author: RyanNg1403

src/oclif/commands/migrate.ts

@@ -60,6 +60,49 @@ public static flags = {
     }),
   }
 
+  // Visibility: `protected` so unit tests can exercise the gate
+  // combinations without spinning up the daemon transport.
+  protected displayForwardResult(report: MigrateRunReport, format: string, dryRun: boolean): void {
+    if (format === 'json') {
+      this.log(JSON.stringify(report, null, 2))
+      return
+    }
+
+    this.log(summarizeReportLine(report))
+    if (report.summary.failed > 0) {
+      this.warn(
+        `${report.summary.failed} file(s) failed — sources moved to the archive at ${report.archiveRoot ?? '(none)'}`,
+      )
+      for (const f of report.files) {
+        if (f.outcome === 'failed') {
+          this.warn(`  - ${f.sourceRelPath}: ${f.reason ?? '(no reason)'}`)
+        }
+      }
+    }
+
+    // VC-sync hint: text mode only (JSON consumers parse the envelope),
+    // skip when nothing actually changed on disk (dry-run, or `migrated=0`
+    // which means no .md topics were eligible). Also skip on partial
+    // failure (`failed > 0`) — the run exits 2 below, so "successfully
+    // migrated" wording would contradict the exit code and might nudge
+    // users to push a tree the command itself reports as failed.
+    if (!dryRun && report.summary.migrated > 0 && report.summary.failed === 0) {
+      this.logVcSyncHint()
+    }
+  }
+
+  // Stderr (not stdout) so the hint stays out of pipes like
+  // `brv migrate | grep migrated`; logToStderr (not `this.warn`)
+  // because it's a tip, not a warning.
+  protected logVcSyncHint(): void {
+    this.logToStderr('')
+    this.logToStderr('Tip: the context tree was successfully migrated. Sync the new HTML topics to ByteRover cloud:')
+    this.logToStderr('  brv vc status')
+    this.logToStderr('  brv vc add . && brv vc commit -m "Migrate context tree to HTML"')
+    this.logToStderr('  brv vc push')
+    this.logToStderr('(Run `brv vc remote add origin <url>` first if no remote is configured.)')
+  }
+
   public async run(): Promise<void> {
     const {flags} = await this.parse(Migrate)
     // Project resolution mirrors `status` / `vc`:
@@ -163,29 +206,14 @@ public static flags = {
       this.emitError(format, formatConnectionError(error))
     }
 
-    const {report} = response
-    if (format === 'json') {
-      this.log(JSON.stringify(report, null, 2))
-    } else {
-      this.log(summarizeReportLine(report))
-      if (report.summary.failed > 0) {
-        this.warn(
-          `${report.summary.failed} file(s) failed — sources moved to the archive at ${report.archiveRoot ?? '(none)'}`,
-        )
-        for (const f of report.files) {
-          if (f.outcome === 'failed') {
-            this.warn(`  - ${f.sourceRelPath}: ${f.reason ?? '(no reason)'}`)
-          }
-        }
-      }
-    }
+    this.displayForwardResult(response.report, format, dryRun)
 
     // Force termination with exit 2 on failure. The daemon-client tail
     // (Socket.IO reconnect timers) keeps the event loop alive, so
     // letting Node exit naturally with `process.exitCode = 2` doesn't
     // work — the loop never drains. Match the Python oracle's
     // behavior (lines 2021-2031): hard-exit with the failure code.
-    if (report.summary.failed > 0) {
+    if (response.report.summary.failed > 0) {
       // eslint-disable-next-line n/no-process-exit, unicorn/no-process-exit
       process.exit(2)
     }

test/commands/migrate.test.ts

@@ -0,0 +1,118 @@
+import type {Config} from '@oclif/core'
+
+import {Config as OclifConfig} from '@oclif/core'
+import {expect} from 'chai'
+import {restore, stub} from 'sinon'
+
+import type {MigrateRunReport} from '../../src/shared/transport/events/migrate-events.js'
+
+import Migrate from '../../src/oclif/commands/migrate.js'
+
+// Tests the gate logic in displayForwardResult that controls when the
+// VC-sync hint fires. Daemon transport is bypassed by calling the
+// `protected` display method directly with a synthetic report — the
+// gate decisions are pure functions of (format, dryRun, summary) and
+// don't need a live daemon round-trip.
+
+class TestableMigrate extends Migrate {
+  public exerciseDisplay(report: MigrateRunReport, format: string, dryRun: boolean): void {
+    return this.displayForwardResult(report, format, dryRun)
+  }
+}
+
+function makeReport(overrides: {failed?: number; migrated?: number;} = {}): MigrateRunReport {
+  return {
+    archiveRoot: '/tmp/archive',
+    completedAt: '2026-05-26T00:00:00.000Z',
+    dryRun: false,
+    files: [],
+    projectRoot: '/tmp/proj',
+    startedAt: '2026-05-26T00:00:00.000Z',
+    summary: {
+      archived: 0,
+      failed: overrides.failed ?? 0,
+      migrated: overrides.migrated ?? 0,
+      skipped: 0,
+    },
+  }
+}
+
+describe('brv migrate — VC-sync hint gate', () => {
+  let config: Config
+  let stdout: string[]
+  let stderr: string[]
+  let warnings: string[]
+
+  before(async () => {
+    config = await OclifConfig.load(import.meta.url)
+  })
+
+  beforeEach(() => {
+    stdout = []
+    stderr = []
+    warnings = []
+  })
+
+  afterEach(() => {
+    restore()
+  })
+
+  function buildCommand(): TestableMigrate {
+    const cmd = new TestableMigrate([], config)
+    stub(cmd, 'log').callsFake((msg?: string) => {
+      if (msg !== undefined) stdout.push(msg)
+    })
+    stub(cmd, 'logToStderr').callsFake((msg?: string) => {
+      if (msg !== undefined) stderr.push(msg)
+    })
+    stub(cmd, 'warn').callsFake((msg: Error | string) => {
+      warnings.push(typeof msg === 'string' ? msg : msg.message)
+      return msg
+    })
+    return cmd
+  }
+
+  it('text + real + migrated>0 + failed=0: prints the hint on stderr', () => {
+    const cmd = buildCommand()
+    cmd.exerciseDisplay(makeReport({failed: 0, migrated: 5}), 'text', false)
+
+    const stderrJoined = stderr.join('\n')
+    expect(stderrJoined).to.include('Tip: the context tree was successfully migrated')
+    expect(stderrJoined).to.include('brv vc status')
+    expect(stderrJoined).to.include('brv vc add')
+    expect(stderrJoined).to.include('brv vc push')
+    expect(stderrJoined).to.include('brv vc remote add origin')
+  })
+
+  it('text + dry-run + migrated>0: does NOT print the hint', () => {
+    const cmd = buildCommand()
+    cmd.exerciseDisplay(makeReport({failed: 0, migrated: 5}), 'text', true)
+
+    expect(stderr.join('\n')).to.not.include('Tip:')
+  })
+
+  it('text + real + migrated=0: does NOT print the hint', () => {
+    const cmd = buildCommand()
+    cmd.exerciseDisplay(makeReport({failed: 0, migrated: 0}), 'text', false)
+
+    expect(stderr.join('\n')).to.not.include('Tip:')
+  })
+
+  it('text + real + migrated>0 + failed>0: does NOT print the hint (exit-code contradiction guard)', () => {
+    const cmd = buildCommand()
+    cmd.exerciseDisplay(makeReport({failed: 2, migrated: 5}), 'text', false)
+
+    // Warnings about failures still fire — only the success hint is suppressed.
+    expect(warnings.join('\n')).to.include('2 file(s) failed')
+    expect(stderr.join('\n')).to.not.include('Tip:')
+  })
+
+  it('json + real + migrated>0: emits the JSON envelope on stdout, nothing on stderr', () => {
+    const cmd = buildCommand()
+    cmd.exerciseDisplay(makeReport({failed: 0, migrated: 5}), 'json', false)
+
+    expect(stdout).to.have.lengthOf(1)
+    expect(() => JSON.parse(stdout[0])).to.not.throw()
+    expect(stderr).to.have.lengthOf(0)
+  })
+})