refactor: address PR review comments (BYK + BugBot round 7)

Code changes:
- bin.ts: Static import for disableDbTracing (db/index.ts is lightweight
  now that telemetry import is lazy)
- introspect.ts: Widen isRouteMap/isCommand to accept `unknown` so
  completions.ts can reuse them
- completions.ts: Remove duplicate type guards (use introspect.ts);
  extractFlagEntries uses for..of; extractSubcommands uses filter.map;
  shellVarName uses whitelist (/[^a-zA-Z0-9_]/); bash/zsh standalone
  command completion at position 2; bash __varname uses same whitelist
- complete.ts: Merge matchOrgSlugs into completeOrgSlugs with suffix
  param (no identity fn); eliminate [...nameMap.keys()] intermediate;
  export command sets for testing
- completions.property.test.ts: Add drift-detection tests verifying
  hardcoded ORG_PROJECT_COMMANDS / ORG_ONLY_COMMANDS stay in sync with
  the actual route tree

Author: BYK

src/bin.ts

@@ -6,6 +6,8 @@
  * the full CLI with telemetry, middleware, and error recovery.
  */
 
+import { disableDbTracing } from "./lib/db/index.js";
+
 // Exit cleanly when downstream pipe consumer closes (e.g., `sentry issue list | head`).
 // EPIPE (errno -32) is normal Unix behavior — not an error. Node.js/Bun ignore SIGPIPE
 // at the process level, so pipe write failures surface as async 'error' events on the
@@ -28,7 +30,6 @@ process.stderr.on("error", handleStreamError);
  * completion engine and SQLite cache modules.
  */
 async function runCompletion(completionArgs: string[]): Promise<void> {
-  const { disableDbTracing } = await import("./lib/db/index.js");
   disableDbTracing();
   const { handleComplete } = await import("./lib/complete.js");
   await handleComplete(completionArgs);

src/lib/complete.ts

@@ -63,8 +63,16 @@ export async function handleComplete(args: string[]): Promise<void> {
   }
 }
 
