Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 74 additions & 1 deletion src/cli/main.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { afterEach, describe, it, expect } from 'vitest';
import { afterEach, describe, it, expect, vi } from 'vitest';
import type { CLIOptions } from './args.js';
import {
createSkillTasks,
formatSkillSource,
mergeSkillRunnerOptions,
processTaskResults,
resolveInvocationCwd,
Expand All @@ -20,6 +21,7 @@ import type { SkillReport } from '../types/index.js';
const tempDirs: string[] = [];

afterEach(() => {
vi.restoreAllMocks();
for (const dir of tempDirs) {
rmSync(dir, { recursive: true, force: true });
}
Expand All @@ -39,6 +41,10 @@ function createTestReporter(): Reporter {
return new Reporter({ isTTY: false, supportsColor: false, columns: 80 }, Verbosity.Quiet);
}

function createVisibleTestReporter(): Reporter {
return new Reporter({ isTTY: true, supportsColor: false, columns: 80 }, Verbosity.Normal);
}

function createCliOptions(overrides: Partial<CLIOptions> = {}): CLIOptions {
return {
json: false,
Expand Down Expand Up @@ -84,6 +90,50 @@ describe('createSkillTasks', () => {
expect(skill.rootDir).toContain('src/builtin-skills/security-review');
});

it('shows a built-in source label instead of the package cache path', async () => {
const repoRoot = mkdtempSync(join(tmpdir(), 'warden-main-repo-'));
const packageRoot = mkdtempSync(join(tmpdir(), 'warden-main-package-'));
tempDirs.push(repoRoot, packageRoot);

const rootDir = join(
packageRoot,
'node_modules',
'@sentry',
'warden',
'src',
'builtin-skills',
'security-review'
);
mkdirSync(rootDir, { recursive: true });
writeFileSync(join(rootDir, 'SKILL.md'), `---
name: security-review
description: Review security issues.
---

Review security issues.
`, 'utf-8');

const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined);
const spec: RunSkillSpec = {
name: rootDir,
skill: rootDir,
context: {} as RunSkillSpec['context'],
runnerOptions: {},
};

await createSkillTasks({
specs: [spec],
repoPath: repoRoot,
options: createCliOptions({ quiet: false }),
parallel: 1,
reporter: createVisibleTestReporter(),
});

const output = errorSpy.mock.calls.map(([message]) => String(message)).join('\n');
expect(output).toContain(' Source built-in (@sentry/warden)');
expect(output).not.toContain(rootDir);
});

it('reports missing generated artifacts for explicit generated skill paths', async () => {
const repoRoot = mkdtempSync(join(tmpdir(), 'warden-main-'));
tempDirs.push(repoRoot);
Expand Down Expand Up @@ -115,6 +165,29 @@ prompt: |-
});
});

describe('formatSkillSource', () => {
it('formats repo-local skill sources relative to the repo root', () => {
expect(formatSkillSource(
{ rootDir: '/repo/.agents/skills/security-review' },
'/repo'
)).toBe('.agents/skills/security-review');
});

it('keeps external custom skill sources as absolute paths', () => {
expect(formatSkillSource(
{ rootDir: '/external/skills/security-review' },
'/repo'
)).toBe('/external/skills/security-review');
});

it('keeps the repo root source path instead of rendering an empty source', () => {
expect(formatSkillSource(
{ rootDir: '/repo' },
'/repo'
)).toBe('/repo');
});
});

describe('mergeSkillRunnerOptions', () => {
it('preserves global defaults when per-skill options are undefined', () => {
const merged = mergeSkillRunnerOptions(
Expand Down
52 changes: 48 additions & 4 deletions src/cli/main.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { existsSync, readFileSync, renameSync, unlinkSync, writeFileSync } from 'node:fs';
import { dirname, join, resolve } from 'node:path';
import { dirname, join, relative, resolve } from 'node:path';
import { config as dotenvConfig } from 'dotenv';
import { Sentry, flushSentry, setGlobalAttributes, emitRunMetric, getTraceId } from '../sentry.js';
import { emptyToUndefined, loadWardenConfig, resolveSkillConfigs } from '../config/loader.js';
Expand All @@ -12,6 +12,7 @@ import { matchTrigger, filterContextByPaths, shouldFail, countFindingsAtOrAbove
import type { SkillReport, SeverityThreshold, ConfidenceThreshold, SkillError, Finding } from '../types/index.js';
import { filterFindings } from '../types/index.js';
import { DEFAULT_CONCURRENCY, getAnthropicApiKey } from '../utils/index.js';
import { isRepoRelativePath, normalizePath } from '../utils/path.js';
import { parseCliArgs, showVersion, classifyTargets, type CLIOptions } from './args.js';
import { showHelp } from './help.js';
import { buildLocalEventContext, buildFileEventContext } from './context.js';
Expand Down Expand Up @@ -511,6 +512,51 @@ type SkillRunnerOptionOverrides = Pick<
'model' | 'maxTurns' | 'runtime' | 'auxiliaryModel' | 'synthesisModel' | 'auxiliaryMaxRetries' | 'verifyFindings'
>;

const BUILTIN_SKILL_SOURCE = 'built-in (@sentry/warden)';

function isBuiltinSkillRoot(rootDir: string, repoPath?: string): boolean {
const normalizedRoot = normalizePath(rootDir);
if (
normalizedRoot.includes('/node_modules/@sentry/warden/src/builtin-skills/')
|| normalizedRoot.includes('/node_modules/@sentry/warden/dist/builtin-skills/')
) {
return true;
}

if (!repoPath) {
return false;
}

const relativeRoot = normalizePath(relative(repoPath, rootDir));
return (
relativeRoot === 'src/builtin-skills'
|| relativeRoot.startsWith('src/builtin-skills/')
|| relativeRoot === 'dist/builtin-skills'
|| relativeRoot.startsWith('dist/builtin-skills/')
);
}

/** Format a skill source path for the CLI run header. */
export function formatSkillSource(
skill: Pick<SkillDefinition, 'rootDir'>,
repoPath?: string
): string | undefined {
if (!skill.rootDir) {
return undefined;
}

if (isBuiltinSkillRoot(skill.rootDir, repoPath)) {
return BUILTIN_SKILL_SOURCE;
}

if (!repoPath) {
return skill.rootDir;
}

const relativeRoot = normalizePath(relative(repoPath, skill.rootDir));
return isRepoRelativePath(relativeRoot) ? relativeRoot : skill.rootDir;
}

/** Apply per-skill runner overrides on top of the shared execution defaults. */
export function mergeSkillRunnerOptions(
base: SkillRunnerOptions,
Expand Down Expand Up @@ -539,9 +585,7 @@ function renderSkillRunHeader(args: {
model?: string;
}): void {
const { reporter, skill, repoPath, runtimeName, model } = args;
const source = skill.rootDir && repoPath && skill.rootDir.startsWith(repoPath)
? skill.rootDir.slice(repoPath.length + 1)
: skill.rootDir;
const source = formatSkillSource(skill, repoPath);

reporter.blank();
reporter.text(` Skill ${skill.name}`);
Expand Down
Loading