Compare commits

..
Author SHA1 Message Date
Garry TanandClaude Fable 5 7b8676be3e ci: raise unit-test matrix shard timeout 15 -> 20 min
Shard 4 runs ~14.5 min on master (dream.test.ts at ~29s/test dominates
its wallclock) and hit the 15-min job timeout twice on this PR with
1328 tests passing and 0 failing — the gate then failed on
'gated job did not succeed (got cancelled)'. Real fix is re-mining
scripts/test-weights.json to rebalance the shards; this unblocks CI.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-22 10:59:25 -07:00
SinabinaandClaude Fable 5 b8376f7327 fix(doctor): stop treating ClawHub workspace skills as required gbrain-routable skills (#1767)
ClawHub-installed skills (detected via .clawhub/origin.json) are external
runtime integrations, not gbrain resolver skills. The derived manifest now
skips them, so doctor resolver_health no longer hard-fails with
unreachable/mece_gap on e.g. an email integration skill. A ClawHub skill
opts back into strict checking by declaring triggers: in its SKILL.md
frontmatter (the same surface that makes it routable); an explicit
manifest.json listing also keeps strict checks. Also keeps dry-fix from
rewriting externally-managed skill files.

Fixes #1767

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-21 14:27:57 -07:00
10 changed files with 126 additions and 252 deletions
+5 -1
View File
@@ -206,7 +206,11 @@ jobs:
needs: cache-check
if: needs.cache-check.outputs.hit != 'true'
runs-on: ubuntu-latest
timeout-minutes: 15
# 20 (was 15): shard 4 runs ~14.5 min on master (dream.test.ts ~29s/test
# dominates it) and hits the 15-min ceiling on slower runners, cancelling
# mid-run with 0 test failures. Rebalancing via
# scripts/mine-shard-weights.ts is the real fix; this stops the bleeding.
timeout-minutes: 20
strategy:
fail-fast: false
matrix:
+1 -42
View File
@@ -38,7 +38,6 @@ import { logSelfUpgrade } from '../core/audit/self-upgrade-audit.ts';
import { detectInstallMethod } from './upgrade.ts';
import { evaluateQuietHours } from '../core/minions/quiet-hours.ts';
import { inspectLock } from '../core/db-lock.ts';
import { registerCleanup } from '../core/process-cleanup.ts';
/**
* v0.37.7.0 #1162 — classify autopilot reconnect-loop errors.
@@ -434,37 +433,6 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) {
let stopping = false;
let childSupervisor: ChildWorkerSupervisor | null = null;
// #1872: graceful engine shutdown. On PGLite the cycle steps run INLINE in
// this process, so a hard `process.exit` mid-write (systemctl stop →
// SIGTERM) kills WASM Postgres with the WAL dirty and can corrupt the
// brain. Two exit paths must both close the engine:
// - autopilot's own shutdown() below (owns SIGINT + internal stops like
// max_crashes / cycle-failure-cap), and
// - process-cleanup's SIGTERM handler (installed at cli.ts module load;
// it runs the cleanup registry with a 3s deadline and then exits) —
// which is why closeEngine is ALSO registered there.
// closeEngine aborts the in-flight inline cycle (runCycle checks the
// signal between phases and threads it into phase sub-work), gives it a
// short bounded window to wind down, then disconnects. PGLite's
// disconnect() drains the pending query and checkpoints before closing;
// a second call is a no-op (disconnect snapshots + nulls the handle), so
// both paths firing is safe.
const shutdownAbort = new AbortController();
let inflightInlineCycle: Promise<unknown> | null = null;
const closeEngine = async () => {
shutdownAbort.abort(new Error('autopilot shutdown'));
if (inflightInlineCycle) {
// ponytail: 2s cap keeps us inside process-cleanup's 3s deadline; a
// between-phase abort resolves instantly, a mid-phase one may not.
await Promise.race([
inflightInlineCycle.catch(() => { /* cycle errors already logged by the loop */ }),
new Promise((r) => setTimeout(r, 2_000)),
]);
}
try { await engine.disconnect(); } catch { /* best-effort */ }
};
const deregisterEngineClose = registerCleanup('autopilot-engine-close', closeEngine);
if (spawnManagedWorker) {
const cliPath = resolveGbrainCliPath();
// Cgroup-aware auto-sized RSS watchdog cap (issue #1678). The old flat
@@ -552,10 +520,6 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) {
childSupervisor.killChild('SIGKILL');
}
}
// #1872: abort the in-flight inline cycle and close the engine BEFORE
// process.exit — a hard exit mid-write corrupts PGLite's WASM Postgres.
await closeEngine();
deregisterEngineClose();
try { unlinkSync(lockPath); } catch { /* already gone */ }
process.exit(0);
};
@@ -1044,21 +1008,16 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) {
// path's phase set). Now both converge on the same primitive.
try {
const { runCycle } = await import('../core/cycle.ts');
// #1872: track the promise so closeEngine can drain it on shutdown,
// and pass the abort signal so the cycle winds down between phases.
const cyclePromise = runCycle(engine, {
const report = await runCycle(engine, {
brainDir: repoPath,
// Autopilot daemon path: pulls by default (matches
// pre-v0.17 autopilot behavior). CLI dream defaults false
// for cron safety; that choice is scoped to dream only.
pull: true,
signal: shutdownAbort.signal,
yieldBetweenPhases: async () => {
await new Promise(r => setImmediate(r));
},
});
inflightInlineCycle = cyclePromise;
const report = await cyclePromise.finally(() => { inflightInlineCycle = null; });
// Only 'failed' (every attempted phase failed) trips the autopilot
// circuit breaker. 'partial' means at least one phase warned or
// failed while others ran — that's a soft signal, not a fatal
+3 -23
View File
@@ -26,7 +26,6 @@
import type { BrainEngine } from '../core/engine.ts';
import {
runCycle,
resolveSourceForDir,
ALL_PHASES,
type CyclePhase,
type CycleReport,
@@ -381,9 +380,9 @@ Options:
--source <id> Scope the cycle to one source so doctor's
cycle_freshness check sees a fresh stamp on
completion. When omitted, gbrain derives the
source from --dir / the configured checkout
when it matches a source's local_path (#1869).
completion. Without this, gbrain dream's
timestamp never lands and federated brains
see "stale cycle" forever.
--source-id <id> Alias for --source. Matches the v0.37.7.0+
naming used by import/extract/graph-query.
@@ -635,25 +634,6 @@ export async function runDream(engine: BrainEngine | null, args: string[]): Prom
);
process.exit(1);
}
// #1869: a path-scoped run (--dir, or the configured sync.repo_path) whose
// directory matches a registered source's local_path IS that source's cycle
// — derive the source id so runCycle writes last_source_cycle_at /
// last_full_cycle_at on success and doctor's cycle_freshness check stops
// reading perpetually stale. Explicit --source still wins (resolved above).
// Fixed here at the command level, NOT in runCycle's stamp gate, so legacy
// global callers (autopilot-global-maintenance runs GLOBAL_PHASES with a
// brainDir and no sourceId) can't falsely stamp per-source freshness.
// A derived match on an archived source is skipped silently (falls back to
// legacy unscoped behavior) — stamping it would mask staleness on restore,
// mirroring the explicit --source archived guard above.
if (resolvedSourceId === undefined && engine !== null && brainDir !== null) {
const derived = await resolveSourceForDir(engine, brainDir);
if (derived !== undefined) {
const src = await fetchSource(engine, derived);
if (src?.archived !== true) resolvedSourceId = derived;
}
}
// ─── issue #1678: bounded single-hold extract_atoms drain ──────────
if (opts.drain) {
if (engine === null) {
+1 -9
View File
@@ -855,16 +855,8 @@ interface SyncPhaseResult extends PhaseResult {
* Resolve the source id for a brain directory by looking up the sources
* table. Returns undefined when no registered source matches (falls back
* to pre-v0.18 global config.sync.* keys).
*
* Exported for dream.ts (#1869): a `gbrain dream --dir <path>` run whose
* path matches a registered source's local_path is a per-source cycle in
* everything but name, so dream derives the source id up front and passes
* it as opts.sourceId — landing the freshness stamp without changing
* runCycle's stamp/lock semantics for legacy global callers (the
* autopilot-global-maintenance handler runs GLOBAL_PHASES with a brainDir
* and MUST NOT stamp per-source freshness; see rejected PR #2549).
*/
export async function resolveSourceForDir(
async function resolveSourceForDir(
engine: BrainEngine,
brainDir: string | null,
): Promise<string | undefined> {
+34 -1
View File
@@ -23,6 +23,15 @@
* hold conventions and shared rule files, not skills. Files like
* `_brain-filing-rules.md` live at the root and are not considered
* skills by either loader.
*
* ClawHub-installed workspace skills (#1767): a skill dir carrying
* `.clawhub/origin.json` is an externally-managed runtime integration
* (e.g. an email or catalog skill), not a gbrain-routable skill. The
* derive path SKIPS those so `gbrain doctor` resolver_health doesn't
* hard-fail on them — UNLESS the skill's SKILL.md frontmatter declares
* `triggers:`, which is the explicit opt-in to gbrain routing (and the
* same surface that makes it reachable). An explicit manifest.json that
* lists a ClawHub skill also keeps strict checking (verbatim path).
*/
import { existsSync, readFileSync, readdirSync, statSync } from 'fs';
@@ -60,9 +69,27 @@ function parseSkillName(skillMdPath: string): string | null {
}
}
/**
* Does the SKILL.md frontmatter declare a `triggers:` key? A ClawHub-
* installed skill that ships gbrain `triggers:` has explicitly opted in
* to gbrain routing and gets full resolver checks (#1767).
*/
function declaresTriggers(skillMdPath: string): boolean {
try {
const content = readFileSync(skillMdPath, 'utf-8');
const fmMatch = content.match(/^---\n([\s\S]*?)\n---/);
if (!fmMatch) return false;
return /^triggers:/m.test(fmMatch[1]);
} catch {
return false;
}
}
/**
* Walk skillsDir, return every `<skillsDir>/<dir>/SKILL.md` as a
* ManifestEntry. Dotfile and underscore-prefixed dirs are skipped.
* ManifestEntry. Dotfile and underscore-prefixed dirs are skipped, as
* are ClawHub-installed external skills that haven't opted in to gbrain
* routing via `triggers:` frontmatter (#1767).
*/
function deriveManifest(skillsDir: string): ManifestEntry[] {
const out: ManifestEntry[] = [];
@@ -93,6 +120,12 @@ function deriveManifest(skillsDir: string): ManifestEntry[] {
const skillMd = join(subdirAbs, 'SKILL.md');
if (!existsSync(skillMd)) continue;
// ClawHub-installed external skill (#1767): skip unless it opts in
// to gbrain routing by declaring `triggers:` in its frontmatter.
if (existsSync(join(subdirAbs, '.clawhub', 'origin.json')) && !declaresTriggers(skillMd)) {
continue;
}
const frontmatterName = parseSkillName(skillMd);
const name = frontmatterName && frontmatterName !== '' ? frontmatterName : entry;
out.push({ name, path: `${entry}/SKILL.md` });
@@ -1,63 +0,0 @@
/**
* #1872 — autopilot SIGTERM/SIGINT must close the engine before exit.
*
* On PGLite the cycle steps run INLINE in the autopilot process, so a hard
* `process.exit` mid-write (systemctl stop → SIGTERM) kills WASM Postgres
* with the WAL dirty and can corrupt the brain. Two exit paths must both
* close the engine:
*
* - autopilot's own shutdown() (owns SIGINT + internal stops like
* max_crashes / cycle-failure-cap), and
* - process-cleanup's SIGTERM handler (installed at cli.ts module load,
* which exits within its 3s cleanup deadline) — reached via the
* registered 'autopilot-engine-close' cleanup callback.
*
* Because the shutdown path is deep inside `runAutopilot()` (a long-running
* daemon loop that ends in process.exit), a behavioral test would have to
* spawn + signal a real daemon. Following the established precedent
* (test/autopilot-supervisor-wiring.test.ts, test/autopilot-fanout-wiring.test.ts),
* these static-shape regressions pin the load-bearing wiring instead.
*/
import { describe, expect, it } from 'bun:test';
import { readFileSync } from 'fs';
import { join } from 'path';
const AUTOPILOT_SRC = readFileSync(
join(import.meta.dir, '..', 'src', 'commands', 'autopilot.ts'),
'utf8',
);
describe('autopilot.ts graceful engine shutdown (#1872)', () => {
it('registers an engine-close callback in the process-cleanup registry (SIGTERM path)', () => {
// process-cleanup owns SIGTERM (installed at cli.ts:10) and hard-exits
// after its cleanup pass; without this registration the engine is never
// closed on `systemctl stop`.
expect(AUTOPILOT_SRC).toContain(
"import { registerCleanup } from '../core/process-cleanup.ts';",
);
expect(AUTOPILOT_SRC).toContain(
"registerCleanup('autopilot-engine-close', closeEngine)",
);
});
it('closeEngine aborts the in-flight inline cycle then disconnects the engine', () => {
// Abort first (runCycle checks the signal between phases and threads it
// into phase sub-work), bounded drain, then disconnect.
expect(AUTOPILOT_SRC).toMatch(
/const closeEngine = async \(\) => \{[\s\S]{0,900}shutdownAbort\.abort\([\s\S]{0,900}engine\.disconnect\(\)/,
);
});
it('the inline runCycle call carries the shutdown abort signal and is tracked as in-flight', () => {
// PGLite / --inline path: the cycle runs in-process, so shutdown must be
// able to (a) signal it to wind down and (b) await it before closing.
expect(AUTOPILOT_SRC).toMatch(/signal:\s*shutdownAbort\.signal/);
expect(AUTOPILOT_SRC).toMatch(/inflightInlineCycle\s*=\s*cyclePromise/);
});
it('shutdown() awaits closeEngine() before process.exit(0) (SIGINT + internal-stop path)', () => {
expect(AUTOPILOT_SRC).toMatch(
/await closeEngine\(\);[\s\S]{0,400}process\.exit\(0\)/,
);
});
});
+29
View File
@@ -382,6 +382,35 @@ describe("DRY detection — checkResolvable", () => {
});
});
describe("#1767 — ClawHub workspace skills are not resolver-required", () => {
let dir: string;
afterEachCleanup(() => dir && rmSync(dir, { recursive: true, force: true }));
test("ClawHub skill without gbrain metadata produces no unreachable/mece_gap", () => {
dir = mkdtempSync(join(tmpdir(), "gbrain-clawhub-"));
// Native gbrain skill: routable via frontmatter triggers. No manifest.json
// (the OpenClaw derive path from the issue repro).
mkdirSync(join(dir, "query"), { recursive: true });
writeFileSync(
join(dir, "query", "SKILL.md"),
`---\nname: query\ndescription: test\ntriggers:\n - "what do we know"\n---\n\n# query\n`
);
// ClawHub-installed integration: no triggers, no resolver row.
mkdirSync(join(dir, "agentmail", ".clawhub"), { recursive: true });
writeFileSync(
join(dir, "agentmail", ".clawhub", "origin.json"),
JSON.stringify({ registry: "https://clawhub.ai", slug: "agentmail" })
);
writeFileSync(join(dir, "agentmail", "SKILL.md"), `---\nname: agentmail\ndescription: email integration\n---\n\n# agentmail\n`);
const report = checkResolvable(dir);
const agentmailIssues = report.issues.filter(i => i.skill === "agentmail");
expect(agentmailIssues).toEqual([]);
expect(report.ok).toBe(true);
expect(report.summary.total_skills).toBe(1);
});
});
describe("v0.22.4 regression — actual repo skills/ has 0 errors", () => {
test("repo skills/ pass check-resolvable cleanly (zero errors AND zero warnings)", () => {
// The v0.22.4 (Part A) contract was zero warnings AND zero errors.
-99
View File
@@ -1,99 +0,0 @@
/**
* #1869 — `gbrain dream --dir <path>` stamps cycle freshness when the path
* matches a registered source's local_path.
*
* Pre-fix, only `--source <id>` runs wrote last_source_cycle_at /
* last_full_cycle_at (runCycle's stamp gate reads opts.sourceId, and dream
* never derived one from --dir), so a path-scoped brain showed doctor's
* cycle_freshness as perpetually stale.
*
* The fix lives in dream.ts (derive the source id from the resolved brain
* dir via resolveSourceForDir), NOT in runCycle's stamp gate — a runCycle-
* wide change would make the autopilot-global-maintenance handler (global
* phases, brainDir set, no sourceId) falsely stamp per-source freshness
* (the #2194 poisoning class; see rejected PR #2549).
*
* Same real-PGLite/no-mocks discipline as test/dream.test.ts; same
* GBRAIN_HOME isolation as test/cycle-last-full-cycle-at.test.ts (the
* cycle's PGLite file lock lives under ~/.gbrain).
*/
import { describe, test, expect, beforeAll, afterAll, beforeEach, afterEach } from 'bun:test';
import { mkdtempSync, rmSync } from 'fs';
import { join } from 'path';
import { tmpdir } from 'os';
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
import { resetPgliteState } from './helpers/reset-pglite.ts';
import { runDream } from '../src/commands/dream.ts';
import { withEnv } from './helpers/with-env.ts';
let engine: PGLiteEngine;
let brainDir: string;
let gbrainHome: string;
beforeAll(async () => {
engine = new PGLiteEngine();
await engine.connect({});
await engine.initSchema();
}, 60_000);
afterAll(async () => {
await engine.disconnect();
});
beforeEach(async () => {
await resetPgliteState(engine);
brainDir = mkdtempSync(join(tmpdir(), 'gbrain-dream-stamp-'));
gbrainHome = mkdtempSync(join(tmpdir(), 'gbrain-dream-stamp-home-'));
}, 60_000);
afterEach(() => {
rmSync(brainDir, { recursive: true, force: true });
rmSync(gbrainHome, { recursive: true, force: true });
});
async function seedSource(id: string, archived = false): Promise<void> {
await engine.executeRaw(
`INSERT INTO sources (id, name, local_path, config, archived, created_at)
VALUES ($1, $2, $3, '{}'::jsonb, $4, NOW())`,
[id, id, brainDir, archived],
);
}
async function readLastFullCycleAt(sourceId: string): Promise<string | null> {
const rows = await engine.executeRaw<{ config: Record<string, unknown> | null }>(
`SELECT config FROM sources WHERE id = $1`,
[sourceId],
);
const raw = rows[0]?.config?.last_full_cycle_at;
return typeof raw === 'string' ? raw : null;
}
describe('gbrain dream --dir <path> freshness stamp (#1869)', () => {
test('--dir matching a source local_path stamps last_full_cycle_at', async () => {
await withEnv({ GBRAIN_HOME: gbrainHome }, async () => {
await seedSource('path-scoped');
expect(await readLastFullCycleAt('path-scoped')).toBeNull();
const report = await runDream(engine, ['--dir', brainDir, '--phase', 'lint', '--json']);
expect(report).toBeTruthy();
if (report) expect(['ok', 'clean']).toContain(report.status);
// Pre-fix this stays null forever: dream never passed a sourceId, so
// runCycle's stamp gate skipped the write.
expect(await readLastFullCycleAt('path-scoped')).not.toBeNull();
});
}, 60_000);
test('--dir matching an ARCHIVED source does not stamp it', async () => {
await withEnv({ GBRAIN_HOME: gbrainHome }, async () => {
await seedSource('mothballed', true);
const report = await runDream(engine, ['--dir', brainDir, '--phase', 'lint', '--json']);
expect(report).toBeTruthy();
// Stamping an archived source would mask data staleness when it is
// later restored (mirrors the explicit --source archived guard).
expect(await readLastFullCycleAt('mothballed')).toBeNull();
});
}, 60_000);
});
+4 -14
View File
@@ -562,22 +562,12 @@ describe('runDream — --source / --source-id (v0.41.13)', () => {
// ─── Back-compat: bare `gbrain dream` does NOT write per-source stamp ─
test('gbrain dream (no --source) stamps only the source whose local_path matches --dir (#1869)', async () => {
// Pre-#1869 this asserted NO source was ever stamped without an explicit
// --source — which is exactly the bug: a path-scoped `gbrain dream --dir`
// run never landed a freshness stamp and doctor's cycle_freshness stayed
// stale forever. New truth: the source whose local_path matches the
// resolved brain dir is derived and stamped; unrelated sources stay
// untouched (cross-source isolation).
await seedSource('alpha'); // local_path = repo → derived + stamped
await engine.executeRaw(
`INSERT INTO sources (id, name, local_path, config, archived, created_at)
VALUES ($1, $2, $3, '{}'::jsonb, false, NOW())`,
['beta', 'beta', '/somewhere/else'],
);
test('gbrain dream (no --source) leaves all sources untouched (back-compat regression)', async () => {
await seedSource('alpha');
await seedSource('beta');
const report = await runDream(engine, ['--dir', repo, '--phase', 'lint', '--json']);
expect(report).toBeTruthy();
expect(await readLastFullCycleAt('alpha')).not.toBeNull();
expect(await readLastFullCycleAt('alpha')).toBeNull();
expect(await readLastFullCycleAt('beta')).toBeNull();
}, 60_000);
+49
View File
@@ -166,6 +166,55 @@ describe('loadOrDeriveManifest', () => {
expect(r.skills.map(s => s.name)).toEqual(['apple', 'mango', 'zebra']);
});
// #1767 — ClawHub-installed workspace skills are external integrations,
// not gbrain-routable skills. The derive path skips them unless they
// opt in via `triggers:` frontmatter.
it('skips ClawHub-origin skills without triggers frontmatter (#1767)', () => {
const dir = scratch();
writeSkill(dir, 'query', 'query');
writeSkill(dir, 'agentmail', 'agentmail');
mkdirSync(join(dir, 'agentmail', '.clawhub'), { recursive: true });
writeFileSync(
join(dir, 'agentmail', '.clawhub', 'origin.json'),
JSON.stringify({ registry: 'https://clawhub.ai', slug: 'agentmail' })
);
const r = loadOrDeriveManifest(dir);
expect(r.derived).toBe(true);
expect(r.skills.map(s => s.name)).toEqual(['query']);
});
it('includes ClawHub-origin skills that opt in via triggers frontmatter (#1767)', () => {
const dir = scratch();
writeSkill(dir, 'agentmail', 'agentmail');
mkdirSync(join(dir, 'agentmail', '.clawhub'), { recursive: true });
writeFileSync(
join(dir, 'agentmail', '.clawhub', 'origin.json'),
JSON.stringify({ registry: 'https://clawhub.ai', slug: 'agentmail' })
);
writeFileSync(
join(dir, 'agentmail', 'SKILL.md'),
`---\nname: agentmail\ndescription: test\ntriggers:\n - "send email"\n---\n\n# agentmail\n`
);
const r = loadOrDeriveManifest(dir);
expect(r.derived).toBe(true);
expect(r.skills.map(s => s.name)).toEqual(['agentmail']);
});
it('keeps ClawHub-origin skills listed in an explicit manifest.json (#1767)', () => {
// Explicit manifest.json is a deliberate declaration — strict checking stays.
const dir = scratch();
writeSkill(dir, 'agentmail', 'agentmail');
mkdirSync(join(dir, 'agentmail', '.clawhub'), { recursive: true });
writeFileSync(
join(dir, 'agentmail', '.clawhub', 'origin.json'),
JSON.stringify({ registry: 'https://clawhub.ai', slug: 'agentmail' })
);
writeManifest(dir, { skills: [{ name: 'agentmail', path: 'agentmail/SKILL.md' }] });
const r = loadOrDeriveManifest(dir);
expect(r.derived).toBe(false);
expect(r.skills.map(s => s.name)).toEqual(['agentmail']);
});
it('treats dirs without SKILL.md as not-a-skill', () => {
const dir = scratch();
writeSkill(dir, 'query', 'query');