From 658ea33cb60add7c9ed55ce413dd4f0abe065793 Mon Sep 17 00:00:00 2001
From: Tom Boucher <trekkie@nomorestars.com>
Date: Tue, 23 Jun 2026 14:52:32 -0400
Subject: [PATCH] fix(#1615): applySurface rewrites commands kind, not just
 skills
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Codex adversarial orthogonal review of PR #1622 surfaced that applySurface (src/surface.cts) only called rewriteStagedSkillBodies for kind='skills', skipping kind='commands'. The gap meant /gsd-surface profile changes on any runtime with commands kinds (windsurf, opencode, kilo, cursor, augment, codebuddy, gemini) wrote raw @~/.claude/... references into synced command/workflow bodies, which fail at invocation time on non-Claude runtimes.

For Windsurf specifically, this left workflow files containing @~/.claude/gsd-core/commands/gsd/X.md after a profile change — paths that don't exist on a Windsurf install. Verified by the new regression test which fails before the fix (workflow bodies contained @~/.claude/) and passes after (workflow bodies reference the install target).

Captures the return value of rewriteStagedCommandBodies (temp dir path — commands rewrite uses copy-then-rewrite to avoid mutating the package source), syncs from the temp dir, then cleans up. Type annotations satisfy typescript-eslint strict mode.

Findings 2 (install ordering) and 3 (legacy .devin cleanup) from the same review are tracked in #1629 — both real but out of scope for #1615.
---
 src/surface.cts                               | 47 ++++++++++++----
 .../runtime-artifact-layout-surface.test.cjs  | 55 +++++++++++++++++++
 2 files changed, 92 insertions(+), 10 deletions(-)

diff --git a/src/surface.cts b/src/surface.cts
index 8422a3871..83e99d383 100644
--- a/src/surface.cts
+++ b/src/surface.cts
@@ -310,17 +310,44 @@ function applySurface(runtimeConfigDir: string, layout: Layout, manifest: Map<st
   // module's deep seam (ADR-1508 / #1511 Phase 2) — no attribution resolver
   // needed here (proven: Co-Authored-By never appears in staged content; see
   // brief PROVEN KEY FACT). No getInstallExports() call required.
-  for (const kind of layout.kinds) {
-    const staged = kind.stage(resolved);
-    if (kind.kind === 'skills') {
-      runtimeArtifactConversion.rewriteStagedSkillBodies(staged, {
-        runtime: layout.runtime,
-        configDir: layout.configDir,
-        scope: layout.scope ?? 'global',
-      });
+  // #1615 adversarial review (PR #1622): commands kind was previously skipped,
+  // leaving raw @~/.claude/... references in Windsurf workflow bodies after a
+  // /gsd-surface profile change. Same gap affected any runtime with commands
+  // kinds (windsurf, opencode, kilo, cursor, augment, codebuddy, gemini).
+  //
+  // Asymmetry note: rewriteStagedSkillBodies mutates in place (returns void),
+  // but rewriteStagedCommandBodies copies to a fresh mkdtemp dir and returns
+  // its path (commands .md files are flat; mutating the staged source would
+  // corrupt the package source on full-profile runs). Caller MUST sync from
+  // the returned dir and clean it up.
+  const tempDirsToClean: string[] = [];
+  try {
+    for (const kind of layout.kinds) {
+      let staged: string = kind.stage(resolved);
+      if (kind.kind === 'skills') {
+        runtimeArtifactConversion.rewriteStagedSkillBodies(staged, {
+          runtime: layout.runtime,
+          configDir: layout.configDir,
+          scope: layout.scope ?? 'global',
+        });
+      } else if (kind.kind === 'commands') {
+        const rewritten: string | undefined = runtimeArtifactConversion.rewriteStagedCommandBodies(staged, {
+          runtime: layout.runtime,
+          configDir: layout.configDir,
+          scope: layout.scope ?? 'global',
+        }) as string | undefined;
+        if (rewritten && rewritten !== staged) {
+          staged = rewritten;
+          tempDirsToClean.push(rewritten);
+        }
+      }
+      const dest = path.join(layout.configDir, kind.destSubpath);
+      _syncGsdDir(staged, dest, kind, skillManifest);
+    }
+  } finally {
+    for (const dir of tempDirsToClean) {
+      try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* best-effort cleanup */ }
     }
-    const dest = path.join(layout.configDir, kind.destSubpath);
-    _syncGsdDir(staged, dest, kind, skillManifest);
   }
   return resolved;
 }
diff --git a/tests/runtime-artifact-layout-surface.test.cjs b/tests/runtime-artifact-layout-surface.test.cjs
index 6965b1803..5c124618f 100644
--- a/tests/runtime-artifact-layout-surface.test.cjs
+++ b/tests/runtime-artifact-layout-surface.test.cjs
@@ -1056,3 +1056,58 @@ describe('listSurface', () => {
     }
   });
 });
