From ad9823223ed219117d3d5cd48f6b84524f844d8c Mon Sep 17 00:00:00 2001 From: Ross Gardler <250240+SorraTheOrc@users.noreply.github.com> Date: Sat, 23 May 2026 00:27:33 +0100 Subject: [PATCH] WL-0MP14WFW6001VE3G: Add priority normalization to create, update and doctor commands - Create src/validators/priority.ts with normalizePriority, isValidPriority, isMappablePriority - Update create.ts to validate and normalize priority on creation, rejecting P* values - Update update.ts to validate and normalize priority on update, rejecting P* values - Update doctor.ts with: - 'doctor priority' subcommand supporting --dry-run and --apply - Priority validation integrated into main doctor findings with --fix support - Add unit tests for priority validator (14 tests) - Add integration tests for doctor priority subcommand (6 tests) --- src/commands/create.ts | 11 +++ src/commands/doctor.ts | 128 +++++++++++++++++++++++++- src/commands/update.ts | 13 ++- src/validators/priority.ts | 49 ++++++++++ tests/cli/doctor-priority.test.ts | 118 ++++++++++++++++++++++++ tests/unit/priority-validator.test.ts | 113 +++++++++++++++++++++++ 6 files changed, 428 insertions(+), 4 deletions(-) create mode 100644 src/validators/priority.ts create mode 100644 tests/cli/doctor-priority.test.ts create mode 100644 tests/unit/priority-validator.test.ts diff --git a/src/commands/create.ts b/src/commands/create.ts index 919e4ed5..1d30cdc8 100644 --- a/src/commands/create.ts +++ b/src/commands/create.ts @@ -10,6 +10,7 @@ import { canValidateStatusStage, validateStatusStageCompatibility, validateStatu import { promises as fs } from 'fs'; import { normalizeActionArgs } from './cli-utils.js'; import { buildAuditEntry, formatInvalidAuditFirstLineMessage, inspectAuditFirstLine, redactAuditText } from '../audit.js'; +import { normalizePriority, CANONICAL_PRIORITIES } from '../validators/priority.js'; export default function register(ctx: PluginContext): void { const { program, output, utils } = ctx; @@ -86,6 +87,16 @@ export default function register(ctx: PluginContext): void { } } + if (normalized.provided.has('priority') && options.priority !== undefined) { + const np = normalizePriority(options.priority); + if (!np) { + const allowed = CANONICAL_PRIORITIES.join(', '); + output.error(`Invalid priority: "${options.priority}". Allowed values: ${allowed} (case-insensitive). P0-P3 values are not accepted at creation time; use "wl doctor" to migrate legacy data.`, { success: false, error: 'invalid-priority' }); + process.exit(1); + } + options.priority = np; + } + let auditTextInput = options.auditText ?? options.audit; if (options.auditFile) { diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 9a2cd9aa..8d79c726 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -11,6 +11,7 @@ import { importFromJsonl } from '../jsonl.js'; import { mergeWorkItems, mergeComments } from '../sync.js'; import * as fs from 'fs'; import * as path from 'path'; +import { normalizePriority, isValidPriority, isMappablePriority, PRIORITY_MAP, CANONICAL_PRIORITIES } from '../validators/priority.js'; interface DoctorOptions { prefix?: string; @@ -230,6 +231,92 @@ export default function register(ctx: PluginContext): void { } }); + doctor + .command('priority') + .description('Detect and fix invalid priority values in the database') + .option('--dry-run', 'Show invalid priorities without modifying them') + .option('--apply', 'Apply priority mapping (P0-P3 -> canonical values)') + .option('--prefix ', 'Override the default prefix') + .action(async (opts: { dryRun?: boolean; apply?: boolean; prefix?: string }) => { + utils.requireInitialized(); + const db = utils.getDatabase(opts.prefix); + const all = db.getAll(); + + const invalid: Array<{ id: string; current: string; mapped?: string }> = []; + + for (const item of all) { + const p = item.priority; + if (p && !isValidPriority(p)) { + const mapped = isMappablePriority(p) ? normalizePriority(p) : undefined; + invalid.push({ id: item.id, current: p, mapped: mapped ?? undefined }); + } + } + + if (invalid.length === 0) { + if (utils.isJsonMode()) { + output.json({ success: true, invalid: [], fixed: [] }); + return; + } + console.log('Doctor priority: no invalid priorities found.'); + return; + } + + if (opts.dryRun || !opts.apply) { + if (utils.isJsonMode()) { + const out: any = { dryRun: true, invalid, count: invalid.length }; + if (!opts.dryRun) out.hint = 'Use --apply to fix invalid priorities'; + output.json(out); + return; + } + console.log(`Doctor priority: found ${invalid.length} work item(s) with invalid priorities.`); + console.log(`Canonical priority values: ${CANONICAL_PRIORITIES.join(', ')}`); + console.log(`P* mapping: P0->critical, P1->high, P2->medium, P3->low`); + console.log(''); + for (const entry of invalid) { + const hint = entry.mapped ? ` (would map to "${entry.mapped}")` : ' (no mapping available)'; + console.log(` - ${entry.id}: current="${entry.current}"${hint}`); + } + if (!opts.dryRun) { + console.log(''); + console.log('Use --dry-run to preview or --apply to apply the P* mapping.'); + } + return; + } + + // --apply: apply mapping for mappable values + const fixed: Array<{ id: string; from: string; to: string }> = []; + const unfixable: Array<{ id: string; current: string }> = []; + + for (const entry of invalid) { + if (entry.mapped) { + try { + db.update(entry.id, { priority: entry.mapped as any }); + fixed.push({ id: entry.id, from: entry.current, to: entry.mapped }); + } catch (err) { + unfixable.push({ id: entry.id, current: entry.current }); + } + } else { + unfixable.push({ id: entry.id, current: entry.current }); + } + } + + if (utils.isJsonMode()) { + output.json({ fixed, unfixable, fixedCount: fixed.length, unfixableCount: unfixable.length }); + return; + } + + console.log(`Doctor priority: fixed ${fixed.length} item(s).`); + for (const f of fixed) { + console.log(` - ${f.id}: "${f.from}" -> "${f.to}"`); + } + if (unfixable.length > 0) { + console.log(`\n${unfixable.length} item(s) with unmappable priorities (requires manual fix):`); + for (const u of unfixable) { + console.log(` - ${u.id}: "${u.current}"`); + } + } + }); + doctor .command('migrate') .description('Migrate from persistent JSONL to SQLite-only architecture (ephemeral JSONL pattern)') @@ -382,9 +469,39 @@ export default function register(ctx: PluginContext): void { } const dependencyEdges = db.getAllDependencyEdges(); - let findings = [ + const priorityFindings: Array<{ + checkId: string; + type: string; + severity: string; + itemId: string; + message: string; + proposedFix: Record | null; + safe: boolean; + context: Record; + }> = []; + for (const item of items) { + const p = item.priority; + if (p && !isValidPriority(p)) { + const mapped = isMappablePriority(p) ? normalizePriority(p) : null; + priorityFindings.push({ + checkId: 'priority.invalid', + type: 'invalid-priority', + severity: 'warning', + itemId: item.id, + message: mapped + ? `Invalid priority "${p}" (maps to "${mapped}" via P* mapping)` + : `Invalid priority "${p}" (not a canonical value: ${CANONICAL_PRIORITIES.join(', ')})`, + proposedFix: mapped ? { priority: mapped } as Record : null, + safe: !!mapped, + context: { current: p, mapped } as Record, + }); + } + } + + let findings: any[] = [ ...validateStatusStageItems(items, rules), ...validateDependencyEdges(items, dependencyEdges), + ...priorityFindings, ]; // If --fix was provided, attempt to apply safe fixes and prompt per non-safe finding @@ -446,6 +563,7 @@ export default function register(ctx: PluginContext): void { const update: any = {}; if ((f.proposedFix as any).status) update.status = (f.proposedFix as any).status; if ((f.proposedFix as any).stage) update.stage = (f.proposedFix as any).stage; + if ((f.proposedFix as any).priority) update.priority = (f.proposedFix as any).priority; if (Object.keys(update).length > 0) { try { db.update(itemId, update); @@ -488,7 +606,8 @@ export default function register(ctx: PluginContext): void { const hasActionableFix = f.proposedFix && typeof f.proposedFix === 'object' && ( Object.prototype.hasOwnProperty.call(f.proposedFix, 'status') || - Object.prototype.hasOwnProperty.call(f.proposedFix, 'stage') + Object.prototype.hasOwnProperty.call(f.proposedFix, 'stage') || + Object.prototype.hasOwnProperty.call(f.proposedFix, 'priority') ); if (!hasActionableFix) { @@ -513,6 +632,7 @@ export default function register(ctx: PluginContext): void { const update: any = {}; if ((f.proposedFix as any).status) update.status = (f.proposedFix as any).status; if ((f.proposedFix as any).stage) update.stage = (f.proposedFix as any).stage; + if ((f.proposedFix as any).priority) update.priority = (f.proposedFix as any).priority; if (Object.keys(update).length > 0) { try { db.update(f.itemId, update); continue; } catch (err) { /* fall through to keep in report */ } } @@ -566,7 +686,8 @@ export default function register(ctx: PluginContext): void { const proposed = f.proposedFix as any; const hasActionableFix = proposed && typeof proposed === 'object' && ( Object.prototype.hasOwnProperty.call(proposed, 'status') || - Object.prototype.hasOwnProperty.call(proposed, 'stage') + Object.prototype.hasOwnProperty.call(proposed, 'stage') || + Object.prototype.hasOwnProperty.call(proposed, 'priority') ); return !!ctx.requiresManualFix || !hasActionableFix; }); @@ -594,6 +715,7 @@ export default function register(ctx: PluginContext): void { if (proposed.allowedStatuses) suggestions.push(`allowedStatuses=${JSON.stringify(proposed.allowedStatuses)}`); if (proposed.stage) suggestions.push(`proposedStage=${String(proposed.stage)}`); if (proposed.status) suggestions.push(`proposedStatus=${String(proposed.status)}`); + if (proposed.priority) suggestions.push(`proposedPriority=${String(proposed.priority)}`); } // Also check context for same keys if (ctx.allowedStages && !suggestions.some(s => s.startsWith('allowedStages='))) { diff --git a/src/commands/update.ts b/src/commands/update.ts index 39ca3d8d..8b507a72 100644 --- a/src/commands/update.ts +++ b/src/commands/update.ts @@ -11,6 +11,7 @@ import { canValidateStatusStage, validateStatusStageCompatibility, validateStatu import { normalizeActionArgs } from './cli-utils.js'; import { buildAuditEntry, formatInvalidAuditFirstLineMessage, inspectAuditFirstLine, redactAuditText } from '../audit.js'; import { submitToOpenBrain } from '../openbrain.js'; +import { normalizePriority, CANONICAL_PRIORITIES } from '../validators/priority.js'; export default function register(ctx: PluginContext): void { const { program, output, utils } = ctx; @@ -116,7 +117,17 @@ export default function register(ctx: PluginContext): void { } } const statusCandidate = hasProvided('status') ? options.status : undefined; - const priorityCandidate = hasProvided('priority') ? options.priority : undefined; + let priorityCandidate = hasProvided('priority') ? options.priority : undefined; + // Validate priority if provided: normalize case, reject P* and unknown tokens + if (priorityCandidate !== undefined) { + const np = normalizePriority(priorityCandidate); + if (!np) { + const allowed = CANONICAL_PRIORITIES.join(', '); + output.error(`Invalid priority: "${priorityCandidate}". Allowed values: ${allowed} (case-insensitive). P0-P3 values are not accepted for update; use "wl doctor" to migrate legacy data.`, { success: false, error: 'invalid-priority' }); + process.exit(1); + } + priorityCandidate = np; + } // Commander populates a `parent` property on option objects (the parent // command), so we must check that the user actually provided the // `--parent` flag. Use hasOwnProperty to detect presence of the option diff --git a/src/validators/priority.ts b/src/validators/priority.ts new file mode 100644 index 00000000..99de08e3 --- /dev/null +++ b/src/validators/priority.ts @@ -0,0 +1,49 @@ +import type { WorkItemPriority } from '../types.js'; + +export const CANONICAL_PRIORITIES: readonly WorkItemPriority[] = ['critical', 'high', 'medium', 'low']; + +export const PRIORITY_MAP: Record = { + P0: 'critical', + P1: 'high', + P2: 'medium', + P3: 'low', +}; + +const MAPPABLE_KEYS = new Set(Object.keys(PRIORITY_MAP)); + +function trimmed(raw: string): string { + if (!raw) return ''; + const t = raw.trim(); + return t; +} + +export function normalizePriority(raw: string): WorkItemPriority | null { + const t = trimmed(raw); + if (!t) return null; + + const lower = t.toLowerCase() as string; + if (CANONICAL_PRIORITIES.includes(lower as WorkItemPriority)) { + return lower as WorkItemPriority; + } + + const upper = t.toUpperCase(); + if (MAPPABLE_KEYS.has(upper)) { + return PRIORITY_MAP[upper]; + } + + return null; +} + +export function isValidPriority(raw: string): boolean { + const t = trimmed(raw); + if (!t) return false; + const lower = t.toLowerCase(); + return CANONICAL_PRIORITIES.includes(lower as WorkItemPriority); +} + +export function isMappablePriority(raw: string): boolean { + const t = trimmed(raw); + if (!t) return false; + const upper = t.toUpperCase(); + return MAPPABLE_KEYS.has(upper); +} diff --git a/tests/cli/doctor-priority.test.ts b/tests/cli/doctor-priority.test.ts new file mode 100644 index 00000000..6c589717 --- /dev/null +++ b/tests/cli/doctor-priority.test.ts @@ -0,0 +1,118 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as path from 'path'; +import { + cliPath, + execAsync, + enterTempDir, + leaveTempDir, + writeConfig, + writeInitSemaphore, + seedWorkItems, +} from './cli-helpers.js'; + +describe('doctor priority command', () => { + let tempState: { tempDir: string; originalCwd: string }; + + beforeEach(() => { + tempState = enterTempDir(); + writeConfig(tempState.tempDir, 'Test Project', 'TEST'); + writeInitSemaphore(tempState.tempDir); + }); + + afterEach(() => { + leaveTempDir(tempState); + }); + + it('reports no invalid priorities when all are canonical', async () => { + seedWorkItems(tempState.tempDir, [ + { id: 'TEST-OK-1', title: 'item 1', priority: 'low' }, + { id: 'TEST-OK-2', title: 'item 2', priority: 'medium' }, + { id: 'TEST-OK-3', title: 'item 3', priority: 'high' }, + { id: 'TEST-OK-4', title: 'item 4', priority: 'critical' }, + ]); + + const { stdout } = await execAsync(`tsx ${cliPath} --json doctor priority`); + const result = JSON.parse(stdout); + expect(result.success).toBe(true); + expect(result.invalid).toEqual([]); + }); + + it('detects invalid P* priority values in dry-run mode', async () => { + seedWorkItems(tempState.tempDir, [ + { id: 'TEST-P0', title: 'P0 item', priority: 'P0' as any }, + { id: 'TEST-P1', title: 'P1 item', priority: 'P1' as any }, + { id: 'TEST-OK', title: 'good item', priority: 'medium' }, + ]); + + const { stdout } = await execAsync(`tsx ${cliPath} --json doctor priority --dry-run`); + const result = JSON.parse(stdout); + expect(result.dryRun).toBe(true); + expect(result.count).toBe(2); + expect(result.invalid).toContainEqual( + expect.objectContaining({ id: 'TEST-P0', current: 'P0', mapped: 'critical' }) + ); + expect(result.invalid).toContainEqual( + expect.objectContaining({ id: 'TEST-P1', current: 'P1', mapped: 'high' }) + ); + }); + + it('detects invalid case-mangled priority values', async () => { + seedWorkItems(tempState.tempDir, [ + { id: 'TEST-HIGH', title: 'High item', priority: 'High' as any }, + { id: 'TEST-LOW', title: 'LOW item', priority: 'LOW' as any }, + ]); + + const { stdout } = await execAsync(`tsx ${cliPath} --json doctor priority --dry-run`); + const result = JSON.parse(stdout); + // "High" and "LOW" are valid after case normalization, so isValidPriority returns true + expect(result.success).toBe(true); + expect(result.invalid).toEqual([]); + }); + + it('fixes P* priorities with --apply', async () => { + seedWorkItems(tempState.tempDir, [ + { id: 'TEST-P0', title: 'P0 item', priority: 'P0' as any }, + { id: 'TEST-P2', title: 'P2 item', priority: 'P2' as any }, + { id: 'TEST-OK', title: 'good item', priority: 'medium' }, + ]); + + const { stdout } = await execAsync(`tsx ${cliPath} --json doctor priority --apply`); + const result = JSON.parse(stdout); + expect(result.fixedCount).toBe(2); + expect(result.fixed).toContainEqual({ id: 'TEST-P0', from: 'P0', to: 'critical' }); + expect(result.fixed).toContainEqual({ id: 'TEST-P2', from: 'P2', to: 'medium' }); + + // Verify persistence by re-reading + const { stdout: listOut } = await execAsync(`tsx ${cliPath} --json list`); + const listResult = JSON.parse(listOut); + const items = listResult.workItems || []; + const p0 = items.find((i: any) => i.id === 'TEST-P0'); + const p2 = items.find((i: any) => i.id === 'TEST-P2'); + expect(p0.priority).toBe('critical'); + expect(p2.priority).toBe('medium'); + }); + + it('reports unmappable invalid priorities', async () => { + seedWorkItems(tempState.tempDir, [ + { id: 'TEST-BAD', title: 'bad priority', priority: 'urgent' as any }, + ]); + + const { stdout } = await execAsync(`tsx ${cliPath} --json doctor priority --dry-run`); + const result = JSON.parse(stdout); + expect(result.count).toBe(1); + expect(result.invalid[0].id).toBe('TEST-BAD'); + expect(result.invalid[0].mapped).toBeUndefined(); + }); + + it('leaves unmappable priorities unfixed after --apply', async () => { + seedWorkItems(tempState.tempDir, [ + { id: 'TEST-BAD', title: 'bad priority', priority: 'urgent' as any }, + ]); + + const { stdout } = await execAsync(`tsx ${cliPath} --json doctor priority --apply`); + const result = JSON.parse(stdout); + expect(result.fixedCount).toBe(0); + expect(result.unfixableCount).toBe(1); + expect(result.unfixable[0].id).toBe('TEST-BAD'); + }); +}); diff --git a/tests/unit/priority-validator.test.ts b/tests/unit/priority-validator.test.ts new file mode 100644 index 00000000..e8442852 --- /dev/null +++ b/tests/unit/priority-validator.test.ts @@ -0,0 +1,113 @@ +import { describe, it, expect } from 'vitest'; +import { + normalizePriority, + isValidPriority, + isMappablePriority, + CANONICAL_PRIORITIES, + PRIORITY_MAP, +} from '../../src/validators/priority.js'; + +describe('isValidPriority', () => { + it('returns true for canonical values (case-insensitive)', () => { + expect(isValidPriority('low')).toBe(true); + expect(isValidPriority('medium')).toBe(true); + expect(isValidPriority('high')).toBe(true); + expect(isValidPriority('critical')).toBe(true); + expect(isValidPriority('LOW')).toBe(true); + expect(isValidPriority('Medium')).toBe(true); + expect(isValidPriority('HIGH')).toBe(true); + expect(isValidPriority('Critical')).toBe(true); + }); + + it('returns false for P* values', () => { + expect(isValidPriority('P0')).toBe(false); + expect(isValidPriority('P1')).toBe(false); + expect(isValidPriority('p2')).toBe(false); + expect(isValidPriority('p3')).toBe(false); + }); + + it('returns false for empty/whitespace', () => { + expect(isValidPriority('')).toBe(false); + expect(isValidPriority(' ')).toBe(false); + }); + + it('returns false for unknown tokens', () => { + expect(isValidPriority('urgent')).toBe(false); + expect(isValidPriority('normal')).toBe(false); + expect(isValidPriority('')).toBe(false); + }); +}); + +describe('isMappablePriority', () => { + it('returns true for P0-P3 (case-insensitive)', () => { + expect(isMappablePriority('P0')).toBe(true); + expect(isMappablePriority('P1')).toBe(true); + expect(isMappablePriority('P2')).toBe(true); + expect(isMappablePriority('P3')).toBe(true); + expect(isMappablePriority('p0')).toBe(true); + expect(isMappablePriority('p1')).toBe(true); + }); + + it('returns false for canonical values', () => { + expect(isMappablePriority('low')).toBe(false); + expect(isMappablePriority('critical')).toBe(false); + }); + + it('returns false for unknown tokens', () => { + expect(isMappablePriority('urgent')).toBe(false); + expect(isMappablePriority('P4')).toBe(false); + expect(isMappablePriority('')).toBe(false); + }); +}); + +describe('normalizePriority', () => { + it('normalizes canonical values case-insensitively', () => { + expect(normalizePriority('low')).toBe('low'); + expect(normalizePriority('LOW')).toBe('low'); + expect(normalizePriority('Low')).toBe('low'); + expect(normalizePriority('MEDIUM')).toBe('medium'); + expect(normalizePriority('Medium')).toBe('medium'); + expect(normalizePriority('High')).toBe('high'); + expect(normalizePriority('CRITICAL')).toBe('critical'); + }); + + it('maps P0-P3 to canonical values', () => { + expect(normalizePriority('P0')).toBe('critical'); + expect(normalizePriority('P1')).toBe('high'); + expect(normalizePriority('P2')).toBe('medium'); + expect(normalizePriority('P3')).toBe('low'); + expect(normalizePriority('p0')).toBe('critical'); + expect(normalizePriority('p1')).toBe('high'); + }); + + it('returns null for unknown tokens', () => { + expect(normalizePriority('urgent')).toBeNull(); + expect(normalizePriority('normal')).toBeNull(); + expect(normalizePriority('')).toBeNull(); + expect(normalizePriority(' ')).toBeNull(); + }); + + it('trims whitespace before normalizing', () => { + expect(normalizePriority(' high ')).toBe('high'); + expect(normalizePriority(' P1 ')).toBe('high'); + }); + + it('returns null for null-like or undefined input coerced to string', () => { + expect(normalizePriority('' as string)).toBeNull(); + }); +}); + +describe('constants', () => { + it('CANONICAL_PRIORITIES contains the four canonical values', () => { + expect(CANONICAL_PRIORITIES).toEqual(['critical', 'high', 'medium', 'low']); + }); + + it('PRIORITY_MAP maps P0-P3 correctly', () => { + expect(PRIORITY_MAP).toEqual({ + P0: 'critical', + P1: 'high', + P2: 'medium', + P3: 'low', + }); + }); +});