* enhance(#4273): add phase.tdd-applicable single-owner predicate One query verb computes TDD-applicability for a plan (CLI flag, plan type: tdd frontmatter, a task's tdd="true" attribute, or the workflow.tdd_mode config default), mirroring phase.mvp-mode's precedence-cascade shape. Foundation for epic #4272 Phase 2, which wires both dispatch backends to consume it instead of restating the predicate independently. Also fixes workflow.tdd_mode, workflow.research, and workflow.nyquist_validation, which never reached cmdInitExecutePhase/cmdInitPlanPhase/cmdInitDebug/cmdInitNewMilestone because loadConfig() never populates config.workflow — a dead accessor found while wiring this verb's own config read, fixed inline per the no-defer rule rather than left alongside it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(#4273): document phase.tdd-applicable's FEATURES.md entry Add a docs/features/ fragment for the new phase.tdd-applicable query verb and regenerate docs/FEATURES.md. docs/COMMANDS.md is left untouched: it documents /gsd-* slash commands only, and the sibling verb phase.tdd-applicable mirrors (phase.mvp-mode) has no formal CLI reference entry anywhere in docs/ either -- only inline prose mentions in docs/reference/workflow-fragments.md -- so there is no COMMANDS.md precedent to extend. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#4273): use PHASE_NOT_FOUND reason code, remove try/finally from tests Two orthogonal code reviews flagged a mistyped error reason and a CONTRIBUTING.md-banned try/finally pattern in the phase.tdd-applicable change; both are corrected here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#4273): stop whitelisting capability-owned config keys centrally workflow.tdd_mode, workflow.research, and workflow.nyquist_validation are each already owned by their own first-party capability's federated config schema (the tdd/research/nyquist capabilities declare them under their own capability.json `config`), resolved via isCapabilityConfigKey. Adding them to gsd-core/bin/shared/config-schema.manifest.json's central validKeys, as the prior commit in this branch did (mirroring workflow.mvp_mode, which genuinely is central-only), declares the same key in two places at once. That collision breaks capability-loader.cts's loadRegistry composition: gsd-test caught this as 84-85 unrelated failures across capability-cli/capability-command-dispatch/capability-lifecycle test files, every one showing "unknown capability: <id>" for a freshly-installed third-party capability that should have resolved fine. Verified directly (not asserted): reverting only this file, keeping the config-loader.cts tdd_mode/research/nyquist_validation flattening and the init.cts call-site fixes from the prior commit, and re-running the exact capability install + capability set repro from tests/capability-cli.test.cjs's "issue-2322" test locally reproduces the failure with the whitelist entries present and clears it without them. loadConfig() still surfaces all three flattened values correctly with no central whitelist entry (confirmed directly against the compiled module) — the whitelist additions were never required for the #4273 fix to work; they were an incorrect over-application of the mvp_mode precedent to keys that aren't central. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#4273): use getNested for tdd_mode (no legacy top-level fallback), allowlist new test file Both fixes address defects found by a gsd-test bench run: tdd_mode routed through get() invented an undocumented top-level alias that silently outranked the canonical workflow.tdd_mode key, and the new phase-tdd-applicable test file was missing from the file-count allowlist. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore(#4273): backfill changeset PR number Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
@@ -915,6 +915,7 @@ function loadConfigResolved(cwd: string, options: Record<string, unknown> = {}):
|
||||
firecrawl: get('firecrawl') ?? defaults.firecrawl,
|
||||
exa_search: get('exa_search') ?? defaults.exa_search,
|
||||
mvp_mode: get('mvp_mode', { section: 'workflow', field: 'mvp_mode' }) ?? false,
|
||||
tdd_mode: getNested('workflow', 'tdd_mode') ?? false,
|
||||
text_mode: get('text_mode', { section: 'workflow', field: 'text_mode' }) ?? defaults.text_mode,
|
||||
auto_advance: get('auto_advance', { section: 'workflow', field: 'auto_advance' }) ?? false,
|
||||
_auto_chain_active: get('_auto_chain_active', { section: 'workflow', field: '_auto_chain_active' }) ?? false,
|
||||
|
||||
24
src/init.cts
24
src/init.cts
@@ -908,8 +908,6 @@ function cmdInitExecutePhase(
|
||||
: null;
|
||||
const phase_req_ids = reqExtracted && reqExtracted !== 'TBD' ? reqExtracted : null;
|
||||
|
||||
const wf = (config.workflow ?? {}) as Record<string, unknown>;
|
||||
|
||||
// #3188: these paths are null when the file is absent, matching the contract
|
||||
// the conditional sibling fields (context_path, patterns_path, ...) already
|
||||
// honour and that ultraplan-phase.md / execute-phase.md gate on. Hoisted so
|
||||
@@ -922,7 +920,7 @@ function cmdInitExecutePhase(
|
||||
executor_model: resolveModelInternal(cwd, 'gsd-executor'),
|
||||
verifier_model: resolveModelInternal(cwd, 'gsd-verifier'),
|
||||
|
||||
tdd_mode: options['tdd'] || Boolean(wf['tdd_mode']) || false,
|
||||
tdd_mode: options['tdd'] || Boolean(config.tdd_mode) || false,
|
||||
commit_docs: config.commit_docs,
|
||||
sub_repos: config.sub_repos,
|
||||
parallelization: config.parallelization,
|
||||
@@ -1088,8 +1086,6 @@ function cmdInitPlanPhase(
|
||||
assertValidGranularityOverride(granularityOverride, error);
|
||||
const granularity = resolveGranularityInternal(cwd, 'planning', granularityOverride || undefined);
|
||||
|
||||
const wf = (config.workflow ?? {}) as Record<string, unknown>;
|
||||
|
||||
// #3188: see cmdInitExecutePhase — null when absent, parity with the
|
||||
// conditional sibling fields in this same result object.
|
||||
const statePath = path.join(planningDir(cwd), 'STATE.md');
|
||||
@@ -1101,11 +1097,11 @@ function cmdInitPlanPhase(
|
||||
planner_model: resolveModelInternal(cwd, 'gsd-planner'),
|
||||
checker_model: resolveModelInternal(cwd, 'gsd-plan-checker'),
|
||||
|
||||
tdd_mode: options['tdd'] || Boolean(wf['tdd_mode']) || false,
|
||||
tdd_mode: options['tdd'] || Boolean(config.tdd_mode) || false,
|
||||
granularity,
|
||||
research_enabled: wf['research'],
|
||||
research_enabled: config.research,
|
||||
plan_checker_enabled: config.plan_checker,
|
||||
nyquist_validation_enabled: wf['nyquist_validation'],
|
||||
nyquist_validation_enabled: config.nyquist_validation,
|
||||
commit_docs: config.commit_docs,
|
||||
text_mode: config.text_mode,
|
||||
auto_advance: !!(config.auto_advance),
|
||||
@@ -1376,15 +1372,13 @@ function cmdInitNewMilestone(cwd: string, raw: boolean, options: Record<string,
|
||||
// window filter (which also never excluded sentinels, unlike the owner).
|
||||
const phaseDirCount = listMilestonePhaseDirs(phasesDir, { cwd }).value.length;
|
||||
|
||||
const wf = (config.workflow ?? {}) as Record<string, unknown>;
|
||||
|
||||
const result: Record<string, unknown> = {
|
||||
researcher_model: resolveModelInternal(cwd, 'gsd-project-researcher'),
|
||||
synthesizer_model: resolveModelInternal(cwd, 'gsd-research-synthesizer'),
|
||||
roadmapper_model: resolveModelInternal(cwd, 'gsd-roadmapper'),
|
||||
|
||||
commit_docs: config.commit_docs,
|
||||
research_enabled: wf['research'],
|
||||
research_enabled: config.research,
|
||||
|
||||
// #3216 review Finding 2: `?? null` so an unresolved milestone still emits
|
||||
// the key with an explicit `null` rather than letting JSON.stringify drop
|
||||
@@ -1610,12 +1604,11 @@ function cmdInitOnboard(
|
||||
options: Record<string, unknown> = {},
|
||||
): void {
|
||||
const config = loadConfig(cwd);
|
||||
const workflowConfig = (config.workflow ?? {}) as Record<string, unknown>;
|
||||
const result = {
|
||||
...buildOnboardProjection(cwd, {
|
||||
commitDocs: !!config.commit_docs,
|
||||
fast: options['fast'] === true,
|
||||
textMode: options['text'] === true || !!config.text_mode || !!workflowConfig['text_mode'],
|
||||
textMode: options['text'] === true || !!config.text_mode,
|
||||
}),
|
||||
...getInitGitState(cwd),
|
||||
};
|
||||
@@ -3034,7 +3027,7 @@ function cmdInitTransition(cwd: string, raw: boolean, options: Record<string, un
|
||||
* location has one source instead of two kept in sync by hand.
|
||||
* - `debugger_model` — `resolveModelInternal`, which IS what `query
|
||||
* resolve-model --pick model` returns (`cmdResolveModel`, src/commands.cts).
|
||||
* - `tdd_mode` — the `Boolean(wf['tdd_mode'])` idiom `cmdInitExecutePhase` and
|
||||
* - `tdd_mode` — the `Boolean(config.tdd_mode)` idiom `cmdInitExecutePhase` and
|
||||
* `cmdInitPlanPhase` already use. `/gsd:debug` has no `--tdd` flag, so the
|
||||
* sibling handlers' `options['tdd'] ||` disjunct is deliberately omitted
|
||||
* rather than carried as a phantom.
|
||||
@@ -3052,7 +3045,6 @@ function cmdInitTransition(cwd: string, raw: boolean, options: Record<string, un
|
||||
*/
|
||||
function cmdInitDebug(cwd: string, raw: boolean, options: Record<string, unknown> = {}): void {
|
||||
const config = loadConfig(cwd);
|
||||
const wf = (config.workflow ?? {}) as Record<string, unknown>;
|
||||
|
||||
const result: Record<string, unknown> = {
|
||||
commit_docs: config.commit_docs,
|
||||
@@ -3061,7 +3053,7 @@ function cmdInitDebug(cwd: string, raw: boolean, options: Record<string, unknown
|
||||
// own cwd may differ from the orchestrator's.
|
||||
debug_dir: toPosixPath(planningPaths(cwd).debug),
|
||||
debugger_model: resolveModelInternal(cwd, 'gsd-debugger'),
|
||||
tdd_mode: Boolean(wf['tdd_mode']),
|
||||
tdd_mode: Boolean(config.tdd_mode),
|
||||
diagnose: options['diagnose'] === true,
|
||||
};
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
* Unsupported in this router:
|
||||
* - scaffold: routed through top-level scaffold command.
|
||||
*
|
||||
* CJS-only subcommands: mvp-mode (dispatched directly, before hub).
|
||||
* CJS-only subcommands: mvp-mode, tdd-applicable (dispatched directly, before hub).
|
||||
*
|
||||
* #3788: dispatch is mediated by CommandRoutingHub. The public entry point
|
||||
* and observable CLI behaviour are unchanged.
|
||||
@@ -32,6 +32,7 @@ const { createDefaultLogger, isAuditEnabled } = observabilityLogger;
|
||||
|
||||
interface PhaseHandlers {
|
||||
cmdPhaseMvpMode: (cwd: string, args: string[], raw: boolean) => void;
|
||||
cmdPhaseTddApplicable: (cwd: string, args: string[], raw: boolean) => void;
|
||||
cmdPhaseNextDecimal: (cwd: string, arg: string | undefined, raw: boolean) => void;
|
||||
cmdPhaseAdd: (cwd: string, desc: string, raw: boolean, customId: string | null) => void;
|
||||
cmdPhaseAddBatch: (cwd: string, descriptions: string[], raw: boolean) => void;
|
||||
@@ -87,6 +88,14 @@ function routePhaseCommand({ phase, args, cwd, raw, error }: RoutePhaseCommandOp
|
||||
return;
|
||||
}
|
||||
|
||||
// `tdd-applicable` (#4273): same CJS-native dispatch shape as `mvp-mode`
|
||||
// immediately above — a precedence cascade over plan/task/config sources
|
||||
// with its own typed JSON result, not routed through the SDK query layer.
|
||||
if (subcommand === 'tdd-applicable') {
|
||||
phase.cmdPhaseTddApplicable(cwd, args.slice(2), raw);
|
||||
return;
|
||||
}
|
||||
|
||||
// ── Build the CJS registry ──────────────────────────────────────────────────
|
||||
// Each handler receives a ctx object from the hub and must return a HubResult.
|
||||
const cjsRegistry = {
|
||||
@@ -240,14 +249,14 @@ function routePhaseCommand({ phase, args, cwd, raw, error }: RoutePhaseCommandOp
|
||||
// ── Build manifest (available subcommands for UnknownCommand detection) ─────
|
||||
// `availableSubcommands` is what the error message shows. It excludes
|
||||
// unsupported commands (already handled above) but does NOT include 'mvp-mode'
|
||||
// because it was absent from PHASE_SUBCOMMANDS in the original and was not
|
||||
// shown in the "Available:" list there either.
|
||||
// or 'tdd-applicable' (#4273) because both are absent from PHASE_SUBCOMMANDS
|
||||
// and were not shown in the "Available:" list there either.
|
||||
//
|
||||
// `manifestSubcommands` is the full routing set for the hub — it includes
|
||||
// 'mvp-mode' (which the original code routed via a handler even without a
|
||||
// manifest entry) so the hub's UnknownCommand check passes for it.
|
||||
// 'mvp-mode' and 'tdd-applicable' (both routed via a handler even without a
|
||||
// manifest entry) so the hub's UnknownCommand check passes for them.
|
||||
const availableSubcommands = PHASE_SUBCOMMANDS.filter(s => !UNSUPPORTED[s]);
|
||||
const manifestSubcommands = ['mvp-mode', ...availableSubcommands];
|
||||
const manifestSubcommands = ['mvp-mode', 'tdd-applicable', ...availableSubcommands];
|
||||
const manifest = { phase: manifestSubcommands };
|
||||
|
||||
// ── Construct hub ──────────────────────────────────────────────────────────
|
||||
|
||||
@@ -448,6 +448,62 @@ function cmdPhaseMvpMode(cwd: string, args: string[], raw: boolean): void {
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* `phase.tdd-applicable <plan-file> [--cli-flag]` (#4273, Phase 1 of epic
|
||||
* #4272) — resolves whether the TDD RED/GREEN/REFACTOR gate applies to a
|
||||
* given plan, in strict precedence order: an explicit `--cli-flag` wins over
|
||||
* the plan's own `type: tdd` frontmatter, which wins over any task in the
|
||||
* plan carrying `tdd="true"` (the #4265 mixed-mode shape), which wins over
|
||||
* the project-wide `workflow.tdd_mode` config default. Mirrors
|
||||
* `cmdPhaseMvpMode`'s precedence-cascade shape immediately above.
|
||||
*/
|
||||
function cmdPhaseTddApplicable(cwd: string, args: string[], raw: boolean): void {
|
||||
const planPath = args[0];
|
||||
if (!planPath) {
|
||||
error('Usage: phase.tdd-applicable <plan-file> [--cli-flag]', ERROR_REASON.USAGE);
|
||||
}
|
||||
|
||||
const resolvedPath = path.isAbsolute(planPath) ? planPath : path.join(cwd, planPath);
|
||||
if (!fs.existsSync(resolvedPath)) {
|
||||
error(`Plan file not found: ${planPath}`, ERROR_REASON.PHASE_NOT_FOUND);
|
||||
}
|
||||
|
||||
const cliFlagPresent = args.includes('--cli-flag');
|
||||
const content = fs.readFileSync(resolvedPath, 'utf-8');
|
||||
const doc = parsePlanDocument(content, resolvedPath);
|
||||
const planType = doc.type;
|
||||
const taskTddAttribute = doc.tasks.some((t) => t.tdd === 'true');
|
||||
const config = loadConfig(cwd);
|
||||
const configTddMode = Boolean(config.tdd_mode);
|
||||
|
||||
let applicable = false;
|
||||
let source = 'none';
|
||||
if (cliFlagPresent) {
|
||||
applicable = true;
|
||||
source = 'cli_flag';
|
||||
} else if (planType === 'tdd') {
|
||||
applicable = true;
|
||||
source = 'plan_frontmatter';
|
||||
} else if (taskTddAttribute) {
|
||||
applicable = true;
|
||||
source = 'task_attribute';
|
||||
} else if (configTddMode) {
|
||||
applicable = true;
|
||||
source = 'config';
|
||||
}
|
||||
|
||||
output(
|
||||
{
|
||||
applicable,
|
||||
source,
|
||||
plan_type: planType,
|
||||
config_tdd_mode: configTddMode,
|
||||
cli_flag_present: cliFlagPresent,
|
||||
},
|
||||
raw,
|
||||
);
|
||||
}
|
||||
|
||||
function cmdFindPhase(cwd: string, phase: string, raw: boolean): void {
|
||||
if (!phase) {
|
||||
error('phase identifier required');
|
||||
@@ -4520,6 +4576,7 @@ export = {
|
||||
cmdPhaseAdd,
|
||||
cmdPhaseAddBatch,
|
||||
cmdPhaseMvpMode,
|
||||
cmdPhaseTddApplicable,
|
||||
cmdPhaseInsert,
|
||||
cmdPhaseRemove,
|
||||
cmdPhaseComplete,
|
||||
|
||||
@@ -76,11 +76,20 @@ interface PlanTask {
|
||||
* normalises to null, same as every other optional attribute here).
|
||||
*/
|
||||
trackerId: string | null;
|
||||
/**
|
||||
* Verbatim `tdd` attribute value on a `<task>` opening tag (e.g. `"true"`),
|
||||
* or null. Never coerced to boolean — read exactly like `trackerId`. Null
|
||||
* for a checkpoint task (never read), an absent attribute, or an
|
||||
* empty-string value. #4273 (epic #4272, ADR-3473's 4th application).
|
||||
*/
|
||||
tdd: string | null;
|
||||
}
|
||||
|
||||
interface PlanDocument {
|
||||
/** `<objective>` body, else frontmatter `objective`, else null. */
|
||||
objective: string | null;
|
||||
/** Frontmatter `type` value, read verbatim (e.g. `"tdd"`, `"standard"`), or null when absent. #4273. */
|
||||
type: string | null;
|
||||
/** Frontmatter `wave` as an integer, or null when absent/unparseable. */
|
||||
declaredWave: number | null;
|
||||
dependsOn: string[];
|
||||
@@ -206,6 +215,7 @@ function parseXmlTasks(content: string): PlanTask[] {
|
||||
acceptanceCriteria: [],
|
||||
done: null,
|
||||
trackerId: null,
|
||||
tdd: null,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -218,6 +228,7 @@ function parseXmlTasks(content: string): PlanTask[] {
|
||||
acceptanceCriteria: splitCriteria(elementBody(block, 'acceptance_criteria')),
|
||||
done: collapseWhitespace(elementBody(block, 'done')),
|
||||
trackerId: tagAttribute(openTag, 'tracker-id'),
|
||||
tdd: tagAttribute(openTag, 'tdd'),
|
||||
};
|
||||
});
|
||||
}
|
||||
@@ -237,6 +248,7 @@ function parseMarkdownTasks(content: string): PlanTask[] {
|
||||
acceptanceCriteria: [],
|
||||
done: null,
|
||||
trackerId: null,
|
||||
tdd: null,
|
||||
}));
|
||||
}
|
||||
|
||||
@@ -324,8 +336,16 @@ function parsePlanDocument(content: string, planPath = ''): PlanDocument {
|
||||
agentHint = hintStr !== '' ? hintStr : null;
|
||||
}
|
||||
|
||||
let planType: string | null = null;
|
||||
const fmType = fm['type'];
|
||||
if (fmType !== undefined) {
|
||||
// eslint-disable-next-line @typescript-eslint/no-base-to-string -- FrontmatterValue scalar-to-string
|
||||
planType = String(fmType);
|
||||
}
|
||||
|
||||
return {
|
||||
objective: extractObjective(content) || (fm['objective'] as string | null) || null,
|
||||
type: planType,
|
||||
declaredWave,
|
||||
dependsOn,
|
||||
autonomous,
|
||||
|
||||
Reference in New Issue
Block a user