Merge pull request #3110 from patrickclery/fix/3100-search-dirs-colon-leaks
fix: replace stale /gsd: references in agents/, sdk/src/, and .clinerules
This commit is contained in:
5
.changeset/noble-jaguars-squeak.md
Normal file
5
.changeset/noble-jaguars-squeak.md
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
type: Fixed
|
||||||
|
pr: 3110
|
||||||
|
---
|
||||||
|
**Stale `/gsd:<cmd>` references no longer leak into model context on non-Gemini runtimes** — `scripts/fix-slash-commands.cjs` SEARCH_DIRS did not cover `agents/`, `sdk/src/`, or top-level files, so 9 colon-form references survived in 6 files. The hit at `agents/gsd-codebase-mapper.md:105` propagated into `~/.claude/agents/` at install time (the fixer is not wired into install) and produced unrunnable `/gsd:<cmd>` suggestions in agent output on Claude Code, Cursor, Windsurf, etc. Closes #3100.
|
||||||
@@ -5,9 +5,9 @@ GSD is a structured AI development workflow system. It coordinates AI agents thr
|
|||||||
|
|
||||||
## Core Rule: Never Edit Outside a GSD Workflow
|
## Core Rule: Never Edit Outside a GSD Workflow
|
||||||
Do not make direct repo edits. All changes must go through a GSD workflow:
|
Do not make direct repo edits. All changes must go through a GSD workflow:
|
||||||
- `/gsd:plan-phase` → plan the work
|
- `/gsd-plan-phase` → plan the work
|
||||||
- `/gsd:execute-phase` → build it
|
- `/gsd-execute-phase` → build it
|
||||||
- `/gsd:verify-work` → verify results
|
- `/gsd-verify-work` → verify results
|
||||||
|
|
||||||
## Architecture
|
## Architecture
|
||||||
- `get-shit-done/bin/lib/` — Core Node.js library (CommonJS .cjs, no external deps)
|
- `get-shit-done/bin/lib/` — Core Node.js library (CommonJS .cjs, no external deps)
|
||||||
|
|||||||
@@ -102,7 +102,7 @@ The prompt may include a line of the form:
|
|||||||
--paths <p1>,<p2>,...
|
--paths <p1>,<p2>,...
|
||||||
```
|
```
|
||||||
|
|
||||||
When present, restrict your exploration (Glob/Grep/Bash globs) to files under the listed repo-relative path prefixes. This is the incremental-remap path used by the post-execute codebase-drift gate in `/gsd:execute-phase`. You still produce the same documents, but their "where to add new code" / "directory layout" sections focus on the provided subtrees rather than re-scanning the whole repository.
|
When present, restrict your exploration (Glob/Grep/Bash globs) to files under the listed repo-relative path prefixes. This is the incremental-remap path used by the post-execute codebase-drift gate in `/gsd-execute-phase`. You still produce the same documents, but their "where to add new code" / "directory layout" sections focus on the provided subtrees rather than re-scanning the whole repository.
|
||||||
|
|
||||||
**Path validation:** Reject any `--paths` value containing `..`, starting with `/`, or containing shell metacharacters (`;`, `` ` ``, `$`, `&`, `|`, `<`, `>`). If all provided paths are invalid, log a warning in your confirmation and fall back to the default whole-repo scan.
|
**Path validation:** Reject any `--paths` value containing `..`, starting with `/`, or containing shell metacharacters (`;`, `` ` ``, `$`, `&`, `|`, `<`, `>`). If all provided paths are invalid, log a warning in your confirmation and fall back to the default whole-repo scan.
|
||||||
|
|
||||||
|
|||||||
@@ -18,8 +18,22 @@ const SEARCH_DIRS = [
|
|||||||
path.join(__dirname, '..', 'get-shit-done', 'templates'),
|
path.join(__dirname, '..', 'get-shit-done', 'templates'),
|
||||||
path.join(__dirname, '..', 'get-shit-done', 'contexts'),
|
path.join(__dirname, '..', 'get-shit-done', 'contexts'),
|
||||||
path.join(__dirname, '..', 'commands', 'gsd'),
|
path.join(__dirname, '..', 'commands', 'gsd'),
|
||||||
|
path.join(__dirname, '..', 'agents'),
|
||||||
|
path.join(__dirname, '..', 'sdk', 'src'),
|
||||||
];
|
];
|
||||||
const EXTENSIONS = new Set(['.md', '.cjs', '.js']);
|
|
||||||
|
const TOP_LEVEL_FILES = [
|
||||||
|
path.join(__dirname, '..', '.clinerules'),
|
||||||
|
];
|
||||||
|
|
||||||
|
const SKIP_DIRS = new Set(['node_modules', 'dist', '.turbo']);
|
||||||
|
const EXTENSIONS = new Set(['.md', '.cjs', '.js', '.ts', '.tsx']);
|
||||||
|
|
||||||
|
// Test files contain intentional fixture strings (e.g. inputs the sanitizer
|
||||||
|
// is expected to strip). Rewriting them changes test semantics.
|
||||||
|
function isTestFile(name) {
|
||||||
|
return /\.test\.(c?js|tsx?)$/.test(name);
|
||||||
|
}
|
||||||
|
|
||||||
function buildPattern(cmdNames) {
|
function buildPattern(cmdNames) {
|
||||||
// Empty input would compile `/gsd:()(?=[^a-zA-Z0-9_-]|$)/g`, which the regex
|
// Empty input would compile `/gsd:()(?=[^a-zA-Z0-9_-]|$)/g`, which the regex
|
||||||
@@ -49,6 +63,19 @@ function readCmdNames() {
|
|||||||
.map(f => f.replace(/\.md$/, ''));
|
.map(f => f.replace(/\.md$/, ''));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function processFile(file, cmdNames) {
|
||||||
|
const pattern = buildPattern(cmdNames);
|
||||||
|
if (!pattern) return;
|
||||||
|
let src;
|
||||||
|
try { src = fs.readFileSync(file, 'utf-8'); } catch { return; }
|
||||||
|
const replaced = transformContent(src, cmdNames);
|
||||||
|
if (replaced !== src) {
|
||||||
|
fs.writeFileSync(file, replaced, 'utf-8');
|
||||||
|
const count = (src.match(pattern) || []).length;
|
||||||
|
console.log(` ${count} replacements: ${path.relative(path.join(__dirname, '..'), file)}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function processDir(dir, cmdNames) {
|
function processDir(dir, cmdNames) {
|
||||||
const pattern = buildPattern(cmdNames);
|
const pattern = buildPattern(cmdNames);
|
||||||
if (!pattern) return;
|
if (!pattern) return;
|
||||||
@@ -57,15 +84,10 @@ function processDir(dir, cmdNames) {
|
|||||||
for (const e of entries) {
|
for (const e of entries) {
|
||||||
const full = path.join(dir, e.name);
|
const full = path.join(dir, e.name);
|
||||||
if (e.isDirectory()) {
|
if (e.isDirectory()) {
|
||||||
|
if (SKIP_DIRS.has(e.name)) continue;
|
||||||
processDir(full, cmdNames);
|
processDir(full, cmdNames);
|
||||||
} else if (EXTENSIONS.has(path.extname(e.name))) {
|
} else if (EXTENSIONS.has(path.extname(e.name)) && !isTestFile(e.name)) {
|
||||||
const src = fs.readFileSync(full, 'utf-8');
|
processFile(full, cmdNames);
|
||||||
const replaced = transformContent(src, cmdNames);
|
|
||||||
if (replaced !== src) {
|
|
||||||
fs.writeFileSync(full, replaced, 'utf-8');
|
|
||||||
const count = (src.match(pattern) || []).length;
|
|
||||||
console.log(` ${count} replacements: ${path.relative(path.join(__dirname, '..'), full)}`);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -75,7 +97,10 @@ if (require.main === module) {
|
|||||||
for (const dir of SEARCH_DIRS) {
|
for (const dir of SEARCH_DIRS) {
|
||||||
processDir(dir, cmdNames);
|
processDir(dir, cmdNames);
|
||||||
}
|
}
|
||||||
|
for (const file of TOP_LEVEL_FILES) {
|
||||||
|
processFile(file, cmdNames);
|
||||||
|
}
|
||||||
console.log('Done.');
|
console.log('Done.');
|
||||||
}
|
}
|
||||||
|
|
||||||
module.exports = { transformContent, buildPattern };
|
module.exports = { transformContent, buildPattern, SKIP_DIRS };
|
||||||
|
|||||||
@@ -186,7 +186,7 @@ describe('loadConfig', () => {
|
|||||||
// resolveModel/MODEL_PROFILES do not emit aliases when resolve_model_ids
|
// resolveModel/MODEL_PROFILES do not emit aliases when resolve_model_ids
|
||||||
// is "omit". Once a project is initialized, config.json is authoritative,
|
// is "omit". Once a project is initialized, config.json is authoritative,
|
||||||
// because buildNewProjectConfig bakes user defaults into project config
|
// because buildNewProjectConfig bakes user defaults into project config
|
||||||
// at /gsd:new-project time.
|
// at /gsd-new-project time.
|
||||||
|
|
||||||
it('pre-project: ignores user defaults and uses built-in defaults', async () => {
|
it('pre-project: ignores user defaults and uses built-in defaults', async () => {
|
||||||
await writeUserDefaults({ resolve_model_ids: 'omit' });
|
await writeUserDefaults({ resolve_model_ids: 'omit' });
|
||||||
|
|||||||
@@ -172,7 +172,7 @@ export async function loadConfig(projectDir: string, workstream?: string): Promi
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Project config exists — user-level defaults are ignored (CJS parity).
|
// Project config exists — user-level defaults are ignored (CJS parity).
|
||||||
// `buildNewProjectConfig` already baked them into config.json at /gsd:new-project.
|
// `buildNewProjectConfig` already baked them into config.json at /gsd-new-project.
|
||||||
return mergeDefaults(parsed);
|
return mergeDefaults(parsed);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1128,7 +1128,7 @@ export const statePlannedPhase: QueryHandler = async (args, projectDir, workstre
|
|||||||
* Query handler for `state.milestone-switch` — resets STATE.md for a new
|
* Query handler for `state.milestone-switch` — resets STATE.md for a new
|
||||||
* milestone cycle (bug #2630 regression guard).
|
* milestone cycle (bug #2630 regression guard).
|
||||||
*
|
*
|
||||||
* The `/gsd:new-milestone` workflow only rewrote STATE.md's body (Current
|
* The `/gsd-new-milestone` workflow only rewrote STATE.md's body (Current
|
||||||
* Position section). The YAML frontmatter (`milestone`, `milestone_name`,
|
* Position section). The YAML frontmatter (`milestone`, `milestone_name`,
|
||||||
* `status`, `progress.*`) was never touched on a mid-flight switch, so queries
|
* `status`, `progress.*`) was never touched on a mid-flight switch, so queries
|
||||||
* that read frontmatter (`state.json`, `getMilestoneInfo`, every handler that
|
* that read frontmatter (`state.json`, `getMilestoneInfo`, every handler that
|
||||||
@@ -1151,7 +1151,7 @@ export const statePlannedPhase: QueryHandler = async (args, projectDir, workstre
|
|||||||
*
|
*
|
||||||
* Sibling CJS parity: `cmdInitNewMilestone` in `init.cjs` is read-only (like
|
* Sibling CJS parity: `cmdInitNewMilestone` in `init.cjs` is read-only (like
|
||||||
* the TS `initNewMilestone`). The workflow-level fix is to call
|
* the TS `initNewMilestone`). The workflow-level fix is to call
|
||||||
* `state.milestone-switch` from `/gsd:new-milestone` Step 5 in place of the
|
* `state.milestone-switch` from `/gsd-new-milestone` Step 5 in place of the
|
||||||
* manual body rewrite.
|
* manual body rewrite.
|
||||||
*/
|
*/
|
||||||
export const stateMilestoneSwitch: QueryHandler = async (args, projectDir, workstream) => {
|
export const stateMilestoneSwitch: QueryHandler = async (args, projectDir, workstream) => {
|
||||||
|
|||||||
@@ -649,7 +649,7 @@ export const verifySchemaDrift: QueryHandler = async (args, projectDir, workstre
|
|||||||
*
|
*
|
||||||
* Non-blocking by contract: every failure mode returns a successful response
|
* Non-blocking by contract: every failure mode returns a successful response
|
||||||
* with `{ skipped: true, reason }`. The post-execute drift gate in
|
* with `{ skipped: true, reason }`. The post-execute drift gate in
|
||||||
* `/gsd:execute-phase` relies on this guarantee.
|
* `/gsd-execute-phase` relies on this guarantee.
|
||||||
*
|
*
|
||||||
* Delegates to the Node-side implementation in `bin/lib/drift.cjs` and
|
* Delegates to the Node-side implementation in `bin/lib/drift.cjs` and
|
||||||
* `bin/lib/verify.cjs` via a child process so the drift logic stays in one
|
* `bin/lib/verify.cjs` via a child process so the drift logic stays in one
|
||||||
|
|||||||
@@ -39,8 +39,20 @@ const SEARCH_DIRS = [
|
|||||||
path.join(ROOT, 'get-shit-done', 'templates'),
|
path.join(ROOT, 'get-shit-done', 'templates'),
|
||||||
path.join(ROOT, 'get-shit-done', 'contexts'),
|
path.join(ROOT, 'get-shit-done', 'contexts'),
|
||||||
COMMANDS_DIR,
|
COMMANDS_DIR,
|
||||||
|
path.join(ROOT, 'agents'),
|
||||||
|
path.join(ROOT, 'sdk', 'src'),
|
||||||
];
|
];
|
||||||
|
|
||||||
|
const TOP_LEVEL_FILES = [
|
||||||
|
path.join(ROOT, '.clinerules'),
|
||||||
|
];
|
||||||
|
|
||||||
|
// Re-use SKIP_DIRS from the production script so the test's directory walker
|
||||||
|
// stays in lockstep with the fixer's. EXTENSIONS legitimately diverges (the
|
||||||
|
// guard scans only `.md`/`.cjs`/`.js` per the no-source-grep standard, while
|
||||||
|
// the fixer also rewrites `.ts`/`.tsx`), so it is not shared.
|
||||||
|
const { SKIP_DIRS } = require(path.join(ROOT, 'scripts', 'fix-slash-commands.cjs'));
|
||||||
|
|
||||||
// Discover user-facing markdown surfaces dynamically so a freshly added
|
// Discover user-facing markdown surfaces dynamically so a freshly added
|
||||||
// doc (a new RELEASE-*.md, a new top-level guide) is automatically scanned
|
// doc (a new RELEASE-*.md, a new top-level guide) is automatically scanned
|
||||||
// for namespace drift. A hand-curated list silently weakens drift detection
|
// for namespace drift. A hand-curated list silently weakens drift detection
|
||||||
@@ -80,6 +92,11 @@ function discoverDocSearchFiles(root) {
|
|||||||
|
|
||||||
const DOC_SEARCH_FILES = discoverDocSearchFiles(ROOT);
|
const DOC_SEARCH_FILES = discoverDocSearchFiles(ROOT);
|
||||||
|
|
||||||
|
// Limited to .md (and pre-existing .cjs/.js) by the no-source-grep standard:
|
||||||
|
// markdown text IS the deployed product, but .ts/.tsx source must be guarded
|
||||||
|
// via runtime behavior. The fixer (scripts/fix-slash-commands.cjs) covers
|
||||||
|
// .ts/.tsx auto-rewrites at build time; idempotency (a no-op second run) is
|
||||||
|
// the runtime guard for those extensions.
|
||||||
const EXTENSIONS = new Set(['.md', '.cjs', '.js']);
|
const EXTENSIONS = new Set(['.md', '.cjs', '.js']);
|
||||||
|
|
||||||
function collectFiles(dir, results = []) {
|
function collectFiles(dir, results = []) {
|
||||||
@@ -87,7 +104,10 @@ function collectFiles(dir, results = []) {
|
|||||||
try { entries = fs.readdirSync(dir, { withFileTypes: true }); } catch { return results; }
|
try { entries = fs.readdirSync(dir, { withFileTypes: true }); } catch { return results; }
|
||||||
for (const e of entries) {
|
for (const e of entries) {
|
||||||
const full = path.join(dir, e.name);
|
const full = path.join(dir, e.name);
|
||||||
if (e.isDirectory()) collectFiles(full, results);
|
if (e.isDirectory()) {
|
||||||
|
if (SKIP_DIRS.has(e.name)) continue;
|
||||||
|
collectFiles(full, results);
|
||||||
|
}
|
||||||
else if (EXTENSIONS.has(path.extname(e.name))) results.push(full);
|
else if (EXTENSIONS.has(path.extname(e.name))) results.push(full);
|
||||||
}
|
}
|
||||||
return results;
|
return results;
|
||||||
@@ -103,7 +123,10 @@ const cmdNames = fs.readdirSync(COMMANDS_DIR)
|
|||||||
const retiredPattern = new RegExp(`/gsd:(${cmdNames.join('|')})(?=[^a-zA-Z0-9_-]|$)`);
|
const retiredPattern = new RegExp(`/gsd:(${cmdNames.join('|')})(?=[^a-zA-Z0-9_-]|$)`);
|
||||||
|
|
||||||
const allFiles = SEARCH_DIRS.flatMap(d => collectFiles(d));
|
const allFiles = SEARCH_DIRS.flatMap(d => collectFiles(d));
|
||||||
const allUserFacingFiles = allFiles.concat(DOC_SEARCH_FILES.filter((file) => fs.existsSync(file)));
|
const topLevelFiles = TOP_LEVEL_FILES.filter((file) => fs.existsSync(file));
|
||||||
|
const allUserFacingFiles = allFiles
|
||||||
|
.concat(topLevelFiles)
|
||||||
|
.concat(DOC_SEARCH_FILES.filter((file) => fs.existsSync(file)));
|
||||||
|
|
||||||
describe('slash-command namespace invariant (#2697)', () => {
|
describe('slash-command namespace invariant (#2697)', () => {
|
||||||
test('commands/gsd/ directory contains known command files', () => {
|
test('commands/gsd/ directory contains known command files', () => {
|
||||||
@@ -183,7 +206,7 @@ describe('slash-command namespace invariant (#2697)', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
test('gsd-sdk and gsd-tools identifiers are not rewritten', () => {
|
test('gsd-sdk and gsd-tools identifiers are not rewritten', () => {
|
||||||
for (const file of allFiles) {
|
for (const file of allUserFacingFiles) {
|
||||||
const src = fs.readFileSync(file, 'utf-8');
|
const src = fs.readFileSync(file, 'utf-8');
|
||||||
assert.ok(
|
assert.ok(
|
||||||
!src.includes('/gsd:sdk'),
|
!src.includes('/gsd:sdk'),
|
||||||
|
|||||||
Reference in New Issue
Block a user