fix(smoke): actually clean up smoke-test resources
341 of 356 projects on the shared mcpd were smoke-test leftovers, plus 11
never-expiring `smoke-*` mcptokens sitting on the real `mcpctl-development`
and `sre` projects. Enough to make the project picker unusable.
Two causes, both silent:
- `delete project X --force` — `--force` is valid on `create project` but NOT
on `delete`, so every cleanup call exited non-zero and deleted nothing.
project-llm-ref's afterAll looked correct and had never worked, which is why
there were 84 each of smoke-proj-{ok,orphan,none}-*.
- mcptoken.smoke had no afterAll at all; its cleanup was an `it()` at the end
of the file, so it was skipped whenever an earlier assertion failed.
Cleanup now lives in `afterAll` (runs on failure too) and is no longer gated
on the health probe: the project is created through mcpd, which can be up when
the gateway probe is not, and deleting a project that was never created is a
no-op.
Adds `pnpm smoke:clean` for the case afterAll cannot cover — a killed process
(CI timeout, Ctrl-C, OOM). Dry-run by default, `--yes` to apply. Only touches
`smoke-*` names and protects the shared `smoke-data` / `smoke-aws-docs`
fixture, which five suites depend on and which must survive a concurrent run.
Verified end to end: created a throwaway smoke project, confirmed the dry run
lists without deleting, then `--yes` removed it and left all 15 real projects
untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017BMXdb2qZbPSh8Q7XpTyjB
This commit is contained in:
@@ -33,7 +33,8 @@
|
|||||||
"mcpd:deploy": "bash deploy.sh",
|
"mcpd:deploy": "bash deploy.sh",
|
||||||
"mcpd:deploy-dry": "bash deploy.sh --dry-run",
|
"mcpd:deploy-dry": "bash deploy.sh --dry-run",
|
||||||
"mcpd:logs": "bash logs.sh",
|
"mcpd:logs": "bash logs.sh",
|
||||||
"typecheck:pi-ext": "tsc -p src/pi-ext/tsconfig.json"
|
"typecheck:pi-ext": "tsc -p src/pi-ext/tsconfig.json",
|
||||||
|
"smoke:clean": "tsx scripts/clean-smoke-resources.ts"
|
||||||
},
|
},
|
||||||
"engines": {
|
"engines": {
|
||||||
"node": ">=20.0.0",
|
"node": ">=20.0.0",
|
||||||
|
|||||||
101
scripts/clean-smoke-resources.ts
Normal file
101
scripts/clean-smoke-resources.ts
Normal file
@@ -0,0 +1,101 @@
|
|||||||
|
#!/usr/bin/env node
|
||||||
|
/**
|
||||||
|
* Sweep per-run smoke-test resources off the shared mcpd.
|
||||||
|
*
|
||||||
|
* The suites clean up in `afterAll`, which covers assertion failures but not a
|
||||||
|
* killed process (CI timeout, Ctrl-C, OOM). Over time that leaked hundreds of
|
||||||
|
* `smoke-*` projects and never-expiring `smoke-*` mcptokens onto the shared
|
||||||
|
* server, to the point where the project picker was unusable.
|
||||||
|
*
|
||||||
|
* Run after a smoke run, or any time the server looks cluttered:
|
||||||
|
* pnpm smoke:clean # show what would go
|
||||||
|
* pnpm smoke:clean --yes # actually delete
|
||||||
|
*
|
||||||
|
* Only names matching SMOKE_PREFIX are ever considered, and the shared fixture
|
||||||
|
* (`smoke-data` / `smoke-aws-docs`, provisioned from
|
||||||
|
* tests/smoke/fixtures/smoke-data.yaml and depended on by five suites) is
|
||||||
|
* protected — deleting it mid-run would break a concurrent suite.
|
||||||
|
*/
|
||||||
|
import { execFileSync } from 'node:child_process';
|
||||||
|
|
||||||
|
const SMOKE_PREFIX = /^smoke-/;
|
||||||
|
|
||||||
|
/** Shared, intentionally long-lived fixture — never swept. */
|
||||||
|
const PROTECTED = new Set(['smoke-data', 'smoke-aws-docs']);
|
||||||
|
|
||||||
|
const APPLY = process.argv.includes('--yes') || process.argv.includes('-y');
|
||||||
|
|
||||||
|
interface Named { id?: string; name?: string; projectName?: string; status?: string }
|
||||||
|
|
||||||
|
function mcpctl(args: string[]): string {
|
||||||
|
return execFileSync('mcpctl', args, { encoding: 'utf-8', maxBuffer: 32 * 1024 * 1024 });
|
||||||
|
}
|
||||||
|
|
||||||
|
function list(resource: string, extra: string[] = []): Named[] {
|
||||||
|
try {
|
||||||
|
const out = mcpctl(['get', resource, '-o', 'json', ...extra]);
|
||||||
|
const parsed: unknown = JSON.parse(out || '[]');
|
||||||
|
return Array.isArray(parsed) ? (parsed as Named[]) : [];
|
||||||
|
} catch {
|
||||||
|
return [];
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function sweepable(rows: Named[]): Named[] {
|
||||||
|
return rows.filter((r) => typeof r.name === 'string' && SMOKE_PREFIX.test(r.name) && !PROTECTED.has(r.name));
|
||||||
|
}
|
||||||
|
|
||||||
|
function remove(resource: string, row: Named, extra: string[] = []): boolean {
|
||||||
|
const name = row.name ?? row.id ?? '';
|
||||||
|
if (!SMOKE_PREFIX.test(name) || PROTECTED.has(name)) return false; // belt and braces
|
||||||
|
try {
|
||||||
|
mcpctl(['delete', resource, name, ...extra]);
|
||||||
|
return true;
|
||||||
|
} catch (err) {
|
||||||
|
process.stderr.write(` ! ${resource} ${name}: ${err instanceof Error ? err.message.split('\n')[0] : String(err)}\n`);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const projects = sweepable(list('projects'));
|
||||||
|
// Tokens live on real projects too (the suites mint smoke-* tokens against
|
||||||
|
// mcpctl-development and sre), so they need sweeping independently of projects.
|
||||||
|
const tokens = sweepable(list('mcptoken'));
|
||||||
|
const servers = sweepable(list('servers'));
|
||||||
|
const llms = sweepable(list('llms'));
|
||||||
|
const agents = sweepable(list('agents'));
|
||||||
|
|
||||||
|
const groups: Array<[string, Named[], string[]]> = [
|
||||||
|
['project', projects, []],
|
||||||
|
['mcptoken', tokens, []],
|
||||||
|
['server', servers, []],
|
||||||
|
['llm', llms, []],
|
||||||
|
['agent', agents, []],
|
||||||
|
];
|
||||||
|
|
||||||
|
let total = 0;
|
||||||
|
for (const [, rows] of groups) total += rows.length;
|
||||||
|
|
||||||
|
if (total === 0) {
|
||||||
|
process.stdout.write('smoke:clean — nothing to sweep\n');
|
||||||
|
process.exit(0);
|
||||||
|
}
|
||||||
|
|
||||||
|
for (const [kind, rows] of groups) {
|
||||||
|
if (rows.length === 0) continue;
|
||||||
|
process.stdout.write(`${kind}: ${String(rows.length)}\n`);
|
||||||
|
for (const r of rows.slice(0, 5)) process.stdout.write(` ${r.name ?? ''}${r.projectName ? ` (in ${r.projectName})` : ''}\n`);
|
||||||
|
if (rows.length > 5) process.stdout.write(` … and ${String(rows.length - 5)} more\n`);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!APPLY) {
|
||||||
|
process.stdout.write(`\n${String(total)} resource(s) would be deleted. Re-run with --yes to apply.\n`);
|
||||||
|
process.exit(0);
|
||||||
|
}
|
||||||
|
|
||||||
|
let deleted = 0;
|
||||||
|
for (const [kind, rows, extra] of groups) {
|
||||||
|
for (const r of rows) if (remove(kind, r, extra)) deleted++;
|
||||||
|
}
|
||||||
|
process.stdout.write(`\nsmoke:clean — deleted ${String(deleted)}/${String(total)}\n`);
|
||||||
|
process.exit(deleted === total ? 0 : 1);
|
||||||
@@ -14,7 +14,7 @@
|
|||||||
*
|
*
|
||||||
* Run with: pnpm test:smoke
|
* Run with: pnpm test:smoke
|
||||||
*/
|
*/
|
||||||
import { describe, it, expect, beforeAll } from 'vitest';
|
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||||
import http from 'node:http';
|
import http from 'node:http';
|
||||||
import https from 'node:https';
|
import https from 'node:https';
|
||||||
import { execSync } from 'node:child_process';
|
import { execSync } from 'node:child_process';
|
||||||
@@ -87,9 +87,23 @@ describe('mcptoken smoke', () => {
|
|||||||
}
|
}
|
||||||
}, 20_000);
|
}, 20_000);
|
||||||
|
|
||||||
|
// Cleanup belongs in afterAll, not an `it()`: a failing assertion earlier in
|
||||||
|
// the file must not strand server-side projects. Deliberately NOT gated on
|
||||||
|
// `gatewayUp` — the project is created through mcpd, which can be reachable
|
||||||
|
// even when the gateway healthz probe is not, and deleting something that was
|
||||||
|
// never created is a harmless no-op.
|
||||||
|
//
|
||||||
|
// `delete project` takes no --force (only `create project` does); passing it
|
||||||
|
// made every one of these calls exit non-zero and silently do nothing, which
|
||||||
|
// is how hundreds of smoke-* projects accumulated on the shared mcpd.
|
||||||
|
afterAll(() => {
|
||||||
|
run(`delete project ${PROJECT_NAME}`);
|
||||||
|
run(`delete project ${OTHER_PROJECT}`);
|
||||||
|
});
|
||||||
|
|
||||||
it('creates the project and a project-scoped mcptoken', () => {
|
it('creates the project and a project-scoped mcptoken', () => {
|
||||||
if (!gatewayUp) return;
|
if (!gatewayUp) return;
|
||||||
run(`delete project ${PROJECT_NAME} --force`); // cleanup leftovers — best-effort
|
run(`delete project ${PROJECT_NAME}`); // cleanup leftovers — best-effort
|
||||||
const createProj = run(`create project ${PROJECT_NAME} --force`);
|
const createProj = run(`create project ${PROJECT_NAME} --force`);
|
||||||
expect(createProj.code).toBe(0);
|
expect(createProj.code).toBe(0);
|
||||||
|
|
||||||
@@ -149,10 +163,8 @@ describe('mcptoken smoke', () => {
|
|||||||
expect(report.error ?? '').toMatch(/401|revoked|Invalid token/i);
|
expect(report.error ?? '').toMatch(/401|revoked|Invalid token/i);
|
||||||
}, 20_000);
|
}, 20_000);
|
||||||
|
|
||||||
it('cleans up test fixtures', () => {
|
it('recorded a tool name for the gated catalogue', () => {
|
||||||
if (!gatewayUp) return;
|
if (!gatewayUp) return;
|
||||||
run(`delete project ${PROJECT_NAME} --force`);
|
|
||||||
run(`delete project ${OTHER_PROJECT} --force`);
|
|
||||||
expect(knownToolName === undefined || typeof knownToolName === 'string').toBe(true);
|
expect(knownToolName === undefined || typeof knownToolName === 'string').toBe(true);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -81,17 +81,20 @@ describe('project-llm-ref smoke', () => {
|
|||||||
}
|
}
|
||||||
}, 30_000);
|
}, 30_000);
|
||||||
|
|
||||||
|
// Not gated on `mcpdUp`: if creation got far enough to leave a project
|
||||||
|
// behind, cleanup must still run. Deleting a non-existent project is a no-op.
|
||||||
|
// (`delete project` takes no --force — passing it made these calls fail
|
||||||
|
// silently and leak a project per run.)
|
||||||
afterAll(() => {
|
afterAll(() => {
|
||||||
if (!mcpdUp) return;
|
run(`delete project ${PROJ_OK}`);
|
||||||
run(`delete project ${PROJ_OK} --force`);
|
run(`delete project ${PROJ_ORPHAN}`);
|
||||||
run(`delete project ${PROJ_ORPHAN} --force`);
|
run(`delete project ${PROJ_NONE}`);
|
||||||
run(`delete project ${PROJ_NONE} --force`);
|
|
||||||
run(`delete llm ${LLM_NAME}`);
|
run(`delete llm ${LLM_NAME}`);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('project with --llm pointing at a registered Llm describes without warning', () => {
|
it('project with --llm pointing at a registered Llm describes without warning', () => {
|
||||||
if (!mcpdUp) return;
|
if (!mcpdUp) return;
|
||||||
run(`delete project ${PROJ_OK} --force`);
|
run(`delete project ${PROJ_OK}`);
|
||||||
const created = run(`create project ${PROJ_OK} --llm ${LLM_NAME}`);
|
const created = run(`create project ${PROJ_OK} --llm ${LLM_NAME}`);
|
||||||
expect(created.code, created.stderr || created.stdout).toBe(0);
|
expect(created.code, created.stderr || created.stdout).toBe(0);
|
||||||
|
|
||||||
@@ -104,7 +107,7 @@ describe('project-llm-ref smoke', () => {
|
|||||||
|
|
||||||
it('project with --llm naming an unregistered Llm shows the warning line', () => {
|
it('project with --llm naming an unregistered Llm shows the warning line', () => {
|
||||||
if (!mcpdUp) return;
|
if (!mcpdUp) return;
|
||||||
run(`delete project ${PROJ_ORPHAN} --force`);
|
run(`delete project ${PROJ_ORPHAN}`);
|
||||||
const created = run(`create project ${PROJ_ORPHAN} --llm claude-ghost-${SUFFIX}`);
|
const created = run(`create project ${PROJ_ORPHAN} --llm claude-ghost-${SUFFIX}`);
|
||||||
expect(created.code, created.stderr || created.stdout).toBe(0);
|
expect(created.code, created.stderr || created.stdout).toBe(0);
|
||||||
|
|
||||||
@@ -117,7 +120,7 @@ describe('project-llm-ref smoke', () => {
|
|||||||
|
|
||||||
it('project with --llm none treats it as an explicit disable (no warning)', () => {
|
it('project with --llm none treats it as an explicit disable (no warning)', () => {
|
||||||
if (!mcpdUp) return;
|
if (!mcpdUp) return;
|
||||||
run(`delete project ${PROJ_NONE} --force`);
|
run(`delete project ${PROJ_NONE}`);
|
||||||
const created = run(`create project ${PROJ_NONE} --llm none`);
|
const created = run(`create project ${PROJ_NONE} --llm none`);
|
||||||
expect(created.code).toBe(0);
|
expect(created.code).toBe(0);
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user