Skip to content
Open
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
4 changes: 4 additions & 0 deletions docs/cli-v1-agent-install/skill-template.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,10 @@ blue 'Submit' button"`. The agent does its own semantic match; a guessed label
- **1–2 assertions, at the end, on content existence** rather than UI copy /
formatting. (Quoting a literal you submitted earlier in the same plan, to verify
it round-trips, is fine.)
- **Keep each assertion single and decisive.** Avoid conditional or multi-branch
wording such as "either A or B", "if A then B", `unless`, or `whether` — it can
exhaust the frontend run budget before a verdict is emitted. `test lint` warns
on these patterns before the plan consumes run credits.
- **Assertion targets name the specific page region** (panel / tab / output area).
Otherwise the agent settles for "some element with that text is visible
anywhere," which passes for wrong reasons.
Expand Down
99 changes: 99 additions & 0 deletions src/commands/test.lint-warning.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { mkdtempSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';

import { describe, expect, it } from 'vitest';

import { runLint } from './test.js';

describe('runLint assertion-complexity warnings', () => {
it('warns when a frontend assertion contains conditional or multi-branch wording', async () => {
const dir = mkdtempSync(join(tmpdir(), 'cli-lint-conditional-'));
const file = join(dir, 'plan.json');
writeFileSync(
file,
JSON.stringify({
projectId: 'project_alice',
type: 'frontend',
name: 'Knowledge Web renders',
planSteps: [
{
type: 'assertion',
description: 'Verify either an interactive graph canvas or a clear empty-state message',
},
],
}),
'utf8',
);

const report = await runLint(
{ profile: 'default', output: 'json', debug: false, planFrom: file },
{ stdout: () => undefined },
);

expect(report).toMatchObject({ checked: 1, valid: 1, issues: [] });
expect(report.warnings).toEqual([
expect.objectContaining({
field: 'planSteps[0].description',
reason: expect.stringContaining('single, decisive assertion'),
}),
]);
});

it('keeps text warnings on stderr and stdout machine-safe', async () => {
const dir = mkdtempSync(join(tmpdir(), 'cli-lint-warning-streams-'));
const file = join(dir, 'plan.json');
writeFileSync(
file,
JSON.stringify({
projectId: 'project_alice',
type: 'frontend',
name: 'Knowledge Web renders',
planSteps: [
{
type: 'assertion',
description: 'Verify either an interactive graph canvas or a clear empty-state message',
},
],
}),
'utf8',
);
const stdout: string[] = [];
const stderr: string[] = [];

await runLint(
{ profile: 'default', output: 'text', debug: false, planFrom: file },
{ stdout: line => stdout.push(line), stderr: line => stderr.push(line) },
);

expect(stdout.join('\n')).toBe('1/1 valid, 0 problem(s)');
expect(stdout.join('\n')).not.toContain('[warning]');
expect(stderr.join('\n')).toContain('[warning]');
expect(stderr.at(-1)).toBe('1 warning(s)');
});

it('does not flag actions or single-outcome assertions', async () => {
const dir = mkdtempSync(join(tmpdir(), 'cli-lint-decisive-'));
const file = join(dir, 'plan.json');
writeFileSync(
file,
JSON.stringify({
projectId: 'project_alice',
type: 'frontend',
name: 'Checkout works',
planSteps: [
{ type: 'action', description: 'Open the cart or return to the catalog' },
{ type: 'assertion', description: 'Verify the order total is visible' },
],
}),
'utf8',
);

const report = await runLint(
{ profile: 'default', output: 'json', debug: false, planFrom: file },
{ stdout: () => undefined },
);

expect(report).toEqual({ checked: 1, valid: 1, issues: [] });
});
});
61 changes: 60 additions & 1 deletion src/commands/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4742,6 +4742,42 @@ export interface CliLintReport {
checked: number;
valid: number;
issues: CliLintIssue[];
/** Non-fatal plan-quality findings; omitted when there are none. */
warnings?: CliLintIssue[];
}

/**
* Conditional and multi-branch assertion wording makes the frontend agent
* explore several outcomes and can exhaust its step budget before it emits a
* verdict. This is deliberately a narrow heuristic: only assertion steps are
* considered, and the finding stays non-fatal because the plan is structurally
* valid and may still be intentional.
*/
const CONDITIONAL_ASSERTION_PATTERN =
/\b(?:either|or|otherwise|unless|whether)\b|\bif\b[^.!?]*\bthen\b/i;

function collectAssertionComplexityWarnings(
parsed: unknown,
prefix = '',
): Array<{ field: string; reason: string }> {
if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) return [];
const obj = parsed as Record<string, unknown>;
if (obj.type !== undefined && obj.type !== 'frontend') return [];
if (!Array.isArray(obj.planSteps)) return [];

const warnings: Array<{ field: string; reason: string }> = [];
obj.planSteps.forEach((step, index) => {
if (typeof step !== 'object' || step === null || Array.isArray(step)) return;
const candidate = step as Record<string, unknown>;
if (candidate.type !== 'assertion' || typeof candidate.description !== 'string') return;
if (!CONDITIONAL_ASSERTION_PATTERN.test(candidate.description)) return;
warnings.push({
field: `${prefix}planSteps[${index}].description`,
reason:
'uses conditional or multi-branch wording; choose one observable outcome and write a single, decisive assertion to avoid exhausting the frontend run budget',
});
});
return warnings;
}