-/** Commands that accept org/project positional args. */
-const ORG_PROJECT_COMMANDS = new Set([
+/**
+ * Commands that accept org/project positional args.
+ *
+ * Hardcoded for fast-path performance — cannot derive from the route map
+ * at runtime because `app.ts` imports `@sentry/bun`. A property test in
+ * `completions.property.test.ts` verifies this set stays in sync.
+ *
+ * @internal Exported for testing only.
+ */
+export const ORG_PROJECT_COMMANDS = new Set([
   "issue list",
   "issue view",
   "issue explain",
@@ -84,8 +92,12 @@ const ORG_PROJECT_COMMANDS = new Set([
   "dashboard list",
 ]);
 
-/** Commands that accept only an org slug (no project). */
-const ORG_ONLY_COMMANDS = new Set([
+/**
+ * Commands that accept only an org slug (no project).
+ *
+ * @internal Exported for testing only.
+ */
+export const ORG_ONLY_COMMANDS = new Set([
   "org view",
   "team list",
   "repo list",
@@ -125,18 +137,18 @@ export async function getCompletions(
 }
 
 /**
- * Fuzzy-match cached org slugs and build completions.
+ * Complete organization slugs with fuzzy matching.
  *
- * Shared by both `completeOrgSlugs` (bare slugs) and the slash variant
- * (slugs with trailing `/`). Avoids duplicating the cache query, fuzzy
- * match, and name lookup logic.
+ * Queries the org_regions cache for all known org slugs and matches
+ * them against the partial input.
  *
  * @param partial - Partial org slug to match
- * @param formatValue - Transform a matched slug into the completion value
+ * @param suffix - Appended to each slug (e.g., "/" for org/project mode)
+ * @returns Completions with org names as descriptions
  */
-async function matchOrgSlugs(
+export async function completeOrgSlugs(
   partial: string,
-  formatValue: (slug: string) => string
+  suffix = ""
 ): Promise<Completion[]> {
   const orgs = await getCachedOrganizations();
   if (orgs.length === 0) {
@@ -147,26 +159,12 @@ async function matchOrgSlugs(
   const matched = fuzzyMatch(partial, slugs);
 
   const nameMap = new Map(orgs.map((o) => [o.slug, o.name]));
-
   return matched.map((slug) => ({
-    value: formatValue(slug),
+    value: `${slug}${suffix}`,
     description: nameMap.get(slug),
   }));
 }
 
-/**
- * Complete organization slugs with fuzzy matching.
- *
- * Queries the org_regions cache for all known org slugs and matches
- * them against the partial input.
- *
- * @param partial - Partial org slug to match
- * @returns Completions with org names as descriptions
- */
-export function completeOrgSlugs(partial: string): Promise<Completion[]> {
-  return matchOrgSlugs(partial, (slug) => slug);
-}
-
 /**
  * Complete the `org/project` positional pattern with fuzzy matching.
  *
@@ -218,7 +216,7 @@ export async function completeOrgSlashProject(
  * `sentry/` so they can continue typing the project name.
  */
 function completeOrgSlugsWithSlash(partial: string): Promise<Completion[]> {
-  return matchOrgSlugs(partial, (slug) => `${slug}/`);
+  return completeOrgSlugs(partial, "/");
 }
 
 /**

src/lib/completions.ts

@@ -15,7 +15,7 @@
 import { existsSync, mkdirSync } from "node:fs";
 import { dirname, join } from "node:path";
 import { routes } from "../app.js";
-import type { FlagDef } from "./introspect.js";
+import { type FlagDef, isCommand, isRouteMap } from "./introspect.js";
 import type { ShellType } from "./shell.js";
 
 /** Where completions are installed */
@@ -26,43 +26,6 @@ export type CompletionLocation = {
   created: boolean;
 };
 
-/**
- * Check if a routing target is a route map (has subcommands).
- * Works with untyped Stricli route targets (avoids generic type constraints).
- */
-function isRouteMap(target: unknown): target is {
-  getAllEntries: () => readonly {
-    name: { original: string };
-    target: unknown;
-    hidden: boolean;
-  }[];
-  brief: string;
-} {
-  return (
-    typeof target === "object" &&
-    target !== null &&
-    typeof (target as Record<string, unknown>).getAllEntries === "function"
-  );
-}
-
-/**
- * Check if a routing target is a command (has parameters with flags).
- * Works with untyped Stricli route targets.
- */
-function isCommandTarget(target: unknown): target is {
-  brief: string;
-  parameters: {
-    flags?: Record<string, FlagDef>;
-  };
-} {
-  return (
-    typeof target === "object" &&
-    target !== null &&
-    "parameters" in target &&
-    !("getAllEntries" in target)
-  );
-}
-
 /** Flag metadata for shell completion */
 type FlagEntry = {
   /** Flag name without `--` prefix (e.g., "limit") */
@@ -108,27 +71,29 @@ function extractFlagEntries(
     return [];
   }
 
-  return Object.entries(flags)
-    .filter(([, def]) => !def.hidden)
-    .map(([name, def]) => {
-      const entry: FlagEntry = { name, brief: def.brief ?? "" };
-      // Extract enum values if available
-      if (def.kind === "enum" && "values" in def) {
-        const enumDef = def as FlagDef & { values?: readonly string[] };
-        if (enumDef.values) {
-          entry.enumValues = [...enumDef.values];
-        }
+  const entries: FlagEntry[] = [];
+  for (const [name, def] of Object.entries(flags)) {
+    if (def.hidden) {
+      continue;
+    }
+    const entry: FlagEntry = { name, brief: def.brief ?? "" };
+    if (def.kind === "enum" && "values" in def) {
+      const enumDef = def as FlagDef & { values?: readonly string[] };
+      if (enumDef.values) {
+        entry.enumValues = [...enumDef.values];
       }
-      return entry;
-    });
+    }
+    entries.push(entry);
+  }
+  return entries;
 }
 
 /**
  * Build a CommandEntry from a route target with its name.
  * Extracts flags if the target is a command.
  */
 function buildCommandEntry(name: string, target: unknown): CommandEntry {
-  const flags = isCommandTarget(target)
+  const flags = isCommand(target)
     ? extractFlagEntries(target.parameters.flags)
     : [];
   return {
@@ -148,13 +113,10 @@ function extractSubcommands(routeMap: {
     hidden: boolean;
   }[];
 }): CommandEntry[] {
-  const subcommands: CommandEntry[] = [];
-  for (const sub of routeMap.getAllEntries()) {
-    if (!sub.hidden) {
-      subcommands.push(buildCommandEntry(sub.name.original, sub.target));
-    }
-  }
-  return subcommands;
+  return routeMap
+    .getAllEntries()
+    .filter((sub) => !sub.hidden)
+    .map((sub) => buildCommandEntry(sub.name.original, sub.target));
 }
 
 /**
@@ -191,10 +153,14 @@ export function extractCommandTree(): CommandTree {
 
 /**
  * Sanitize a string for use in a shell variable name.
- * Replaces hyphens and dots with underscores.
+ *
+ * Uses a whitelist approach — replaces any character that is not a valid
+ * shell identifier character (`[a-zA-Z0-9_]`) with an underscore.
+ * Input comes from Stricli route/flag names (alphanumeric + hyphen),
+ * but the whitelist guards against unexpected characters.
  */
 function shellVarName(s: string): string {
-  return s.replace(/[-. ]/g, "_");
+  return s.replace(/[^a-zA-Z0-9_]/g, "_");
 }
 
 /** Collected flag and enum variable declarations for bash scripts. */
@@ -314,8 +280,8 @@ export function generateBashCompletion(binaryName: string): string {
 # Auto-generated from command definitions
 
 # Sanitize a string for use as a bash variable name suffix.
-# Matches shellVarName() in completions.ts: replaces hyphens, dots, spaces.
-__${binaryName}_varname() { echo "\${1//[-. ]/_}"; }
+# Mirrors shellVarName() in completions.ts — replaces non-identifier chars.
+__${binaryName}_varname() { echo "\${1//[^a-zA-Z0-9_]/_}"; }
 
 _${binaryName}_completions() {
   local cur prev words cword
@@ -333,6 +299,25 @@ ${enumVarLines.join("\n")}
     2)
       case "\${prev}" in
 ${caseBranches}
+        *)
+          # Standalone command: check for flags at position 2
+          if [[ "\${cur}" == --* ]]; then
+            local standalone_var="$(__${binaryName}_varname "\${prev}")_flags"
+            if [[ -n "\${!standalone_var+x}" ]]; then
+              COMPREPLY=($(compgen -W "\${!standalone_var}" -- "\${cur}"))
+            fi
+          else
+            # Dynamic value completion for standalone commands
+            local IFS=$'\\n'
+            COMPREPLY=($(${binaryName} __complete "\${COMP_WORDS[@]:1}" 2>/dev/null))
+            if [[ \${#COMPREPLY[@]} -gt 0 ]]; then
+              local i
+              for i in "\${!COMPREPLY[@]}"; do
+                COMPREPLY[$i]="\${COMPREPLY[$i]%%$'\\t'*}"
+              done
+            fi
+          fi
+          ;;
       esac
       ;;
     *)
@@ -430,8 +415,11 @@ ${subArrays}
     subcommand)
       case "$line[1]" in
 ${caseBranches}
+        *)
+          # Standalone command — delegate to dynamic completion
+          ;;
       esac
-      ;;
+      ;;&
     args)
       # Dynamic completion for positional args (org slugs, project names)
       # In the args state, $line[1] and $line[2] hold the parsed command

src/lib/introspect.ts

@@ -111,18 +111,28 @@ export type ResolvedPath =
 
 /**
  * Check if a routing target is a RouteMap (has subcommands).
- * RouteMap has getAllEntries(), Command does not.
+ *
+ * Accepts `unknown` so callers (e.g., completions.ts) can use it on raw
+ * Stricli route entries without first narrowing to {@link RouteTarget}.
  */
-export function isRouteMap(target: RouteTarget): target is RouteMap {
-  return "getAllEntries" in target;
+export function isRouteMap(target: unknown): target is RouteMap {
+  return (
+    typeof target === "object" && target !== null && "getAllEntries" in target
+  );
 }
 
 /**
  * Check if a routing target is a Command (has parameters).
- * Commands have parameters but no getAllEntries.
+ *
+ * Accepts `unknown` for the same reason as {@link isRouteMap}.
  */
-export function isCommand(target: RouteTarget): target is Command {
-  return "parameters" in target && !("getAllEntries" in target);
+export function isCommand(target: unknown): target is Command {
+  return (
+    typeof target === "object" &&
+    target !== null &&
+    "parameters" in target &&
+    !("getAllEntries" in target)
+  );
 }
 
 // ---------------------------------------------------------------------------