golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

feat(#1132): complete contribution hook prerequisite

Browse Source 
Closes #1132
This commit is contained in:
Tom Boucher
2026-06-12 18:39:57 -04:00
committed by GitHub
parent 1d90ad3c30
commit fd01e7a12e
8 changed files with 347 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

67
scripts/gen-capability-registry.cjs
View File

@@ -970,6 +970,22 @@ function validateContribution(contrib, prefix) {
errors.push(prefix + '.into must be a string (agent role name)');
}
if (!Array.isArray(contrib.produces)) {
errors.push(prefix + '.produces must be an array');
} else {
for (const p of contrib.produces) {
if (typeof p !== 'string') errors.push(prefix + '.produces entries must be strings');
}
}
if (!Array.isArray(contrib.consumes)) {
errors.push(prefix + '.consumes must be an array');
} else {
for (const c of contrib.consumes) {
if (typeof c !== 'string') errors.push(prefix + '.consumes entries must be strings');
}
}
if (typeof contrib.fragment !== 'object' || contrib.fragment === null) {
errors.push(prefix + '.fragment must be an object with path or inline key');
} else {
@@ -978,6 +994,14 @@ function validateContribution(contrib, prefix) {
if (!hasPath && !hasInline) {
errors.push(prefix + '.fragment must have a "path" or "inline" key');
}
if (hasInline) {
const inline = contrib.fragment.inline;
if (typeof inline !== 'string') {
errors.push(prefix + '.fragment.inline must be a string');
} else if (inline === '') {
errors.push(prefix + '.fragment.inline must be a non-empty string');
}
}
// S1: fragment.path traversal guard — must be a relative path with no ".." segments
if (hasPath) {
const p = contrib.fragment.path;
@@ -1440,14 +1464,7 @@ function computeRequiresClosure(id, capMap) {
// ─── Topological ordering ─────────────────────────────────────────────────────
/**
* Topologically sort steps at a given point by produces/consumes.
* Capability-id tiebreak for determinism.
*
* @param {{ capId: string, step: object }[]} entries
* @returns {{ capId: string, step: object }[]}
*/
function topoSortSteps(entries) {
function topoSortHookEntries(entries, hookKey, hookKind) {
if (entries.length <= 1) return entries;
// Build adjacency: entry A must come before entry B if B consumes something A produces
@@ -1456,10 +1473,10 @@ function topoSortSteps(entries) {
const adj = Array.from({ length: n }, () => []);
for (let i = 0; i < n; i++) {
const producesI = new Set(entries[i].step.produces || []);
const producesI = new Set(entries[i][hookKey].produces || []);
for (let j = 0; j < n; j++) {
if (i === j) continue;
const consumesJ = entries[j].step.consumes || [];
const consumesJ = entries[j][hookKey].consumes || [];
for (const artifact of consumesJ) {
if (producesI.has(artifact)) {
adj[i].push(j);
@@ -1497,8 +1514,8 @@ function topoSortSteps(entries) {
if (result.length < n) {
const sortedIds = entries.map((e) => e.capId).join(', ');
throw new Error(
'produces/consumes cycle detected in steps at point "' +
(entries[0] && entries[0].step ? entries[0].step.point : '?') +
'produces/consumes cycle detected in ' + hookKind + ' at point "' +
(entries[0] && entries[0][hookKey] ? entries[0][hookKey].point : '?') +
'" among capabilities [' + sortedIds + ']: ' +
'a cycle in hook produces/consumes prevents deterministic ordering',
);
@@ -1506,6 +1523,21 @@ function topoSortSteps(entries) {
return result;
}
/**
* Topologically sort steps at a given point by produces/consumes.
* Capability-id tiebreak for determinism.
*
* @param {{ capId: string, step: object }[]} entries
* @returns {{ capId: string, step: object }[]}
*/
function topoSortSteps(entries) {
return topoSortHookEntries(entries, 'step', 'steps');
}
function topoSortContributions(entries) {
return topoSortHookEntries(entries, 'contrib', 'contributions');
}
// ─── ADR-857 Phase 4a: Derived views ─────────────────────────────────────────
// FIX 5 (lazy requires): paths are declared at top level but the actual require()
@@ -1948,14 +1980,9 @@ function buildRegistry(capMap) {
...e.step,
}));
// Contributions: group by into, then capability-id order within group
const contribs = pointContribs.get(point);
contribs.sort((a, b) => {
const intoCompare = a.contrib.into.localeCompare(b.contrib.into);
if (intoCompare !== 0) return intoCompare;
return a.capId.localeCompare(b.capId);
});
byLoopPoint[point].contributions = contribs.map((e) => ({
// Contributions: topological sort by produces/consumes, cap-id tiebreak
const sortedContribs = topoSortContributions(pointContribs.get(point));
byLoopPoint[point].contributions = sortedContribs.map((e) => ({
capId: e.capId,
...e.contrib,
}));

8
scripts/run-tests.cjs
View File

@@ -342,6 +342,9 @@ function main() {
const MAX_CMDLINE_CHARS = process.env.RUN_TESTS_MAX_CMDLINE_CHARS
? Number(process.env.RUN_TESTS_MAX_CMDLINE_CHARS)
: 28000; // headroom below the 32,767 Windows ceiling
const MAX_FILES_PER_CHUNK = process.env.RUN_TESTS_MAX_FILES_PER_CHUNK
? Number(process.env.RUN_TESTS_MAX_FILES_PER_CHUNK)
: 180;
// node:test does not exit until the event loop drains. A unit test that leaks
// an open handle (un-terminated Worker, un-killed child_process, ref'd timer)
@@ -362,7 +365,10 @@ function main() {
let currentLen = FIXED_OVERHEAD;
for (const file of selected) {
const add = file.length + 1; // +1 for the inter-arg separator
if (current.length > 0 && currentLen + add > MAX_CMDLINE_CHARS) {
if (
current.length > 0 &&
(currentLen + add > MAX_CMDLINE_CHARS || current.length >= MAX_FILES_PER_CHUNK)
) {
chunks.push(current);
current = [];
currentLen = FIXED_OVERHEAD;

24
src/loop-resolver.cts
View File

@@ -231,6 +231,7 @@ interface RawHook {
point?: unknown;
ref?: unknown;
into?: unknown;
fragment?: unknown;
produces?: unknown;
consumes?: unknown;
when?: unknown;
@@ -246,6 +247,7 @@ interface ActiveHook {
kind: HookKind;
ref?: HookRef;
into?: string;
fragment?: { inline?: string; path?: string };
when?: string;
produces?: string[];
consumes?: string[];
@@ -325,6 +327,15 @@ function resolveLoopHooks(input: ResolveLoopHooksInput): ResolveLoopHooksResult
return v.filter((x): x is string => typeof x === 'string');
}
function toFragment(v: unknown): { inline?: string; path?: string } | undefined {
if (!v || typeof v !== 'object' || Array.isArray(v)) return undefined;
const raw = v as Record<string, unknown>;
const fragment: { inline?: string; path?: string } = {};
if (typeof raw.inline === 'string') fragment.inline = raw.inline;
if (typeof raw.path === 'string') fragment.path = raw.path;
return Object.keys(fragment).length > 0 ? fragment : undefined;
}
// Process steps
const stepsRaw = entryMap['steps'];
const steps: RawHook[] = Array.isArray(stepsRaw) ? (stepsRaw as RawHook[]) : [];
@@ -356,12 +367,14 @@ function resolveLoopHooks(input: ResolveLoopHooksInput): ResolveLoopHooksResult
if (!isActive(hook)) continue;
const capId = typeof hook['capId'] === 'string' ? hook['capId'] : '';
const into = typeof hook['into'] === 'string' ? hook['into'] : undefined;
const fragment = toFragment(hook['fragment']);
const when = typeof hook['when'] === 'string' ? hook['when'] : undefined;
const produces = toStringArray(hook['produces']);
const consumes = toStringArray(hook['consumes']);
const onError = typeof hook['onError'] === 'string' ? hook['onError'] : undefined;
const active: ActiveHook = { capId, kind: 'contribution' };
if (into !== undefined) active.into = into;
if (fragment !== undefined) active.fragment = fragment;
if (when !== undefined) active.when = when;
if (produces.length > 0) active.produces = produces;
if (consumes.length > 0) active.consumes = consumes;
@@ -432,7 +445,12 @@ function renderLoopHooks(resolved: ResolveLoopHooksResult): string {
}
lines.push('');
} else if (hook.kind === 'contribution') {
lines.push(`<contribution from="${hook.capId}" into="${hook.into ?? '(unset)'}"/>`);
lines.push(`<contribution from="${hook.capId}" into="${hook.into ?? '(unset)'}">`);
if (hook.fragment?.inline) {
lines.push(hook.fragment.inline);
} else if (hook.fragment?.path) {
lines.push(`_Contribution fragment path is declared but not rendered by loop-resolver: ${hook.fragment.path}_`);
}
if (hook.produces && hook.produces.length > 0) {
lines.push(`- produces: ${hook.produces.join(', ')}`);
}
@@ -442,6 +460,10 @@ function renderLoopHooks(resolved: ResolveLoopHooksResult): string {
if (hook.when) {
lines.push(`- when: \`${hook.when}\``);
}
if (hook.onError) {
lines.push(`- onError: ${hook.onError}`);
}
lines.push('</contribution>');
lines.push('');
} else if (hook.kind === 'gate') {
let checkStr = '(none)';

182
tests/capability-registry.test.cjs
View File

@@ -373,6 +373,116 @@ describe('topological step ordering', () => {
assert.strictEqual(sorted[0].capId, 'a-cap', 'a-cap should come first (alphabetical tiebreak)');
assert.strictEqual(sorted[1].capId, 'z-cap');
});
test('contributions at one point use produces/consumes dependency order', () => {
const capMap = new Map([
['a-consumer', {
id: 'a-consumer',
role: 'feature',
title: 'Consumer',
tier: 'full',
requires: [],
skills: [],
agents: [],
hooks: [],
config: {},
steps: [],
contributions: [{
point: 'plan:pre',
into: 'planner',
fragment: { inline: 'Consume produced planning note.' },
produces: [],
consumes: ['PLAN-NOTE.md'],
onError: 'skip',
}],
gates: [],
}],
['b-producer', {
id: 'b-producer',
role: 'feature',
title: 'Producer',
tier: 'full',
requires: [],
skills: [],
agents: [],
hooks: [],
config: {},
steps: [],
contributions: [{
point: 'plan:pre',
into: 'planner',
fragment: { inline: 'Produce planning note.' },
produces: ['PLAN-NOTE.md'],
consumes: [],
onError: 'skip',
}],
gates: [],
}],
]);
const registry = buildRegistry(capMap);
assert.deepEqual(
registry.byLoopPoint['plan:pre'].contributions.map((c) => c.capId),
['b-producer', 'a-consumer'],
);
});
test('contribution produces/consumes cycle throws a clear error', () => {
const capMap = new Map([
['cap-a', {
id: 'cap-a',
role: 'feature',
title: 'A',
tier: 'full',
requires: [],
skills: [],
agents: [],
hooks: [],
config: {},
steps: [],
contributions: [{
point: 'plan:pre',
into: 'planner',
fragment: { inline: 'A.' },
produces: ['A.md'],
consumes: ['B.md'],
onError: 'skip',
}],
gates: [],
}],
['cap-b', {
id: 'cap-b',
role: 'feature',
title: 'B',
tier: 'full',
requires: [],
skills: [],
agents: [],
hooks: [],
config: {},
steps: [],
contributions: [{
point: 'plan:pre',
into: 'planner',
fragment: { inline: 'B.' },
produces: ['B.md'],
consumes: ['A.md'],
onError: 'skip',
}],
gates: [],
}],
]);
assert.throws(
() => buildRegistry(capMap),
(err) => {
assert.ok(err instanceof Error);
assert.match(err.message, /contributions/);
assert.match(err.message, /cycle/);
return true;
},
);
});
});
// ─── 4. --check drift detection ──────────────────────────────────────────────
@@ -1011,6 +1121,78 @@ describe('S1: fragment.path traversal guard', () => {
assert.ok(errors.length > 0, 'Expected rejection for empty path');
assert.ok(errors.some((e) => e.includes('fragment.path')));
});
test('non-string fragment.inline is rejected', () => {
const cap = {
...UI_CAP,
contributions: [
{
point: 'plan:pre',
into: 'planner',
fragment: { inline: 42 },
when: 'workflow.ui_phase',
onError: 'skip',
},
],
};
const errors = validateCapability(cap, 'ui');
assert.ok(errors.some((e) => e.includes('fragment.inline') && e.includes('string')));
});
test('empty fragment.inline string is rejected', () => {
const cap = {
...UI_CAP,
contributions: [
{
point: 'plan:pre',
into: 'planner',
fragment: { inline: '' },
when: 'workflow.ui_phase',
onError: 'skip',
},
],
};
const errors = validateCapability(cap, 'ui');
assert.ok(errors.some((e) => e.includes('fragment.inline') && e.includes('non-empty')));
});
test('non-array contribution produces is rejected', () => {
const cap = {
...UI_CAP,
contributions: [
{
point: 'plan:pre',
into: 'planner',
fragment: { inline: 'Plan with UI context.' },
produces: 'PLAN-NOTE.md',
consumes: [],
when: 'workflow.ui_phase',
onError: 'skip',
},
],
};
const errors = validateCapability(cap, 'ui');
assert.ok(errors.some((e) => e.includes('produces') && e.includes('array')));
});
test('non-string contribution consumes entry is rejected', () => {
const cap = {
...UI_CAP,
contributions: [
{
point: 'plan:pre',
into: 'planner',
fragment: { inline: 'Plan with UI context.' },
produces: [],
consumes: [42],
when: 'workflow.ui_phase',
onError: 'skip',
},
],
};
const errors = validateCapability(cap, 'ui');
assert.ok(errors.some((e) => e.includes('consumes entries') && e.includes('strings')));
});
});
// ─── 8. Security: prototype pollution (S2) ────────────────────────────────────

30
tests/loop-render-hooks.test.cjs
View File

@@ -650,12 +650,42 @@ describe('renderLoopHooks', () => {
capId: 'contrib-cap',
kind: 'contribution',
into: 'planner',
fragment: { inline: 'Apply the project-specific planning guardrails.' },
}],
};
const rendered = renderLoopHooks(resolved);
assert.match(rendered, /contribution/);
assert.match(rendered, /contrib-cap/);
assert.match(rendered, /planner/);
assert.match(rendered, /Apply the project-specific planning guardrails\./);
assert.match(rendered, /<contribution from="contrib-cap" into="planner">/);
assert.match(rendered, /<\/contribution>/);
assert.doesNotMatch(rendered, /<contribution[^>]+\/>/);
});
test('resolveLoopHooks preserves contribution fragment data', () => {
const registry = makeRegistry({
point: 'plan:pre',
contributions: [{
capId: 'contrib-cap',
point: 'plan:pre',
into: 'planner',
fragment: { inline: 'Use artifact-backed evidence.' },
produces: ['PLAN-NOTES.md'],
consumes: ['CONTEXT.md'],
when: 'workflow.contrib',
onError: 'halt',
}],
configSchema: {
'workflow.contrib': { type: 'boolean', default: true, description: 'Enable test contribution.' },
},
});
const resolved = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
assert.strictEqual(resolved.activeHooks.length, 1);
assert.deepEqual(resolved.activeHooks[0].fragment, { inline: 'Use artifact-backed evidence.' });
assert.deepEqual(resolved.activeHooks[0].produces, ['PLAN-NOTES.md']);
assert.deepEqual(resolved.activeHooks[0].consumes, ['CONTEXT.md']);
assert.strictEqual(resolved.activeHooks[0].onError, 'halt');
});
test('gate hook renders check, blocking, onError', () => {

24
tests/run-tests-harness.test.cjs
View File

@@ -314,6 +314,30 @@ test('ambient GSD workstream vars are stripped by the runner', () => {
`expected chunking marker in stderr; STDERR (tail):\n${r.stderr.split('\n').slice(-20).join('\n')}`,
);
});
test('chunks by file count even when argv length is below the ceiling', () => {
const names = Array.from({ length: 7 }, (_, i) => `tiny-${String(i).padStart(2, '0')}.test.cjs`);
seed(tmpDir, names);
const r = runHarness(tmpDir, [], {
RUN_TESTS_MAX_CMDLINE_CHARS: '100000',
RUN_TESTS_MAX_FILES_PER_CHUNK: '3',
});
assert.strictEqual(
r.status,
0,
`expected zero exit; got status=${r.status} signal=${r.signal}\nSTDERR:\n${r.stderr}`,
);
assert.match(
r.stderr,
/run-tests: chunk 1\/3 — 3 files/,
`expected file-count chunking marker in stderr; STDERR:\n${r.stderr}`,
);
assert.match(
r.stderr,
/run-tests: chunk 3\/3 — 1 files/,
`expected final file-count chunking marker in stderr; STDERR:\n${r.stderr}`,
);
});
});
describe('per-chunk timeout + force-exit (windows hang guard, #1051)', () => {

28
tests/tsconfig-noemit.test.cjs Normal file
View File

@@ -0,0 +1,28 @@
'use strict';
const { test } = require('node:test');
const assert = require('node:assert/strict');
const path = require('node:path');
const { spawnSync } = require('node:child_process');
test('root tsconfig supports the default no-emit typecheck command', () => {
const root = path.join(__dirname, '..');
const tscBin = path.join(root, 'node_modules', 'typescript', 'bin', 'tsc');
const result = spawnSync(process.execPath, [tscBin, '--noEmit'], {
cwd: root,
encoding: 'utf8',
});
assert.equal(
result.status,
0,
[
'Expected the default root TypeScript typecheck to pass.',
'stdout:',
result.stdout,
'stderr:',
result.stderr,
].join('\n'),
);
});

8
tsconfig.json
View File

@@ -1,4 +1,8 @@
{
"files": [],
"references": []
"//": "Default editor/CI typecheck config. The emitting publish build stays in tsconfig.build.json.",
"extends": "./tsconfig.build.json",
"compilerOptions": {
"noEmit": true
},
"include": ["src/**/*.cts"]
}