Packet — Skills root resolution + diagnostic surface

Files touched

Path Action Reason
src/config.ts Edit Add COLIBRI_SKILLS_ROOT field + TSDoc.
src/startup.ts Edit New resolution chain + WARN diagnostic.
src/tools/health.ts Edit New COUNT_SKILLS_SQL, countSkills, schema field, payload field.
src/__tests__/config.test.ts Edit Cover env-var parse + reject branches.
src/__tests__/startup.test.ts Edit Cover resolution chain + WARN branches.
src/__tests__/tools/health.test.ts Edit Cover countSkills + skill_count in payload + E2E.
docs/audits/fix-skills-root-env-audit.md Already shipped (Step 1).  
docs/contracts/fix-skills-root-env-contract.md Already shipped (Step 2).  
docs/packets/fix-skills-root-env-packet.md This file (Step 3).  
docs/verification/fix-skills-root-env-verification.md Step 5 will add.  

No new files in src/. No new migration. No new MCP tool.

Implementation order

  1. src/config.ts — add field, run npm run build to confirm typecheck.
  2. src/startup.ts — resolve via the new chain, add WARN line.
  3. src/tools/health.ts — schema + helper + payload.
  4. Update src/__tests__/tools/health.test.ts 6-field key-set assertion to 7 fields (so existing tests don’t break before the new tests are added).
  5. src/__tests__/config.test.ts — add the three new cases.
  6. src/__tests__/startup.test.ts — add the resolution + WARN describes.
  7. src/__tests__/tools/health.test.ts — add the countSkills + payload cases.
  8. Run npm run build && npm run lint && npm test — must be all green.

Concrete diffs (literals)

src/config.ts

Insert after the COLIBRI_DB_PATH field (logical grouping: paths together):

/**
 * Optional override for the ε Skill Registry on-disk root.
 *
 * When set, replaces the default `process.cwd() + .agents/skills` path
 * that `loadSkillsFromDisk` (`src/domains/skills/repository.ts`) reads at
 * boot. Useful for production deployments started from a working
 * directory other than the repo root — the cwd-relative default would
 * silently load zero skills there, which is the C3 misconfiguration
 * surfaced by the post-R83 code review.
 *
 * Accepted values:
 *   - Absolute path (`/srv/colibri/skills`) — used verbatim.
 *   - Relative path (`./skills`) — resolved by `fs` against
 *     `process.cwd()` at the call site, identical to the default.
 *   - Empty string — rejected at module-load time by `min(1)`.
 *
 * Namespace: `COLIBRI_SKILLS_ROOT`. The donor `AMS_*` prefix is rejected
 * by `assertNoDonorNamespace`.
 */
COLIBRI_SKILLS_ROOT: z.string().min(1).optional(),

src/startup.ts

Replace (line 283-284):

const skillsRootPath =
  options.skillsRoot ?? pathJoin(process.cwd(), '.agents', 'skills');

with:

const skillsRootPath =
  options.skillsRoot ??
  config.COLIBRI_SKILLS_ROOT ??
  pathJoin(process.cwd(), '.agents', 'skills');

Add immediately after the existing [Startup] Skills: loaded=... line:

if (
  skillsResult.loaded === 0 &&
  skillsResult.total_on_disk === 0 &&
  ctx.mode === 'FULL'
) {
  logger(
    '[Startup] WARNING: 0 skills loaded — check COLIBRI_SKILLS_ROOT or process.cwd()',
  );
}

src/tools/health.ts

After COUNT_TABLES_SQL, add:

/**
 * SQL query — counts rows in the ε Skill Registry table.
 *
 * Returns 0 when the table does not exist (Phase 1 boot before the 004
 * migration), when the DB handle is closed/locked, or when the row shape
 * is unexpected. Same defensive contract as `countTables`.
 */
const COUNT_SKILLS_SQL = 'SELECT COUNT(*) AS c FROM skills';

Update healthPayloadSchema:

