feat(#1416): formalize Resolution convention + agent-skills value envelope (Resolution Provenance P3) (#1425)
Narrows P3 of ADR-1411 (Resolution Provenance, epic #1411) based on an adversarial fit-analysis that showed a single Resolution<T> envelope adopted by agent-skills, capability-state, and capability-writer fails the deletion test: configured/reason are meaningless for capability verbs, and capability-writer's errors[] (operation-not-applied) cannot fold into warnings[]. The only genuinely shared seam is warnings: string[]. Changes: - src/resolution.cts: new pure types+builder leaf — exports Resolution<T> {value, configured, reason, warnings}, makeResolution<T>() builder, and AgentSkillsValue {block, skills_count}. No other src/ imports. - src/init.cts: cmdAgentSkills --json IR gains additive value:{block, skills_count} field (built via makeResolution). All existing flat fields (agent_type, block, skills_count, warnings, configured, reason, source, degraded) are retained unchanged for back-compat. - src/capability-state.cts: doc comment on ResolveCapabilityRuntimeStateResult naming it the canonical read-verb envelope. No JSON change. - src/capability-writer.cts: doc comment on SetCapabilityStateResult naming it the canonical mutation-verb result (warnings=advisory, errors=operation- not-applied). No JSON change. - CONTEXT.md: new ### Resolution Convention glossary entry after ### Resolution Provenance. - docs/adr/1411-resolution-provenance.md: P3 narrowing amendment appended. - tests/resolution.test.cjs: 9 unit tests for makeResolution (new). - tests/agent-skills.test.cjs: 2 P3 tests for value.block/value.skills_count and back-compat of all flat fields. All 277 tests pass (5 suites). npm run lint clean. All lint checks pass. Part of #1411 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
@@ -1570,4 +1570,53 @@ describe('agent-skills — Resolution Provenance (#1415)', () => {
|
||||
`Should emit WARNING for empty-string configured_empty, got stderr: ${r.stderr}`,
|
||||
);
|
||||
});
|
||||
|
||||
// ─── Resolution Convention P3 (#1416) ────────────────────────────────────────
|
||||
// The --json IR gains an additive `value: { block, skills_count }` field
|
||||
// (Resolution<AgentSkillsValue> envelope). All existing flat fields are retained
|
||||
// for back-compat. RED: value field absent before build; GREEN: after build:lib.
|
||||
|
||||
test('P3 (#1416): --json IR includes value.block and value.skills_count matching flat fields (back-compat)', () => {
|
||||
const skillDir = path.join(tmpDir, 'skills', 'p3-skill');
|
||||
fs.mkdirSync(skillDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(skillDir, 'SKILL.md'), '# P3 Skill\n');
|
||||
writeConfig(tmpDir, {
|
||||
agent_skills: { 'gsd-executor': ['skills/p3-skill'] },
|
||||
});
|
||||
const r = runAgentSkillsJson(['agent-skills', 'gsd-executor'], tmpDir);
|
||||
assert.ok(r.success, `Command failed: ${r.error}`);
|
||||
|
||||
// value field must exist and be an object
|
||||
assert.ok(r.ir.value !== undefined && r.ir.value !== null, 'ir.value must be present (Resolution<AgentSkillsValue>)');
|
||||
assert.strictEqual(typeof r.ir.value, 'object', 'ir.value must be an object');
|
||||
|
||||
// value.block must match flat block
|
||||
assert.strictEqual(r.ir.value.block, r.ir.block, 'value.block must match flat block field');
|
||||
assert.ok(r.ir.value.block.includes('<agent_skills>'), 'value.block must contain <agent_skills>');
|
||||
|
||||
// value.skills_count must match flat skills_count
|
||||
assert.strictEqual(r.ir.value.skills_count, r.ir.skills_count, 'value.skills_count must match flat skills_count field');
|
||||
assert.strictEqual(r.ir.value.skills_count, 1, 'value.skills_count must be 1 for one configured path');
|
||||
|
||||
// All existing flat fields must still be present (back-compat)
|
||||
assert.strictEqual(typeof r.ir.agent_type, 'string', 'flat agent_type must still be present');
|
||||
assert.strictEqual(typeof r.ir.block, 'string', 'flat block must still be present');
|
||||
assert.strictEqual(typeof r.ir.skills_count, 'number', 'flat skills_count must still be present');
|
||||
assert.ok(Array.isArray(r.ir.warnings), 'flat warnings must still be present');
|
||||
assert.strictEqual(typeof r.ir.configured, 'boolean', 'flat configured must still be present');
|
||||
assert.strictEqual(typeof r.ir.reason, 'string', 'flat reason must still be present');
|
||||
assert.ok('source' in r.ir, 'flat source must still be present');
|
||||
assert.ok('degraded' in r.ir, 'flat degraded must still be present');
|
||||
});
|
||||
|
||||
test('P3 (#1416): value.block and value.skills_count are consistent when unconfigured', () => {
|
||||
// No config → not_configured; value must still be present with empty block and 0 count
|
||||
const r = runAgentSkillsJson(['agent-skills', 'gsd-executor'], tmpDir);
|
||||
assert.ok(r.success, `Command failed: ${r.error}`);
|
||||
assert.ok(r.ir.value !== undefined, 'ir.value must be present even when unconfigured');
|
||||
assert.strictEqual(r.ir.value.block, r.ir.block, 'value.block must match flat block (empty)');
|
||||
assert.strictEqual(r.ir.value.skills_count, r.ir.skills_count, 'value.skills_count must match flat skills_count (0)');
|
||||
assert.strictEqual(r.ir.value.block, '', 'value.block must be empty when unconfigured');
|
||||
assert.strictEqual(r.ir.value.skills_count, 0, 'value.skills_count must be 0 when unconfigured');
|
||||
});
|
||||
});
|
||||
|
||||
89
tests/resolution.test.cjs
Normal file
89
tests/resolution.test.cjs
Normal file
@@ -0,0 +1,89 @@
|
||||
/**
|
||||
* Unit tests for src/resolution.cts → gsd-core/bin/lib/resolution.cjs
|
||||
*
|
||||
* Tests the `makeResolution` builder and the `Resolution<T>` envelope shape
|
||||
* introduced for ADR-1411 P3 (Resolution Provenance, #1416).
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const path = require('path');
|
||||
|
||||
const RESOLUTION_PATH = path.join(__dirname, '../gsd-core/bin/lib/resolution.cjs');
|
||||
|
||||
describe('resolution module — makeResolution builder', () => {
|
||||
test('module loads and exports makeResolution', () => {
|
||||
const mod = require(RESOLUTION_PATH);
|
||||
assert.ok(mod, 'module must be truthy');
|
||||
assert.strictEqual(typeof mod.makeResolution, 'function', 'makeResolution must be a function');
|
||||
});
|
||||
|
||||
test('makeResolution builds the Resolution<T> shape', () => {
|
||||
const { makeResolution } = require(RESOLUTION_PATH);
|
||||
const result = makeResolution(
|
||||
{ block: '<agent_skills />', skills_count: 1 },
|
||||
{ configured: true, reason: 'resolved', warnings: [] },
|
||||
);
|
||||
assert.ok(result, 'result must be truthy');
|
||||
assert.ok('value' in result, 'result must have value field');
|
||||
assert.ok('configured' in result, 'result must have configured field');
|
||||
assert.ok('reason' in result, 'result must have reason field');
|
||||
assert.ok('warnings' in result, 'result must have warnings field');
|
||||
});
|
||||
|
||||
test('makeResolution preserves the value as-is', () => {
|
||||
const { makeResolution } = require(RESOLUTION_PATH);
|
||||
const value = { block: '<agent_skills>test</agent_skills>', skills_count: 2 };
|
||||
const result = makeResolution(value, { configured: true, reason: 'resolved', warnings: [] });
|
||||
assert.deepStrictEqual(result.value, value, 'value must be preserved');
|
||||
assert.strictEqual(result.value.block, '<agent_skills>test</agent_skills>');
|
||||
assert.strictEqual(result.value.skills_count, 2);
|
||||
});
|
||||
|
||||
test('makeResolution carries configured field', () => {
|
||||
const { makeResolution } = require(RESOLUTION_PATH);
|
||||
const trueResult = makeResolution({ block: '', skills_count: 0 }, { configured: true, reason: 'configured_empty', warnings: [] });
|
||||
const falseResult = makeResolution({ block: '', skills_count: 0 }, { configured: false, reason: 'not_configured', warnings: [] });
|
||||
assert.strictEqual(trueResult.configured, true, 'configured:true must be preserved');
|
||||
assert.strictEqual(falseResult.configured, false, 'configured:false must be preserved');
|
||||
});
|
||||
|
||||
test('makeResolution carries reason field', () => {
|
||||
const { makeResolution } = require(RESOLUTION_PATH);
|
||||
const reasons = ['resolved', 'not_configured', 'configured_empty', 'configured_unresolved'];
|
||||
for (const reason of reasons) {
|
||||
const result = makeResolution({ block: '', skills_count: 0 }, { configured: false, reason, warnings: [] });
|
||||
assert.strictEqual(result.reason, reason, `reason '${reason}' must be preserved`);
|
||||
}
|
||||
});
|
||||
|
||||
test('makeResolution carries warnings array', () => {
|
||||
const { makeResolution } = require(RESOLUTION_PATH);
|
||||
const warnings = ['path /foo/bar not found', 'path /baz not found'];
|
||||
const result = makeResolution({ block: '', skills_count: 0 }, { configured: true, reason: 'configured_unresolved', warnings });
|
||||
assert.deepStrictEqual(result.warnings, warnings, 'warnings must be preserved');
|
||||
assert.strictEqual(result.warnings.length, 2);
|
||||
});
|
||||
|
||||
test('makeResolution with empty warnings array', () => {
|
||||
const { makeResolution } = require(RESOLUTION_PATH);
|
||||
const result = makeResolution({ block: '<agent_skills />', skills_count: 1 }, { configured: true, reason: 'resolved', warnings: [] });
|
||||
assert.deepStrictEqual(result.warnings, [], 'empty warnings array must be preserved');
|
||||
});
|
||||
|
||||
test('makeResolution works with non-AgentSkillsValue generics (string)', () => {
|
||||
const { makeResolution } = require(RESOLUTION_PATH);
|
||||
const result = makeResolution('hello', { configured: true, reason: 'resolved', warnings: [] });
|
||||
assert.strictEqual(result.value, 'hello', 'string value must be preserved');
|
||||
assert.strictEqual(result.configured, true);
|
||||
});
|
||||
|
||||
test('makeResolution result has exactly the four envelope fields plus value', () => {
|
||||
const { makeResolution } = require(RESOLUTION_PATH);
|
||||
const result = makeResolution(42, { configured: false, reason: 'not_configured', warnings: [] });
|
||||
const keys = Object.keys(result).sort();
|
||||
assert.deepStrictEqual(keys, ['configured', 'reason', 'value', 'warnings'], 'envelope must have exactly 4 fields');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user