fix: address post-merge review concerns from PR #1268

Three non-blocking findings from the adversarial re-review of the
workstream namespacing PR, addressed as a follow-up:

1. setActiveWorkstream now validates names with the same regex used
   at CLI entry and cmdWorkstreamSet — defense-in-depth so future
   callers can't poison the active-workstream file

2. Replaced tautological test assertion (result.success || !result.success
   was always true) with actual validation that cmdWorkstreamSet returns
   invalid_name error for path traversal attempts. Added 8 new tests
   for setActiveWorkstream's own validation.

3. Updated stale comment in copilot-install.test.cjs (said 31, actual 56)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

get-shit-done/bin/lib/core.cjs

@@ -589,6 +589,9 @@ function setActiveWorkstream(cwd, name) {
     try { fs.unlinkSync(filePath); } catch {}
     return;
   }
+  if (!/^[a-zA-Z0-9_-]+$/.test(name)) {
+    throw new Error('Invalid workstream name: must be alphanumeric, hyphens, and underscores only');
+  }
   fs.writeFileSync(filePath, name + '\n', 'utf-8');
 }
 

tests/copilot-install.test.cjs

@@ -617,7 +617,7 @@ describe('copyCommandsAsCopilotSkills', () => {
       assert.ok(fs.existsSync(path.join(tempDir, 'gsd-help')), 'gsd-help folder exists');
       assert.ok(fs.existsSync(path.join(tempDir, 'gsd-progress')), 'gsd-progress folder exists');
 
-      // Count gsd-* directories — should be 31
+      // Count gsd-* directories — should be 56
       const dirs = fs.readdirSync(tempDir, { withFileTypes: true })
         .filter(e => e.isDirectory() && e.name.startsWith('gsd-'));
       assert.strictEqual(dirs.length, 56, `expected 56 skill folders, got ${dirs.length}`);

tests/workstream.test.cjs

@@ -408,12 +408,11 @@ describe('path traversal rejection', () => {
     for (const name of maliciousNames) {
       test(`rejects set ${name}`, () => {
         const result = runGsdTools(['workstream', 'set', name, '--raw'], tmpDir);
-        // set validates independently — should return error or invalid_name
-        assert.ok(result.success || !result.success, 'should handle gracefully');
-        if (result.success) {
-          const data = JSON.parse(result.output);
-          assert.strictEqual(data.error, 'invalid_name', `should return invalid_name error for: ${name}`);
-        }
+        // cmdWorkstreamSet validates the positional arg and returns invalid_name error
+        assert.ok(result.success, `command should exit cleanly for: ${name}`);
+        const data = JSON.parse(result.output);
+        assert.strictEqual(data.error, 'invalid_name', `should return invalid_name error for: ${name}`);
+        assert.strictEqual(data.active, null, `active should be null for: ${name}`);
       });
     }
   });
@@ -436,4 +435,17 @@ describe('path traversal rejection', () => {
       try { fs.unlinkSync(path.join(tmpDir, '.planning', 'active-workstream')); } catch {}
     });
   });
+
+  describe('setActiveWorkstream rejects invalid names directly', () => {
+    const { setActiveWorkstream } = require('../get-shit-done/bin/lib/core.cjs');
+    for (const name of maliciousNames) {
+      test(`throws for ${name}`, () => {
+        assert.throws(
+          () => setActiveWorkstream(tmpDir, name),
+          { message: /Invalid workstream name/ },
+          `should throw for: ${name}`
+        );
+      });
+    }
+  });
 });