+
+// ─── #1615: applySurface must rewrite commands kind (Windsurf workflows) ─────
+// Adversarial review of PR #1622 found that applySurface only rewrites 'skills'
+// kinds, skipping 'commands'. Windsurf's capability now stages workflow files
+// as kind='commands'; without the rewrite, /gsd-surface would write workflow
+// bodies containing raw @~/.claude/... references that don't exist on a
+// Windsurf install. The same gap affected any runtime with commands kinds.
+describe('applySurface — commands kind path rewrite (#1615 adversarial review)', () => {
+  test('windsurf workflow bodies are rewritten to install target (no raw ~/.claude/)', (t) => {
+    const base = createTempDir('gsd-surface-cmds-windsurf-');
+    t.after(() => cleanup(base));
+    const runtimeConfigDir = base;
+
+    // Stage the canonical command body the workflow delegates to.
+    const canonicalDir = path.join(runtimeConfigDir, 'gsd-core', 'commands', 'gsd');
+    fs.mkdirSync(canonicalDir, { recursive: true });
+    fs.writeFileSync(path.join(canonicalDir, 'help.md'),
+      '---\nname: help\ndescription: Show help\n---\n\nHelp body\n');
+
+    const manifest = loadSkillsManifest(REAL_COMMANDS_DIR);
+    const layout = resolveRuntimeArtifactLayout('windsurf', runtimeConfigDir, 'local');
+
+    // Sanity: layout must have a commands kind (workflows) — pre-condition
+    // introduced by PR #1622; if a future refactor removes it, this test
+    // would silently pass without exercising the rewrite path.
+    const commandsKind = layout.kinds.find((k) => k.kind === 'commands');
+    assert.ok(commandsKind, 'pre-condition: windsurf layout has a commands kind');
+
+    applySurface(runtimeConfigDir, layout, manifest, CLUSTERS);
+
+    // Workflow files should be written to <configDir>/workflows/gsd-*.md
+    const workflowsDir = path.join(runtimeConfigDir, 'workflows');
+    const workflowFiles = fs.existsSync(workflowsDir)
+      ? fs.readdirSync(workflowsDir).filter((f) => f.startsWith('gsd-') && f.endsWith('.md'))
+      : [];
+    assert.ok(workflowFiles.length > 0,
+      `expected at least one gsd-*.md workflow under ${workflowsDir}; got [${workflowFiles.join(', ')}]`);
+
+    // Every workflow body must reference the install target, NOT the raw
+    // ~/.claude/ path. This is the regression: pre-fix, the commands kind
+    // was skipped and raw @~/.claude/... survived into the synced file.
+    for (const fileName of workflowFiles) {
+      const workflowPath = path.join(workflowsDir, fileName);
+      const content = fs.readFileSync(workflowPath, 'utf8');
+      assert.ok(
+        !content.includes('~/.claude/'),
+        `${fileName} must not contain raw ~/.claude/ after applySurface rewrite (got: ${content.slice(0, 200)})`,
+      );
+      assert.ok(
+        !content.includes('$HOME/.claude/'),
+        `${fileName} must not contain raw $HOME/.claude/ after applySurface rewrite`,
+      );
+    }
+  });
+});