Code review High Finding C3: a Colibri server started from any directory other
than the repo root silently loads zero skills. The resolution site at
src/startup.ts:284 is hardcoded to path.join(process.cwd(), '.agents', 'skills').
loadSkillsFromDisk returns {loaded: 0, ...} and Phase 2 proceeds. The
operator has no diagnostic surface short of calling skill_list and observing
an empty array.
The options.skillsRoot test seam exists already (line 109-110) — only the
operator-facing override is missing.
Config schema (insertion site)
Path
Lines
Purpose
src/config.ts
43-118
Phase 0 environment Zod schema. New COLIBRI_SKILLS_ROOT field belongs alongside COLIBRI_DB_PATH (similar shape: optional path string).
src/config.ts
65, 79
Pattern of z.string().min(1).optional() already used for COLIBRI_WEBHOOK_URL (URL flavor) and ANTHROPIC_API_KEY.
Health surface (insertion site for skill_count)
Path
Lines
Purpose
src/tools/health.ts
48-50
COUNT_TABLES_SQL constant — model for new COUNT_SKILLS_SQL.
src/tools/health.ts
72-79
healthPayloadSchema — current 6 fields. New skill_count adds field 7.
src/tools/health.ts
100-112
countTables(db) — pattern model for new countSkills(db). Try/catch returns 0 on missing table or closed handle.
src/tools/health.ts
130-139
buildHealthPayload(ctx) — assembly site for the new field.
Skills storage
Path
Purpose
src/db/migrations/004_skills.sql
Defines the skills table, name TEXT PRIMARY KEY, plus idx_skills_greek index. The COUNT query target.
src/domains/skills/repository.ts
loadSkillsFromDisk(db, root, logger) returns {loaded, skipped, pruned, total_on_disk} — already structured for the diagnostic warning.
Test fixtures already aware of the loader signature
Path
Lines
Detail
src/__tests__/startup.test.ts
230
loadSkillsFn: () => ({ loaded: 0, skipped: 0, pruned: 0, total_on_disk: 0 }) — the no-op stub used by every existing startup test. New tests for COLIBRI_SKILLS_ROOT use the same stub but assert the path argument.
src/__tests__/tools/health.test.ts
299-306
Existing 6-field key assertion expect(Object.keys(payload).sort()).toEqual(['db_tables', 'mode', 'phase', 'status', 'uptime_ms', 'version'].sort()) — must update to include skill_count.
src/__tests__/tools/health.test.ts
528-529
Existing loadSkillsFn: () => ({ loaded: 0, skipped: 0, pruned: 0, total_on_disk: 0 }) — already structured to allow assertions of skill_count: 0 from a DB without the skills table.
Acceptance check (against task spec)
Criterion
Path of change
Verdict
1. COLIBRI_SKILLS_ROOT optional in config schema
src/config.ts
Path clear; z.string().min(1).optional() precedent at L65, L79.
5. Three new test surfaces (config, startup, health)
existing test files
Co-locate with existing config/startup/health tests.
6. npm run build && npm run lint && npm test green
repo gates
Standard.
Constraints honored
14-tool MCP surface unchanged — skill_count is a payload field on the
existing server_health tool.
No new SQL migration — the skills table already exists at
src/db/migrations/004_skills.sql.
countSkills follows the countTables defensive contract: returns 0 if
table is missing (Phase 1 startup-before-migration), DB is closed, or row
shape is unexpected.
100ms SLA preserved — SELECT COUNT(*) AS c FROM skills is a single index
COUNT (~<1ms).
Non-scope
Renaming loadSkillsFromDisk or its return shape.
Adding a new MCP tool (e.g. skill_count_tool) — the task explicitly
forbids this.
Validation of relative-vs-absolute path semantics for COLIBRI_SKILLS_ROOT
— the consumer (fs.readdirSync) accepts both. We accept any non-empty
string and let the OS error propagate at call time, consistent with how
COLIBRI_DB_PATH is treated.
Resolving relative paths against any specific anchor — process.cwd() is
the implicit anchor for relative paths, identical to how a bare
.agents/skills works today.
Diagnostic for a SET-but-empty COLIBRI_SKILLS_ROOT directory — covered by
the same WARN line: if the operator points at an empty dir,
total_on_disk === 0 still trips and the message still prints.