/**
Expand Down Expand Up @@ -4795,6 +4831,7 @@ export async function runLint(opts: LintOptions, deps: TestDeps = {}): Promise<C
}

const issues: CliLintIssue[] = [];
const warnings: CliLintIssue[] = [];
let checked = 0;

const lintPlanFile = (file: string, path: string, specIndex?: number): void => {
Expand All @@ -4809,6 +4846,10 @@ export async function runLint(opts: LintOptions, deps: TestDeps = {}): Promise<C
for (const issue of collectPlanIssues(parsed, { specIndex })) {
issues.push({ file, ...issue });
}
const prefix = specIndex === undefined ? '' : `specs[${specIndex}].`;
for (const warning of collectAssertionComplexityWarnings(parsed, prefix)) {
warnings.push({ file, ...warning });
}
};

const lintStepsFile = (file: string, path: string): void => {
Expand All @@ -4823,6 +4864,9 @@ export async function runLint(opts: LintOptions, deps: TestDeps = {}): Promise<C
for (const issue of collectPlanStepsIssues(parsed)) {
issues.push({ file, ...issue });
}
for (const warning of collectAssertionComplexityWarnings(parsed)) {
warnings.push({ file, ...warning });
}
};

if (opts.planFrom !== undefined) {
Expand Down Expand Up @@ -4883,11 +4927,26 @@ export async function runLint(opts: LintOptions, deps: TestDeps = {}): Promise<C
for (const issue of collectPlanIssues(parsed, { specIndex: lineNo - 1 })) {
issues.push({ file, ...issue });
}
for (const warning of collectAssertionComplexityWarnings(parsed, `specs[${lineNo - 1}].`)) {
warnings.push({ file, ...warning });
}
}
}

const filesWithIssues = new Set(issues.map(issue => issue.file)).size;
const report: CliLintReport = { checked, valid: checked - filesWithIssues, issues };
const report: CliLintReport = {
checked,
valid: checked - filesWithIssues,
issues,
...(warnings.length > 0 ? { warnings } : {}),
};
if (opts.output === 'text' && warnings.length > 0) {
const stderr = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`));
for (const warning of warnings) {
stderr(`[warning] ${warning.file}: ${warning.field}: ${warning.reason}`);
}
stderr(`${warnings.length} warning(s)`);
}
out.print(report, () =>
[
...issues.map(issue => `${issue.file}: ${issue.field}: ${issue.reason}`),
Expand Down
Loading