export const healthPayloadSchema = z.object({
  status: z.literal('ok'),
  version: z.string().min(1),
  uptime_ms: z.number().int().nonnegative(),
  db_tables: z.number().int().nonnegative(),
  skill_count: z.number().int().nonnegative(),
  phase: z.enum(['phase1', 'phase2']),
  mode: z.enum(RUNTIME_MODES),
});

Add countSkills after countTables:

export function countSkills(db: ColibriServerContext['db']): number {
  if (db === undefined) {
    return 0;
  }
  try {
    const row = db.prepare(COUNT_SKILLS_SQL).get() as
      | { c?: number }
      | undefined;
    return typeof row?.c === 'number' ? row.c : 0;
  } catch {
    return 0;
  }
}

Update buildHealthPayload:

export function buildHealthPayload(ctx: ColibriServerContext): HealthPayload {
  return {
    status: 'ok',
    version: ctx.version,
    uptime_ms: Math.floor(ctx.nowMs() - ctx.bootStartMs),
    db_tables: countTables(ctx.db),
    skill_count: countSkills(ctx.db),
    phase: ctx.phase ?? 'phase1',
    mode: ctx.mode,
  };
}

src/__tests__/config.test.ts

In the loadConfig describe block (around the existing positive cases), add:

it('parses COLIBRI_SKILLS_ROOT when set to an absolute path', () => {
  const cfg = loadConfig({
    NODE_ENV: 'test',
    COLIBRI_SKILLS_ROOT: '/srv/colibri/skills',
  });
  expect(cfg.COLIBRI_SKILLS_ROOT).toBe('/srv/colibri/skills');
});

it('parses COLIBRI_SKILLS_ROOT when set to a relative path', () => {
  const cfg = loadConfig({
    NODE_ENV: 'test',
    COLIBRI_SKILLS_ROOT: './custom/skills',
  });
  expect(cfg.COLIBRI_SKILLS_ROOT).toBe('./custom/skills');
});

it('leaves COLIBRI_SKILLS_ROOT undefined when env var is not set', () => {
  const cfg = loadConfig({ NODE_ENV: 'test' });
  expect(cfg.COLIBRI_SKILLS_ROOT).toBeUndefined();
});

it('rejects an empty COLIBRI_SKILLS_ROOT', () => {
  expect(() =>
    loadConfig({ NODE_ENV: 'test', COLIBRI_SKILLS_ROOT: '' }),
  ).toThrow(/COLIBRI_SKILLS_ROOT/);
});

it('freezes COLIBRI_SKILLS_ROOT into the returned Config', () => {
  const cfg = loadConfig({
    NODE_ENV: 'test',
    COLIBRI_SKILLS_ROOT: '/abs/path',
  });
  expect(Object.isFrozen(cfg)).toBe(true);
  expect(() => {
    (cfg as { COLIBRI_SKILLS_ROOT: string }).COLIBRI_SKILLS_ROOT = '/other';
  }).toThrow();
});

src/__tests__/startup.test.ts

Add a new describe block after the existing startup — happy path:

describe('startup — skills root resolution', () => {
  it('options.skillsRoot wins over config.COLIBRI_SKILLS_ROOT', async () => {
    const captured: string[] = [];
    const loadSkillsFn = (
      _db: Database.Database,
      root: string,
    ) => {
      captured.push(root);
      return { loaded: 0, skipped: 0, pruned: 0, total_on_disk: 0 };
    };
    await startup(
      defaultOptions({
        skillsRoot: '/explicit/option',
        loadSkillsFn,
      }),
    );
    expect(captured).toEqual(['/explicit/option']);
  });

  it('uses cwd default when neither options.skillsRoot nor env var is set', async () => {
    const captured: string[] = [];
    const loadSkillsFn = (
      _db: Database.Database,
      root: string,
    ) => {
      captured.push(root);
      return { loaded: 0, skipped: 0, pruned: 0, total_on_disk: 0 };
    };
    await startup(defaultOptions({ loadSkillsFn }));
    expect(captured).toHaveLength(1);
    expect(captured[0]).toContain('.agents');
    expect(captured[0]).toContain('skills');
  });
});

