perf(completion): keep Tab completion off telemetry and heavy imports

.changeset/faster-tab-completion.md

@@ -0,0 +1,5 @@
+---
+"@fission-ai/openspec": patch
+---
+
+Shell Tab completion is faster. `openspec __complete`, which the completion scripts run on every Tab press, now loads 48 modules instead of 344 when completing changes or specs, and no longer records usage, so a press never waits on the telemetry request and can't use up the first-run telemetry notice where nobody sees it. Completion output is unchanged.

src/cli/index.ts

@@ -123,6 +123,14 @@ export function isCompletionRun(commandPath: string): boolean {
   return commandPath.split(':')[0] === 'completion' || commandPath === '__complete';
 }
 
+/**
+ * True for the hidden `__complete` resolver alone. It runs on every Tab press,
+ * so the hooks skip telemetry and the completion tip for it entirely.
+ */
+export function isTabCompletionRun(commandPath: string): boolean {
+  return commandPath === '__complete';
+}
+
 /**
  * True when the first-run completions tip must be deferred rather than shown.
  *
@@ -196,6 +204,13 @@ program.hook('preAction', async (thisCommand, actionCommand) => {
     process.env.NO_COLOR = '1';
   }
 
+  // `__complete` runs on every Tab press, with stderr sent to /dev/null by the
+  // generated scripts. Tracking it would hold each press open for the request,
+  // and the first-run notice would be marked seen without anyone reading it.
+  if (isTabCompletionRun(getCommandPath(actionCommand))) {
+    return;
+  }
+
   // Show first-run telemetry notice (if not seen). It's written to stderr, so it
   // never pollutes stdout — but --json runs still defer it (see isJsonRun) so the
   // very first invocation stays free of any incidental output on either stream.
@@ -210,6 +225,12 @@ program.hook('preAction', async (thisCommand, actionCommand) => {
 
 // Shutdown telemetry after command completes
 program.hook('postAction', async (_thisCommand, actionCommand) => {
+  // Nothing to flush (preAction tracked nothing), and the tip is always
+  // deferred for completion runs, so skip loading either module.
+  if (isTabCompletionRun(getCommandPath(actionCommand))) {
+    return;
+  }
+
   // Show the first-run shell-completions tip (on stderr, so piped stdout stays
   // clean). postAction, not preAction: the tip trails the command's own output
   // instead of pushing an error message or `init`'s setup summary down the

src/commands/completion.ts

@@ -1,4 +1,3 @@
-import ora from 'ora';
 import { CompletionFactory } from '../core/completions/factory.js';
 import { COMMAND_REGISTRY } from '../core/completions/command-registry.js';
 import { detectShell, SupportedShell } from '../utils/shell-detection.js';
@@ -125,6 +124,9 @@ export class CompletionCommand {
     const generator = CompletionFactory.createGenerator(shell);
     const installer = CompletionFactory.createInstaller(shell);
 
+    // Loaded here, not at the top: `__complete` shares this module and runs on
+    // every Tab press, so it must not pay for the spinner.
+    const { default: ora } = await import('ora');
     const spinner = ora(`Installing ${shell} completion script...`).start();
 
     try {
@@ -234,6 +236,7 @@ export class CompletionCommand {
       }
     }
 
+    const { default: ora } = await import('ora');
     const spinner = ora(`Uninstalling ${shell} completion script...`).start();
 
     try {

src/core/completions/completion-provider.ts

@@ -1,5 +1,4 @@
 import { getActiveChangeIds, getSpecIds } from '../../utils/item-discovery.js';
-import { listSchemas } from '../artifact-graph/index.js';
 
 /**
  * Cache entry for completion data
@@ -96,7 +95,10 @@ export class CompletionProvider {
       return this.schemaCache.data;
     }
 
-    // Fetch fresh data
+    // Fetch fresh data. The resolver (and the yaml and zod it parses schemas
+    // with) loads only here: completing changes or specs never needs it, and
+    // the artifact-graph barrel would also pull in fast-glob.
+    const { listSchemas } = await import('../artifact-graph/resolver.js');
     const schemaNames = listSchemas(this.projectRoot);
 
     // Update cache

test/cli-e2e/startup-modules.test.ts

@@ -1,6 +1,6 @@
 import { afterAll, describe, expect, it } from 'vitest';
 import { spawnSync } from 'node:child_process';
-import { mkdtempSync, readFileSync, realpathSync, rmSync } from 'node:fs';
+import { existsSync, mkdtempSync, readFileSync, realpathSync, rmSync } from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
 import { fileURLToPath, pathToFileURL } from 'node:url';
@@ -118,6 +118,101 @@ describe('CLI startup loads only what the command needs', () => {
     }
   );
 
+  // Shell completion scripts run `__complete` on every Tab press. It needs no
+  // package beyond commander, and never telemetry: tracking would hold each
+  // press open for the request.
+  it.each(['changes', 'specs'])(
+    '`openspec __complete %s` loads only commander and no telemetry',
+    async (type) => {
+      await ensureCliBuilt();
+      const { own, packages, status } = loadedModules(['__complete', type]);
+
+      expect(status).toBe(0);
+      expect([...packages]).toEqual(['commander']);
+      expect(implementationsLoaded(own)).toEqual(['completion']);
+      expect(own).not.toContain('telemetry/index.js');
+    }
+  );
+
+  it('`openspec __complete schemas` loads the schema reader but not fast-glob or ora', async () => {
+    await ensureCliBuilt();
+    const { own, packages, status } = loadedModules(['__complete', 'schemas']);
+
+    expect(status).toBe(0);
+    expect([...packages]).not.toContain('fast-glob');
+    expect([...packages]).not.toContain('ora');
+    expect(own).not.toContain('telemetry/index.js');
+    // The name promises a schema reader, so pin it: the resolver directly, not
+    // the artifact-graph barrel, which re-exports it but also pulls outputs.js
+    // and with it fast-glob.
+    expect(own).toContain('core/artifact-graph/resolver.js');
+    expect(own).not.toContain('core/artifact-graph/index.js');
+  });
+
+  // Negative control for the two cases above. `isTabCompletionRun` matches the
+  // command path exactly, unlike the adjacent `isCompletionRun`, which
+  // prefix-matches `completion:*`. Widening one into the other would silently
+  // stop tracking every user-facing command in that group, and every assertion
+  // above would still pass.
+  it.each([['list --json'], ['completion generate zsh']])(
+    '`openspec %s` still reaches the telemetry hooks',
+    async (invocation) => {
+      await ensureCliBuilt();
+      const { own } = loadedModules(invocation.split(' '));
+
+      expect(own).toContain('telemetry/index.js');
+      expect(own).toContain('core/completion-tip.js');
+    }
+  );
+
+  // The module-count assertions above pin that telemetry is not LOADED. They do
+  // not pin the user-visible consequence, which is the reason the skip exists:
+  // the first-run disclosure goes to stderr, every generated completion script
+  // sends `__complete` stderr to /dev/null, and `maybeShowTelemetryNotice` marks
+  // `noticeSeen` before `trackCommand` re-reads it. So before this change a
+  // user's first-ever interaction being a Tab press burned the disclosure where
+  // nobody could read it and began collecting on that same press.
+  //
+  // Telemetry is deliberately left ENABLED here: the point is that an enabled
+  // run still records nothing. HOME must be isolated as well as XDG_CONFIG_HOME,
+  // or `migrateLegacyTelemetryConfig` imports the developer's real config from
+  // ~/.config/openspec/config.json and the test passes vacuously.
+  it('`openspec __complete` leaves the first-run telemetry notice unconsumed', async () => {
+    await ensureCliBuilt();
+    const home = mkdtempSync(path.join(os.tmpdir(), 'openspec-notice-'));
+    workDirs.push(home);
+    const configHome = path.join(home, 'config');
+
+    // These have to be DELETED, not blanked. Both opt-outs read as "set to
+    // anything that is not an explicit on-value", so `OPENSPEC_TELEMETRY: ''`
+    // and `DO_NOT_TRACK: ''` would switch telemetry OFF and the assertions
+    // below would hold on any build, passing vacuously.
+    const env = {
+      ...process.env,
+      HOME: home,
+      USERPROFILE: home,
+      XDG_CONFIG_HOME: configHome,
+      XDG_DATA_HOME: path.join(home, 'data'),
+    };
+    delete env.OPENSPEC_TELEMETRY;
+    delete env.DO_NOT_TRACK;
+    delete env.CI;
+
+    const result = spawnSync(process.execPath, [binPath, '__complete', 'changes'], {
+      cwd: home,
+      env,
+      encoding: 'utf-8',
+      timeout: 30_000,
+      windowsHide: true,
+    });
+
+    expect(result.status).toBe(0);
+    // No disclosure written to a stream the completion scripts discard.
+    expect(result.stderr).toBe('');
+    // And no config file, so neither noticeSeen nor an anonymousId was minted.
+    expect(existsSync(path.join(configHome, 'openspec', 'config.json'))).toBe(false);
+  });
+
   it.each([
     ['config list --json', 'config'],
     ['config path', 'config'],

test/core/cli-is-json-run.test.ts

@@ -1,7 +1,12 @@
 import { describe, it, expect } from 'vitest';
 import { Command, Option } from 'commander';
 
-import { isJsonRun, isCompletionRun, shouldDeferCompletionTip } from '../../src/cli/index.js';
+import {
+  isJsonRun,
+  isCompletionRun,
+  isTabCompletionRun,
+  shouldDeferCompletionTip,
+} from '../../src/cli/index.js';
 
 /**
  * Reproduce the three ways `--json` reaches a command in the real CLI, so a
@@ -106,6 +111,23 @@ describe('isCompletionRun', () => {
   );
 });
 
+describe('isTabCompletionRun', () => {
+  /**
+   * Only the hidden resolver skips telemetry. The user-facing `completion`
+   * commands are run by a person and are tracked like any other command.
+   */
+  it('matches the hidden resolver', () => {
+    expect(isTabCompletionRun('__complete')).toBe(true);
+  });
+
+  it.each(['completion', 'completion:install', 'completion:generate', 'list'])(
+    'does not match "%s"',
+    (commandPath) => {
+      expect(isTabCompletionRun(commandPath)).toBe(false);
+    }
+  );
+});
+
 describe('shouldDeferCompletionTip', () => {
   /**
    * The tip must survive every run that cannot display it. Deferring (rather

test/core/completions/generators/zsh-generator.test.ts

@@ -421,5 +421,35 @@ describe('ZshGenerator', () => {
       expect(script).toContain('_openspec_view() {');
       expect(script).toContain('_arguments');
     });
+
+    // The bash, fish and powershell generator tests each pin this redirect;
+    // zsh was the one that did not, and it has the most `__complete` call
+    // sites and is the default shell on macOS. The redirect is what keeps a
+    // Tab press silent, so anything `__complete` ever writes to stderr - a
+    // deprecation notice, a Node experimental warning - would otherwise print
+    // into the middle of the user's prompt with no test objecting.
+    it('sends every __complete call stderr to /dev/null', () => {
+      const commands: CommandDefinition[] = [
+        { name: 'archive', description: 'Archive a change', flags: [] },
+        { name: 'show', description: 'Show an item', flags: [] },
+        { name: 'new', description: 'Create a change', flags: [] },
+      ];
+
+      const script = generator.generate(commands);
+
+      expect(script).toContain('openspec __complete changes 2>/dev/null');
+      expect(script).toContain('openspec __complete specs 2>/dev/null');
+      expect(script).toContain('openspec __complete schemas 2>/dev/null');
+      // No invocation may survive a template edit that drops a redirect. Only
+      // real call sites count: the script also explains itself in comments
+      // that name the command without running it.
+      const invocations = script
+        .split('\n')
+        .filter((line) => line.includes('openspec __complete') && !line.trim().startsWith('#'));
+      expect(invocations.length).toBeGreaterThan(0);
+      for (const line of invocations) {
+        expect(line, `unredirected __complete call: ${line.trim()}`).toContain('2>/dev/null');
+      }
+    });
   });
 });