fix(3663): address CodeRabbit surface/layout follow-ups
This commit is contained in:
@@ -118,9 +118,9 @@ Valid cluster names: `core_loop`, `audit_review`, `milestone`, `research_ideate`
|
||||
# Claude Code
|
||||
RUNTIME_CONFIG_DIR=~/.claude/skills
|
||||
|
||||
# Resolve commandsDir and agentsDir
|
||||
COMMANDS_DIR=~/.claude/commands/gsd
|
||||
AGENTS_DIR=~/.claude/agents
|
||||
# Artifact destinations are derived from runtime layout
|
||||
# via resolveRuntimeArtifactLayout(runtime, RUNTIME_CONFIG_DIR, scope)
|
||||
# then applySurface(RUNTIME_CONFIG_DIR, layout, manifest, CLUSTERS)
|
||||
```
|
||||
|
||||
All paths can be overridden by reading the `CLAUDE_CONFIG_DIR` env var if set.
|
||||
|
||||
@@ -18,7 +18,7 @@ The root problem is the absence of a typed seam for "where does runtime R put ar
|
||||
- The `kinds` array is empty for runtimes with no GSD surface (a hypothetical future runtime with no integration). The `skills` kind is **absent** for runtimes that don't materialize skill directories (Cline; Gemini today). The `commands` kind is **absent** for runtimes that consume only the skills/agents layout (Claude global, Codex, etc.).
|
||||
- Per-runtime quirks live in the layout's record fields, not in caller branches:
|
||||
- **Hermes**: `{ kind: 'skills', destSubpath: 'skills/gsd', prefix: '' }` — preserves the nested namespace from #2841.
|
||||
- **Cline**: `kinds: [ { kind: 'commands', … } ]` — no skills kind in the array.
|
||||
- **Cline**: `kinds: []` — Cline resolves to zero kinds in Phase 1 (no `commands` kind).
|
||||
- **Gemini**: `kinds: [ { kind: 'commands', destSubpath: 'commands/gsd', prefix: 'gsd-' } ]` — no agents, no skills.
|
||||
- `applySurface` migrates from `(runtimeConfigDir, commandsDir, agentsDir, manifest, clusterMap)` to `(runtimeConfigDir, layout, manifest, clusterMap)`. Body collapses to `for (const kind of layout.kinds) _syncGsdDir(kind.stage(resolved), path.join(layout.configDir, kind.destSubpath), kind.kind)`.
|
||||
- `_findInstallSource` and `_findAgentsSource` in `surface.cjs` are removed. The layout owns source resolution.
|
||||
@@ -140,6 +140,7 @@ function applySurface(runtimeConfigDir, layout, manifest, clusterMap) {
|
||||
|
||||
Phase 1 implementation landed on `feat/3663-runtime-artifact-layout-module-phase-1-m`:
|
||||
- `get-shit-done/bin/lib/runtime-artifact-layout.cjs` — 15-runtime layout table (grok intentionally excluded), `resolveRuntimeArtifactLayout(runtime, configDir, scope) → Layout`, walk-up `findInstallSourceRoot` helper.
|
||||
- Clarification: in this Phase 1 implementation, **Cline resolves to zero kinds** (`kinds: []`), so it carries no `commands` kind in the layout table.
|
||||
- `get-shit-done/bin/lib/install-profiles.cjs` — new `stageSkillsForRuntimeAsSkills(srcCommandsDir, resolvedProfile, converter, prefix) → stagedDir` helper.
|
||||
- `get-shit-done/bin/lib/surface.cjs` — `applySurface(runtimeConfigDir, layout, manifest, clusterMap)` signature migration; `_findInstallSource` + `_findAgentsSource` deleted; `_syncGsdDir` extended to handle the `skills` kind via directory iteration.
|
||||
- Tests: `runtime-artifact-layout-resolve.test.cjs` (16), `runtime-artifact-layout-edge-cases.test.cjs` (10), `runtime-artifact-layout-stage.test.cjs` (5), `install-profiles-stage.test.cjs` (+7 new), `surface-apply.test.cjs` (updated 5 call sites + new skills-kind test).
|
||||
|
||||
@@ -204,7 +204,10 @@ function resolveSurface(runtimeConfigDir, manifest, clusterMap) {
|
||||
* @param {Object} [clusterMap]
|
||||
*/
|
||||
function applySurface(runtimeConfigDir, layout, manifest, clusterMap) {
|
||||
const resolved = resolveSurface(runtimeConfigDir, manifest, clusterMap);
|
||||
if (path.resolve(runtimeConfigDir) !== path.resolve(layout.configDir)) {
|
||||
throw new TypeError('applySurface runtimeConfigDir must match layout.configDir');
|
||||
}
|
||||
const resolved = resolveSurface(layout.configDir, manifest, clusterMap);
|
||||
for (const kind of layout.kinds) {
|
||||
const staged = kind.stage(resolved);
|
||||
const dest = path.join(layout.configDir, kind.destSubpath);
|
||||
|
||||
@@ -46,18 +46,20 @@ describe('applySurface', () => {
|
||||
});
|
||||
const manifest = loadSkillsManifest(REAL_COMMANDS_DIR);
|
||||
const layout = resolveRuntimeArtifactLayout('claude', runtimeConfigDir, 'local');
|
||||
applySurface(runtimeConfigDir, layout, manifest, CLUSTERS);
|
||||
const resolved = applySurface(runtimeConfigDir, layout, manifest, CLUSTERS);
|
||||
|
||||
const files = fs.readdirSync(commandsDir).filter(f => f.endsWith('.md'));
|
||||
// Every file should be a real stem we know about
|
||||
for (const file of files) {
|
||||
assert.ok(fs.existsSync(path.join(REAL_COMMANDS_DIR, file)), `unexpected file: ${file}`);
|
||||
}
|
||||
// At minimum core skills should be present
|
||||
const coreStems = ['new-project', 'discuss-phase', 'plan-phase', 'execute-phase', 'help', 'update'];
|
||||
for (const stem of coreStems) {
|
||||
assert.ok(files.includes(`${stem}.md`), `core skill "${stem}" should be in commandsDir`);
|
||||
}
|
||||
// Core profile should materialize exactly the resolved core command set.
|
||||
const expectedCore = [...resolved.skills].map(stem => `${stem}.md`).sort();
|
||||
assert.deepStrictEqual(
|
||||
[...files].sort(),
|
||||
expectedCore,
|
||||
'commandsDir should contain exactly core commands'
|
||||
);
|
||||
});
|
||||
|
||||
test('removes superseded files when profile shrinks', (t) => {
|
||||
@@ -84,17 +86,23 @@ describe('applySurface', () => {
|
||||
explicitAdds: [],
|
||||
explicitRemoves: [],
|
||||
});
|
||||
applySurface(runtimeConfigDir, layout, manifest, CLUSTERS);
|
||||
const resolvedCore = applySurface(runtimeConfigDir, layout, manifest, CLUSTERS);
|
||||
|
||||
const afterCore = new Set(fs.readdirSync(commandsDir).filter(f => f.endsWith('.md')));
|
||||
|
||||
// core should be a subset of standard
|
||||
assert.ok(afterCore.size <= afterStandard.size, 'core should have fewer or equal files than standard');
|
||||
|
||||
// Files removed should not be in core set
|
||||
const coreStems = new Set(['new-project', 'discuss-phase', 'plan-phase', 'execute-phase', 'help', 'update']);
|
||||
// Core profile should materialize exactly the resolved core command set.
|
||||
const expectedCore = [...resolvedCore.skills].map(stem => `${stem}.md`).sort();
|
||||
assert.deepStrictEqual(
|
||||
[...afterCore].sort(),
|
||||
expectedCore,
|
||||
'afterCore should contain exactly core commands'
|
||||
);
|
||||
|
||||
// All files should still map to known real skills.
|
||||
for (const file of afterCore) {
|
||||
const stem = file.slice(0, -3);
|
||||
assert.ok(
|
||||
fs.existsSync(path.join(REAL_COMMANDS_DIR, file)),
|
||||
`file in commandsDir not a real skill: ${file}`
|
||||
|
||||
Reference in New Issue
Block a user