Feat(ship): add configurable PR body sections (#3391)
* feat: add configurable ship PR body sections * chore: add changeset for ship PR sections * docs: avoid prompt scanner trigger in PR body guide * docs: escape pr body source separator
This commit is contained in:
@@ -389,6 +389,43 @@ describe('configSet', () => {
|
||||
const raw = JSON.parse(await readFile(join(tmpDir, '.planning', 'config.json'), 'utf-8'));
|
||||
expect(raw.commit_docs).toBe(true);
|
||||
});
|
||||
|
||||
it('validates ship.pr_body_sections arrays (#3167)', async () => {
|
||||
const { configSet } = await import('./config-mutation.js');
|
||||
await writeFile(
|
||||
join(tmpDir, '.planning', 'config.json'),
|
||||
JSON.stringify({}),
|
||||
);
|
||||
|
||||
const sections = JSON.stringify([
|
||||
{
|
||||
heading: 'Risks & Rollback',
|
||||
enabled: true,
|
||||
source: 'PLAN.md ## Risks || PLAN.md ## Rollback',
|
||||
fallback: '- Rollback: revert this PR.',
|
||||
},
|
||||
{
|
||||
heading: 'Stakeholder Sign-off',
|
||||
enabled: false,
|
||||
template: '- Product owner: {phase_name}',
|
||||
},
|
||||
]);
|
||||
const result = await configSet(['ship.pr_body_sections', sections], tmpDir);
|
||||
expect((result.data as { updated: boolean }).updated).toBe(true);
|
||||
|
||||
const raw = JSON.parse(await readFile(join(tmpDir, '.planning', 'config.json'), 'utf-8'));
|
||||
expect(raw.ship.pr_body_sections).toHaveLength(2);
|
||||
expect(raw.ship.pr_body_sections[1].enabled).toBe(false);
|
||||
|
||||
await expect(configSet(
|
||||
['ship.pr_body_sections', JSON.stringify([{ heading: 'Bad', enabled: 'yes', fallback: '- item' }])],
|
||||
tmpDir,
|
||||
)).rejects.toThrow(/enabled/);
|
||||
await expect(configSet(
|
||||
['ship.pr_body_sections', JSON.stringify([{ heading: 'Bad', template: '- {unknown}' }])],
|
||||
tmpDir,
|
||||
)).rejects.toThrow(/Unsupported template token/);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── configSetModelProfile ─────────────────────────────────────────────────
|
||||
@@ -452,6 +489,22 @@ describe('configNewProject', () => {
|
||||
expect(raw.commit_docs).toBe(true);
|
||||
});
|
||||
|
||||
it('validates ship.pr_body_sections choices before writing config', async () => {
|
||||
const { configNewProject } = await import('./config-mutation.js');
|
||||
const choices = JSON.stringify({
|
||||
ship: {
|
||||
pr_body_sections: [
|
||||
{
|
||||
heading: 'Invalid source',
|
||||
source: 'package.json ## Scripts',
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
|
||||
await expect(configNewProject([choices], tmpDir)).rejects.toThrow(GSDError);
|
||||
});
|
||||
|
||||
it('does not overwrite existing config', async () => {
|
||||
const { configNewProject } = await import('./config-mutation.js');
|
||||
await writeFile(
|
||||
|
||||
@@ -72,6 +72,81 @@ const CONFIG_KEY_SUGGESTIONS: Record<string, string> = {
|
||||
'plan_checker': 'workflow.plan_check',
|
||||
};
|
||||
|
||||
const SHIP_PR_BODY_SECTION_KEYS = new Set(['heading', 'enabled', 'source', 'fallback', 'template']);
|
||||
const SHIP_PR_BODY_TEMPLATE_TOKENS = new Set([
|
||||
'phase_number',
|
||||
'phase_name',
|
||||
'phase_dir',
|
||||
'base_branch',
|
||||
'padded_phase',
|
||||
]);
|
||||
const SHIP_PR_BODY_SOURCE_RE = /^(ROADMAP|PLAN|SUMMARY|VERIFICATION|STATE|REQUIREMENTS|CONTEXT)\.md\s+##\s+[^\r\n#][^\r\n]*$/;
|
||||
|
||||
function validateShipPrBodySections(value: unknown): void {
|
||||
if (!Array.isArray(value)) {
|
||||
throw new GSDError(
|
||||
'Invalid ship.pr_body_sections value. Expected a JSON array of section objects.',
|
||||
ErrorClassification.Validation,
|
||||
);
|
||||
}
|
||||
|
||||
value.forEach((section, index) => {
|
||||
const prefix = `Invalid ship.pr_body_sections[${index}]`;
|
||||
if (!section || typeof section !== 'object' || Array.isArray(section)) {
|
||||
throw new GSDError(`${prefix}. Expected an object.`, ErrorClassification.Validation);
|
||||
}
|
||||
|
||||
const record = section as Record<string, unknown>;
|
||||
const unknownKeys = Object.keys(record).filter((key) => !SHIP_PR_BODY_SECTION_KEYS.has(key));
|
||||
if (unknownKeys.length > 0) {
|
||||
throw new GSDError(`${prefix}. Unknown field(s): ${unknownKeys.join(', ')}.`, ErrorClassification.Validation);
|
||||
}
|
||||
|
||||
if (typeof record.heading !== 'string' || record.heading.trim() === '') {
|
||||
throw new GSDError(`${prefix}. heading must be a non-empty string.`, ErrorClassification.Validation);
|
||||
}
|
||||
if (/[\r\n]/.test(record.heading)) {
|
||||
throw new GSDError(`${prefix}. heading must be a single line.`, ErrorClassification.Validation);
|
||||
}
|
||||
|
||||
if ('enabled' in record && typeof record.enabled !== 'boolean') {
|
||||
throw new GSDError(`${prefix}. enabled must be true or false.`, ErrorClassification.Validation);
|
||||
}
|
||||
|
||||
for (const field of ['source', 'fallback', 'template']) {
|
||||
if (field in record && typeof record[field] !== 'string') {
|
||||
throw new GSDError(`${prefix}. ${field} must be a string.`, ErrorClassification.Validation);
|
||||
}
|
||||
}
|
||||
|
||||
const hasContent = ['source', 'fallback', 'template'].some((field) => {
|
||||
return typeof record[field] === 'string' && record[field].trim() !== '';
|
||||
});
|
||||
if (!hasContent) {
|
||||
throw new GSDError(`${prefix}. Provide at least one of source, fallback, or template.`, ErrorClassification.Validation);
|
||||
}
|
||||
|
||||
if (typeof record.source === 'string' && record.source.trim() !== '') {
|
||||
const selectors = record.source.split('||').map((selector) => selector.trim()).filter(Boolean);
|
||||
if (selectors.length === 0 || selectors.some((selector) => !SHIP_PR_BODY_SOURCE_RE.test(selector))) {
|
||||
throw new GSDError(
|
||||
`${prefix}. source must use selectors like "PLAN.md ## Risks", separated with "||".`,
|
||||
ErrorClassification.Validation,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
if (typeof record.template === 'string') {
|
||||
const tokens = record.template.matchAll(/\{([a-zA-Z][a-zA-Z0-9_]*)\}/g);
|
||||
for (const match of tokens) {
|
||||
if (!SHIP_PR_BODY_TEMPLATE_TOKENS.has(match[1])) {
|
||||
throw new GSDError(`${prefix}. Unsupported template token: {${match[1]}}.`, ErrorClassification.Validation);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// ─── isValidConfigKey ─────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
@@ -215,6 +290,10 @@ export const configSet: QueryHandler = async (args, projectDir, workstream) => {
|
||||
);
|
||||
}
|
||||
|
||||
if (keyPath === 'ship.pr_body_sections') {
|
||||
validateShipPrBodySections(parsedValue);
|
||||
}
|
||||
|
||||
// D6: Lock protection for read-modify-write (match CJS config.cjs:296)
|
||||
const paths = planningPaths(projectDir, workstream);
|
||||
const lockPath = await acquireStateLock(paths.config);
|
||||
@@ -395,6 +474,9 @@ export const configNewProject: QueryHandler = async (args, projectDir, workstrea
|
||||
code_review: true,
|
||||
code_review_depth: 'standard',
|
||||
},
|
||||
ship: {
|
||||
pr_body_sections: [],
|
||||
},
|
||||
hooks: {
|
||||
context_warnings: true,
|
||||
},
|
||||
@@ -419,6 +501,11 @@ export const configNewProject: QueryHandler = async (args, projectDir, workstrea
|
||||
...((globalDefaults.workflow as Record<string, unknown>) || {}),
|
||||
...((userChoices.workflow as Record<string, unknown>) || {}),
|
||||
},
|
||||
ship: {
|
||||
...(defaults.ship as Record<string, unknown>),
|
||||
...((globalDefaults.ship as Record<string, unknown>) || {}),
|
||||
...((userChoices.ship as Record<string, unknown>) || {}),
|
||||
},
|
||||
hooks: {
|
||||
...(defaults.hooks as Record<string, unknown>),
|
||||
...((globalDefaults.hooks as Record<string, unknown>) || {}),
|
||||
@@ -436,6 +523,9 @@ export const configNewProject: QueryHandler = async (args, projectDir, workstrea
|
||||
},
|
||||
};
|
||||
|
||||
const ship = config.ship as Record<string, unknown>;
|
||||
validateShipPrBodySections(ship.pr_body_sections);
|
||||
|
||||
await atomicWriteConfig(paths.config, config);
|
||||
|
||||
return { data: { created: true, path: paths.config } };
|
||||
|
||||
@@ -45,6 +45,7 @@ export const VALID_CONFIG_KEYS: ReadonlySet<string> = new Set([
|
||||
'workflow.security_block_on',
|
||||
'workflow.drift_threshold',
|
||||
'workflow.drift_action',
|
||||
'ship.pr_body_sections',
|
||||
'git.branching_strategy', 'git.base_branch', 'git.phase_branch_template', 'git.milestone_branch_template', 'git.quick_branch_template',
|
||||
'planning.commit_docs', 'planning.search_gitignored', 'planning.sub_repos',
|
||||
'review.ollama_host', 'review.lm_studio_host', 'review.llama_cpp_host',
|
||||
|
||||
Reference in New Issue
Block a user