describe('startup — skills root via COLIBRI_SKILLS_ROOT (subprocess)', () => {
  // Hermetic subprocess — config validates env at module-load, so the only
  // way to prove the env var feeds into startup is to spawn a fresh node.
  it('passes COLIBRI_SKILLS_ROOT to loadSkillsFromDisk', () => {
    const cleanEnv: NodeJS.ProcessEnv = {};
    for (const [k, v] of Object.entries(process.env)) {
      if (k.startsWith('AMS_') || k.startsWith('COLIBRI_')) {
        continue;
      }
      if (v !== undefined) {
        cleanEnv[k] = v;
      }
    }
    const result = spawnSync(
      process.execPath,
      ['--import', 'tsx', SERVER_MODULE_FS_PATH],
      {
        env: {
          ...cleanEnv,
          NODE_ENV: 'test',
          COLIBRI_STARTUP_TIMEOUT_MS: '10000',
          COLIBRI_DB_PATH: path.join(
            process.env.TEMP ?? process.env.TMPDIR ?? '/tmp',
            `colibri-skills-root-${Date.now()}.db`,
          ),
          COLIBRI_SKILLS_ROOT: path.join(
            process.env.TEMP ?? process.env.TMPDIR ?? '/tmp',
            `colibri-empty-skills-${Date.now()}`,
          ),
        },
        encoding: 'utf8',
        timeout: 4000,
        input: '',
      },
    );
    const stderr = result.stderr ?? '';
    // The empty (non-existent) skills root produces total_on_disk=0,
    // loaded=0; in FULL mode the WARN line should fire.
    expect(stderr).toMatch(
      /\[Startup\] WARNING: 0 skills loaded — check COLIBRI_SKILLS_ROOT or process\.cwd\(\)/,
    );
  }, 15000);
});

describe('startup — empty-skills WARN diagnostic', () => {
  it('emits WARN when loaded=0 AND total_on_disk=0 AND mode=FULL', async () => {
    const { logger, logs } = makeLogger();
    await startup(
      defaultOptions({
        logger,
        loadSkillsFn: () => ({ loaded: 0, skipped: 0, pruned: 0, total_on_disk: 0 }),
        createOptions: { mode: 'FULL', installGlobalHandlers: false },
      }),
    );
    expect(logsAsString(logs)).toMatch(
      /\[Startup\] WARNING: 0 skills loaded — check COLIBRI_SKILLS_ROOT or process\.cwd\(\)/,
    );
  });

  it('does NOT emit WARN when total_on_disk > 0 (parser-error scenario)', async () => {
    const { logger, logs } = makeLogger();
    await startup(
      defaultOptions({
        logger,
        loadSkillsFn: () => ({ loaded: 0, skipped: 5, pruned: 0, total_on_disk: 5 }),
        createOptions: { mode: 'FULL', installGlobalHandlers: false },
      }),
    );
    expect(logsAsString(logs)).not.toMatch(/\[Startup\] WARNING:/);
  });

  it('does NOT emit WARN when mode is not FULL', async () => {
    const { logger, logs } = makeLogger();
    await startup(
      defaultOptions({
        logger,
        loadSkillsFn: () => ({ loaded: 0, skipped: 0, pruned: 0, total_on_disk: 0 }),
        createOptions: { mode: 'READONLY', installGlobalHandlers: false },
      }),
    );
    expect(logsAsString(logs)).not.toMatch(/\[Startup\] WARNING:/);
  });

  it('does NOT emit WARN when at least one skill loaded', async () => {
    const { logger, logs } = makeLogger();
    await startup(
      defaultOptions({
        logger,
        loadSkillsFn: () => ({ loaded: 3, skipped: 0, pruned: 0, total_on_disk: 3 }),
        createOptions: { mode: 'FULL', installGlobalHandlers: false },
      }),
    );
    expect(logsAsString(logs)).not.toMatch(/\[Startup\] WARNING:/);
  });
});

src/__tests__/tools/health.test.ts

Update the 6-field key set at L299-306:

expect(Object.keys(payload).sort()).toEqual(
  ['db_tables', 'mode', 'phase', 'skill_count', 'status', 'uptime_ms', 'version'].sort(),
);

