mirror of
https://github.com/glittercowboy/get-shit-done
synced 2026-04-25 17:25:23 +02:00
Compare commits
8 Commits
fix/2559-s
...
fix/2544-c
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
726003563a | ||
|
|
dd06a26e2e | ||
|
|
efebf07625 | ||
|
|
32d1e1b939 | ||
|
|
39f7f47a2b | ||
|
|
9a3f735c17 | ||
|
|
e972f598f1 | ||
|
|
ca8d389549 |
@@ -6780,6 +6780,16 @@ function installSdkIfNeeded() {
|
||||
emitSdkFatal('Failed to `npm run build` in sdk/.', { globalBin: null, exitCode: 1 });
|
||||
}
|
||||
|
||||
// Ensure tsc-emitted cli.js is executable before npm install -g creates the bin symlink.
|
||||
// tsc emits .js files as 644; npm install -g creates the symlink but does not chmod the
|
||||
// target, so the kernel refuses to exec the file on macOS (issue #2525).
|
||||
const cliPath = path.join(sdkDir, 'dist', 'cli.js');
|
||||
if (fs.existsSync(cliPath)) {
|
||||
try { fs.chmodSync(cliPath, 0o755); } catch (e) {
|
||||
if (process.platform !== 'win32') throw e;
|
||||
}
|
||||
}
|
||||
|
||||
// 3. Install the built package globally so `gsd-sdk` lands on PATH.
|
||||
const globalResult = spawnSync(npmCmd, ['install', '-g', '.'], { cwd: sdkDir, stdio: 'inherit' });
|
||||
if (globalResult.status !== 0) {
|
||||
|
||||
@@ -888,6 +888,30 @@ function cmdPhaseComplete(cwd, phaseNum, raw) {
|
||||
);
|
||||
}
|
||||
|
||||
// Diff: scan body for all **REQ-ID** patterns, warn about any missing from the Traceability table
|
||||
const bodyReqIds = [];
|
||||
const bodyReqPattern = /\*\*([A-Z][A-Z0-9]*-\d+)\*\*/g;
|
||||
let bodyMatch;
|
||||
while ((bodyMatch = bodyReqPattern.exec(reqContent)) !== null) {
|
||||
const id = bodyMatch[1];
|
||||
if (!bodyReqIds.includes(id)) bodyReqIds.push(id);
|
||||
}
|
||||
|
||||
// Collect REQ-IDs already present in the Traceability table
|
||||
const tableReqIds = new Set();
|
||||
const tableRowPattern = /^\|\s*([A-Z][A-Z0-9]*-\d+)\s*\|/gm;
|
||||
let tableMatch;
|
||||
while ((tableMatch = tableRowPattern.exec(reqContent)) !== null) {
|
||||
tableReqIds.add(tableMatch[1]);
|
||||
}
|
||||
|
||||
const unregistered = bodyReqIds.filter(id => !tableReqIds.has(id));
|
||||
if (unregistered.length > 0) {
|
||||
warnings.push(
|
||||
`REQUIREMENTS.md: ${unregistered.length} REQ-ID(s) found in body but missing from Traceability table: ${unregistered.join(', ')} — add them manually to keep traceability in sync`
|
||||
);
|
||||
}
|
||||
|
||||
atomicWriteFileSync(reqPath, reqContent);
|
||||
requirementsUpdated = true;
|
||||
}
|
||||
|
||||
@@ -74,6 +74,8 @@ AGENT_SKILLS=$(gsd-sdk query agent-skills gsd-executor 2>/dev/null)
|
||||
|
||||
Parse JSON for: `executor_model`, `verifier_model`, `commit_docs`, `parallelization`, `branching_strategy`, `branch_name`, `phase_found`, `phase_dir`, `phase_number`, `phase_name`, `phase_slug`, `plans`, `incomplete_plans`, `plan_count`, `incomplete_count`, `state_exists`, `roadmap_exists`, `phase_req_ids`, `response_language`.
|
||||
|
||||
**Model resolution:** If `executor_model` is `"inherit"`, omit the `model=` parameter from executor `Task()` calls — do NOT pass `model="inherit"` to Task. Omitting the `model=` parameter causes Claude Code to inherit the current orchestrator model automatically. Only set `model=` when `executor_model` is an explicit model name (e.g., `"claude-sonnet-4-6"`, `"claude-opus-4-7"`).
|
||||
|
||||
**If `response_language` is set:** Include `response_language: {value}` in all spawned subagent prompts so any user-facing output stays in the configured language.
|
||||
|
||||
Read worktree config:
|
||||
@@ -421,7 +423,10 @@ Execute each selected wave in sequence. Within a wave: parallel if `PARALLELIZAT
|
||||
Task(
|
||||
subagent_type="gsd-executor",
|
||||
description="Execute plan {plan_number} of phase {phase_number}",
|
||||
model="{executor_model}",
|
||||
# Only include model= when executor_model is an explicit model name.
|
||||
# When executor_model is "inherit", omit this parameter entirely so
|
||||
# Claude Code inherits the orchestrator model automatically.
|
||||
model="{executor_model}", # omit this line when executor_model == "inherit"
|
||||
isolation="worktree",
|
||||
prompt="
|
||||
<objective>
|
||||
|
||||
@@ -862,6 +862,7 @@ Build file list:
|
||||
- If `$DISCUSS_MODE` and context file exists: `${QUICK_DIR}/${quick_id}-CONTEXT.md`
|
||||
- If `$RESEARCH_MODE` and research file exists: `${QUICK_DIR}/${quick_id}-RESEARCH.md`
|
||||
- If `$VALIDATE_MODE` and verification file exists: `${QUICK_DIR}/${quick_id}-VERIFICATION.md`
|
||||
- If `${QUICK_DIR}/${quick_id}-deferred-items.md` exists: `${QUICK_DIR}/${quick_id}-deferred-items.md`
|
||||
|
||||
```bash
|
||||
# Explicitly stage all artifacts before commit — PLAN.md may be untracked
|
||||
|
||||
@@ -367,9 +367,34 @@ If a requirement specifies a quantity of test cases (e.g., "30 calculations"), c
|
||||
</step>
|
||||
|
||||
<step name="identify_human_verification">
|
||||
**Always needs human:** Visual appearance, user flow completion, real-time behavior (WebSocket/SSE), external service integration, performance feel, error message clarity.
|
||||
**First: determine if this is an infrastructure/foundation phase.**
|
||||
|
||||
**Needs human if uncertain:** Complex wiring grep can't trace, dynamic state-dependent behavior, edge cases.
|
||||
Infrastructure and foundation phases — code foundations, database schema, internal APIs, data models, build tooling, CI/CD, internal service integrations — have no user-facing elements by definition. For these phases:
|
||||
|
||||
- Do NOT invent artificial manual steps (e.g., "manually run git commits", "manually invoke methods", "manually check database state").
|
||||
- Mark human verification as **N/A** with rationale: "Infrastructure/foundation phase — no user-facing elements to test manually."
|
||||
- Set `human_verification: []` and do **not** produce a `human_needed` status solely due to lack of user-facing features.
|
||||
- Only add human verification items if the phase goal or success criteria explicitly describe something a user would interact with (UI, CLI command output visible to end users, external service UX).
|
||||
|
||||
**How to determine if a phase is infrastructure/foundation:**
|
||||
- Phase goal or name contains: "foundation", "infrastructure", "schema", "database", "internal API", "data model", "scaffolding", "pipeline", "tooling", "CI", "migrations", "service layer", "backend", "core library"
|
||||
- Phase success criteria describe only technical artifacts (files exist, tests pass, schema is valid) with no user interaction required
|
||||
- There is no UI, CLI output visible to end users, or real-time behavior to observe
|
||||
|
||||
**If the phase IS infrastructure/foundation:** auto-pass UAT — skip the human verification items list entirely. Log:
|
||||
|
||||
```markdown
|
||||
## Human Verification
|
||||
|
||||
N/A — Infrastructure/foundation phase with no user-facing elements.
|
||||
All acceptance criteria are verifiable programmatically.
|
||||
```
|
||||
|
||||
**If the phase IS user-facing:** Only flag items that genuinely require a human. Do not invent steps.
|
||||
|
||||
**Always needs human (user-facing phases only):** Visual appearance, user flow completion, real-time behavior (WebSocket/SSE), external service integration, performance feel, error message clarity.
|
||||
|
||||
**Needs human if uncertain (user-facing phases only):** Complex wiring grep can't trace, dynamic state-dependent behavior, edge cases.
|
||||
|
||||
Format each as: Test Name → What to do → Expected result → Why can't verify programmatically.
|
||||
</step>
|
||||
|
||||
@@ -58,6 +58,43 @@ describe('configGet', () => {
|
||||
await expect(configGet(['nonexistent.key'], tmpDir)).rejects.toThrow(GSDError);
|
||||
});
|
||||
|
||||
it('throws GSDError with Execution classification for missing key (exit code 1 not 10)', async () => {
|
||||
// Regression for #2544: missing key must exit 1, not 10 (Validation).
|
||||
// Callers like `gsd-sdk query config-get k || default` rely on non-zero exit.
|
||||
// ErrorClassification.Execution maps to exit code 1 (matches git config --get).
|
||||
const { configGet } = await import('./config-query.js');
|
||||
const { ErrorClassification } = await import('../errors.js');
|
||||
await writeFile(
|
||||
join(tmpDir, '.planning', 'config.json'),
|
||||
JSON.stringify({ model_profile: 'quality' }),
|
||||
);
|
||||
let thrown: unknown;
|
||||
try {
|
||||
await configGet(['nonexistent.key'], tmpDir);
|
||||
} catch (err) {
|
||||
thrown = err;
|
||||
}
|
||||
expect(thrown).toBeInstanceOf(GSDError);
|
||||
expect((thrown as GSDError).classification).toBe(ErrorClassification.Execution);
|
||||
});
|
||||
|
||||
it('throws GSDError with Execution classification when traversal hits non-object', async () => {
|
||||
const { configGet } = await import('./config-query.js');
|
||||
const { ErrorClassification } = await import('../errors.js');
|
||||
await writeFile(
|
||||
join(tmpDir, '.planning', 'config.json'),
|
||||
JSON.stringify({ workflow: 'a-string-not-an-object' }),
|
||||
);
|
||||
let thrown: unknown;
|
||||
try {
|
||||
await configGet(['workflow.auto_advance'], tmpDir);
|
||||
} catch (err) {
|
||||
thrown = err;
|
||||
}
|
||||
expect(thrown).toBeInstanceOf(GSDError);
|
||||
expect((thrown as GSDError).classification).toBe(ErrorClassification.Execution);
|
||||
});
|
||||
|
||||
it('reads raw config without merging defaults', async () => {
|
||||
const { configGet } = await import('./config-query.js');
|
||||
// Write config with only model_profile -- no workflow section
|
||||
|
||||
@@ -90,12 +90,12 @@ export const configGet: QueryHandler = async (args, projectDir) => {
|
||||
let current: unknown = config;
|
||||
for (const key of keys) {
|
||||
if (current === undefined || current === null || typeof current !== 'object') {
|
||||
throw new GSDError(`Key not found: ${keyPath}`, ErrorClassification.Validation);
|
||||
throw new GSDError(`Key not found: ${keyPath}`, ErrorClassification.Execution);
|
||||
}
|
||||
current = (current as Record<string, unknown>)[key];
|
||||
}
|
||||
if (current === undefined) {
|
||||
throw new GSDError(`Key not found: ${keyPath}`, ErrorClassification.Validation);
|
||||
throw new GSDError(`Key not found: ${keyPath}`, ErrorClassification.Execution);
|
||||
}
|
||||
|
||||
return { data: current };
|
||||
|
||||
@@ -110,7 +110,7 @@ export async function extractCurrentMilestone(content: string, projectDir: strin
|
||||
|
||||
// Fallback: derive from ROADMAP in-progress marker
|
||||
if (!version) {
|
||||
const inProgressMatch = content.match(/🚧\s*\*\*v(\d+\.\d+)\s/);
|
||||
const inProgressMatch = content.match(/🚧\s*\*\*v(\d+\.\d+(?:\.\d+)?)\s/);
|
||||
if (inProgressMatch) {
|
||||
version = 'v' + inProgressMatch[1];
|
||||
}
|
||||
@@ -135,7 +135,7 @@ export async function extractCurrentMilestone(content: string, projectDir: strin
|
||||
const headingLevel = headingLevelMatch ? headingLevelMatch[1].length : 2;
|
||||
const restContent = content.slice(sectionStart + sectionMatch[0].length);
|
||||
const nextMilestonePattern = new RegExp(
|
||||
`^#{1,${headingLevel}}\\s+(?:.*v\\d+\\.\\d+|✅|📋|🚧)`,
|
||||
`^#{1,${headingLevel}}\\s+(?:.*v\\d+\\.\\d+(?:\\.\\d+)?|✅|📋|🚧)`,
|
||||
'mi'
|
||||
);
|
||||
const nextMatch = restContent.match(nextMilestonePattern);
|
||||
|
||||
143
tests/bug-2504-uat-foundation-phases.test.cjs
Normal file
143
tests/bug-2504-uat-foundation-phases.test.cjs
Normal file
@@ -0,0 +1,143 @@
|
||||
/**
|
||||
* Regression test for bug #2504
|
||||
*
|
||||
* When UAT testing is mandated and a phase has no user-facing elements
|
||||
* (e.g., code foundations, database schema, internal APIs), the agent
|
||||
* invented artificial UAT steps — things like "manually run git commits",
|
||||
* "manually invoke methods", "manually check database state" — and left
|
||||
* work half-finished specifically to create things for a human to do.
|
||||
*
|
||||
* Fix: The verify-phase workflow's identify_human_verification step must
|
||||
* explicitly handle phases with no user-facing elements by auto-passing UAT
|
||||
* with a logged rationale instead of inventing manual steps.
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const VERIFY_PHASE_PATH = path.join(
|
||||
__dirname, '..', 'get-shit-done', 'workflows', 'verify-phase.md'
|
||||
);
|
||||
|
||||
/**
|
||||
* Extract a named section from a markdown/workflow document.
|
||||
* Returns the text from `heading` up to (but not including) the next `## ` heading,
|
||||
* or to end-of-file if no subsequent heading exists.
|
||||
*/
|
||||
function extractSection(content, heading) {
|
||||
const start = content.indexOf(heading);
|
||||
if (start === -1) return '';
|
||||
const nextHeading = content.indexOf('\n## ', start + 1);
|
||||
return nextHeading === -1 ? content.slice(start) : content.slice(start, nextHeading);
|
||||
}
|
||||
|
||||
describe('bug #2504: UAT auto-pass for foundation/infrastructure phases', () => {
|
||||
test('verify-phase workflow file exists', () => {
|
||||
assert.ok(
|
||||
fs.existsSync(VERIFY_PHASE_PATH),
|
||||
'get-shit-done/workflows/verify-phase.md should exist'
|
||||
);
|
||||
});
|
||||
|
||||
test('identify_human_verification step handles phases with no user-facing elements', () => {
|
||||
const content = fs.readFileSync(VERIFY_PHASE_PATH, 'utf-8');
|
||||
const section = extractSection(content, 'identify_human_verification');
|
||||
// The step must explicitly call out the infrastructure/foundation case
|
||||
const hasInfrastructureHandling =
|
||||
section.includes('infrastructure') ||
|
||||
section.includes('foundation') ||
|
||||
section.includes('no user-facing') ||
|
||||
section.includes('no user facing') ||
|
||||
section.includes('internal API') ||
|
||||
section.includes('internal APIs') ||
|
||||
section.includes('database schema') ||
|
||||
section.includes('code foundation');
|
||||
|
||||
assert.ok(
|
||||
hasInfrastructureHandling,
|
||||
'verify-phase.md identify_human_verification step must explicitly handle ' +
|
||||
'infrastructure/foundation phases that have no user-facing elements. Without ' +
|
||||
'this, agents invent artificial manual steps to satisfy UAT requirements ' +
|
||||
'(root cause of #2504).'
|
||||
);
|
||||
});
|
||||
|
||||
test('workflow includes auto-pass or skip UAT language for non-user-facing phases', () => {
|
||||
const content = fs.readFileSync(VERIFY_PHASE_PATH, 'utf-8');
|
||||
const section = extractSection(content, 'identify_human_verification');
|
||||
const hasAutoPass =
|
||||
section.includes('auto-pass') ||
|
||||
section.includes('auto pass') ||
|
||||
section.includes('automatically pass') ||
|
||||
section.includes('skip UAT') ||
|
||||
section.includes('skip the UAT') ||
|
||||
section.includes('UAT does not apply') ||
|
||||
section.includes('UAT not applicable') ||
|
||||
section.includes('no UAT required');
|
||||
|
||||
assert.ok(
|
||||
hasAutoPass,
|
||||
'verify-phase.md identify_human_verification step must contain language about ' +
|
||||
'auto-passing or skipping UAT for phases without user-facing elements. Agents ' +
|
||||
'must not invent manual steps when there is nothing user-facing to test ' +
|
||||
'(root cause of #2504).'
|
||||
);
|
||||
});
|
||||
|
||||
test('workflow prohibits inventing artificial manual steps for infrastructure phases', () => {
|
||||
const content = fs.readFileSync(VERIFY_PHASE_PATH, 'utf-8');
|
||||
const section = extractSection(content, 'identify_human_verification');
|
||||
// The workflow must tell the agent NOT to invent steps when there's nothing to test.
|
||||
// Look for explicit prohibition or the inverse: "do not invent" or "must not create"
|
||||
// or equivalent framing like "only require human testing when..."
|
||||
const hasProhibition =
|
||||
section.includes('do not invent') ||
|
||||
section.includes('must not invent') ||
|
||||
section.includes('never invent') ||
|
||||
section.includes('Do not invent') ||
|
||||
section.includes('Must not invent') ||
|
||||
section.includes('Never invent') ||
|
||||
section.includes('only require human') ||
|
||||
section.includes('only add human') ||
|
||||
(section.includes('only flag') && section.includes('user-facing')) ||
|
||||
// Or via "N/A" framing
|
||||
(section.includes('N/A') && (
|
||||
section.includes('infrastructure') ||
|
||||
section.includes('foundation') ||
|
||||
section.includes('no user-facing')
|
||||
));
|
||||
|
||||
assert.ok(
|
||||
hasProhibition,
|
||||
'verify-phase.md identify_human_verification step must explicitly prohibit ' +
|
||||
'inventing artificial manual UAT steps for infrastructure phases. The current ' +
|
||||
'wording causes agents to create fake "manually run git commits" steps to ' +
|
||||
'satisfy UAT mandates (root cause of #2504).'
|
||||
);
|
||||
});
|
||||
|
||||
test('workflow includes a concept of N/A or not-applicable UAT state', () => {
|
||||
const content = fs.readFileSync(VERIFY_PHASE_PATH, 'utf-8');
|
||||
const section = extractSection(content, 'identify_human_verification');
|
||||
const hasNaState =
|
||||
section.includes('N/A') ||
|
||||
section.includes('not applicable') ||
|
||||
section.includes('not_applicable') ||
|
||||
section.includes('no_uat') ||
|
||||
section.includes('uat_not_applicable') ||
|
||||
section.includes('infrastructure phase') ||
|
||||
section.includes('foundation phase');
|
||||
|
||||
assert.ok(
|
||||
hasNaState,
|
||||
'verify-phase.md identify_human_verification step must include some concept of ' +
|
||||
'a "not applicable" or N/A UAT state for phases with no user-facing elements. ' +
|
||||
'This prevents agents from blocking phase completion on invented manual steps ' +
|
||||
'(root cause of #2504).'
|
||||
);
|
||||
});
|
||||
});
|
||||
127
tests/bug-2516-inherit-model-execute-phase.test.cjs
Normal file
127
tests/bug-2516-inherit-model-execute-phase.test.cjs
Normal file
@@ -0,0 +1,127 @@
|
||||
/**
|
||||
* Regression test for bug #2516
|
||||
*
|
||||
* When `.planning/config.json` has `model_profile: "inherit"`, the
|
||||
* `init.execute-phase` query returns `executor_model: "inherit"`. The
|
||||
* execute-phase workflow was passing this literal string directly to the
|
||||
* Task tool via `model="{executor_model}"`, causing Task to fall back to
|
||||
* its default model instead of inheriting the orchestrator model.
|
||||
*
|
||||
* Fix: the workflow must document that when `executor_model` is `"inherit"`,
|
||||
* the `model=` parameter must be OMITTED from Task() calls entirely.
|
||||
* Omitting `model=` causes Claude Code to inherit the current orchestrator
|
||||
* model automatically.
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const WORKFLOW_PATH = path.join(
|
||||
__dirname,
|
||||
'..',
|
||||
'get-shit-done',
|
||||
'workflows',
|
||||
'execute-phase.md'
|
||||
);
|
||||
|
||||
describe('bug #2516: executor_model "inherit" must not be passed literally to Task()', () => {
|
||||
test('workflow file exists', () => {
|
||||
assert.ok(fs.existsSync(WORKFLOW_PATH), 'get-shit-done/workflows/execute-phase.md should exist');
|
||||
});
|
||||
|
||||
test('workflow contains instructions for handling the "inherit" case', () => {
|
||||
assert.ok(fs.existsSync(WORKFLOW_PATH), 'get-shit-done/workflows/execute-phase.md should exist');
|
||||
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
||||
|
||||
const hasInheritInstruction =
|
||||
content.includes('"inherit"') &&
|
||||
(content.includes('omit') || content.includes('Omit') || content.includes('omitting') || content.includes('Omitting'));
|
||||
assert.ok(
|
||||
hasInheritInstruction,
|
||||
'execute-phase.md must document that when executor_model is "inherit", ' +
|
||||
'the model= parameter must be omitted from Task() calls. ' +
|
||||
'Found "inherit" mention: ' + content.includes('"inherit"') + '. ' +
|
||||
'Found omit mention: ' + (content.includes('omit') || content.includes('Omit'))
|
||||
);
|
||||
});
|
||||
|
||||
test('workflow does not instruct passing model="inherit" literally to Task', () => {
|
||||
assert.ok(fs.existsSync(WORKFLOW_PATH), 'get-shit-done/workflows/execute-phase.md should exist');
|
||||
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
||||
|
||||
// The workflow must not have an unconditional model="{executor_model}" template
|
||||
// that would pass "inherit" through. It should document conditional logic.
|
||||
const hasConditionalModelParam =
|
||||
content.includes('inherit') &&
|
||||
(
|
||||
content.includes('Only set `model=`') ||
|
||||
content.includes('only set `model=`') ||
|
||||
content.includes('Only set model=') ||
|
||||
content.includes('omit the `model=`') ||
|
||||
content.includes('omit the model=') ||
|
||||
content.includes('omit `model=`') ||
|
||||
content.includes('omit model=')
|
||||
);
|
||||
assert.ok(
|
||||
hasConditionalModelParam,
|
||||
'execute-phase.md must document omitting model= when executor_model is "inherit". ' +
|
||||
'The unconditional model="{executor_model}" template would pass the literal ' +
|
||||
'string "inherit" to Task(), which falls back to the default model instead ' +
|
||||
'of the orchestrator model (root cause of #2516).'
|
||||
);
|
||||
|
||||
// Assert that no unsafe unconditional template line exists:
|
||||
// a line that contains model="{executor_model}" or model='{executor_model}'
|
||||
// and is NOT inside a "do NOT" / "do not" / "NEVER" instruction context.
|
||||
const unsafeTemplateLines = content.split('\n').filter(line => {
|
||||
const hasTemplate =
|
||||
line.includes('model="{executor_model}"') ||
|
||||
line.includes("model='{executor_model}'");
|
||||
if (!hasTemplate) return false;
|
||||
const isNegated = /do\s+not|NEVER|omit/i.test(line);
|
||||
return !isNegated;
|
||||
});
|
||||
assert.strictEqual(
|
||||
unsafeTemplateLines.length,
|
||||
0,
|
||||
'execute-phase workflow must not contain an unconditional model="{executor_model}" template outside of a "do not" / "NEVER" instruction context. ' +
|
||||
'Unsafe lines found: ' + unsafeTemplateLines.join(' | ')
|
||||
);
|
||||
|
||||
// Direct negative: scan line-by-line for model="inherit" as an actual Task argument.
|
||||
// Skip lines that are part of instructional "do NOT" context.
|
||||
const lines = content.split('\n');
|
||||
for (const line of lines) {
|
||||
if (/do\s+not|must\s+not|never|don't|NEVER/i.test(line)) continue;
|
||||
assert.ok(
|
||||
!line.includes('model="inherit"'),
|
||||
`execute-phase.md must not pass model="inherit" as a literal Task argument. ` +
|
||||
`Found on line: ${line.trim()}`
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('workflow documents that omitting model= causes inheritance from orchestrator', () => {
|
||||
assert.ok(fs.existsSync(WORKFLOW_PATH), 'get-shit-done/workflows/execute-phase.md should exist');
|
||||
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
||||
|
||||
const hasInheritanceExplanation =
|
||||
content.includes('inherit') &&
|
||||
(
|
||||
content.includes('orchestrator model') ||
|
||||
content.includes('orchestrator\'s model') ||
|
||||
content.includes('inherits the') ||
|
||||
content.includes('inherit the current')
|
||||
);
|
||||
assert.ok(
|
||||
hasInheritanceExplanation,
|
||||
'execute-phase.md must explain that omitting model= causes Claude Code to ' +
|
||||
'inherit the current orchestrator model — this is the mechanism that makes ' +
|
||||
'"inherit" work correctly.'
|
||||
);
|
||||
});
|
||||
});
|
||||
82
tests/bug-2525-sdk-executable-bit.test.cjs
Normal file
82
tests/bug-2525-sdk-executable-bit.test.cjs
Normal file
@@ -0,0 +1,82 @@
|
||||
/**
|
||||
* Regression test for bug #2525
|
||||
*
|
||||
* After running the GSD installer on macOS with a Homebrew npm prefix,
|
||||
* `gsd-sdk` is installed but `command -v gsd-sdk` returns nothing because
|
||||
* `dist/cli.js` is installed with mode 644 (no executable bit). tsc emits
|
||||
* .js files as 644, and `npm install -g .` creates the bin symlink without
|
||||
* chmod-ing the target. The kernel then refuses to exec the file.
|
||||
*
|
||||
* Fix: between the `npm run build` step and `npm install -g .`, chmod
|
||||
* dist/cli.js to 0o755. This mirrors the pattern already used for hook
|
||||
* files at lines 5838, 5846, 5959, and 5965.
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const INSTALL_PATH = path.join(__dirname, '..', 'bin', 'install.js');
|
||||
|
||||
describe('bug #2525: dist/cli.js chmod 0o755 after tsc build', () => {
|
||||
const content = fs.readFileSync(INSTALL_PATH, 'utf-8');
|
||||
|
||||
test('install.js exists', () => {
|
||||
assert.ok(fs.existsSync(INSTALL_PATH), 'bin/install.js should exist');
|
||||
});
|
||||
|
||||
test('chmodSync is called for dist/cli.js after the build step', () => {
|
||||
// Find the installSdkIfNeeded function body
|
||||
const fnStart = content.indexOf('function installSdkIfNeeded()');
|
||||
assert.ok(fnStart !== -1, 'installSdkIfNeeded function must exist in bin/install.js');
|
||||
|
||||
// Find the closing brace of the function (next top-level function definition)
|
||||
const fnEnd = content.indexOf('\nfunction ', fnStart + 1);
|
||||
assert.ok(fnEnd !== -1, 'installSdkIfNeeded function must have a closing boundary');
|
||||
|
||||
const fnBody = content.slice(fnStart, fnEnd);
|
||||
|
||||
// Locate the build step
|
||||
const buildStep = fnBody.indexOf("'run', 'build'");
|
||||
assert.ok(buildStep !== -1, "installSdkIfNeeded must contain the 'run', 'build' spawn call");
|
||||
|
||||
// Locate the global install step
|
||||
const globalStep = fnBody.indexOf("'install', '-g', '.'");
|
||||
assert.ok(globalStep !== -1, "installSdkIfNeeded must contain the 'install', '-g', '.' spawn call");
|
||||
|
||||
// Locate chmodSync for dist/cli.js
|
||||
const chmodIdx = fnBody.indexOf("chmodSync");
|
||||
assert.ok(chmodIdx !== -1, "installSdkIfNeeded must call chmodSync to set the executable bit on dist/cli.js");
|
||||
|
||||
// The path may be assembled via path.join(sdkDir, 'dist', 'cli.js') so check
|
||||
// for the component strings rather than the joined slash form.
|
||||
const cliPathIdx = fnBody.indexOf("'cli.js'");
|
||||
assert.ok(cliPathIdx !== -1, "installSdkIfNeeded must reference 'cli.js' (via path.join or literal) for the chmod call");
|
||||
|
||||
// chmodSync must appear AFTER the build step
|
||||
assert.ok(
|
||||
chmodIdx > buildStep,
|
||||
'chmodSync for dist/cli.js must appear AFTER the npm run build step'
|
||||
);
|
||||
|
||||
// chmodSync must appear BEFORE the global install step
|
||||
assert.ok(
|
||||
chmodIdx < globalStep,
|
||||
'chmodSync for dist/cli.js must appear BEFORE the npm install -g . step'
|
||||
);
|
||||
});
|
||||
|
||||
test('chmod mode is 0o755', () => {
|
||||
const fnStart = content.indexOf('function installSdkIfNeeded()');
|
||||
const fnEnd = content.indexOf('\nfunction ', fnStart + 1);
|
||||
const fnBody = content.slice(fnStart, fnEnd);
|
||||
|
||||
assert.ok(
|
||||
fnBody.includes('0o755'),
|
||||
"chmodSync call in installSdkIfNeeded must use mode 0o755 (not 0o644 or a bare number)"
|
||||
);
|
||||
});
|
||||
});
|
||||
271
tests/bug-2526-phase-complete-req-discovery.test.cjs
Normal file
271
tests/bug-2526-phase-complete-req-discovery.test.cjs
Normal file
@@ -0,0 +1,271 @@
|
||||
/**
|
||||
* Regression tests for bug #2526
|
||||
*
|
||||
* phase complete must warn about REQ-IDs that appear in the REQUIREMENTS.md
|
||||
* body but are missing from the Traceability table.
|
||||
*
|
||||
* Root cause: cmdPhaseComplete() only flips status for REQ-IDs already in
|
||||
* the Traceability table (from the roadmap **Requirements:** line). REQ-IDs
|
||||
* added to the REQUIREMENTS.md body after roadmap creation are never
|
||||
* discovered or reflected in the table.
|
||||
*
|
||||
* Fix (Option A — warning only): scan the REQUIREMENTS.md body for all
|
||||
* REQ-IDs, check which are absent from the Traceability table, and emit
|
||||
* a warning listing the missing IDs.
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const { describe, test, beforeEach, afterEach } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const os = require('node:os');
|
||||
const { execFileSync } = require('node:child_process');
|
||||
|
||||
const gsdTools = path.resolve(__dirname, '..', 'get-shit-done', 'bin', 'gsd-tools.cjs');
|
||||
|
||||
describe('bug #2526: phase complete warns about unregistered REQ-IDs', () => {
|
||||
let tmpDir;
|
||||
let planningDir;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2526-'));
|
||||
planningDir = path.join(tmpDir, '.planning');
|
||||
fs.mkdirSync(planningDir, { recursive: true });
|
||||
|
||||
// Minimal config
|
||||
fs.writeFileSync(
|
||||
path.join(planningDir, 'config.json'),
|
||||
JSON.stringify({ project_code: '' })
|
||||
);
|
||||
|
||||
// Minimal STATE.md
|
||||
fs.writeFileSync(
|
||||
path.join(planningDir, 'STATE.md'),
|
||||
'---\ncurrent_phase: 1\nstatus: executing\n---\n# State\n'
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test('emits warning for REQ-IDs in body but missing from Traceability table', () => {
|
||||
// Set up phase directory with a plan and summary
|
||||
const phasesDir = path.join(planningDir, 'phases', '01-foundation');
|
||||
fs.mkdirSync(phasesDir, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
path.join(phasesDir, '01-1-PLAN.md'),
|
||||
'---\nphase: 1\nplan: 1\n---\n# Plan 1\n'
|
||||
);
|
||||
fs.writeFileSync(
|
||||
path.join(phasesDir, '01-1-SUMMARY.md'),
|
||||
'---\nstatus: complete\n---\n# Summary\nDone.'
|
||||
);
|
||||
|
||||
// ROADMAP.md — phase 1 lists only REQ-001 in its Requirements line
|
||||
const roadmapPath = path.join(planningDir, 'ROADMAP.md');
|
||||
fs.writeFileSync(roadmapPath, [
|
||||
'# Roadmap',
|
||||
'',
|
||||
'### Phase 1: Foundation',
|
||||
'',
|
||||
'**Goal:** Build core',
|
||||
'**Requirements:** REQ-001',
|
||||
'**Plans:** 1 plans',
|
||||
'',
|
||||
'Plans:',
|
||||
'- [x] 01-1-PLAN.md',
|
||||
'',
|
||||
'| Phase | Plans | Status | Completed |',
|
||||
'|-------|-------|--------|-----------|',
|
||||
'| 1. Foundation | 0/1 | Pending | - |',
|
||||
].join('\n'));
|
||||
|
||||
// REQUIREMENTS.md — body has REQ-001 (in table) and REQ-002, REQ-003 (missing from table)
|
||||
const reqPath = path.join(planningDir, 'REQUIREMENTS.md');
|
||||
fs.writeFileSync(reqPath, [
|
||||
'# Requirements',
|
||||
'',
|
||||
'## Functional Requirements',
|
||||
'',
|
||||
'- [x] **REQ-001**: Core data model',
|
||||
'- [ ] **REQ-002**: User authentication',
|
||||
'- [ ] **REQ-003**: API endpoints',
|
||||
'',
|
||||
'## Traceability',
|
||||
'',
|
||||
'| REQ-ID | Phase | Status |',
|
||||
'|--------|-------|--------|',
|
||||
'| REQ-001 | 1 | Pending |',
|
||||
].join('\n'));
|
||||
|
||||
let stdout = '';
|
||||
let stderr = '';
|
||||
try {
|
||||
const result = execFileSync('node', [gsdTools, 'phase', 'complete', '1'], {
|
||||
cwd: tmpDir,
|
||||
timeout: 10000,
|
||||
encoding: 'utf-8',
|
||||
});
|
||||
stdout = result;
|
||||
} catch (err) {
|
||||
stdout = err.stdout || '';
|
||||
stderr = err.stderr || '';
|
||||
throw err;
|
||||
}
|
||||
|
||||
const combined = stdout + stderr;
|
||||
assert.match(
|
||||
combined,
|
||||
/REQ-002/,
|
||||
'output should mention REQ-002 as missing from Traceability table'
|
||||
);
|
||||
assert.match(
|
||||
combined,
|
||||
/REQ-003/,
|
||||
'output should mention REQ-003 as missing from Traceability table'
|
||||
);
|
||||
});
|
||||
|
||||
test('no warning when all body REQ-IDs are present in Traceability table', () => {
|
||||
// Set up phase directory
|
||||
const phasesDir = path.join(planningDir, 'phases', '01-foundation');
|
||||
fs.mkdirSync(phasesDir, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
path.join(phasesDir, '01-1-PLAN.md'),
|
||||
'---\nphase: 1\nplan: 1\n---\n# Plan 1\n'
|
||||
);
|
||||
fs.writeFileSync(
|
||||
path.join(phasesDir, '01-1-SUMMARY.md'),
|
||||
'---\nstatus: complete\n---\n# Summary\nDone.'
|
||||
);
|
||||
|
||||
const roadmapPath = path.join(planningDir, 'ROADMAP.md');
|
||||
fs.writeFileSync(roadmapPath, [
|
||||
'# Roadmap',
|
||||
'',
|
||||
'### Phase 1: Foundation',
|
||||
'',
|
||||
'**Goal:** Build core',
|
||||
'**Requirements:** REQ-001, REQ-002',
|
||||
'**Plans:** 1 plans',
|
||||
'',
|
||||
'Plans:',
|
||||
'- [x] 01-1-PLAN.md',
|
||||
'',
|
||||
'| Phase | Plans | Status | Completed |',
|
||||
'|-------|-------|--------|-----------|',
|
||||
'| 1. Foundation | 0/1 | Pending | - |',
|
||||
].join('\n'));
|
||||
|
||||
// All body REQ-IDs are present in the Traceability table
|
||||
const reqPath = path.join(planningDir, 'REQUIREMENTS.md');
|
||||
fs.writeFileSync(reqPath, [
|
||||
'# Requirements',
|
||||
'',
|
||||
'## Functional Requirements',
|
||||
'',
|
||||
'- [x] **REQ-001**: Core data model',
|
||||
'- [x] **REQ-002**: User authentication',
|
||||
'',
|
||||
'## Traceability',
|
||||
'',
|
||||
'| REQ-ID | Phase | Status |',
|
||||
'|--------|-------|--------|',
|
||||
'| REQ-001 | 1 | Pending |',
|
||||
'| REQ-002 | 1 | Pending |',
|
||||
].join('\n'));
|
||||
|
||||
let stdout = '';
|
||||
let stderr = '';
|
||||
try {
|
||||
const result = execFileSync('node', [gsdTools, 'phase', 'complete', '1'], {
|
||||
cwd: tmpDir,
|
||||
timeout: 10000,
|
||||
encoding: 'utf-8',
|
||||
});
|
||||
stdout = result;
|
||||
} catch (err) {
|
||||
stdout = err.stdout || '';
|
||||
stderr = err.stderr || '';
|
||||
throw err;
|
||||
}
|
||||
|
||||
const combined = stdout + stderr;
|
||||
assert.doesNotMatch(
|
||||
combined,
|
||||
/unregistered|missing.*traceability|not in.*traceability/i,
|
||||
'no warning should appear when all REQ-IDs are in the table'
|
||||
);
|
||||
});
|
||||
|
||||
test('warning includes all missing REQ-IDs, not just the first', () => {
|
||||
const phasesDir = path.join(planningDir, 'phases', '01-foundation');
|
||||
fs.mkdirSync(phasesDir, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
path.join(phasesDir, '01-1-PLAN.md'),
|
||||
'---\nphase: 1\nplan: 1\n---\n# Plan 1\n'
|
||||
);
|
||||
fs.writeFileSync(
|
||||
path.join(phasesDir, '01-1-SUMMARY.md'),
|
||||
'---\nstatus: complete\n---\n# Summary\nDone.'
|
||||
);
|
||||
|
||||
const roadmapPath = path.join(planningDir, 'ROADMAP.md');
|
||||
fs.writeFileSync(roadmapPath, [
|
||||
'# Roadmap',
|
||||
'',
|
||||
'### Phase 1: Foundation',
|
||||
'',
|
||||
'**Goal:** Build core',
|
||||
'**Requirements:** REQ-001',
|
||||
'**Plans:** 1 plans',
|
||||
'',
|
||||
'Plans:',
|
||||
'- [x] 01-1-PLAN.md',
|
||||
'',
|
||||
'| Phase | Plans | Status | Completed |',
|
||||
'|-------|-------|--------|-----------|',
|
||||
'| 1. Foundation | 0/1 | Pending | - |',
|
||||
].join('\n'));
|
||||
|
||||
// Body has 4 REQ-IDs; table only has 1
|
||||
const reqPath = path.join(planningDir, 'REQUIREMENTS.md');
|
||||
fs.writeFileSync(reqPath, [
|
||||
'# Requirements',
|
||||
'',
|
||||
'- [x] **REQ-001**: Core data model',
|
||||
'- [ ] **REQ-002**: User auth',
|
||||
'- [ ] **REQ-003**: API',
|
||||
'- [ ] **REQ-004**: Reports',
|
||||
'',
|
||||
'## Traceability',
|
||||
'',
|
||||
'| REQ-ID | Phase | Status |',
|
||||
'|--------|-------|--------|',
|
||||
'| REQ-001 | 1 | Pending |',
|
||||
].join('\n'));
|
||||
|
||||
let stdout = '';
|
||||
let stderr = '';
|
||||
try {
|
||||
const result = execFileSync('node', [gsdTools, 'phase', 'complete', '1'], {
|
||||
cwd: tmpDir,
|
||||
timeout: 10000,
|
||||
encoding: 'utf-8',
|
||||
});
|
||||
stdout = result;
|
||||
} catch (err) {
|
||||
stdout = err.stdout || '';
|
||||
stderr = err.stderr || '';
|
||||
throw err;
|
||||
}
|
||||
|
||||
const combined = stdout + stderr;
|
||||
assert.match(combined, /REQ-002/, 'should warn about REQ-002');
|
||||
assert.match(combined, /REQ-003/, 'should warn about REQ-003');
|
||||
assert.match(combined, /REQ-004/, 'should warn about REQ-004');
|
||||
});
|
||||
});
|
||||
44
tests/bug-2544-config-get-exit-code.test.cjs
Normal file
44
tests/bug-2544-config-get-exit-code.test.cjs
Normal file
@@ -0,0 +1,44 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Bug #2544: `gsd-sdk query config-get` exits 0 on missing key.
|
||||
*
|
||||
* Callers that use `gsd-sdk query config-get k || fallback` rely on a
|
||||
* non-zero exit code when the key is absent. The fix changes the
|
||||
* ErrorClassification for "Key not found" from Validation (exit 10)
|
||||
* to Execution (exit 1), matching the UNIX convention of `git config --get`.
|
||||
*/
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
const CONFIG_QUERY_SRC = path.join(
|
||||
__dirname, '..', 'sdk', 'src', 'query', 'config-query.ts',
|
||||
);
|
||||
|
||||
describe('gsd-sdk config-get exit code for missing key (#2544)', () => {
|
||||
test('config-query.ts source exists', () => {
|
||||
assert.ok(fs.existsSync(CONFIG_QUERY_SRC), 'sdk/src/query/config-query.ts must exist');
|
||||
});
|
||||
|
||||
test('"Key not found" throws with Execution classification, not Validation', () => {
|
||||
const src = fs.readFileSync(CONFIG_QUERY_SRC, 'utf-8');
|
||||
// Find the "Key not found" throw lines and confirm they use Execution, not Validation
|
||||
const keyNotFoundLines = src
|
||||
.split('\n')
|
||||
.filter(line => line.includes('Key not found'));
|
||||
assert.ok(keyNotFoundLines.length > 0, 'Source must contain "Key not found" throw(s)');
|
||||
for (const line of keyNotFoundLines) {
|
||||
assert.ok(
|
||||
line.includes('Execution'),
|
||||
`"Key not found" throw must use ErrorClassification.Execution (exit 1), got: ${line.trim()}`
|
||||
);
|
||||
assert.ok(
|
||||
!line.includes('Validation'),
|
||||
`"Key not found" throw must NOT use ErrorClassification.Validation (exit 10), got: ${line.trim()}`
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -28,7 +28,7 @@ const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
|
||||
const AGENTS_DIR = path.join(__dirname, '..', 'agents');
|
||||
const COMMANDS_DIR = path.join(__dirname, '..', 'commands', 'gsd');
|
||||
const WORKFLOWS_DIR = path.join(__dirname, '..', 'get-shit-done', 'workflows');
|
||||
const CONFIG_PATH = path.join(__dirname, '..', 'get-shit-done', 'bin', 'lib', 'config.cjs');
|
||||
const CONFIG_PATH = path.join(__dirname, '..', 'get-shit-done', 'bin', 'lib', 'config-schema.cjs');
|
||||
|
||||
// Plugin directory resolution (cross-platform safe)
|
||||
const PLUGIN_WORKFLOWS_DIR = process.env.GSD_PLUGIN_ROOT || path.join(os.homedir(), '.claude', 'get-shit-done', 'workflows');
|
||||
|
||||
@@ -128,7 +128,7 @@ describe('use_worktrees config: cross-workflow structural coverage', () => {
|
||||
const DIAGNOSE_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'diagnose-issues.md');
|
||||
const EXECUTE_PLAN_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'execute-plan.md');
|
||||
const PLANNING_CONFIG_PATH = path.join(__dirname, '..', 'get-shit-done', 'references', 'planning-config.md');
|
||||
const CONFIG_CJS_PATH = path.join(__dirname, '..', 'get-shit-done', 'bin', 'lib', 'config.cjs');
|
||||
const CONFIG_CJS_PATH = path.join(__dirname, '..', 'get-shit-done', 'bin', 'lib', 'config-schema.cjs');
|
||||
|
||||
test('quick workflow reads USE_WORKTREES from config', () => {
|
||||
const content = fs.readFileSync(QUICK_PATH, 'utf-8');
|
||||
|
||||
@@ -20,7 +20,7 @@ const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
|
||||
const repoRoot = path.resolve(__dirname, '..');
|
||||
const executePlanPath = path.join(repoRoot, 'get-shit-done', 'workflows', 'execute-plan.md');
|
||||
const planningConfigPath = path.join(repoRoot, 'get-shit-done', 'references', 'planning-config.md');
|
||||
const configCjsPath = path.join(repoRoot, 'get-shit-done', 'bin', 'lib', 'config.cjs');
|
||||
const configCjsPath = path.join(repoRoot, 'get-shit-done', 'bin', 'lib', 'config-schema.cjs');
|
||||
|
||||
describe('inline_plan_threshold config key (#1979)', () => {
|
||||
let tmpDir;
|
||||
|
||||
@@ -17,7 +17,7 @@ const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const GSD_ROOT = path.join(__dirname, '..', 'get-shit-done');
|
||||
const CONFIG_CJS_PATH = path.join(GSD_ROOT, 'bin', 'lib', 'config.cjs');
|
||||
const CONFIG_CJS_PATH = path.join(GSD_ROOT, 'bin', 'lib', 'config-schema.cjs');
|
||||
const CONFIG_TEMPLATE_PATH = path.join(GSD_ROOT, 'templates', 'config.json');
|
||||
const PLAN_PHASE_PATH = path.join(GSD_ROOT, 'workflows', 'plan-phase.md');
|
||||
|
||||
|
||||
@@ -68,7 +68,7 @@ describe('Thinking Partner Integration (#1726)', () => {
|
||||
describe('Config integration', () => {
|
||||
test('features.thinking_partner is in VALID_CONFIG_KEYS', () => {
|
||||
const configSrc = fs.readFileSync(
|
||||
path.join(GSD_ROOT, 'bin', 'lib', 'config.cjs'),
|
||||
path.join(GSD_ROOT, 'bin', 'lib', 'config-schema.cjs'),
|
||||
'utf-8'
|
||||
);
|
||||
assert.ok(
|
||||
|
||||
Reference in New Issue
Block a user