From 1b2ecc390fb5bd8c0bf5110de627fcb9e56cdb13 Mon Sep 17 00:00:00 2001
From: Nick Wesselman <27013789+nickwesselman@users.noreply.github.com>
Date: Wed, 2 Sep 2026 17:00:24 -0400
Subject: [PATCH] Skip hidden commands in breaking change detection

`extractManifestSurface` treated every manifest command as public surface, so
renaming a flag on a `hidden` command failed the breaking change check even
though CONTRIBUTING.md scopes the stable command surface to what the CLI
actually exposes. Skip hidden commands when building the surface, which drops
them from the removed command, flag and env var scans alike.

Hidden-ness is read from each manifest independently, so a command that was
visible in the baseline and is hidden now still reports as removed: a PR can't
hide a command and strip its flags in one move without being flagged. An env
var shared with a visible command also stays tracked through that command.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Assisted-By: devx/a27b4557-3ac8-448a-8860-7a5ed6b20687
---
 workspace/src/major-change-check.js      | 15 +++++-
 workspace/src/major-change-check.test.js | 61 +++++++++++++++++++++++-
 2 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/workspace/src/major-change-check.js b/workspace/src/major-change-check.js
index 9691a8dd34c..3b411aad54d 100644
--- a/workspace/src/major-change-check.js
+++ b/workspace/src/major-change-check.js
@@ -163,11 +163,24 @@ async function parseManifest(directory) {
   }
 }
 
-function extractManifestSurface(manifest) {
+export function extractManifestSurface(manifest) {
   const surface = {commands: {}, envVars: {}}
   if (!manifest?.commands) return surface
 
   for (const [cmdName, cmd] of Object.entries(manifest.commands)) {
+    // Hidden commands are not part of the stable command surface that
+    // CONTRIBUTING.md defines, so renaming or removing their flags is not a
+    // breaking change. Dropping them here excludes them from the removed
+    // command, flag and env var scans alike, since all three derive from this
+    // surface. An env var shared with a visible command stays tracked through
+    // that command.
+    //
+    // Hidden-ness is read from each manifest independently, on purpose: a
+    // command that was visible in the baseline and is hidden now still reports
+    // as a removed command, so a PR cannot hide a command and strip its flags
+    // in one move without being flagged.
+    if (cmd.hidden) continue
+
     const flags = {}
     if (cmd.flags) {
       for (const [flagName, flag] of Object.entries(cmd.flags)) {
diff --git a/workspace/src/major-change-check.test.js b/workspace/src/major-change-check.test.js
index 0af5762bd17..e5ac46440a6 100644
--- a/workspace/src/major-change-check.test.js
+++ b/workspace/src/major-change-check.test.js
@@ -17,7 +17,13 @@ import {mkdtemp, rm, writeFile, mkdir} from 'node:fs/promises'
 import os from 'node:os'
 import * as path from 'pathe'
 
-import {checkChangesets, extractSchemaFields, resolveContext, stripStringsAndComments} from './major-change-check.js'
+import {
+  checkChangesets,
+  extractManifestSurface,
+  extractSchemaFields,
+  resolveContext,
+  stripStringsAndComments,
+} from './major-change-check.js'
 
 test('extracts top-level keys from a flat .object({...})', () => {
   const src = `
@@ -270,3 +276,56 @@ test('resolveContext: no GITHUB_BASE_REF falls back to scanning main (local invo
   assert.equal(ctx.changedFiles, null)
   assert.equal(called, false, 'must not shell out to git when no base ref is known')
 })
+
+test('manifest surface omits hidden commands and their flags', () => {
+  const surface = extractManifestSurface({
+    commands: {
+      'app:dev': {flags: {store: {type: 'option'}}},
+      'store:create:dev': {hidden: true, flags: {'with-demo-data': {type: 'boolean'}}},
+    },
+  })
+
+  assert.deepEqual(Object.keys(surface.commands), ['app:dev'])
+  assert.equal(surface.commands['store:create:dev'], undefined)
+})
+
+test('manifest surface omits env vars used only by hidden commands', () => {
+  const surface = extractManifestSurface({
+    commands: {
+      'store:create:dev': {
+        hidden: true,
+        flags: {'with-demo-data': {type: 'boolean', env: 'SHOPIFY_FLAG_STORE_WITH_DEMO_DATA'}},
+      },
+    },
+  })
+
+  assert.deepEqual(surface.envVars, {})
+})
+
+test('manifest surface keeps an env var shared with a visible command', () => {
+  // Only the hidden command drops out; the visible command keeps the env var
+  // under watch, so removing it there is still reported.
+  const surface = extractManifestSurface({
+    commands: {
+      'app:dev': {flags: {store: {type: 'option', env: 'SHOPIFY_FLAG_STORE'}}},
+      'store:create:dev': {hidden: true, flags: {store: {type: 'option', env: 'SHOPIFY_FLAG_STORE'}}},
+    },
+  })
+
+  assert.deepEqual(surface.envVars.SHOPIFY_FLAG_STORE, [{command: 'app:dev', flag: 'store'}])
+})
+
+test('manifest surface still tracks a command that is visible in one manifest and hidden in another', () => {
+  // Reading `hidden` per manifest is what stops a PR from hiding a command and
+  // stripping its flags in one move: the baseline still lists it, so the
+  // removal surfaces.
+  const baseline = extractManifestSurface({
+    commands: {'app:dev': {flags: {store: {type: 'option'}}}},
+  })
+  const current = extractManifestSurface({
+    commands: {'app:dev': {hidden: true, flags: {}}},
+  })
+
+  assert.deepEqual(Object.keys(baseline.commands), ['app:dev'])
+  assert.deepEqual(Object.keys(current.commands), [])
+})