Add describe('countSkills', ...) mirroring countTables describe block.

Add to existing buildHealthPayload describe:

it('returns skill_count=0 when ctx.db is undefined', () => {
  const ctx = makeCtx();
  expect(buildHealthPayload(ctx).skill_count).toBe(0);
});

it('returns skill_count=0 when DB has no skills table', () => {
  const ctx = makeCtx();
  const db = new Database(':memory:');
  try {
    ctx.db = db;
    expect(buildHealthPayload(ctx).skill_count).toBe(0);
  } finally {
    db.close();
  }
});

it('returns skill_count from a populated skills table', () => {
  const ctx = makeCtx();
  const db = new Database(':memory:');
  try {
    db.exec(`CREATE TABLE skills (
      name TEXT PRIMARY KEY,
      description TEXT NOT NULL DEFAULT '',
      version TEXT,
      entrypoint TEXT,
      capabilities TEXT NOT NULL DEFAULT '[]',
      greek_letter TEXT,
      body TEXT NOT NULL DEFAULT '',
      source_path TEXT NOT NULL DEFAULT '',
      frontmatter_json TEXT NOT NULL DEFAULT '{}',
      loaded_at TEXT NOT NULL DEFAULT ''
    )`);
    db.exec("INSERT INTO skills(name) VALUES ('alpha'),('beta'),('gamma')");
    ctx.db = db;
    expect(buildHealthPayload(ctx).skill_count).toBe(3);
  } finally {
    db.close();
  }
});

Add E2E case in describe 3:

it('returns skill_count reflecting a populated skills table', async () => {
  const db = new Database(':memory:');
  db.exec(`CREATE TABLE skills (
    name TEXT PRIMARY KEY,
    description TEXT NOT NULL DEFAULT '',
    version TEXT,
    entrypoint TEXT,
    capabilities TEXT NOT NULL DEFAULT '[]',
    greek_letter TEXT,
    body TEXT NOT NULL DEFAULT '',
    source_path TEXT NOT NULL DEFAULT '',
    frontmatter_json TEXT NOT NULL DEFAULT '{}',
    loaded_at TEXT NOT NULL DEFAULT ''
  )`);
  db.exec("INSERT INTO skills(name) VALUES ('alpha'),('beta')");
  const { client, cleanup } = await makeLinkedPair({
    register: (c) => {
      c.phase = 'phase2';
      c.db = db;
      registerHealthTool(c);
    },
  });
  try {
    const response = await client.callTool({
      name: 'server_health',
      arguments: {},
    });
    const data = (response.structuredContent as { data: HealthPayload }).data;
    expect(data.skill_count).toBe(2);
  } finally {
    await cleanup();
    db.close();
  }
});

Risks + mitigations

Risk Mitigation
Existing 6-field key-set assertion at L299-306 fails after schema bump Update to 7 fields in same commit as schema change.
WARN trips in unrelated startup tests using the no-op stub The default fixture uses mode: 'FULL' and stub returns total_on_disk: 0. The WARN line is benign (logger receives it; assertions look for specific other lines). Verify no test asserts log-line count or absence of WARN.
Subprocess test brittleness on Windows Mirror the existing startup — subprocess smoke pattern (already passes on Windows under flaky-load). Use path.join(TEMP, ...) for both DB and skills root.
Tests depend on a real skills table schema The countSkills and E2E tests construct an in-memory schema mirroring 004_skills.sql. The schema is small (10 columns, 1 index in production; tests use a minimal subset with NOT NULL DEFAULT for unused columns).

Gate

  • After Step 4 (implement) all three of npm run build && npm run lint && npm test pass.
  • After Step 5 (verify) the verification doc cites the green test count.
  • Commit signatures follow the table from CLAUDE.md §6 byte-for-byte.

Approval

This packet matches the contract in §C1-C7 and the audit’s acceptance check. Step 4 is unblocked.

Back to top

Colibri — documentation-first MCP runtime. Apache 2.0 + Commons Clause.

This site uses Just the Docs, a documentation theme for Jekyll.