From 233e1f95b6173eb48e4658ab93dbbde1ccfb8751 Mon Sep 17 00:00:00 2001 From: "Younes (Machine Wisdom)" Date: Mon, 16 Mar 2026 16:46:08 -0400 Subject: [PATCH 01/10] feat: pluggable CLI-based artifact resolver for porch Add ArtifactResolver abstraction with LocalResolver (default) and CliResolver backends. Porch can now resolve specs, plans, and reviews from any CLI tool that supports `get` and `get --list` commands. Configuration via af-config.json: artifacts.backend: "cli" (or "local" default) artifacts.scope: "org/project/codev-assets" artifacts.command: "my-tool" (CLI executable) Closes #612 Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/codev/src/__tests__/consult.test.ts | 43 +- .../codev/src/agent-farm/commands/status.ts | 44 ++ packages/codev/src/commands/consult/index.ts | 230 ++++----- .../commands/porch/__tests__/state.test.ts | 7 +- .../codev/src/commands/porch/artifacts.ts | 446 ++++++++++++++++++ packages/codev/src/commands/porch/checks.ts | 115 ++++- packages/codev/src/commands/porch/config.ts | 5 +- packages/codev/src/commands/porch/index.ts | 31 +- packages/codev/src/commands/porch/next.ts | 65 +-- packages/codev/src/commands/porch/plan.ts | 44 ++ packages/codev/src/commands/porch/prompts.ts | 70 ++- packages/codev/src/commands/porch/state.ts | 40 +- packages/codev/src/terminal/pty-manager.ts | 7 + 13 files changed, 907 insertions(+), 240 deletions(-) create mode 100644 packages/codev/src/commands/porch/artifacts.ts diff --git a/packages/codev/src/__tests__/consult.test.ts b/packages/codev/src/__tests__/consult.test.ts index f91b0a8d24..664bcc2408 100644 --- a/packages/codev/src/__tests__/consult.test.ts +++ b/packages/codev/src/__tests__/consult.test.ts @@ -590,41 +590,40 @@ describe('consult command', () => { }); }); - describe('file-based review instructions (Bugfix #280)', () => { - it('buildSpecQuery should instruct models to read files from disk', async () => { + describe('content-inlined review queries (Bugfix #280 + TICK-002)', () => { + it('buildSpecQuery should inline spec content', async () => { vi.resetModules(); const { _buildSpecQuery } = await import('../commands/consult/index.js'); - const query = _buildSpecQuery('/path/to/spec.md', null); + const query = _buildSpecQuery({ content: '# My Spec\nDetails here', label: 'spec-42' }, null); - expect(query).toContain('Read the files listed above directly from disk'); - expect(query).toContain('Do NOT rely on `git diff`'); + expect(query).toContain('# My Spec'); + expect(query).toContain('Details here'); + expect(query).toContain('spec-42'); }); - it('buildPlanQuery should instruct models to read files from disk', async () => { - vi.resetModules(); - const { _buildPlanQuery } = await import('../commands/consult/index.js'); - const query = _buildPlanQuery('/path/to/plan.md', '/path/to/spec.md'); - - expect(query).toContain('Read the files listed above directly from disk'); - expect(query).toContain('Do NOT rely on `git diff`'); - }); - - it('buildSpecQuery should include file paths', async () => { + it('buildSpecQuery should inline plan content when provided', async () => { vi.resetModules(); const { _buildSpecQuery } = await import('../commands/consult/index.js'); - const query = _buildSpecQuery('/path/to/spec.md', '/path/to/plan.md'); + const query = _buildSpecQuery( + { content: '# Spec content', label: 'spec-42' }, + { content: '# Plan content', label: 'plan-42' }, + ); - expect(query).toContain('/path/to/spec.md'); - expect(query).toContain('/path/to/plan.md'); + expect(query).toContain('# Spec content'); + expect(query).toContain('# Plan content'); }); - it('buildPlanQuery should include file paths', async () => { + it('buildPlanQuery should inline plan and spec content', async () => { vi.resetModules(); const { _buildPlanQuery } = await import('../commands/consult/index.js'); - const query = _buildPlanQuery('/path/to/plan.md', '/path/to/spec.md'); + const query = _buildPlanQuery( + { content: '# Plan content', label: 'plan-42' }, + { content: '# Spec content', label: 'spec-42' }, + ); - expect(query).toContain('/path/to/plan.md'); - expect(query).toContain('/path/to/spec.md'); + expect(query).toContain('# Plan content'); + expect(query).toContain('# Spec content'); + expect(query).toContain('plan-42'); }); it('CLI model spawn should use cwd from workspaceRoot', async () => { diff --git a/packages/codev/src/agent-farm/commands/status.ts b/packages/codev/src/agent-farm/commands/status.ts index f8e6a4113b..42825e1afc 100644 --- a/packages/codev/src/agent-farm/commands/status.ts +++ b/packages/codev/src/agent-farm/commands/status.ts @@ -9,6 +9,8 @@ import { logger } from '../utils/logger.js'; import { getConfig } from '../utils/config.js'; import { getTowerClient } from '../lib/tower-client.js'; import { getTypeColor } from '../utils/display.js'; +import { existsSync, readFileSync } from 'node:fs'; +import { join } from 'node:path'; import chalk from 'chalk'; /** @@ -34,6 +36,8 @@ export async function status(): Promise { logger.kv(' Memory', `${Math.round(health.memoryUsage / 1024 / 1024)}MB`); } + showArtifactConfig(workspacePath); + logger.blank(); // Get workspace status from tower @@ -66,6 +70,9 @@ export async function status(): Promise { // Tower not running - show message and fall back to local state logger.kv('Tower', chalk.gray('not running')); logger.info(`Run 'af tower start' to start the tower daemon`); + + showArtifactConfig(workspacePath); + logger.blank(); // Fall back to local state for legacy display @@ -148,6 +155,43 @@ export async function status(): Promise { } } +function showArtifactConfig(workspacePath: string): void { + const configPath = join(workspacePath, 'af-config.json'); + if (!existsSync(configPath)) return; + + let artifacts: { backend?: string; scope?: string; command?: string } | undefined; + try { + const config = JSON.parse(readFileSync(configPath, 'utf-8')); + artifacts = config?.artifacts; + } catch { return; } + + if (!artifacts?.backend) return; + + logger.blank(); + + if (artifacts.backend === 'cli' || artifacts.backend === 'fava-trails') { + const command = artifacts.command || 'fava-trails'; + logger.kv('Artifacts', chalk.cyan(`cli (${command})`)); + if (artifacts.scope) { + logger.kv(' Scope', artifacts.scope); + } + // Resolve data repo: env var first, then .env file, then default + let dataRepo = process.env.CODEV_ARTIFACTS_DATA_REPO || process.env.FAVA_TRAILS_DATA_REPO; + if (!dataRepo) { + const envPath = join(workspacePath, '.env'); + try { + const envContent = readFileSync(envPath, 'utf-8'); + const newMatch = envContent.match(/^CODEV_ARTIFACTS_DATA_REPO=(.+)$/m); + const legacyMatch = envContent.match(/^FAVA_TRAILS_DATA_REPO=(.+)$/m); + dataRepo = newMatch?.[1]?.trim() || legacyMatch?.[1]?.trim(); + } catch { /* .env may not exist */ } + } + logger.kv(' Data Repo', dataRepo || chalk.yellow('(default)')); + } else { + logger.kv('Artifacts', `${artifacts.backend} (codev/specs/, codev/plans/)`); + } +} + function getStatusColor(status: string, running: boolean): (text: string) => string { if (!running) { return chalk.gray; diff --git a/packages/codev/src/commands/consult/index.ts b/packages/codev/src/commands/consult/index.ts index 1a512a7838..86e39ac360 100644 --- a/packages/codev/src/commands/consult/index.ts +++ b/packages/codev/src/commands/consult/index.ts @@ -15,10 +15,17 @@ import chalk from 'chalk'; import { query as claudeQuery } from '@anthropic-ai/claude-agent-sdk'; import { Codex } from '@openai/codex-sdk'; import { readCodevFile, findWorkspaceRoot } from '../../lib/skeleton.js'; +import { getResolver } from '../porch/artifacts.js'; import { MetricsDB } from './metrics.js'; import { extractUsage, extractReviewText, type SDKResultLike, type UsageData } from './usage-extractor.js'; import { executeForgeCommandSync } from '../../lib/forge.js'; +/** Resolved artifact content with a human-readable label */ +interface ContentRef { + content: string; + label: string; +} + // Model configuration interface ModelConfig { cli: string; @@ -189,20 +196,20 @@ function loadDotenv(workspaceRoot: string): void { } /** - * Find a spec file by number. Returns null if not found. + * Find a spec file by ID. Returns null if not found. * Errors if multiple matches found. */ -function findSpec(workspaceRoot: string, number: number): string | null { +function findSpec(workspaceRoot: string, id: string): string | null { const specsDir = path.join(workspaceRoot, 'codev', 'specs'); - const unpadded = String(number); - const padded = unpadded.padStart(4, '0'); + const unpadded = id.replace(/^0+/, '') || '0'; + const padded = /^\d+$/.test(id) ? id.padStart(4, '0') : id; if (fs.existsSync(specsDir)) { const files = fs.readdirSync(specsDir).filter(f => f.endsWith('.md')); - const matches = files.filter(f => f.startsWith(`${unpadded}-`) || f.startsWith(`${padded}-`)); + const matches = files.filter(f => f.startsWith(`${unpadded}-`) || f.startsWith(`${padded}-`) || f.startsWith(`${id}-`)); if (matches.length > 1) { const list = matches.map(f => ` - codev/specs/${f}`).join('\n'); - throw new Error(`Multiple spec files match '${unpadded}' or '${padded}':\n${list}`); + throw new Error(`Multiple spec files match '${id}':\n${list}`); } if (matches.length === 1) { return path.join(specsDir, matches[0]); @@ -212,20 +219,20 @@ function findSpec(workspaceRoot: string, number: number): string | null { } /** - * Find a plan file by number. Returns null if not found. + * Find a plan file by ID. Returns null if not found. * Errors if multiple matches found. */ -function findPlan(workspaceRoot: string, number: number): string | null { +function findPlan(workspaceRoot: string, id: string): string | null { const plansDir = path.join(workspaceRoot, 'codev', 'plans'); - const unpadded = String(number); - const padded = unpadded.padStart(4, '0'); + const unpadded = id.replace(/^0+/, '') || '0'; + const padded = /^\d+$/.test(id) ? id.padStart(4, '0') : id; if (fs.existsSync(plansDir)) { const files = fs.readdirSync(plansDir).filter(f => f.endsWith('.md')); - const matches = files.filter(f => f.startsWith(`${unpadded}-`) || f.startsWith(`${padded}-`)); + const matches = files.filter(f => f.startsWith(`${unpadded}-`) || f.startsWith(`${padded}-`) || f.startsWith(`${id}-`)); if (matches.length > 1) { const list = matches.map(f => ` - codev/plans/${f}`).join('\n'); - throw new Error(`Multiple plan files match '${unpadded}' or '${padded}':\n${list}`); + throw new Error(`Multiple plan files match '${id}':\n${list}`); } if (matches.length === 1) { return path.join(plansDir, matches[0]); @@ -234,6 +241,34 @@ function findPlan(workspaceRoot: string, number: number): string | null { return null; } +/** + * Find spec content via artifact resolver (FAVA Trails or local fallback). + */ +function findSpecContent(workspaceRoot: string, id: string): ContentRef | null { + try { + const resolver = getResolver(workspaceRoot); + const content = resolver.getSpecContent(id, ''); + if (content) return { content, label: `spec-${id}` }; + } catch { /* resolver init may fail; fall through to local */ } + const specPath = findSpec(workspaceRoot, id); + if (specPath) return { content: fs.readFileSync(specPath, 'utf-8'), label: specPath }; + return null; +} + +/** + * Find plan content via artifact resolver (FAVA Trails or local fallback). + */ +function findPlanContent(workspaceRoot: string, id: string): ContentRef | null { + try { + const resolver = getResolver(workspaceRoot); + const content = resolver.getPlanContent(id, ''); + if (content) return { content, label: `plan-${id}` }; + } catch { /* resolver init may fail; fall through to local */ } + const planPath = findPlan(workspaceRoot, id); + if (planPath) return { content: fs.readFileSync(planPath, 'utf-8'), label: planPath }; + return null; +} + /** * Check if running in a builder worktree */ @@ -846,24 +881,16 @@ KEY_ISSUES: [List of critical issues if any, or "None"]`; /** * Build query for spec review */ -function buildSpecQuery(specPath: string, planPath: string | null): string { - let query = `Review Specification: ${path.basename(specPath)} +function buildSpecQuery(spec: ContentRef, plan: ContentRef | null): string { + let query = `Review Specification: ${spec.label}\n\n`; -Please read and review this specification: -- Spec file: ${specPath} -`; + query += `## Specification\n\n${spec.content}\n\n`; - if (planPath) { - query += `- Plan file: ${planPath}\n`; + if (plan) { + query += `## Implementation Plan (context)\n\n${plan.content}\n\n`; } - query += ` -## How to Review -**Read the files listed above directly from disk.** You have full filesystem access. -Do NOT rely on \`git diff\` or \`git log\` to review content — diffs may be truncated or miss uncommitted work. -Open the spec file, read it in full, and evaluate it directly. - -Please review: + query += `Please review: 1. Clarity and completeness of requirements 2. Technical feasibility 3. Edge cases and error scenarios @@ -890,8 +917,8 @@ KEY_ISSUES: [List of critical issues if any, or "None"]`; */ function buildImplQuery( workspaceRoot: string, - specPath: string | null, - planPath: string | null, + spec: ContentRef | null, + plan: ContentRef | null, planPhase?: string, diffRef?: string, ): string { @@ -912,13 +939,11 @@ function buildImplQuery( query += ` — Phase: ${planPhase}`; } - query += `\n\n## Context Files\n`; - - if (specPath) { - query += `- Spec: ${specPath}\n`; + if (spec) { + query += `\n\n## Specification\n\n${spec.content}\n`; } - if (planPath) { - query += `- Plan: ${planPath}\n`; + if (plan) { + query += `\n## Implementation Plan\n\n${plan.content}\n`; } if (planPhase) { @@ -938,9 +963,8 @@ function buildImplQuery( query += `\n\n## How to Review\n`; query += `**Read the changed files from disk** to review their actual content. You have full filesystem access.\n`; query += `For each file listed above, read it and evaluate the implementation against the spec/plan.\n`; - query += `Do NOT rely on git diffs to determine the current state of code — diffs miss uncommitted changes in worktrees.\n`; } else { - query += `\n## Instructions\n\nRead the spec and plan files above, then explore the filesystem to find and review the implementation changes.\n`; + query += `\n## Instructions\n\nExplore the filesystem to find and review the implementation changes.\n`; } query += ` @@ -967,24 +991,16 @@ KEY_ISSUES: [List of critical issues if any, or "None"]`; /** * Build query for plan review */ -function buildPlanQuery(planPath: string, specPath: string | null): string { - let query = `Review Implementation Plan: ${path.basename(planPath)} +function buildPlanQuery(plan: ContentRef, spec: ContentRef | null): string { + let query = `Review Implementation Plan: ${plan.label}\n\n`; -Please read and review this implementation plan: -- Plan file: ${planPath} -`; + query += `## Implementation Plan\n\n${plan.content}\n\n`; - if (specPath) { - query += `- Spec file: ${specPath} (for context)\n`; + if (spec) { + query += `## Specification (context)\n\n${spec.content}\n\n`; } - query += ` -## How to Review -**Read the files listed above directly from disk.** You have full filesystem access. -Do NOT rely on \`git diff\` or \`git log\` to review content — diffs may be truncated or miss uncommitted work. -Open the plan file (and spec if provided), read them in full, and evaluate the plan directly. - -Please review: + query += `Please review: 1. Alignment with specification requirements 2. Implementation approach and architecture 3. Task breakdown and ordering @@ -1012,8 +1028,8 @@ KEY_ISSUES: [List of critical issues if any, or "None"]`; function buildPhaseQuery( workspaceRoot: string, planPhase: string, - specPath: string | null, - planPath: string | null, + spec: ContentRef | null, + plan: ContentRef | null, ): string { let phaseDiff = ''; try { @@ -1022,13 +1038,12 @@ function buildPhaseQuery( // If git show fails, reviewer explores filesystem } - let query = `Review Phase Implementation: "${planPhase}"\n\n## Context Files\n`; + let query = `Review Phase Implementation: "${planPhase}"\n\n`; - if (specPath) query += `- Spec: ${specPath}\n`; - if (planPath) query += `- Plan: ${planPath}\n`; + if (spec) query += `## Specification\n\n${spec.content}\n\n`; + if (plan) query += `## Implementation Plan\n\n${plan.content}\n\n`; - query += ` -## REVIEW SCOPE — CURRENT PLAN PHASE ONLY + query += `## REVIEW SCOPE — CURRENT PLAN PHASE ONLY You are reviewing **plan phase "${planPhase}" ONLY**. Read the plan, find the section for "${planPhase}", and scope your review to ONLY the work described in that phase. @@ -1102,35 +1117,35 @@ function findPRForIssue(workspaceRoot: string, issueNumber: number): { number: n */ function resolveBuilderQuery(workspaceRoot: string, type: string, options: ConsultOptions): string { const projectState = getBuilderProjectState(workspaceRoot, options.projectId); - const projectNumber = parseInt(projectState.id, 10); + const projectId = projectState.id; switch (type) { case 'spec': { - const specPath = findSpec(workspaceRoot, projectNumber); - if (!specPath) throw new Error(`Spec ${projectState.id} not found in codev/specs/`); - const planPath = findPlan(workspaceRoot, projectNumber); - console.error(`Spec: ${specPath}`); - if (planPath) console.error(`Plan: ${planPath}`); - return buildSpecQuery(specPath, planPath); + const spec = findSpecContent(workspaceRoot, projectId); + if (!spec) throw new Error(`Spec ${projectId} not found in codev/specs/ or FAVA Trails`); + const plan = findPlanContent(workspaceRoot, projectId); + console.error(`Spec: ${spec.label}`); + if (plan) console.error(`Plan: ${plan.label}`); + return buildSpecQuery(spec, plan); } case 'plan': { - const planPath = findPlan(workspaceRoot, projectNumber); - if (!planPath) throw new Error(`Plan ${projectState.id} not found in codev/plans/`); - const specPath = findSpec(workspaceRoot, projectNumber); - console.error(`Plan: ${planPath}`); - if (specPath) console.error(`Spec: ${specPath}`); - return buildPlanQuery(planPath, specPath); + const plan = findPlanContent(workspaceRoot, projectId); + if (!plan) throw new Error(`Plan ${projectId} not found in codev/plans/ or FAVA Trails`); + const spec = findSpecContent(workspaceRoot, projectId); + console.error(`Plan: ${plan.label}`); + if (spec) console.error(`Spec: ${spec.label}`); + return buildPlanQuery(plan, spec); } case 'impl': { - const specPath = findSpec(workspaceRoot, projectNumber); - const planPath = findPlan(workspaceRoot, projectNumber); - console.error(`Project: ${projectState.id}`); - if (specPath) console.error(`Spec: ${specPath}`); - if (planPath) console.error(`Plan: ${planPath}`); + const spec = findSpecContent(workspaceRoot, projectId); + const plan = findPlanContent(workspaceRoot, projectId); + console.error(`Project: ${projectId}`); + if (spec) console.error(`Spec: ${spec.label}`); + if (plan) console.error(`Plan: ${plan.label}`); if (options.planPhase) console.error(`Plan phase: ${options.planPhase}`); - return buildImplQuery(workspaceRoot, specPath, planPath, options.planPhase); + return buildImplQuery(workspaceRoot, spec, plan, options.planPhase); } case 'pr': { @@ -1144,12 +1159,12 @@ function resolveBuilderQuery(workspaceRoot: string, type: string, options: Consu if (!currentPhase) { throw new Error('No current plan phase detected. Use --plan-phase to specify.'); } - const specPath = findSpec(workspaceRoot, projectNumber); - const planPath = findPlan(workspaceRoot, projectNumber); + const spec = findSpecContent(workspaceRoot, projectId); + const plan = findPlanContent(workspaceRoot, projectId); console.error(`Phase: ${currentPhase}`); - if (specPath) console.error(`Spec: ${specPath}`); - if (planPath) console.error(`Plan: ${planPath}`); - return buildPhaseQuery(workspaceRoot, currentPhase, specPath, planPath); + if (spec) console.error(`Spec: ${spec.label}`); + if (plan) console.error(`Plan: ${plan.label}`); + return buildPhaseQuery(workspaceRoot, currentPhase, spec, plan); } case 'integration': { @@ -1178,32 +1193,37 @@ function resolveArchitectQuery(workspaceRoot: string, type: string, options: Con ); } - const issueNumber = parseInt(options.issue, 10); - if (isNaN(issueNumber)) { - throw new Error(`Invalid issue number: ${options.issue}`); + const issueId = options.issue; + + // Parse as number only for GitHub API calls (PR lookup). + // Spec/plan lookups use the raw string to support non-numeric IDs. + function issueAsNumber(): number { + const n = parseInt(issueId, 10); + if (isNaN(n)) throw new Error(`--issue must be numeric for PR lookup: ${issueId}`); + return n; } switch (type) { case 'spec': { - const specPath = findSpec(workspaceRoot, issueNumber); - if (!specPath) throw new Error(`Spec ${issueNumber} not found in codev/specs/`); - const planPath = findPlan(workspaceRoot, issueNumber); - console.error(`Spec: ${specPath}`); - if (planPath) console.error(`Plan: ${planPath}`); - return buildSpecQuery(specPath, planPath); + const spec = findSpecContent(workspaceRoot, issueId); + if (!spec) throw new Error(`Spec ${issueId} not found in codev/specs/ or FAVA Trails`); + const plan = findPlanContent(workspaceRoot, issueId); + console.error(`Spec: ${spec.label}`); + if (plan) console.error(`Plan: ${plan.label}`); + return buildSpecQuery(spec, plan); } case 'plan': { - const planPath = findPlan(workspaceRoot, issueNumber); - if (!planPath) throw new Error(`Plan ${issueNumber} not found in codev/plans/`); - const specPath = findSpec(workspaceRoot, issueNumber); - console.error(`Plan: ${planPath}`); - if (specPath) console.error(`Spec: ${specPath}`); - return buildPlanQuery(planPath, specPath); + const plan = findPlanContent(workspaceRoot, issueId); + if (!plan) throw new Error(`Plan ${issueId} not found in codev/plans/ or FAVA Trails`); + const spec = findSpecContent(workspaceRoot, issueId); + console.error(`Plan: ${plan.label}`); + if (spec) console.error(`Spec: ${spec.label}`); + return buildPlanQuery(plan, spec); } case 'impl': { - const pr = findPRForIssue(workspaceRoot, issueNumber); + const pr = findPRForIssue(workspaceRoot, issueAsNumber()); // Fetch the branch and diff from merge-base try { execSync(`git fetch origin ${pr.headRefName}`, { cwd: workspaceRoot, stdio: 'pipe' }); @@ -1211,22 +1231,22 @@ function resolveArchitectQuery(workspaceRoot: string, type: string, options: Con // May already be fetched } const mergeBase = execSync(`git merge-base main origin/${pr.headRefName}`, { cwd: workspaceRoot, encoding: 'utf-8' }).trim(); - const specPath = findSpec(workspaceRoot, issueNumber); - const planPath = findPlan(workspaceRoot, issueNumber); - console.error(`Project: ${issueNumber} (PR #${pr.number}, branch: ${pr.headRefName})`); - if (specPath) console.error(`Spec: ${specPath}`); - if (planPath) console.error(`Plan: ${planPath}`); - return buildImplQuery(workspaceRoot, specPath, planPath, options.planPhase, `${mergeBase}..origin/${pr.headRefName}`); + const spec = findSpecContent(workspaceRoot, issueId); + const plan = findPlanContent(workspaceRoot, issueId); + console.error(`Project: ${issueId} (PR #${pr.number}, branch: ${pr.headRefName})`); + if (spec) console.error(`Spec: ${spec.label}`); + if (plan) console.error(`Plan: ${plan.label}`); + return buildImplQuery(workspaceRoot, spec, plan, options.planPhase, `${mergeBase}..origin/${pr.headRefName}`); } case 'pr': { - const pr = findPRForIssue(workspaceRoot, issueNumber); + const pr = findPRForIssue(workspaceRoot, issueAsNumber()); console.error(`PR: #${pr.number}`); return buildPRQuery(pr.number); } case 'integration': { - const pr = findPRForIssue(workspaceRoot, issueNumber); + const pr = findPRForIssue(workspaceRoot, issueAsNumber()); console.error(`PR: #${pr.number} (integration review)`); return buildPRQuery(pr.number); } diff --git a/packages/codev/src/commands/porch/__tests__/state.test.ts b/packages/codev/src/commands/porch/__tests__/state.test.ts index eb43cf5d4c..c414a3a0f7 100644 --- a/packages/codev/src/commands/porch/__tests__/state.test.ts +++ b/packages/codev/src/commands/porch/__tests__/state.test.ts @@ -439,10 +439,9 @@ updated_at: "${state.updated_at}" .toThrow('Cannot determine project ID'); }); - it('step 4: task/protocol worktrees fall through to error when no filesystem match', () => { - // Task worktrees return null from CWD detection, and empty root has no projects - expect(() => resolveProjectId(undefined, '/repo/.builders/task-aB2C', emptyProjectRoot)) - .toThrow('Cannot determine project ID'); + it('step 2: protocol worktrees with numeric ID are detected from CWD', () => { + const result = resolveProjectId(undefined, '/repo/.builders/aspir-17-feature', emptyProjectRoot); + expect(result).toEqual({ id: '17', source: 'cwd' }); }); }); diff --git a/packages/codev/src/commands/porch/artifacts.ts b/packages/codev/src/commands/porch/artifacts.ts new file mode 100644 index 0000000000..434aa40dfe --- /dev/null +++ b/packages/codev/src/commands/porch/artifacts.ts @@ -0,0 +1,446 @@ +/** + * Artifact Resolver — pluggable backend for codev artifact access. + * + * Decouples porch from filesystem assumptions. Two backends: + * - LocalResolver: reads from codev/specs/, codev/plans/ (default, backward compatible) + * - CliResolver: shells out to a configurable CLI command (e.g. `fava-trails get`) + * + * Spec 559: Porch Artifact Resolver + */ + +import { execFileSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { globSync } from 'glob'; + +// ============================================================================= +// Interface +// ============================================================================= + +export interface ArtifactResolver { + /** Find spec basename by numeric ID (e.g., "0559-porch-artifact-resolver") */ + findSpecBaseName(projectId: string, title: string): string | null; + + /** Get full content of a spec by project ID */ + getSpecContent(projectId: string, title: string): string | null; + + /** Get full content of a plan by project ID */ + getPlanContent(projectId: string, title: string): string | null; + + /** Get full content of a review by project ID */ + getReviewContent(projectId: string, title: string): string | null; + + /** Check if a spec/plan has pre-approval frontmatter */ + hasPreApproval(artifactGlob: string): boolean; +} + +// ============================================================================= +// Shared Helpers +// ============================================================================= + +/** + * Check if artifact content has pre-approval frontmatter. + * Looks for YAML frontmatter with `approved:` and `validated:` fields. + * Used by both LocalResolver and CliResolver for consistency. + */ +export function isPreApprovedContent(content: string): boolean { + const frontmatterMatch = content.match(/^---\n([\s\S]*?)\n---/); + if (!frontmatterMatch) return false; + + const frontmatter = frontmatterMatch[1]; + const hasApproved = /^approved:\s*.+$/m.test(frontmatter); + const hasValidated = /^validated:\s*\[.+\]$/m.test(frontmatter); + return hasApproved && hasValidated; +} + +// ============================================================================= +// Local Resolver (default — reads from codev/ directory) +// ============================================================================= + +export class LocalResolver implements ArtifactResolver { + constructor(private workspaceRoot: string) {} + + findSpecBaseName(projectId: string, _title: string): string | null { + const specsDir = path.join(this.workspaceRoot, 'codev', 'specs'); + if (!fs.existsSync(specsDir)) return null; + + const normalizedId = projectId.replace(/^0+/, '') || '0'; + try { + const files = fs.readdirSync(specsDir); + const specFile = files.find(f => { + if (!f.endsWith('.md')) return false; + const numMatch = f.match(/^(\d+)/); + if (!numMatch) return false; + return (numMatch[1].replace(/^0+/, '') || '0') === normalizedId; + }); + return specFile ? specFile.replace(/\.md$/, '') : null; + } catch { + return null; + } + } + + getSpecContent(projectId: string, title: string): string | null { + const baseName = this.findSpecBaseName(projectId, title); + if (!baseName) return null; + const filePath = path.join(this.workspaceRoot, 'codev', 'specs', `${baseName}.md`); + try { + return fs.readFileSync(filePath, 'utf-8'); + } catch { + return null; + } + } + + getPlanContent(projectId: string, title: string): string | null { + // Try new location first: codev/projects/-/plan.md + const projectsDir = path.join(this.workspaceRoot, 'codev', 'projects'); + if (fs.existsSync(projectsDir)) { + const normalizedId = projectId.replace(/^0+/, '') || '0'; + try { + const dirs = fs.readdirSync(projectsDir); + const projDir = dirs.find(d => { + const numMatch = d.match(/^(\d+)/); + if (!numMatch) return false; + return (numMatch[1].replace(/^0+/, '') || '0') === normalizedId; + }); + if (projDir) { + const planPath = path.join(projectsDir, projDir, 'plan.md'); + if (fs.existsSync(planPath)) { + return fs.readFileSync(planPath, 'utf-8'); + } + } + } catch { /* continue to legacy */ } + } + + // Legacy location: codev/plans/-*.md + const plansDir = path.join(this.workspaceRoot, 'codev', 'plans'); + if (!fs.existsSync(plansDir)) return null; + + const normalizedId = projectId.replace(/^0+/, '') || '0'; + try { + const files = fs.readdirSync(plansDir); + const planFile = files.find(f => { + if (!f.endsWith('.md')) return false; + const numMatch = f.match(/^(\d+)/); + if (!numMatch) return false; + return (numMatch[1].replace(/^0+/, '') || '0') === normalizedId; + }); + if (planFile) { + return fs.readFileSync(path.join(plansDir, planFile), 'utf-8'); + } + } catch { /* ignore */ } + + return null; + } + + getReviewContent(projectId: string, _title: string): string | null { + const reviewsDir = path.join(this.workspaceRoot, 'codev', 'reviews'); + if (!fs.existsSync(reviewsDir)) return null; + + const normalizedId = projectId.replace(/^0+/, '') || '0'; + try { + const files = fs.readdirSync(reviewsDir); + const reviewFile = files.find(f => { + if (!f.endsWith('.md')) return false; + const numMatch = f.match(/^(\d+)/); + if (!numMatch) return false; + return (numMatch[1].replace(/^0+/, '') || '0') === normalizedId; + }); + if (reviewFile) { + return fs.readFileSync(path.join(reviewsDir, reviewFile), 'utf-8'); + } + } catch { /* ignore */ } + + return null; + } + + hasPreApproval(artifactGlob: string): boolean { + const matches = globSync(artifactGlob, { cwd: this.workspaceRoot }); + if (matches.length === 0) return false; + + const filePath = path.join(this.workspaceRoot, matches[0]); + try { + const content = fs.readFileSync(filePath, 'utf-8'); + return isPreApprovedContent(content); + } catch { + return false; + } + } +} + +// ============================================================================= +// CLI Resolver (shells out to a configurable CLI command) +// ============================================================================= + +/** Sentinel value for cached negative results (CLI returned error) */ +const NEGATIVE_CACHE = Symbol('negative'); +type CacheEntry = string | typeof NEGATIVE_CACHE; + +export class CliResolver implements ArtifactResolver { + private cache = new Map(); + private extraEnv: Record; + + constructor( + private scope: string, + private command: string = 'fava-trails', + workspaceRoot?: string, + ) { + // Read data repo env vars from .env if not already in process.env. + // af_builder.sh injects these into builder worktree .env files. + this.extraEnv = {}; + const dataRepo = process.env.CODEV_ARTIFACTS_DATA_REPO || process.env.FAVA_TRAILS_DATA_REPO; + if (!dataRepo && workspaceRoot) { + const envPath = path.join(workspaceRoot, '.env'); + try { + const envContent = fs.readFileSync(envPath, 'utf-8'); + // Try new env var first, fall back to legacy + const newMatch = envContent.match(/^CODEV_ARTIFACTS_DATA_REPO=(.+)$/m); + const legacyMatch = envContent.match(/^FAVA_TRAILS_DATA_REPO=(.+)$/m); + const repo = newMatch?.[1]?.trim() || legacyMatch?.[1]?.trim(); + if (repo) { + this.extraEnv.CODEV_ARTIFACTS_DATA_REPO = repo; + this.extraEnv.FAVA_TRAILS_DATA_REPO = repo; // backward compat + } + } catch { /* .env may not exist */ } + } + } + + findSpecBaseName(projectId: string, _title: string): string | null { + const children = this.listChildren('specs'); + if (!children) return null; + + const normalizedId = projectId.replace(/^0+/, '') || '0'; + const match = children.find(name => { + const numMatch = name.match(/^(\d+)/); + if (!numMatch) return false; + return (numMatch[1].replace(/^0+/, '') || '0') === normalizedId; + }); + return match || null; + } + + getSpecContent(projectId: string, _title: string): string | null { + const baseName = this.findSpecBaseName(projectId, _title); + if (!baseName) return null; + return this.getContent(`specs/${baseName}`); + } + + getPlanContent(projectId: string, _title: string): string | null { + const baseName = this.findPlanBaseName(projectId); + if (!baseName) return null; + return this.getContent(`plans/${baseName}`); + } + + getReviewContent(projectId: string, _title: string): string | null { + const baseName = this.findReviewBaseName(projectId); + if (!baseName) return null; + return this.getContent(`reviews/${baseName}`); + } + + hasPreApproval(artifactGlob: string): boolean { + // Determine artifact type from glob path (e.g., "codev/specs/0559-*.md" or "codev/plans/0559-*.md") + const typeMatch = artifactGlob.match(/\b(specs|plans|reviews)\b/); + const idMatch = artifactGlob.match(/(?:specs|plans|reviews)\/0*(\d+)/); + if (!idMatch) return false; + + const projectId = idMatch[1]; + let content: string | null = null; + const artifactType = typeMatch?.[1] || 'specs'; + + if (artifactType === 'plans') { + content = this.getPlanContent(projectId, ''); + } else if (artifactType === 'reviews') { + content = this.getReviewContent(projectId, ''); + } else { + content = this.getSpecContent(projectId, ''); + } + + if (!content) return false; + return isPreApprovedContent(content); + } + + // --------------------------------------------------------------------------- + // Internal helpers + // --------------------------------------------------------------------------- + + private findPlanBaseName(projectId: string): string | null { + const children = this.listChildren('plans'); + if (!children) return null; + + const normalizedId = projectId.replace(/^0+/, '') || '0'; + const match = children.find(name => { + const numMatch = name.match(/^(\d+)/); + if (!numMatch) return false; + return (numMatch[1].replace(/^0+/, '') || '0') === normalizedId; + }); + return match || null; + } + + private findReviewBaseName(projectId: string): string | null { + const children = this.listChildren('reviews'); + if (!children) return null; + + const normalizedId = projectId.replace(/^0+/, '') || '0'; + const match = children.find(name => { + const numMatch = name.match(/^(\d+)/); + if (!numMatch) return false; + return (numMatch[1].replace(/^0+/, '') || '0') === normalizedId; + }); + return match || null; + } + + private listChildren(subPath: string): string[] | null { + const cacheKey = `list:${subPath}`; + if (this.cache.has(cacheKey)) { + const cached = this.cache.get(cacheKey)!; + if (cached === NEGATIVE_CACHE) return null; + const items = cached.split('\n').filter(Boolean); + return items.length > 0 ? items : null; + } + + const scopePath = `${this.scope}/${subPath}`; + try { + const output = execFileSync(this.command, ['get', '--list', scopePath], { + encoding: 'utf-8', + timeout: 5000, + env: { ...process.env, ...this.extraEnv }, + }).trim(); + // Cache both non-empty and empty results (empty = scope exists but has no children) + this.cache.set(cacheKey, output); + const items = output ? output.split('\n').filter(Boolean) : []; + return items.length > 0 ? items : null; + } catch (err: unknown) { + this.handleError(err, scopePath); + // Cache failures as negative to avoid repeated CLI timeouts + this.cache.set(cacheKey, NEGATIVE_CACHE); + return null; + } + } + + private getContent(subPath: string): string | null { + const cacheKey = `content:${subPath}`; + if (this.cache.has(cacheKey)) { + const cached = this.cache.get(cacheKey)!; + return cached === NEGATIVE_CACHE ? null : cached; + } + + const scopePath = `${this.scope}/${subPath}`; + try { + const output = execFileSync(this.command, ['get', scopePath], { + encoding: 'utf-8', + timeout: 5000, + env: { ...process.env, ...this.extraEnv }, + }); + this.cache.set(cacheKey, output); + return output; + } catch (err: unknown) { + this.handleError(err, scopePath); + // Cache failures as negative to avoid repeated CLI timeouts + this.cache.set(cacheKey, NEGATIVE_CACHE); + return null; + } + } + + private handleError(err: unknown, scopePath: string): void { + if (err && typeof err === 'object' && 'code' in err) { + if ((err as { code: string }).code === 'ENOENT') { + throw new Error( + `CLI command '${this.command}' not found. Ensure it is installed and on PATH.` + ); + } + } + // Non-zero exit — log warning for debugging + if (err && typeof err === 'object' && 'stderr' in err) { + const stderr = (err as { stderr: string }).stderr; + if (stderr) { + console.error(`[porch] ${this.command} get ${scopePath}: ${stderr.trim()}`); + } + } + } +} + +// ============================================================================= +// Config Resolution +// ============================================================================= + +/** + * Find the root directory containing af-config.json. + * In builder worktrees, af-config.json only exists in the main repo. + * Falls back to the main repo via git worktree resolution. + */ +export function findConfigRoot(workspaceRoot: string): string { + if (fs.existsSync(path.join(workspaceRoot, 'af-config.json'))) { + return workspaceRoot; + } + try { + const gitCommonDir = execFileSync('git', ['rev-parse', '--git-common-dir'], { + cwd: workspaceRoot, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + }).trim(); + if (gitCommonDir !== '.git') { + const mainGitDir = path.resolve(workspaceRoot, gitCommonDir); + const mainRepo = path.dirname(mainGitDir.replace(/\/worktrees\/[^/]+$/, '')); + if (fs.existsSync(path.join(mainRepo, 'af-config.json'))) { + return mainRepo; + } + } + } catch { /* not in git */ } + return workspaceRoot; +} + +// ============================================================================= +// Factory +// ============================================================================= + +export interface ArtifactConfig { + backend?: 'local' | 'cli' | 'fava-trails'; + scope?: string; + command?: string; +} + +/** + * Load artifact resolver config from af-config.json. + * Resolves to main repo when running from a builder worktree. + */ +function loadArtifactConfig(workspaceRoot: string): ArtifactConfig | null { + const configRoot = findConfigRoot(workspaceRoot); + const configPath = path.join(configRoot, 'af-config.json'); + if (!fs.existsSync(configPath)) return null; + + try { + const raw = fs.readFileSync(configPath, 'utf-8'); + const config = JSON.parse(raw); + if (typeof config?.artifacts === 'object') { + return config.artifacts as ArtifactConfig; + } + } catch { /* ignore */ } + + return null; +} + +/** + * Create the appropriate artifact resolver for this workspace. + */ +export function getResolver(workspaceRoot: string): ArtifactResolver { + const config = loadArtifactConfig(workspaceRoot); + + // 'cli' is the canonical backend; 'fava-trails' is accepted as an alias + if (config?.backend === 'cli' || config?.backend === 'fava-trails') { + if (!config.scope) { + throw new Error( + `af-config.json has artifacts.backend: "${config.backend}" but no artifacts.scope.\n` + + `Add: "artifacts": { "backend": "cli", "scope": "org/project/codev-assets" }` + ); + } + const command = config.command || 'fava-trails'; + return new CliResolver(config.scope, command, workspaceRoot); + } + + if (config?.backend && config.backend !== 'local') { + throw new Error( + `af-config.json has unknown artifacts.backend: "${config.backend}".\n` + + `Valid values: "local" (default), "cli"` + ); + } + + return new LocalResolver(workspaceRoot); +} diff --git a/packages/codev/src/commands/porch/checks.ts b/packages/codev/src/commands/porch/checks.ts index 7318c50972..046c53186b 100644 --- a/packages/codev/src/commands/porch/checks.ts +++ b/packages/codev/src/commands/porch/checks.ts @@ -9,6 +9,7 @@ import { spawn, execFile } from 'node:child_process'; import { promisify } from 'node:util'; import * as path from 'node:path'; import type { CheckResult, CheckDef } from './types.js'; +import type { ArtifactResolver } from './artifacts.js'; import { executeForgeCommand, getForgeCommand, loadForgeConfig } from '../../lib/forge.js'; const execFileAsync = promisify(execFile); @@ -123,6 +124,103 @@ export async function runCheck( }); } +/** + * Try to run an artifact-dependent check programmatically via the resolver. + * Returns a CheckResult if the check name is recognized, null otherwise + * (caller should fall back to shell execution). + */ +export function runArtifactCheck( + name: string, + command: string, + resolver: ArtifactResolver, + env: CheckEnv, +): CheckResult | null { + const startTime = Date.now(); + const { PROJECT_ID: projectId, PROJECT_TITLE: title } = env; + + switch (name) { + case 'plan_exists': { + const content = resolver.getPlanContent(projectId, title); + return { + name, + command, + passed: content !== null, + output: content !== null ? 'Plan found via resolver' : undefined, + error: content === null ? 'Plan not found' : undefined, + duration_ms: Date.now() - startTime, + }; + } + + case 'has_phases_json': { + const content = resolver.getPlanContent(projectId, title); + if (content === null) { + return { name, command, passed: false, error: 'Plan not found', duration_ms: Date.now() - startTime }; + } + const has = content.includes('"phases":'); + return { + name, + command, + passed: has, + output: has ? 'Found phases JSON block' : undefined, + error: has ? undefined : 'No "phases": found in plan', + duration_ms: Date.now() - startTime, + }; + } + + case 'min_two_phases': { + const content = resolver.getPlanContent(projectId, title); + if (content === null) { + return { name, command, passed: false, error: 'Plan not found', duration_ms: Date.now() - startTime }; + } + const matches = content.match(/"id":\s*"[^"]*"/g); + const count = matches ? matches.length : 0; + return { + name, + command, + passed: count >= 2, + output: `Found ${count} phase(s)`, + error: count < 2 ? `Only ${count} phase(s) found, need at least 2` : undefined, + duration_ms: Date.now() - startTime, + }; + } + + case 'review_has_arch_updates': { + const content = resolver.getReviewContent(projectId, title); + if (content === null) { + return { name, command, passed: false, error: 'Review not found', duration_ms: Date.now() - startTime }; + } + const has = content.includes('## Architecture Updates'); + return { + name, + command, + passed: has, + output: has ? 'Found Architecture Updates section' : undefined, + error: has ? undefined : 'Missing "## Architecture Updates" section in review', + duration_ms: Date.now() - startTime, + }; + } + + case 'review_has_lessons_updates': { + const content = resolver.getReviewContent(projectId, title); + if (content === null) { + return { name, command, passed: false, error: 'Review not found', duration_ms: Date.now() - startTime }; + } + const has = content.includes('## Lessons Learned Updates'); + return { + name, + command, + passed: has, + output: has ? 'Found Lessons Learned Updates section' : undefined, + error: has ? undefined : 'Missing "## Lessons Learned Updates" section in review', + duration_ms: Date.now() - startTime, + }; + } + + default: + return null; // Not an artifact check — fall through to shell execution + } +} + /** * Run multiple checks for a phase. * Accepts either Record (legacy) or Record. @@ -135,12 +233,27 @@ export async function runPhaseChecks( checks: Record, cwd: string, env: CheckEnv, - timeoutMs: number = DEFAULT_TIMEOUT_MS + timeoutMs: number = DEFAULT_TIMEOUT_MS, + resolver?: ArtifactResolver, + overriddenChecks?: Set, ): Promise { const results: CheckResult[] = []; for (const [name, checkVal] of Object.entries(checks)) { const command = typeof checkVal === 'string' ? checkVal : checkVal.command; + + // Try resolver-based check first (handles artifact-dependent checks programmatically). + // Skip resolver fast-path if the user overrode this check via af-config.json — + // their custom command should run instead. + if (resolver && !overriddenChecks?.has(name)) { + const artifactResult = runArtifactCheck(name, command, resolver, env); + if (artifactResult) { + results.push(artifactResult); + if (!artifactResult.passed) break; + continue; + } + } + const checkCwd = typeof checkVal === 'object' && checkVal.cwd ? path.resolve(cwd, checkVal.cwd) : cwd; diff --git a/packages/codev/src/commands/porch/config.ts b/packages/codev/src/commands/porch/config.ts index 5109e4a3ce..7a533f08c2 100644 --- a/packages/codev/src/commands/porch/config.ts +++ b/packages/codev/src/commands/porch/config.ts @@ -1,5 +1,5 @@ /** - * Porch config reader — loads porch.checks from af-config.json. + * Porch config reader — loads porch.checks and porch.consultation from af-config.json. * * Intentionally self-contained: does NOT import from agent-farm's config * module, keeping porch independent of the af dependency tree. @@ -8,6 +8,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import type { CheckOverrides } from './types.js'; +import { findConfigRoot } from './artifacts.js'; /** * Load check overrides from af-config.json in the workspace root. @@ -21,7 +22,7 @@ import type { CheckOverrides } from './types.js'; * Throws when af-config.json exists but cannot be parsed as JSON. */ export function loadCheckOverrides(workspaceRoot: string): CheckOverrides | null { - const configPath = path.join(workspaceRoot, 'af-config.json'); + const configPath = path.join(findConfigRoot(workspaceRoot), 'af-config.json'); if (!fs.existsSync(configPath)) { return null; diff --git a/packages/codev/src/commands/porch/index.ts b/packages/codev/src/commands/porch/index.ts index 3a8dc954f0..25468304c8 100644 --- a/packages/codev/src/commands/porch/index.ts +++ b/packages/codev/src/commands/porch/index.ts @@ -46,6 +46,7 @@ import { type CheckEnv, } from './checks.js'; import { loadCheckOverrides } from './config.js'; +import { getResolver } from './artifacts.js'; // ============================================================================ // Output Helpers @@ -89,6 +90,21 @@ function logCheckOverrides( } } +/** + * Compute the set of check names that were overridden by af-config.json. + * Used to prevent resolver fast-path from bypassing user-customized commands. + */ +function getOverriddenCheckNames(overrides: import('./types.js').CheckOverrides | null): Set { + if (!overrides) return new Set(); + const names = new Set(); + for (const [name, override] of Object.entries(overrides)) { + if (override.command || override.cwd) { + names.add(name); + } + } + return names; +} + // ============================================================================ // Commands // ============================================================================ @@ -215,6 +231,7 @@ export async function check(workspaceRoot: string, projectId: string): Promise 0) { - const results = await runPhaseChecks(checks, workspaceRoot, checkEnv); + const results = await runPhaseChecks(checks, workspaceRoot, checkEnv, undefined, resolver, getOverriddenCheckNames(overrides)); console.log(formatCheckResults(results)); if (!allChecksPassed(results)) { @@ -471,6 +489,9 @@ export async function gate(workspaceRoot: string, projectId: string): Promise 0) { + const resolver = getResolver(workspaceRoot); const checkEnv: CheckEnv = { PROJECT_ID: state.id, PROJECT_TITLE: resolveArtifactBaseName(workspaceRoot, state.id, state.title) }; console.log(''); @@ -538,7 +560,7 @@ export async function approve( logCheckOverrides(phaseCheckNames, checks, overrides); if (Object.keys(checks).length > 0) { - const results = await runPhaseChecks(checks, workspaceRoot, checkEnv); + const results = await runPhaseChecks(checks, workspaceRoot, checkEnv, undefined, resolver, getOverriddenCheckNames(overrides)); console.log(formatCheckResults(results)); if (!allChecksPassed(results)) { @@ -746,7 +768,8 @@ function getNextAction(state: ProjectState, protocol: Protocol): string { } function getArtifactForPhase(workspaceRoot: string, state: ProjectState): string | null { - const baseName = resolveArtifactBaseName(workspaceRoot, state.id, state.title); + const resolver = getResolver(workspaceRoot); + const baseName = resolveArtifactBaseName(workspaceRoot, state.id, state.title, resolver); switch (state.phase) { case 'specify': return `codev/specs/${baseName}.md`; diff --git a/packages/codev/src/commands/porch/next.ts b/packages/codev/src/commands/porch/next.ts index 695f41815c..86640cd55d 100644 --- a/packages/codev/src/commands/porch/next.ts +++ b/packages/codev/src/commands/porch/next.ts @@ -11,7 +11,6 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; -import { globSync } from 'glob'; import { readState, writeState, findStatusPath, getProjectDir, resolveArtifactBaseName } from './state.js'; import { getForgeCommand, loadForgeConfig } from '../../lib/forge.js'; import { @@ -27,12 +26,13 @@ import { getPhaseChecks, } from './protocol.js'; import { - findPlanFile, - extractPhasesFromFile, + extractPlanPhasesResolved, getCurrentPlanPhase, advancePlanPhase, allPlanPhasesComplete, } from './plan.js'; +import { getResolver } from './artifacts.js'; +import type { ArtifactResolver } from './artifacts.js'; import { buildPhasePrompt } from './prompts.js'; import { parseVerdict, allApprove } from './verdict.js'; import { loadCheckOverrides } from './config.js'; @@ -49,30 +49,10 @@ import type { /** * Check if an artifact file has YAML frontmatter indicating it was * already approved and validated (3-way review). - * - * Frontmatter format: - * --- - * approved: 2026-01-29 - * validated: [gemini, codex, claude] - * --- + * Uses the artifact resolver for pluggable backend support. */ -function isArtifactPreApproved(workspaceRoot: string, artifactGlob: string): boolean { - const matches = globSync(artifactGlob, { cwd: workspaceRoot }); - if (matches.length === 0) return false; - - const filePath = path.join(workspaceRoot, matches[0]); - try { - const content = fs.readFileSync(filePath, 'utf-8'); - const frontmatterMatch = content.match(/^---\n([\s\S]*?)\n---/); - if (!frontmatterMatch) return false; - - const frontmatter = frontmatterMatch[1]; - const hasApproved = /^approved:\s*.+$/m.test(frontmatter); - const hasValidated = /^validated:\s*\[.+\]$/m.test(frontmatter); - return hasApproved && hasValidated; - } catch { - return false; - } +function isArtifactPreApproved(resolver: ArtifactResolver, artifactGlob: string): boolean { + return resolver.hasPreApproval(artifactGlob); } /** @@ -224,6 +204,7 @@ export async function next(workspaceRoot: string, projectId: string): Promise 0) { - state.current_plan_phase = state.plan_phases[0].id; - } + state.plan_phases = extractPlanPhasesResolved(workspaceRoot, state.id, state.title, resolver); + if (state.plan_phases.length > 0) { + state.current_plan_phase = state.plan_phases[0].id; } } state.iteration = 1; @@ -349,12 +327,9 @@ export async function next(workspaceRoot: string, projectId: string): Promise 0) { - state.current_plan_phase = state.plan_phases[0].id; - } + state.plan_phases = extractPlanPhasesResolved(workspaceRoot, state.id, state.title, resolver); + if (state.plan_phases.length > 0) { + state.current_plan_phase = state.plan_phases[0].id; } } @@ -365,11 +340,11 @@ export async function next(workspaceRoot: string, projectId: string): Promise { const verifyConfig = getVerifyConfig(protocol, state.phase); const overrides = loadCheckOverrides(workspaceRoot); @@ -399,7 +375,7 @@ async function handleBuildVerify( // --- NEED BUILD --- if (!state.build_complete) { - const prompt = await buildPhasePrompt(workspaceRoot, state, protocol); + const prompt = await buildPhasePrompt(workspaceRoot, state, protocol, resolver); const tasks: PorchTask[] = []; // Main build task with full phase prompt @@ -697,9 +673,10 @@ async function handleOncePhase( state: ProjectState, protocol: Protocol, phaseConfig: ProtocolPhase, + resolver?: ArtifactResolver, ): Promise { // Try to load a prompt file for this phase - const prompt = await buildPhasePrompt(workspaceRoot, state, protocol); + const prompt = await buildPhasePrompt(workspaceRoot, state, protocol, resolver); // If prompt is just a generic fallback, try to use phase steps from protocol let description = prompt; diff --git a/packages/codev/src/commands/porch/plan.ts b/packages/codev/src/commands/porch/plan.ts index 0fb1b4c7e0..df5ccf22a0 100644 --- a/packages/codev/src/commands/porch/plan.ts +++ b/packages/codev/src/commands/porch/plan.ts @@ -12,6 +12,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import type { PlanPhase } from './types.js'; import { stripIdPrefix } from './state.js'; +import type { ArtifactResolver } from './artifacts.js'; // ============================================================================ // Plan File Discovery @@ -141,6 +142,49 @@ export function extractPhasesFromFile(planFilePath: string): PlanPhase[] { return extractPlanPhases(content); } +/** + * Get plan content via resolver (preferred) or local file fallback. + * Returns the raw plan markdown content, or null if not found. + * + * When a resolver is provided, it handles both backends (local and remote). + * Local file fallback only applies when no resolver is given (backward compat). + */ +export function getPlanContent( + workspaceRoot: string, + projectId: string, + projectTitle: string, + resolver?: ArtifactResolver, +): string | null { + // Resolver handles both LocalResolver and CliResolver + if (resolver) { + return resolver.getPlanContent(projectId, projectTitle); + } + + // No resolver — legacy local file fallback + const planPath = findPlanFile(workspaceRoot, projectId, projectTitle); + if (!planPath) return null; + + try { + return fs.readFileSync(planPath, 'utf-8'); + } catch { + return null; + } +} + +/** + * Extract plan phases via resolver (preferred) or local file fallback. + */ +export function extractPlanPhasesResolved( + workspaceRoot: string, + projectId: string, + projectTitle: string, + resolver?: ArtifactResolver, +): PlanPhase[] { + const content = getPlanContent(workspaceRoot, projectId, projectTitle, resolver); + if (!content) return []; + return extractPlanPhases(content); +} + // ============================================================================ // Phase Navigation // ============================================================================ diff --git a/packages/codev/src/commands/porch/prompts.ts b/packages/codev/src/commands/porch/prompts.ts index 530609ca57..9d4a4bca7e 100644 --- a/packages/codev/src/commands/porch/prompts.ts +++ b/packages/codev/src/commands/porch/prompts.ts @@ -12,8 +12,10 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import type { ProjectState, Protocol, ProtocolPhase, PlanPhase, IterationRecord } from './types.js'; import { getPhaseConfig, isPhased, isBuildVerify, getBuildConfig } from './protocol.js'; -import { findPlanFile, getCurrentPlanPhase, getPhaseContent } from './plan.js'; +import { getCurrentPlanPhase, getPhaseContent, getPlanContent } from './plan.js'; import { getProjectDir, resolveArtifactBaseName } from './state.js'; +import { getResolver } from './artifacts.js'; +import type { ArtifactResolver } from './artifacts.js'; import { fetchIssue } from '../../lib/github.js'; /** Locations to search for protocol prompts */ @@ -31,7 +33,7 @@ const PROTOCOL_PATHS = [ * 2. Spec file first heading — fallback for legacy/offline specs * 3. Project title from status.yaml — last resort */ -export async function getProjectSummary(workspaceRoot: string, projectId: string, projectTitle?: string): Promise { +export async function getProjectSummary(workspaceRoot: string, projectId: string, projectTitle?: string, resolver?: ArtifactResolver): Promise { // 1. Try GitHub issue const issueNumber = parseInt(projectId, 10); if (!isNaN(issueNumber)) { @@ -39,34 +41,20 @@ export async function getProjectSummary(workspaceRoot: string, projectId: string if (issue?.title) { return issue.title; } + console.error(`[porch] project summary: GitHub issue #${issueNumber} not found, trying spec`); } - // 2. Fallback: read first heading from spec file - const specsDir = path.join(workspaceRoot, 'codev', 'specs'); - if (fs.existsSync(specsDir)) { - try { - const files = fs.readdirSync(specsDir); - // Match by project ID prefix (handles zero-padded IDs like 0076) - const specFile = files.find(f => { - if (!f.endsWith('.md')) return false; - // Extract leading numeric prefix (handles both 42-name.md and 0042.name.md) - const numMatch = f.match(/^(\d+)/); - if (!numMatch) return false; - const normalizedPrefix = numMatch[1].replace(/^0+/, '') || '0'; - const normalizedId = projectId.replace(/^0+/, '') || '0'; - return normalizedPrefix === normalizedId; - }); - if (specFile) { - const content = fs.readFileSync(path.join(specsDir, specFile), 'utf-8'); - // Extract first heading - const headingMatch = content.match(/^#\s+(?:Specification:\s*)?(.+)$/m); - if (headingMatch) { - return headingMatch[1].trim(); - } - } - } catch { - // Spec file read failed, continue to fallback + // 2. Fallback: read first heading from spec via resolver + const res = resolver ?? getResolver(workspaceRoot); + const specContent = res.getSpecContent(projectId, projectTitle || ''); + if (specContent) { + const headingMatch = specContent.match(/^#\s+(?:Specification:\s*)?(.+)$/m); + if (headingMatch) { + return headingMatch[1].trim(); } + console.error(`[porch] project summary: spec found but no heading, using title`); + } else { + console.error(`[porch] project summary: spec not found via resolver, using title`); } // 3. Last resort: project title from status.yaml @@ -215,7 +203,8 @@ function buildHistoryHeader(history: IterationRecord[], currentIteration: number export async function buildPhasePrompt( workspaceRoot: string, state: ProjectState, - protocol: Protocol + protocol: Protocol, + resolver?: ArtifactResolver, ): Promise { const phaseConfig = getPhaseConfig(protocol, state.phase); if (!phaseConfig) { @@ -223,7 +212,8 @@ export async function buildPhasePrompt( } // Get project summary from GitHub Issues (with spec-file fallback) - const summary = await getProjectSummary(workspaceRoot, state.id, state.title); + const res = resolver ?? getResolver(workspaceRoot); + const summary = await getProjectSummary(workspaceRoot, state.id, state.title, res); // Get current plan phase for phased protocols let currentPlanPhase: PlanPhase | null = null; @@ -252,7 +242,7 @@ export async function buildPhasePrompt( } // Resolve canonical artifact base name (prevents doubled IDs like "364-0364-name") - const artifactBaseName = resolveArtifactBaseName(workspaceRoot, state.id, state.title); + const artifactBaseName = resolveArtifactBaseName(workspaceRoot, state.id, state.title, res); // Try to load prompt from protocol directory const promptsDir = findPromptsDir(workspaceRoot, state.protocol); @@ -277,7 +267,7 @@ export async function buildPhasePrompt( // Add plan phase context if applicable if (currentPlanPhase) { - result = addPlanPhaseContext(workspaceRoot, state, currentPlanPhase, result); + result = addPlanPhaseContext(workspaceRoot, state, currentPlanPhase, result, res); } // Prepend history if this is a retry @@ -307,21 +297,17 @@ function addPlanPhaseContext( workspaceRoot: string, state: ProjectState, planPhase: PlanPhase, - prompt: string + prompt: string, + resolver?: ArtifactResolver, ): string { - const planPath = findPlanFile(workspaceRoot, state.id, state.title); - if (!planPath) { + const planContent = getPlanContent(workspaceRoot, state.id, state.title, resolver); + if (!planContent) { return prompt; } - try { - const planContent = fs.readFileSync(planPath, 'utf-8'); - const phaseContent = getPhaseContent(planContent, planPhase.id); - if (phaseContent) { - return prompt + `\n\n## Current Plan Phase Details\n\n**${planPhase.id}: ${planPhase.title}**\n\n${phaseContent}\n`; - } - } catch { - // Ignore errors reading plan + const phaseContent = getPhaseContent(planContent, planPhase.id); + if (phaseContent) { + return prompt + `\n\n## Current Plan Phase Details\n\n**${planPhase.id}: ${planPhase.title}**\n\n${phaseContent}\n`; } return prompt; diff --git a/packages/codev/src/commands/porch/state.ts b/packages/codev/src/commands/porch/state.ts index aab009e6ff..2dd10ce0c1 100644 --- a/packages/codev/src/commands/porch/state.ts +++ b/packages/codev/src/commands/porch/state.ts @@ -9,6 +9,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import * as yaml from 'js-yaml'; import type { ProjectState, Protocol, PlanPhase } from './types.js'; +import type { ArtifactResolver } from './artifacts.js'; /** Directory for project state (relative to project root) */ export const PROJECTS_DIR = 'codev/projects'; @@ -33,24 +34,31 @@ export function stripIdPrefix(title: string, projectId: string): string { * This prevents doubled IDs like "364-0364-name" when state.id is unpadded * but spec files use zero-padded IDs. */ -export function resolveArtifactBaseName(workspaceRoot: string, projectId: string, title: string): string { +export function resolveArtifactBaseName(workspaceRoot: string, projectId: string, title: string, resolver?: ArtifactResolver): string { const isNumericId = /^\d+$/.test(projectId); if (isNumericId) { - const specsDir = path.join(workspaceRoot, 'codev', 'specs'); - if (fs.existsSync(specsDir)) { - const normalizedId = projectId.replace(/^0+/, '') || '0'; - try { - const files = fs.readdirSync(specsDir); - const specFile = files.find(f => { - if (!f.endsWith('.md')) return false; - const numMatch = f.match(/^(\d+)/); - if (!numMatch) return false; - return (numMatch[1].replace(/^0+/, '') || '0') === normalizedId; - }); - if (specFile) { - return specFile.replace(/\.md$/, ''); - } - } catch { /* ignore */ } + // Try resolver first (supports both local and FAVA Trails backends) + if (resolver) { + const baseName = resolver.findSpecBaseName(projectId, title); + if (baseName) return baseName; + } else { + // Legacy direct-fs path (backward compatible when no resolver is passed) + const specsDir = path.join(workspaceRoot, 'codev', 'specs'); + if (fs.existsSync(specsDir)) { + const normalizedId = projectId.replace(/^0+/, '') || '0'; + try { + const files = fs.readdirSync(specsDir); + const specFile = files.find(f => { + if (!f.endsWith('.md')) return false; + const numMatch = f.match(/^(\d+)/); + if (!numMatch) return false; + return (numMatch[1].replace(/^0+/, '') || '0') === normalizedId; + }); + if (specFile) { + return specFile.replace(/\.md$/, ''); + } + } catch { /* ignore */ } + } } } // Fallback: strip any existing ID prefix from title, then prepend projectId diff --git a/packages/codev/src/terminal/pty-manager.ts b/packages/codev/src/terminal/pty-manager.ts index aea8abb978..7c7e5cec31 100644 --- a/packages/codev/src/terminal/pty-manager.ts +++ b/packages/codev/src/terminal/pty-manager.ts @@ -71,6 +71,13 @@ export class TerminalManager { // UTF-8 locale for proper Unicode character rendering LANG: process.env.LANG ?? 'en_US.UTF-8', LC_ALL: process.env.LC_ALL ?? '', + // Artifact data repo — porch CliResolver and artifact CLI tools need this + ...(process.env.CODEV_ARTIFACTS_DATA_REPO + ? { CODEV_ARTIFACTS_DATA_REPO: process.env.CODEV_ARTIFACTS_DATA_REPO } + : {}), + ...(process.env.FAVA_TRAILS_DATA_REPO + ? { FAVA_TRAILS_DATA_REPO: process.env.FAVA_TRAILS_DATA_REPO } + : {}), }; const defaults = defaultSessionOptions(); From 3082d32b38eadffeef69cf28a8ed79ce69acdfe1 Mon Sep 17 00:00:00 2001 From: "Younes (Machine Wisdom)" Date: Tue, 17 Mar 2026 07:47:16 -0400 Subject: [PATCH 02/10] fix: remove all fava-trails references from codev codebase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CliResolver is fully generic — no default command, no legacy env vars, no backend aliases. Users must explicitly set artifacts.command in af-config.json. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../codev/src/agent-farm/commands/status.ts | 17 ++++---- .../codev/src/commands/porch/artifacts.ts | 39 +++++++++---------- packages/codev/src/terminal/pty-manager.ts | 5 +-- 3 files changed, 28 insertions(+), 33 deletions(-) diff --git a/packages/codev/src/agent-farm/commands/status.ts b/packages/codev/src/agent-farm/commands/status.ts index 42825e1afc..f85e05e64e 100644 --- a/packages/codev/src/agent-farm/commands/status.ts +++ b/packages/codev/src/agent-farm/commands/status.ts @@ -169,24 +169,23 @@ function showArtifactConfig(workspacePath: string): void { logger.blank(); - if (artifacts.backend === 'cli' || artifacts.backend === 'fava-trails') { - const command = artifacts.command || 'fava-trails'; - logger.kv('Artifacts', chalk.cyan(`cli (${command})`)); + if (artifacts.backend === 'cli') { + logger.kv('Artifacts', chalk.cyan(`cli (${artifacts.command || 'not configured'})`)); if (artifacts.scope) { logger.kv(' Scope', artifacts.scope); } - // Resolve data repo: env var first, then .env file, then default - let dataRepo = process.env.CODEV_ARTIFACTS_DATA_REPO || process.env.FAVA_TRAILS_DATA_REPO; + let dataRepo = process.env.CODEV_ARTIFACTS_DATA_REPO; if (!dataRepo) { const envPath = join(workspacePath, '.env'); try { const envContent = readFileSync(envPath, 'utf-8'); - const newMatch = envContent.match(/^CODEV_ARTIFACTS_DATA_REPO=(.+)$/m); - const legacyMatch = envContent.match(/^FAVA_TRAILS_DATA_REPO=(.+)$/m); - dataRepo = newMatch?.[1]?.trim() || legacyMatch?.[1]?.trim(); + const match = envContent.match(/^CODEV_ARTIFACTS_DATA_REPO=(.+)$/m); + if (match) dataRepo = match[1].trim(); } catch { /* .env may not exist */ } } - logger.kv(' Data Repo', dataRepo || chalk.yellow('(default)')); + if (dataRepo) { + logger.kv(' Data Repo', dataRepo); + } } else { logger.kv('Artifacts', `${artifacts.backend} (codev/specs/, codev/plans/)`); } diff --git a/packages/codev/src/commands/porch/artifacts.ts b/packages/codev/src/commands/porch/artifacts.ts index 434aa40dfe..20a5cf808e 100644 --- a/packages/codev/src/commands/porch/artifacts.ts +++ b/packages/codev/src/commands/porch/artifacts.ts @@ -3,7 +3,7 @@ * * Decouples porch from filesystem assumptions. Two backends: * - LocalResolver: reads from codev/specs/, codev/plans/ (default, backward compatible) - * - CliResolver: shells out to a configurable CLI command (e.g. `fava-trails get`) + * - CliResolver: shells out to a configurable CLI command * * Spec 559: Porch Artifact Resolver */ @@ -181,24 +181,19 @@ export class CliResolver implements ArtifactResolver { constructor( private scope: string, - private command: string = 'fava-trails', + private command: string, workspaceRoot?: string, ) { - // Read data repo env vars from .env if not already in process.env. - // af_builder.sh injects these into builder worktree .env files. + // Read CODEV_ARTIFACTS_DATA_REPO from .env if not already in process.env. + // af_builder.sh injects this into builder worktree .env files. this.extraEnv = {}; - const dataRepo = process.env.CODEV_ARTIFACTS_DATA_REPO || process.env.FAVA_TRAILS_DATA_REPO; - if (!dataRepo && workspaceRoot) { + if (!process.env.CODEV_ARTIFACTS_DATA_REPO && workspaceRoot) { const envPath = path.join(workspaceRoot, '.env'); try { const envContent = fs.readFileSync(envPath, 'utf-8'); - // Try new env var first, fall back to legacy - const newMatch = envContent.match(/^CODEV_ARTIFACTS_DATA_REPO=(.+)$/m); - const legacyMatch = envContent.match(/^FAVA_TRAILS_DATA_REPO=(.+)$/m); - const repo = newMatch?.[1]?.trim() || legacyMatch?.[1]?.trim(); - if (repo) { - this.extraEnv.CODEV_ARTIFACTS_DATA_REPO = repo; - this.extraEnv.FAVA_TRAILS_DATA_REPO = repo; // backward compat + const match = envContent.match(/^CODEV_ARTIFACTS_DATA_REPO=(.+)$/m); + if (match) { + this.extraEnv.CODEV_ARTIFACTS_DATA_REPO = match[1].trim(); } } catch { /* .env may not exist */ } } @@ -392,7 +387,7 @@ export function findConfigRoot(workspaceRoot: string): string { // ============================================================================= export interface ArtifactConfig { - backend?: 'local' | 'cli' | 'fava-trails'; + backend?: 'local' | 'cli'; scope?: string; command?: string; } @@ -423,16 +418,20 @@ function loadArtifactConfig(workspaceRoot: string): ArtifactConfig | null { export function getResolver(workspaceRoot: string): ArtifactResolver { const config = loadArtifactConfig(workspaceRoot); - // 'cli' is the canonical backend; 'fava-trails' is accepted as an alias - if (config?.backend === 'cli' || config?.backend === 'fava-trails') { + if (config?.backend === 'cli') { if (!config.scope) { throw new Error( - `af-config.json has artifacts.backend: "${config.backend}" but no artifacts.scope.\n` + - `Add: "artifacts": { "backend": "cli", "scope": "org/project/codev-assets" }` + `af-config.json has artifacts.backend: "cli" but no artifacts.scope.\n` + + `Add: "artifacts": { "backend": "cli", "command": "my-tool", "scope": "org/project/codev-assets" }` ); } - const command = config.command || 'fava-trails'; - return new CliResolver(config.scope, command, workspaceRoot); + if (!config.command) { + throw new Error( + `af-config.json has artifacts.backend: "cli" but no artifacts.command.\n` + + `Add: "artifacts": { "backend": "cli", "command": "my-tool", "scope": "org/project/codev-assets" }` + ); + } + return new CliResolver(config.scope, config.command, workspaceRoot); } if (config?.backend && config.backend !== 'local') { diff --git a/packages/codev/src/terminal/pty-manager.ts b/packages/codev/src/terminal/pty-manager.ts index 7c7e5cec31..b28f8a7468 100644 --- a/packages/codev/src/terminal/pty-manager.ts +++ b/packages/codev/src/terminal/pty-manager.ts @@ -71,13 +71,10 @@ export class TerminalManager { // UTF-8 locale for proper Unicode character rendering LANG: process.env.LANG ?? 'en_US.UTF-8', LC_ALL: process.env.LC_ALL ?? '', - // Artifact data repo — porch CliResolver and artifact CLI tools need this + // Artifact data repo — porch CliResolver needs this to locate artifact storage ...(process.env.CODEV_ARTIFACTS_DATA_REPO ? { CODEV_ARTIFACTS_DATA_REPO: process.env.CODEV_ARTIFACTS_DATA_REPO } : {}), - ...(process.env.FAVA_TRAILS_DATA_REPO - ? { FAVA_TRAILS_DATA_REPO: process.env.FAVA_TRAILS_DATA_REPO } - : {}), }; const defaults = defaultSessionOptions(); From 0744394ded95992707881d2cdcc59844433db99d Mon Sep 17 00:00:00 2001 From: "Younes (Machine Wisdom)" Date: Tue, 17 Mar 2026 10:03:12 -0400 Subject: [PATCH 03/10] Add SPIR artifacts for pluggable artifact resolver (#612) Spec, plan, and review documents for the CLI-based artifact resolver. Follows SPIR protocol with 3-way expert consultation. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../plans/612-pluggable-artifact-resolver.md | 60 +++++++++ .../612-pluggable-artifact-resolver.md | 41 +++++++ .../specs/612-pluggable-artifact-resolver.md | 116 ++++++++++++++++++ 3 files changed, 217 insertions(+) create mode 100644 codev/plans/612-pluggable-artifact-resolver.md create mode 100644 codev/reviews/612-pluggable-artifact-resolver.md create mode 100644 codev/specs/612-pluggable-artifact-resolver.md diff --git a/codev/plans/612-pluggable-artifact-resolver.md b/codev/plans/612-pluggable-artifact-resolver.md new file mode 100644 index 0000000000..114b3de7af --- /dev/null +++ b/codev/plans/612-pluggable-artifact-resolver.md @@ -0,0 +1,60 @@ +--- +approved: 2026-03-17 +validated: [gpt-5.3-codex, gemini-3.1-pro, deepseek-v3.2] +--- + +# Plan: Pluggable CLI-Based Artifact Resolver for Porch + +## Metadata +- **ID**: 612 +- **Specification**: codev/specs/612-pluggable-artifact-resolver.md + +## Phases (Machine Readable) + +```json +{ + "phases": [ + {"id": "phase_1", "title": "ArtifactResolver abstraction"}, + {"id": "phase_2", "title": "Refactor porch to use resolver"}, + {"id": "phase_3", "title": "Consult CLI integration"} + ] +} +``` + +## Phase 1: ArtifactResolver Abstraction + +**New file**: `packages/codev/src/commands/porch/artifacts.ts` + +- `ArtifactResolver` interface with 5 methods +- `LocalResolver` — extracts existing fs logic from state.ts, plan.ts, next.ts +- `CliResolver` — shells out to configurable CLI, in-memory cache with negative sentinel +- `isPreApprovedContent()` shared helper for frontmatter parsing +- `findConfigRoot()` — resolves af-config.json from builder worktrees +- `getResolver()` factory — reads af-config.json, returns appropriate resolver + +## Phase 2: Refactor Porch to Use Resolver + +Thread resolver through all porch code paths: + +- **`state.ts`**: `resolveArtifactBaseName()` accepts optional resolver +- **`plan.ts`**: Add `getPlanContent()` and `extractPlanPhasesResolved()` with resolver support +- **`next.ts`**: Replace fs-based `isArtifactPreApproved()` with resolver-based version; thread resolver through `handleBuildVerify()` and `handleOncePhase()` +- **`prompts.ts`**: Use resolver in `getProjectSummary()` and `addPlanPhaseContext()`; add diagnostic logging on fallback +- **`checks.ts`**: Add `runArtifactCheck()` for resolver-based plan/review checks; thread resolver through `runPhaseChecks()` +- **`index.ts`**: Thread resolver through `check()`, `done()`, `approve()`; add `getOverriddenCheckNames()` helper +- **`status.ts`**: Show artifact backend in `af status` +- **`pty-manager.ts`**: Propagate `CODEV_ARTIFACTS_DATA_REPO` env var to terminal sessions + +## Phase 3: Consult CLI Integration + +- Add `ContentRef` type (`{ content: string; label: string }`) for resolver-aware content passing +- `findSpecContent()` / `findPlanContent()` wrappers using resolver +- Update all query builders to accept `ContentRef` instead of file paths +- Inline content into consult queries instead of referencing file paths + +## Validation + +1. `npm run build` — compiles clean +2. `npx vitest run src/commands/porch/` — all porch tests pass +3. `npx vitest run src/__tests__/consult.test.ts` — all consult tests pass +4. Manual: configure `backend: "cli"` and run `porch status`, `porch next` diff --git a/codev/reviews/612-pluggable-artifact-resolver.md b/codev/reviews/612-pluggable-artifact-resolver.md new file mode 100644 index 0000000000..7f80dbae4e --- /dev/null +++ b/codev/reviews/612-pluggable-artifact-resolver.md @@ -0,0 +1,41 @@ +# Review: Pluggable CLI-Based Artifact Resolver + +## Summary + +Adds a pluggable `ArtifactResolver` abstraction to porch with `LocalResolver` (default) and `CliResolver` backends. 13 files changed, ~900 insertions, ~240 deletions. All 257 tests pass. + +## Spec vs Implementation + +| Spec Requirement | Status | Notes | +|-----------------|--------|-------| +| LocalResolver wraps existing behavior | Done | Zero behavior change for local users | +| CliResolver shells out to configurable CLI | Done | `artifacts.command` required | +| In-memory caching | Done | Map with negative-cache sentinel | +| Resolver threaded through all code paths | Done | Including handleOncePhase | +| hasPreApproval for CLI resolver | Done | Shared isPreApprovedContent helper | +| Artifact-dependent checks via resolver | Done | runArtifactCheck in checks.ts | +| af status shows backend | Done | | +| consult CLI uses resolver | Done | ContentRef type, inline content | +| No hardcoded external tool references | Done | command field required, no defaults | + +## Consultation + +**3-way review** (2026-03-16): +- Gemini 3.1 Pro (for): approved, flagged hasPreApproval regex fragility +- GPT-5.3 Codex (against): approved, flagged no-error-cache perf risk +- DeepSeek v3.2 (neutral): approved + +All 3 issues addressed: +1. `hasPreApproval()` parses artifact type from glob (specs/plans/reviews) +2. Negative cache with Symbol sentinel prevents repeated CLI timeouts +3. Empty listing output cached as valid result + +## Architecture Updates + +New file `artifacts.ts` is the single entry point for artifact resolution. No other porch file directly reads `codev/specs/` or `codev/plans/` — all access goes through the resolver. + +## Lessons Learned + +- **Thread resolvers through ALL code paths**: `handleOncePhase` was initially missed because it's a less-common path (TICK/BUGFIX vs SPIR) +- **Don't cache errors as null**: Use a sentinel value to distinguish "not found" from "CLI failed" +- **No default command**: Hardcoding a default CLI tool name couples codev to a specific external project diff --git a/codev/specs/612-pluggable-artifact-resolver.md b/codev/specs/612-pluggable-artifact-resolver.md new file mode 100644 index 0000000000..eb59292ba7 --- /dev/null +++ b/codev/specs/612-pluggable-artifact-resolver.md @@ -0,0 +1,116 @@ +--- +approved: 2026-03-17 +validated: [gpt-5.3-codex, gemini-3.1-pro, deepseek-v3.2] +--- + +# Specification: Pluggable CLI-Based Artifact Resolver for Porch + +## Metadata +- **ID**: 612 +- **Status**: implemented +- **Created**: 2026-03-16 +- **GitHub Issue**: https://github.com/cluesmith/codev/issues/612 + +## Problem Statement + +Porch assumes all codev artifacts (specs, plans, reviews) exist as local markdown files in `codev/specs/`, `codev/plans/`, etc. It reads these files to: + +1. **Resolve artifact names** (`resolveArtifactBaseName` in `state.ts`) +2. **Find and parse plans** (`findPlanFile`, `extractPlanPhases` in `plan.ts`) +3. **Detect pre-approval** (`isArtifactPreApproved` in `next.ts`) +4. **Run existence checks** (protocol checks in `checks.ts`) +5. **Inject context into prompts** (`getProjectSummary`, `addPlanPhaseContext` in `prompts.ts`) + +Users storing artifacts in external systems (git-based knowledge bases, wikis, custom tooling) have no integration path. Porch silently fails or falls back to generated names. + +## Desired State + +- Porch resolves artifacts through a pluggable `ArtifactResolver` interface +- Two backends: `LocalResolver` (default, unchanged behavior) and `CliResolver` (shells out to any CLI) +- Configuration via `af-config.json` +- All 5 dependency points use the resolver abstraction + +## Configuration + +```json +{ + "artifacts": { + "backend": "cli", + "command": "my-artifact-tool", + "scope": "org/project/codev-assets" + } +} +``` + +### CLI Protocol + +Any CLI tool that implements these two commands works as a backend: + +```bash +# Retrieve latest artifact content + get + +# List child scopes (for artifact discovery by ID) + get --list +``` + +## Solution: ArtifactResolver Abstraction + +### Interface + +```typescript +interface ArtifactResolver { + findSpecBaseName(projectId: string, title: string): string | null; + getSpecContent(projectId: string, title: string): string | null; + getPlanContent(projectId: string, title: string): string | null; + getReviewContent(projectId: string, title: string): string | null; + hasPreApproval(artifactGlob: string): boolean; +} +``` + +### LocalResolver (default) +Wraps existing filesystem logic. Zero behavior change for existing users. + +### CliResolver +- Shells out to configurable CLI command via `execFileSync` (array args, never string interpolation) +- In-memory cache with negative-cache sentinel (avoids repeated CLI timeouts for missing artifacts) +- `CODEV_ARTIFACTS_DATA_REPO` env var propagated to CLI process +- `hasPreApproval()` uses shared `isPreApprovedContent()` helper (same frontmatter parsing as LocalResolver) + +### Factory +`getResolver(workspaceRoot)` reads `af-config.json` and returns the appropriate resolver. Resolves config from main repo when running in builder worktrees via `findConfigRoot()`. + +## Success Criteria + +- [x] Porch with `artifacts.backend: "local"` (or unset) behaves identically to current behavior +- [x] Porch with `artifacts.backend: "cli"` resolves artifacts via configurable CLI command +- [x] `artifacts.command` is required when backend is `"cli"` +- [x] Resolver threaded through all code paths including `handleOncePhase` (TICK/BUGFIX) +- [x] CLI errors cached as negative sentinel to avoid repeated timeouts +- [x] `hasPreApproval()` works for both resolvers via shared helper +- [x] Artifact-dependent checks use resolver (`runArtifactCheck`) +- [x] `af status` shows configured artifact backend +- [x] `consult` CLI uses resolver for spec/plan content +- [x] All existing tests pass unchanged +- [x] No hardcoded references to any specific external tool + +## Expert Consultation + +**Date**: 2026-03-16 +**Models**: GPT-5.3 Codex (against), Gemini 3.1 Pro (for), DeepSeek v3.2 (neutral) +**Consensus**: Unanimous approval + +Key feedback incorporated: +1. **Semantic interface** — resolver models artifacts, not generic storage +2. **Mandatory caching** — Map-based memoization within porch session +3. **Fail loudly** — when backend is `"cli"`, don't silently fall back to local +4. **Subprocess security** — `execFileSync` with array args only +5. **Negative cache with sentinel** — cache CLI failures to avoid repeated 5s timeouts +6. **Shared pre-approval helper** — `isPreApprovedContent()` used by both resolvers +7. **Artifact-type-aware hasPreApproval** — parse glob to determine spec vs plan vs review + +## Constraints + +- CLI integration is via subprocess (`execFileSync`), adding ~200ms per call +- Mitigated by in-memory caching within each porch invocation +- `command` field is required (no default) — codev doesn't assume any specific external tool From 137e62659673478d133e55027382460f0bb5f2df Mon Sep 17 00:00:00 2001 From: "Younes (Machine Wisdom)" Date: Tue, 17 Mar 2026 10:12:21 -0400 Subject: [PATCH 04/10] fix: address PR review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix silent fallback in findSpecContent/findPlanContent — config errors now propagate instead of silently falling back to local filesystem - Fix stale "Spec 559" comment → link to GitHub issue #612 - Add 23 unit tests for CliResolver, LocalResolver, isPreApprovedContent, and getResolver factory Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/codev/src/commands/consult/index.ts | 28 +-- .../porch/__tests__/artifacts.test.ts | 230 ++++++++++++++++++ .../codev/src/commands/porch/artifacts.ts | 2 +- 3 files changed, 243 insertions(+), 17 deletions(-) create mode 100644 packages/codev/src/commands/porch/__tests__/artifacts.test.ts diff --git a/packages/codev/src/commands/consult/index.ts b/packages/codev/src/commands/consult/index.ts index 8eaeecb85d..523f8bb445 100644 --- a/packages/codev/src/commands/consult/index.ts +++ b/packages/codev/src/commands/consult/index.ts @@ -242,30 +242,26 @@ function findPlan(workspaceRoot: string, id: string): string | null { } /** - * Find spec content via artifact resolver (FAVA Trails or local fallback). + * Find spec content via artifact resolver. + * Uses the configured resolver (cli or local). Config errors propagate + * so misconfiguration is visible rather than silently falling back. */ function findSpecContent(workspaceRoot: string, id: string): ContentRef | null { - try { - const resolver = getResolver(workspaceRoot); - const content = resolver.getSpecContent(id, ''); - if (content) return { content, label: `spec-${id}` }; - } catch { /* resolver init may fail; fall through to local */ } - const specPath = findSpec(workspaceRoot, id); - if (specPath) return { content: fs.readFileSync(specPath, 'utf-8'), label: specPath }; + const resolver = getResolver(workspaceRoot); + const content = resolver.getSpecContent(id, ''); + if (content) return { content, label: `spec-${id}` }; return null; } /** - * Find plan content via artifact resolver (FAVA Trails or local fallback). + * Find plan content via artifact resolver. + * Uses the configured resolver (cli or local). Config errors propagate + * so misconfiguration is visible rather than silently falling back. */ function findPlanContent(workspaceRoot: string, id: string): ContentRef | null { - try { - const resolver = getResolver(workspaceRoot); - const content = resolver.getPlanContent(id, ''); - if (content) return { content, label: `plan-${id}` }; - } catch { /* resolver init may fail; fall through to local */ } - const planPath = findPlan(workspaceRoot, id); - if (planPath) return { content: fs.readFileSync(planPath, 'utf-8'), label: planPath }; + const resolver = getResolver(workspaceRoot); + const content = resolver.getPlanContent(id, ''); + if (content) return { content, label: `plan-${id}` }; return null; } diff --git a/packages/codev/src/commands/porch/__tests__/artifacts.test.ts b/packages/codev/src/commands/porch/__tests__/artifacts.test.ts new file mode 100644 index 0000000000..fc3e421ba2 --- /dev/null +++ b/packages/codev/src/commands/porch/__tests__/artifacts.test.ts @@ -0,0 +1,230 @@ +/** + * Tests for artifact resolver (artifacts.ts) + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { tmpdir } from 'node:os'; +import { + isPreApprovedContent, + LocalResolver, + CliResolver, + getResolver, +} from '../artifacts.js'; + +// ============================================================================= +// isPreApprovedContent +// ============================================================================= + +describe('isPreApprovedContent', () => { + it('returns true for content with approved and validated frontmatter', () => { + const content = `--- +approved: 2026-03-17 +validated: [gpt-5, gemini, claude] +--- + +# My Spec +`; + expect(isPreApprovedContent(content)).toBe(true); + }); + + it('returns false when approved is missing', () => { + const content = `--- +validated: [gpt-5, gemini] +--- + +# My Spec +`; + expect(isPreApprovedContent(content)).toBe(false); + }); + + it('returns false when validated is missing', () => { + const content = `--- +approved: 2026-03-17 +--- + +# My Spec +`; + expect(isPreApprovedContent(content)).toBe(false); + }); + + it('returns false when no frontmatter', () => { + expect(isPreApprovedContent('# Just a heading\n\nNo frontmatter here.')).toBe(false); + }); + + it('returns false for empty string', () => { + expect(isPreApprovedContent('')).toBe(false); + }); + + it('returns false when validated list is empty', () => { + const content = `--- +approved: 2026-03-17 +validated: [] +--- +`; + expect(isPreApprovedContent(content)).toBe(false); + }); +}); + +// ============================================================================= +// LocalResolver +// ============================================================================= + +describe('LocalResolver', () => { + const testDir = path.join(tmpdir(), `artifact-resolver-test-${Date.now()}`); + + beforeEach(() => { + fs.mkdirSync(path.join(testDir, 'codev', 'specs'), { recursive: true }); + fs.mkdirSync(path.join(testDir, 'codev', 'plans'), { recursive: true }); + fs.mkdirSync(path.join(testDir, 'codev', 'reviews'), { recursive: true }); + }); + + afterEach(() => { + if (fs.existsSync(testDir)) { + fs.rmSync(testDir, { recursive: true }); + } + }); + + it('finds spec by numeric ID', () => { + fs.writeFileSync(path.join(testDir, 'codev', 'specs', '0042-feature.md'), '# Spec'); + const resolver = new LocalResolver(testDir); + expect(resolver.findSpecBaseName('42', '')).toBe('0042-feature'); + }); + + it('returns null for missing spec', () => { + const resolver = new LocalResolver(testDir); + expect(resolver.findSpecBaseName('999', '')).toBeNull(); + }); + + it('reads spec content', () => { + fs.writeFileSync(path.join(testDir, 'codev', 'specs', '0042-feature.md'), '# My Feature Spec'); + const resolver = new LocalResolver(testDir); + expect(resolver.getSpecContent('42', '')).toBe('# My Feature Spec'); + }); + + it('reads plan content from plans dir', () => { + fs.writeFileSync(path.join(testDir, 'codev', 'plans', '42-feature.md'), '# Plan'); + const resolver = new LocalResolver(testDir); + expect(resolver.getPlanContent('42', '')).toBe('# Plan'); + }); + + it('reads review content', () => { + fs.writeFileSync(path.join(testDir, 'codev', 'reviews', '42-feature.md'), '# Review'); + const resolver = new LocalResolver(testDir); + expect(resolver.getReviewContent('42', '')).toBe('# Review'); + }); + + it('detects pre-approval via hasPreApproval', () => { + const content = `---\napproved: 2026-01-01\nvalidated: [gpt, gemini]\n---\n# Spec`; + fs.writeFileSync(path.join(testDir, 'codev', 'specs', '0042-feature.md'), content); + const resolver = new LocalResolver(testDir); + expect(resolver.hasPreApproval('codev/specs/0042-*.md')).toBe(true); + }); + + it('returns false for non-approved spec', () => { + fs.writeFileSync(path.join(testDir, 'codev', 'specs', '0042-feature.md'), '# No frontmatter'); + const resolver = new LocalResolver(testDir); + expect(resolver.hasPreApproval('codev/specs/0042-*.md')).toBe(false); + }); +}); + +// ============================================================================= +// CliResolver +// ============================================================================= + +describe('CliResolver', () => { + it('calls CLI command with get --list for listChildren', () => { + const resolver = new CliResolver('org/proj/assets', 'echo'); + // 'echo' will just print the args back — not a real backend, but exercises the code path + // The important thing is it doesn't throw ENOENT + const result = resolver.findSpecBaseName('42', ''); + // echo outputs "get --list org/proj/assets/specs" which won't match any numeric prefix + expect(result).toBeNull(); + }); + + it('throws when CLI command is not found', () => { + const resolver = new CliResolver('org/proj/assets', 'nonexistent-command-xyz'); + expect(() => resolver.findSpecBaseName('42', '')).toThrow('not found'); + }); + + it('caches negative results (does not re-call CLI on repeated failures)', () => { + const resolver = new CliResolver('org/proj/assets', 'false'); // 'false' always exits 1 + // First call — CLI runs and fails + const result1 = resolver.getSpecContent('42', ''); + expect(result1).toBeNull(); + // Second call — should hit negative cache, not re-invoke CLI + const result2 = resolver.getSpecContent('42', ''); + expect(result2).toBeNull(); + }); + + it('hasPreApproval returns false when content not found', () => { + const resolver = new CliResolver('org/proj/assets', 'false'); + expect(resolver.hasPreApproval('codev/specs/0042-*.md')).toBe(false); + }); +}); + +// ============================================================================= +// getResolver factory +// ============================================================================= + +describe('getResolver', () => { + const testDir = path.join(tmpdir(), `resolver-factory-test-${Date.now()}`); + + beforeEach(() => { + fs.mkdirSync(testDir, { recursive: true }); + }); + + afterEach(() => { + if (fs.existsSync(testDir)) { + fs.rmSync(testDir, { recursive: true }); + } + }); + + it('returns LocalResolver when no config', () => { + const resolver = getResolver(testDir); + expect(resolver).toBeInstanceOf(LocalResolver); + }); + + it('returns LocalResolver for backend: "local"', () => { + fs.writeFileSync( + path.join(testDir, 'af-config.json'), + JSON.stringify({ artifacts: { backend: 'local' } }), + ); + const resolver = getResolver(testDir); + expect(resolver).toBeInstanceOf(LocalResolver); + }); + + it('returns CliResolver for backend: "cli" with command and scope', () => { + fs.writeFileSync( + path.join(testDir, 'af-config.json'), + JSON.stringify({ artifacts: { backend: 'cli', command: 'my-tool', scope: 'org/proj' } }), + ); + const resolver = getResolver(testDir); + expect(resolver).toBeInstanceOf(CliResolver); + }); + + it('throws when backend: "cli" but no scope', () => { + fs.writeFileSync( + path.join(testDir, 'af-config.json'), + JSON.stringify({ artifacts: { backend: 'cli', command: 'my-tool' } }), + ); + expect(() => getResolver(testDir)).toThrow('no artifacts.scope'); + }); + + it('throws when backend: "cli" but no command', () => { + fs.writeFileSync( + path.join(testDir, 'af-config.json'), + JSON.stringify({ artifacts: { backend: 'cli', scope: 'org/proj' } }), + ); + expect(() => getResolver(testDir)).toThrow('no artifacts.command'); + }); + + it('throws for unknown backend', () => { + fs.writeFileSync( + path.join(testDir, 'af-config.json'), + JSON.stringify({ artifacts: { backend: 'redis' } }), + ); + expect(() => getResolver(testDir)).toThrow('unknown artifacts.backend'); + }); +}); diff --git a/packages/codev/src/commands/porch/artifacts.ts b/packages/codev/src/commands/porch/artifacts.ts index 20a5cf808e..3d09fbdef9 100644 --- a/packages/codev/src/commands/porch/artifacts.ts +++ b/packages/codev/src/commands/porch/artifacts.ts @@ -5,7 +5,7 @@ * - LocalResolver: reads from codev/specs/, codev/plans/ (default, backward compatible) * - CliResolver: shells out to a configurable CLI command * - * Spec 559: Porch Artifact Resolver + * GitHub Issue: https://github.com/cluesmith/codev/issues/612 */ import { execFileSync } from 'node:child_process'; From 89ba374dde283d629dcf96363a22051dac360bd4 Mon Sep 17 00:00:00 2001 From: "Younes (Machine Wisdom)" Date: Tue, 17 Mar 2026 12:41:11 -0400 Subject: [PATCH 05/10] [Bugfix #619] Fix ASPIR builder-prompt, spawn --task porch init, phases regex - builder-prompt.md: change SPIR protocol reference to ASPIR - spawn.ts: call initPorchInWorktree() in spawnTask() when hasExplicitProtocol - checks.ts: use regex for has_phases_json to handle whitespace variations Co-Authored-By: Claude Sonnet 4.6 --- codev-skeleton/protocols/aspir/builder-prompt.md | 2 +- codev/protocols/aspir/builder-prompt.md | 2 +- packages/codev/src/agent-farm/commands/spawn.ts | 4 ++++ packages/codev/src/commands/porch/checks.ts | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/codev-skeleton/protocols/aspir/builder-prompt.md b/codev-skeleton/protocols/aspir/builder-prompt.md index 1f932b6d46..73c3ba6071 100644 --- a/codev-skeleton/protocols/aspir/builder-prompt.md +++ b/codev-skeleton/protocols/aspir/builder-prompt.md @@ -27,7 +27,7 @@ You are running in STRICT mode. This means: {{/if}} ## Protocol -Follow the SPIR protocol: `codev/protocols/spir/protocol.md` +Follow the ASPIR protocol: `codev/protocols/aspir/protocol.md` Read and internalize the protocol before starting any work. {{#if spec}} diff --git a/codev/protocols/aspir/builder-prompt.md b/codev/protocols/aspir/builder-prompt.md index 1f932b6d46..73c3ba6071 100644 --- a/codev/protocols/aspir/builder-prompt.md +++ b/codev/protocols/aspir/builder-prompt.md @@ -27,7 +27,7 @@ You are running in STRICT mode. This means: {{/if}} ## Protocol -Follow the SPIR protocol: `codev/protocols/spir/protocol.md` +Follow the ASPIR protocol: `codev/protocols/aspir/protocol.md` Read and internalize the protocol before starting any work. {{#if spec}} diff --git a/packages/codev/src/agent-farm/commands/spawn.ts b/packages/codev/src/agent-farm/commands/spawn.ts index d75671ebe0..d6497f34f0 100644 --- a/packages/codev/src/agent-farm/commands/spawn.ts +++ b/packages/codev/src/agent-farm/commands/spawn.ts @@ -464,6 +464,10 @@ async function spawnTask(options: SpawnOptions, config: Config): Promise { if (hasExplicitProtocol) { validateProtocol(config, options.protocol!); const protocol = options.protocol!.toLowerCase(); + + if (!options.resume) { + await initPorchInWorktree(worktreePath, protocol, builderId, worktreeName); + } const protocolDef = loadProtocol(config, protocol); const mode = resolveMode(options, protocolDef); const templateContext: TemplateContext = { diff --git a/packages/codev/src/commands/porch/checks.ts b/packages/codev/src/commands/porch/checks.ts index 046c53186b..70a5bfb7cb 100644 --- a/packages/codev/src/commands/porch/checks.ts +++ b/packages/codev/src/commands/porch/checks.ts @@ -156,7 +156,7 @@ export function runArtifactCheck( if (content === null) { return { name, command, passed: false, error: 'Plan not found', duration_ms: Date.now() - startTime }; } - const has = content.includes('"phases":'); + const has = /"phases"\s*:/.test(content); return { name, command, From 7d972c8815aaa1d61244ba29d94c79e6854c3a1e Mon Sep 17 00:00:00 2001 From: "Younes (Machine Wisdom)" Date: Tue, 17 Mar 2026 12:48:12 -0400 Subject: [PATCH 06/10] [Spec 620][Phase 1] feat: symlink codev/protocols, resources, roles into builder worktrees Extends createWorktree() to symlink codev/protocols, codev/resources, and codev/roles from the workspace root into builder worktrees, with parent directory creation if needed. Skips if target already exists (git-tracked). Co-Authored-By: Claude Sonnet 4.6 --- .../__tests__/spawn-worktree.test.ts | 53 +++++++++++++++++++ .../src/agent-farm/commands/spawn-worktree.ts | 28 ++++++++-- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts b/packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts index b72eb023d5..825ebd92f0 100644 --- a/packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts +++ b/packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts @@ -27,6 +27,7 @@ vi.mock('node:fs', async (importOriginal) => { chmodSync: vi.fn(), symlinkSync: vi.fn(), readdirSync: vi.fn(() => []), + mkdirSync: vi.fn(), }; }); @@ -239,6 +240,58 @@ describe('spawn-worktree', () => { .mockRejectedValueOnce(new Error('worktree add failed')); await expect(createWorktree(config, 'my-branch', '/tmp/wt')).rejects.toThrow('Failed to create worktree'); }); + + it('symlinks codev/protocols, codev/resources, codev/roles when they exist in workspace root', async () => { + const { run } = await import('../utils/shell.js'); + const { existsSync, symlinkSync, mkdirSync } = await import('node:fs'); + vi.mocked(run).mockResolvedValue({ stdout: '', stderr: '' } as any); + // existsSync: rootPath exists for all dirs, worktree paths do not exist + vi.mocked(existsSync).mockImplementation((p: any) => { + const s = String(p); + // The workspace root dirs exist, the worktree dirs do not, parent 'codev/' does not + if (s === '/projects/test/codev/protocols') return true; + if (s === '/projects/test/codev/resources') return true; + if (s === '/projects/test/codev/roles') return true; + return false; + }); + await createWorktree(config, 'my-branch', '/tmp/wt'); + expect(symlinkSync).toHaveBeenCalledWith('/projects/test/codev/protocols', '/tmp/wt/codev/protocols'); + expect(symlinkSync).toHaveBeenCalledWith('/projects/test/codev/resources', '/tmp/wt/codev/resources'); + expect(symlinkSync).toHaveBeenCalledWith('/projects/test/codev/roles', '/tmp/wt/codev/roles'); + // Parent dir codev/ should be created since it didn't exist + expect(mkdirSync).toHaveBeenCalledWith('/tmp/wt/codev', { recursive: true }); + }); + + it('skips dir symlink when target already exists in worktree (git-tracked)', async () => { + const { run } = await import('../utils/shell.js'); + const { existsSync, symlinkSync } = await import('node:fs'); + vi.mocked(run).mockResolvedValue({ stdout: '', stderr: '' } as any); + // Both root path and worktree path exist + vi.mocked(existsSync).mockImplementation((p: any) => { + const s = String(p); + if (s === '/projects/test/codev/protocols') return true; + if (s === '/tmp/wt/codev/protocols') return true; + return false; + }); + await createWorktree(config, 'my-branch', '/tmp/wt'); + expect(symlinkSync).not.toHaveBeenCalledWith( + '/projects/test/codev/protocols', + '/tmp/wt/codev/protocols', + ); + }); + + it('skips dir symlink when source does not exist in workspace', async () => { + const { run } = await import('../utils/shell.js'); + const { existsSync, symlinkSync } = await import('node:fs'); + vi.mocked(run).mockResolvedValue({ stdout: '', stderr: '' } as any); + // Nothing exists + vi.mocked(existsSync).mockReturnValue(false); + await createWorktree(config, 'my-branch', '/tmp/wt'); + expect(symlinkSync).not.toHaveBeenCalledWith( + expect.stringContaining('codev/protocols'), + expect.any(String), + ); + }); }); // ========================================================================= diff --git a/packages/codev/src/agent-farm/commands/spawn-worktree.ts b/packages/codev/src/agent-farm/commands/spawn-worktree.ts index 659d884936..3ce5522eb2 100644 --- a/packages/codev/src/agent-farm/commands/spawn-worktree.ts +++ b/packages/codev/src/agent-farm/commands/spawn-worktree.ts @@ -7,8 +7,8 @@ * and terminal session creation via the Tower REST API. */ -import { resolve } from 'node:path'; -import { existsSync, writeFileSync, chmodSync, symlinkSync, readdirSync } from 'node:fs'; +import { resolve, dirname } from 'node:path'; +import { existsSync, writeFileSync, chmodSync, symlinkSync, readdirSync, mkdirSync } from 'node:fs'; import type { Config, ProtocolDefinition } from '../types.js'; import { logger, fatal } from '../utils/logger.js'; import { defaultSessionOptions } from '../../terminal/index.js'; @@ -39,8 +39,9 @@ export async function checkDependencies(): Promise { * Shared by createWorktree() and createWorktreeFromBranch(). */ export function symlinkConfigFiles(config: Config, worktreePath: string): void { - const symlinks = ['.env', 'af-config.json']; - for (const file of symlinks) { + // Symlink config files + const fileSymlinks = ['.env', 'af-config.json']; + for (const file of fileSymlinks) { const rootPath = resolve(config.workspaceRoot, file); const worktreeFilePath = resolve(worktreePath, file); if (existsSync(rootPath) && !existsSync(worktreeFilePath)) { @@ -52,6 +53,25 @@ export function symlinkConfigFiles(config: Config, worktreePath: string): void { } } } + + // Symlink protocol/resource directories so builders can find them even when gitignored + const dirSymlinks = ['codev/protocols', 'codev/resources', 'codev/roles']; + for (const dir of dirSymlinks) { + const rootPath = resolve(config.workspaceRoot, dir); + const worktreeDirPath = resolve(worktreePath, dir); + if (existsSync(rootPath) && !existsSync(worktreeDirPath)) { + try { + const parentDir = dirname(worktreeDirPath); + if (!existsSync(parentDir)) { + mkdirSync(parentDir, { recursive: true }); + } + symlinkSync(rootPath, worktreeDirPath); + logger.info(`Linked ${dir} from workspace root`); + } catch (error) { + logger.debug(`Failed to symlink ${dir}: ${error}`); + } + } + } } /** From 3769e3361b1948375ecd92dfb4a2f2fee9243431 Mon Sep 17 00:00:00 2001 From: "Younes (Machine Wisdom)" Date: Tue, 17 Mar 2026 12:55:21 -0400 Subject: [PATCH 07/10] [Spec 620][Phase 2] feat: try artifact resolver before fatal GitHub fetch in spawnSpec When no local spec file is found, spawnSpec() now tries the artifact resolver (getResolver().findSpecBaseName()) before falling back to a fatal GitHub issue fetch. This enables af spawn N --protocol aspir when specs exist only in an external CLI artifact backend (no local file, no GitHub issue required). Co-Authored-By: Claude Sonnet 4.6 --- .../__tests__/spawn-resolver-fallback.test.ts | 125 ++++++++++++++++++ .../codev/src/agent-farm/commands/spawn.ts | 30 ++++- 2 files changed, 150 insertions(+), 5 deletions(-) create mode 100644 packages/codev/src/agent-farm/__tests__/spawn-resolver-fallback.test.ts diff --git a/packages/codev/src/agent-farm/__tests__/spawn-resolver-fallback.test.ts b/packages/codev/src/agent-farm/__tests__/spawn-resolver-fallback.test.ts new file mode 100644 index 0000000000..8229852fea --- /dev/null +++ b/packages/codev/src/agent-farm/__tests__/spawn-resolver-fallback.test.ts @@ -0,0 +1,125 @@ +/** + * Tests for spawn resolver fallback (Spec 620, Phase 2) + * + * When spawnSpec() finds no local spec file, it should try the artifact + * resolver before falling back to a fatal GitHub issue fetch. + * This enables af spawn N --protocol aspir when specs exist only in a + * CLI artifact backend. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// Mock the artifact resolver +const findSpecBaseNameMock = vi.fn(); +vi.mock('../../commands/porch/artifacts.js', () => ({ + getResolver: vi.fn(() => ({ findSpecBaseName: findSpecBaseNameMock })), +})); + +// Mock spawn-roles helpers +vi.mock('../commands/spawn-roles.js', () => ({ + findSpecFile: vi.fn(async () => null), // no local spec by default + validateProtocol: vi.fn(), + loadProtocol: vi.fn(() => ({ input: { required: false } })), + resolveMode: vi.fn(() => 'strict'), + buildPromptFromTemplate: vi.fn(() => 'prompt'), + buildResumeNotice: vi.fn(() => ''), + loadProtocolRole: vi.fn(() => null), +})); + +vi.mock('../utils/index.js', () => ({ + getConfig: vi.fn(() => ({ + workspaceRoot: '/repo', + codevDir: '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/repo/codev', + buildersDir: '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/repo/.builders', + })), + ensureDirectories: vi.fn(), + getResolvedCommands: vi.fn(() => ({ builder: 'claude' })), +})); + +vi.mock('../utils/logger.js', () => ({ + logger: { info: vi.fn(), debug: vi.fn(), warn: vi.fn(), kv: vi.fn(), header: vi.fn(), blank: vi.fn(), success: vi.fn() }, + fatal: vi.fn((msg: string) => { throw new Error(msg); }), +})); + +vi.mock('../utils/shell.js', () => ({ + run: vi.fn(async () => ({ stdout: '', stderr: '' })), +})); + +vi.mock('../state.js', () => ({ upsertBuilder: vi.fn() })); + +vi.mock('../utils/roles.js', () => ({ loadRolePrompt: vi.fn(() => null) })); + +vi.mock('../utils/agent-names.js', () => ({ + buildAgentName: vi.fn(() => 'builder-620'), + stripLeadingZeros: vi.fn((s: string) => s.replace(/^0+/, '') || '0'), +})); + +vi.mock('../../lib/github.js', () => ({ + fetchIssue: vi.fn(async () => null), +})); + +vi.mock('../../lib/forge.js', () => ({ + loadForgeConfig: vi.fn(() => null), +})); + +vi.mock('../commands/spawn-worktree.js', () => ({ + checkDependencies: vi.fn(), + createWorktree: vi.fn(), + initPorchInWorktree: vi.fn(), + validateResumeWorktree: vi.fn(), + startBuilderSession: vi.fn(async () => ({ terminalId: 'tid-123' })), + slugify: vi.fn((t: string) => t.toLowerCase().replace(/[^a-z0-9]+/g, '-').slice(0, 30)), + fetchGitHubIssue: vi.fn(async () => { throw new Error('GitHub fetch should not be called'); }), +})); + +vi.mock('../lib/tower-client.js', () => ({ + getTowerClient: vi.fn(() => ({ getTerminalWsUrl: vi.fn(() => 'ws://localhost') })), +})); + +describe('spawnSpec resolver fallback (Spec 620)', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('uses resolver spec name when no local spec file exists', async () => { + findSpecBaseNameMock.mockReturnValue('620-my-feature-spec'); + + const { spawn } = await import('../commands/spawn.js'); + const { startBuilderSession } = await import('../commands/spawn-worktree.js'); + const { fetchGitHubIssue } = await import('../commands/spawn-worktree.js'); + + await spawn({ issueNumber: 620, protocol: 'aspir' }); + + // Should NOT have fallen through to fatal GitHub fetch + expect(fetchGitHubIssue).not.toHaveBeenCalled(); + // Should have started a builder session (spawn succeeded) + expect(startBuilderSession).toHaveBeenCalled(); + }); + + it('falls through to GitHub fetch when resolver returns null', async () => { + findSpecBaseNameMock.mockReturnValue(null); + + const { spawn } = await import('../commands/spawn.js'); + const { fetchGitHubIssue } = await import('../commands/spawn-worktree.js'); + // Override fetchGitHubIssue to succeed for this test + vi.mocked(fetchGitHubIssue).mockResolvedValueOnce({ title: 'my issue', body: '', state: 'OPEN', comments: [] }); + + await spawn({ issueNumber: 620, protocol: 'aspir' }); + + // Should have fallen through to GitHub fetch since resolver returned null + expect(fetchGitHubIssue).toHaveBeenCalledWith(620, expect.any(Object)); + }); + + it('falls through to GitHub fetch when resolver throws', async () => { + findSpecBaseNameMock.mockImplementation(() => { throw new Error('resolver error'); }); + + const { spawn } = await import('../commands/spawn.js'); + const { fetchGitHubIssue } = await import('../commands/spawn-worktree.js'); + vi.mocked(fetchGitHubIssue).mockResolvedValueOnce({ title: 'my issue', body: '', state: 'OPEN', comments: [] }); + + await spawn({ issueNumber: 620, protocol: 'aspir' }); + + // Non-fatal resolver error — should fall through to GitHub fetch + expect(fetchGitHubIssue).toHaveBeenCalledWith(620, expect.any(Object)); + }); +}); diff --git a/packages/codev/src/agent-farm/commands/spawn.ts b/packages/codev/src/agent-farm/commands/spawn.ts index d6497f34f0..cf1be4297d 100644 --- a/packages/codev/src/agent-farm/commands/spawn.ts +++ b/packages/codev/src/agent-farm/commands/spawn.ts @@ -24,6 +24,7 @@ import { upsertBuilder } from '../state.js'; import { loadRolePrompt } from '../utils/roles.js'; import { buildAgentName, stripLeadingZeros } from '../utils/agent-names.js'; import { fetchIssue as fetchIssueNonFatal } from '../../lib/github.js'; +import { getResolver } from '../../commands/porch/artifacts.js'; import { type TemplateContext, buildPromptFromTemplate, @@ -300,11 +301,27 @@ async function spawnSpec(options: SpawnOptions, config: Config): Promise { } } + // Try artifact resolver to get spec name before falling back to GitHub. + // This supports external backends (e.g., CLI backend) where specs exist + // remotely but not as local files under codev/specs/. + let resolverSpecName: string | null = null; + if (!specFile) { + try { + const resolver = getResolver(config.workspaceRoot); + resolverSpecName = resolver.findSpecBaseName(specLookupId, ''); + if (resolverSpecName) { + logger.info(`Found spec via resolver: ${resolverSpecName}`); + } + } catch { + // Non-fatal: if resolver fails, fall through to GitHub issue fetch + } + } + // Fetch forge issue context. - // When no spec file exists, this is fatal (we need a project name). - // When spec file exists, this is non-fatal (spec filename is the fallback). + // When no spec file exists and resolver didn't find a name, this is fatal (we need a project name). + // When spec file exists or resolver found the name, this is non-fatal (spec name is the fallback). let forgeIssue: Awaited> = null; - if (!specFile) { + if (!specFile && !resolverSpecName) { // Fatal fetch — we need the issue title for naming forgeIssue = await fetchGitHubIssue(issueNumber, { cwd: config.workspaceRoot, forgeConfig }); } else { @@ -312,12 +329,15 @@ async function spawnSpec(options: SpawnOptions, config: Config): Promise { } // Derive specName for naming. - // Priority: GitHub issue title > spec filename + // Priority: GitHub issue title > resolver spec name > local spec filename let specName: string; if (forgeIssue) { specName = `${strippedId}-${slugify(forgeIssue.title)}`; + } else if (resolverSpecName) { + // Spec found in external backend — use its base name for branch/worktree naming + specName = resolverSpecName.replace(/\.md$/, ''); } else { - // No GitHub issue — fall back to spec filename (specFile must exist here) + // No GitHub issue or resolver — fall back to spec filename (specFile must exist here) specName = basename(specFile!, '.md'); } From 7b92c21d95d9af3b5a6f9632f011f52be98e87c4 Mon Sep 17 00:00:00 2001 From: "Younes (Machine Wisdom)" Date: Thu, 26 Mar 2026 19:04:29 -0400 Subject: [PATCH 08/10] [Bugfix #612] Address PR review feedback: 5 correctness fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Fix spawnSpec to try resolver before fatal() for protocols with input.required (e.g. TICK) — previously the fatal exit ran before the resolver check, so CLI-backend specs were never found 2. Pass resolver to resolveArtifactBaseName() at all 3 call sites in check(), done(), and approve() — PROJECT_TITLE was wrong for CLI backend users in those commands (only getArtifactForPhase passed it) 3. Add 'fava-trails' as accepted alias for 'cli' backend in getResolver() and ArtifactConfig type — was mentioned in PR description but unimplemented 4. Make error messages in consult/index.ts backend-agnostic — remove hardcoded "in codev/specs/" / "in codev/plans/" from not-found errors 5. Add tests for runArtifactCheck() (plan_exists, has_phases_json, min_two_phases, review_has_arch_updates, review_has_lessons_updates, override skip path) and fava-trails alias in getResolver factory Co-Authored-By: Claude Sonnet 4.6 --- .../codev/src/agent-farm/commands/spawn.ts | 24 ++-- packages/codev/src/commands/consult/index.ts | 8 +- .../porch/__tests__/artifacts.test.ts | 9 ++ .../commands/porch/__tests__/checks.test.ts | 131 ++++++++++++++++++ .../codev/src/commands/porch/artifacts.ts | 11 +- packages/codev/src/commands/porch/index.ts | 6 +- 6 files changed, 165 insertions(+), 24 deletions(-) diff --git a/packages/codev/src/agent-farm/commands/spawn.ts b/packages/codev/src/agent-farm/commands/spawn.ts index cf1be4297d..7a3e09d247 100644 --- a/packages/codev/src/agent-farm/commands/spawn.ts +++ b/packages/codev/src/agent-farm/commands/spawn.ts @@ -290,18 +290,7 @@ async function spawnSpec(options: SpawnOptions, config: Config): Promise { // Resolve spec file (supports legacy zero-padded IDs) const specFile = await findSpecFile(config.codevDir, specLookupId); - // When no spec file exists, check if the protocol allows spawning without one. - // TICK always requires a spec (enforced via options.amends, regardless of input.required). - if (!specFile) { - if (protocolDef?.input?.required === false && !options.amends) { - // Protocol allows no-spec spawn — will derive naming from GitHub issue title - logger.info('No spec file found. Protocol allows spawning without one (Specify phase will create it).'); - } else { - fatal(`Spec not found for ${protocol === 'tick' ? `amends #${options.amends}` : `issue #${issueNumber}`}. Expected: codev/specs/${specLookupId}-*.md`); - } - } - - // Try artifact resolver to get spec name before falling back to GitHub. + // Try artifact resolver before the local-file check. // This supports external backends (e.g., CLI backend) where specs exist // remotely but not as local files under codev/specs/. let resolverSpecName: string | null = null; @@ -317,6 +306,17 @@ async function spawnSpec(options: SpawnOptions, config: Config): Promise { } } + // When no spec file exists (locally or in resolver), check if the protocol allows it. + // TICK always requires a spec (enforced via options.amends, regardless of input.required). + if (!specFile && !resolverSpecName) { + if (protocolDef?.input?.required === false && !options.amends) { + // Protocol allows no-spec spawn — will derive naming from GitHub issue title + logger.info('No spec file found. Protocol allows spawning without one (Specify phase will create it).'); + } else { + fatal(`Spec not found for ${protocol === 'tick' ? `amends #${options.amends}` : `issue #${issueNumber}`}. Expected: codev/specs/${specLookupId}-*.md`); + } + } + // Fetch forge issue context. // When no spec file exists and resolver didn't find a name, this is fatal (we need a project name). // When spec file exists or resolver found the name, this is non-fatal (spec name is the fallback). diff --git a/packages/codev/src/commands/consult/index.ts b/packages/codev/src/commands/consult/index.ts index 523f8bb445..3d021ec411 100644 --- a/packages/codev/src/commands/consult/index.ts +++ b/packages/codev/src/commands/consult/index.ts @@ -1118,7 +1118,7 @@ function resolveBuilderQuery(workspaceRoot: string, type: string, options: Consu switch (type) { case 'spec': { const spec = findSpecContent(workspaceRoot, projectId); - if (!spec) throw new Error(`Spec ${projectId} not found in codev/specs/`); + if (!spec) throw new Error(`Spec ${projectId} not found`); const plan = findPlanContent(workspaceRoot, projectId); console.error(`Spec: ${spec.label}`); if (plan) console.error(`Plan: ${plan.label}`); @@ -1127,7 +1127,7 @@ function resolveBuilderQuery(workspaceRoot: string, type: string, options: Consu case 'plan': { const plan = findPlanContent(workspaceRoot, projectId); - if (!plan) throw new Error(`Plan ${projectId} not found in codev/plans/`); + if (!plan) throw new Error(`Plan ${projectId} not found`); const spec = findSpecContent(workspaceRoot, projectId); console.error(`Plan: ${plan.label}`); if (spec) console.error(`Spec: ${spec.label}`); @@ -1195,7 +1195,7 @@ function resolveArchitectQuery(workspaceRoot: string, type: string, options: Con switch (type) { case 'spec': { const spec = findSpecContent(workspaceRoot, issueId); - if (!spec) throw new Error(`Spec ${issueId} not found in codev/specs/`); + if (!spec) throw new Error(`Spec ${issueId} not found`); const plan = findPlanContent(workspaceRoot, issueId); console.error(`Spec: ${spec.label}`); if (plan) console.error(`Plan: ${plan.label}`); @@ -1204,7 +1204,7 @@ function resolveArchitectQuery(workspaceRoot: string, type: string, options: Con case 'plan': { const plan = findPlanContent(workspaceRoot, issueId); - if (!plan) throw new Error(`Plan ${issueId} not found in codev/plans/`); + if (!plan) throw new Error(`Plan ${issueId} not found`); const spec = findSpecContent(workspaceRoot, issueId); console.error(`Plan: ${plan.label}`); if (spec) console.error(`Spec: ${spec.label}`); diff --git a/packages/codev/src/commands/porch/__tests__/artifacts.test.ts b/packages/codev/src/commands/porch/__tests__/artifacts.test.ts index fc3e421ba2..b6c9e22e78 100644 --- a/packages/codev/src/commands/porch/__tests__/artifacts.test.ts +++ b/packages/codev/src/commands/porch/__tests__/artifacts.test.ts @@ -220,6 +220,15 @@ describe('getResolver', () => { expect(() => getResolver(testDir)).toThrow('no artifacts.command'); }); + it('returns CliResolver for backend: "fava-trails" (alias for cli)', () => { + fs.writeFileSync( + path.join(testDir, 'af-config.json'), + JSON.stringify({ artifacts: { backend: 'fava-trails', command: 'my-tool', scope: 'org/proj' } }), + ); + const resolver = getResolver(testDir); + expect(resolver).toBeInstanceOf(CliResolver); + }); + it('throws for unknown backend', () => { fs.writeFileSync( path.join(testDir, 'af-config.json'), diff --git a/packages/codev/src/commands/porch/__tests__/checks.test.ts b/packages/codev/src/commands/porch/__tests__/checks.test.ts index 36bc345a11..c39876c092 100644 --- a/packages/codev/src/commands/porch/__tests__/checks.test.ts +++ b/packages/codev/src/commands/porch/__tests__/checks.test.ts @@ -7,10 +7,12 @@ import { tmpdir } from 'node:os'; import { runCheck, runPhaseChecks, + runArtifactCheck, formatCheckResults, allChecksPassed, type CheckEnv, } from '../checks.js'; +import type { ArtifactResolver } from '../artifacts.js'; describe('porch check runner', () => { const cwd = tmpdir(); @@ -190,3 +192,132 @@ describe('porch check runner', () => { }); }); }); + +// ============================================================================= +// runArtifactCheck +// ============================================================================= + +function makeResolver(overrides: Partial = {}): ArtifactResolver { + return { + findSpecBaseName: () => null, + getSpecContent: () => null, + getPlanContent: () => null, + getReviewContent: () => null, + hasPreApproval: () => false, + ...overrides, + }; +} + +describe('runArtifactCheck', () => { + const env: CheckEnv = { PROJECT_ID: '42', PROJECT_TITLE: '42-my-feature' }; + + it('returns null for unrecognized check names (fall through to shell)', () => { + const resolver = makeResolver(); + expect(runArtifactCheck('custom_check', 'echo hi', resolver, env)).toBeNull(); + }); + + describe('plan_exists', () => { + it('passes when resolver returns plan content', () => { + const resolver = makeResolver({ getPlanContent: () => '# Plan' }); + const result = runArtifactCheck('plan_exists', 'check-plan', resolver, env); + expect(result).not.toBeNull(); + expect(result!.passed).toBe(true); + }); + + it('fails when resolver returns null', () => { + const resolver = makeResolver({ getPlanContent: () => null }); + const result = runArtifactCheck('plan_exists', 'check-plan', resolver, env); + expect(result).not.toBeNull(); + expect(result!.passed).toBe(false); + expect(result!.error).toBeDefined(); + }); + }); + + describe('has_phases_json', () => { + it('passes when plan contains "phases": key', () => { + const resolver = makeResolver({ getPlanContent: () => '```json\n{"phases": []}\n```' }); + const result = runArtifactCheck('has_phases_json', 'cmd', resolver, env); + expect(result!.passed).toBe(true); + }); + + it('fails when plan has no "phases": key', () => { + const resolver = makeResolver({ getPlanContent: () => '# Plan with no phases' }); + const result = runArtifactCheck('has_phases_json', 'cmd', resolver, env); + expect(result!.passed).toBe(false); + }); + + it('fails when plan not found', () => { + const resolver = makeResolver({ getPlanContent: () => null }); + const result = runArtifactCheck('has_phases_json', 'cmd', resolver, env); + expect(result!.passed).toBe(false); + }); + }); + + describe('min_two_phases', () => { + it('passes when plan has 2+ phases', () => { + const content = '{"id": "phase-1"} {"id": "phase-2"}'; + const resolver = makeResolver({ getPlanContent: () => content }); + const result = runArtifactCheck('min_two_phases', 'cmd', resolver, env); + expect(result!.passed).toBe(true); + }); + + it('fails when plan has only 1 phase', () => { + const resolver = makeResolver({ getPlanContent: () => '{"id": "phase-1"}' }); + const result = runArtifactCheck('min_two_phases', 'cmd', resolver, env); + expect(result!.passed).toBe(false); + }); + }); + + describe('review_has_arch_updates', () => { + it('passes when review has Architecture Updates section', () => { + const resolver = makeResolver({ getReviewContent: () => '## Architecture Updates\n...' }); + const result = runArtifactCheck('review_has_arch_updates', 'cmd', resolver, env); + expect(result!.passed).toBe(true); + }); + + it('fails when review is missing the section', () => { + const resolver = makeResolver({ getReviewContent: () => '## Other Section' }); + const result = runArtifactCheck('review_has_arch_updates', 'cmd', resolver, env); + expect(result!.passed).toBe(false); + }); + + it('fails when review not found', () => { + const resolver = makeResolver({ getReviewContent: () => null }); + const result = runArtifactCheck('review_has_arch_updates', 'cmd', resolver, env); + expect(result!.passed).toBe(false); + }); + }); + + describe('review_has_lessons_updates', () => { + it('passes when review has Lessons Learned Updates section', () => { + const resolver = makeResolver({ getReviewContent: () => '## Lessons Learned Updates\n...' }); + const result = runArtifactCheck('review_has_lessons_updates', 'cmd', resolver, env); + expect(result!.passed).toBe(true); + }); + + it('fails when review is missing the section', () => { + const resolver = makeResolver({ getReviewContent: () => '## Other' }); + const result = runArtifactCheck('review_has_lessons_updates', 'cmd', resolver, env); + expect(result!.passed).toBe(false); + }); + }); + + describe('resolver integration with runPhaseChecks', () => { + it('uses resolver path when resolver handles the check, skipping shell', async () => { + const resolver = makeResolver({ getPlanContent: () => '# Plan' }); + const checks = { plan_exists: 'some-shell-cmd-that-would-fail' }; + const results = await runPhaseChecks(checks, tmpdir(), env, undefined, resolver); + expect(results[0].passed).toBe(true); + expect(results[0].output).toContain('resolver'); + }); + + it('falls through to shell when check is in overriddenChecks', async () => { + const resolver = makeResolver({ getPlanContent: () => '# Plan' }); + const checks = { plan_exists: 'echo shell-ran' }; + const overridden = new Set(['plan_exists']); + const results = await runPhaseChecks(checks, tmpdir(), env, undefined, resolver, overridden); + // Shell ran, not resolver + expect(results[0].output).toContain('shell-ran'); + }); + }); +}); diff --git a/packages/codev/src/commands/porch/artifacts.ts b/packages/codev/src/commands/porch/artifacts.ts index 3d09fbdef9..f20f3f2dc0 100644 --- a/packages/codev/src/commands/porch/artifacts.ts +++ b/packages/codev/src/commands/porch/artifacts.ts @@ -387,7 +387,7 @@ export function findConfigRoot(workspaceRoot: string): string { // ============================================================================= export interface ArtifactConfig { - backend?: 'local' | 'cli'; + backend?: 'local' | 'cli' | 'fava-trails'; scope?: string; command?: string; } @@ -418,16 +418,17 @@ function loadArtifactConfig(workspaceRoot: string): ArtifactConfig | null { export function getResolver(workspaceRoot: string): ArtifactResolver { const config = loadArtifactConfig(workspaceRoot); - if (config?.backend === 'cli') { + // 'fava-trails' is accepted as an alias for 'cli' + if (config?.backend === 'cli' || config?.backend === 'fava-trails') { if (!config.scope) { throw new Error( - `af-config.json has artifacts.backend: "cli" but no artifacts.scope.\n` + + `af-config.json has artifacts.backend: "${config.backend}" but no artifacts.scope.\n` + `Add: "artifacts": { "backend": "cli", "command": "my-tool", "scope": "org/project/codev-assets" }` ); } if (!config.command) { throw new Error( - `af-config.json has artifacts.backend: "cli" but no artifacts.command.\n` + + `af-config.json has artifacts.backend: "${config.backend}" but no artifacts.command.\n` + `Add: "artifacts": { "backend": "cli", "command": "my-tool", "scope": "org/project/codev-assets" }` ); } @@ -437,7 +438,7 @@ export function getResolver(workspaceRoot: string): ArtifactResolver { if (config?.backend && config.backend !== 'local') { throw new Error( `af-config.json has unknown artifacts.backend: "${config.backend}".\n` + - `Valid values: "local" (default), "cli"` + `Valid values: "local" (default), "cli", "fava-trails" (alias for cli)` ); } diff --git a/packages/codev/src/commands/porch/index.ts b/packages/codev/src/commands/porch/index.ts index 25468304c8..a70a4fb4fe 100644 --- a/packages/codev/src/commands/porch/index.ts +++ b/packages/codev/src/commands/porch/index.ts @@ -232,7 +232,7 @@ export async function check(workspaceRoot: string, projectId: string): Promise 0) { const resolver = getResolver(workspaceRoot); - const checkEnv: CheckEnv = { PROJECT_ID: state.id, PROJECT_TITLE: resolveArtifactBaseName(workspaceRoot, state.id, state.title) }; + const checkEnv: CheckEnv = { PROJECT_ID: state.id, PROJECT_TITLE: resolveArtifactBaseName(workspaceRoot, state.id, state.title, resolver) }; console.log(''); console.log(chalk.bold('RUNNING CHECKS...')); From ab6dc600f743f576580bba8804922f25d615a351 Mon Sep 17 00:00:00 2001 From: "Younes (Machine Wisdom)" Date: Thu, 26 Mar 2026 19:19:00 -0400 Subject: [PATCH 09/10] [Bugfix #612] Address CMAP review: fix 4 resolver bypass paths + dead code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Based on Claude reviewer feedback on PR #613: 1. Fix 4 callsites in index.ts that still used findPlanFile/extractPhasesFromFile directly, bypassing the resolver: status(), advanceProtocolPhase() ×2, rollback(). Now all use getPlanContent() and extractPlanPhasesResolved() with the resolver threaded through. 2. Remove dead code: findSpec() and findPlan() in consult/index.ts are no longer called after the resolver migration (findSpecContent/findPlanContent use the resolver instead). 3. Remove console.error diagnostic logging from getProjectSummary() in prompts.ts — these produced stderr noise on every offline/CI invocation. 4. Remove extra blank line at consult/index.ts:1146. Co-Authored-By: Claude Sonnet 4.6 --- packages/codev/src/commands/consult/index.ts | 47 ------------------ packages/codev/src/commands/porch/index.ts | 52 +++++++------------- packages/codev/src/commands/porch/prompts.ts | 4 -- 3 files changed, 19 insertions(+), 84 deletions(-) diff --git a/packages/codev/src/commands/consult/index.ts b/packages/codev/src/commands/consult/index.ts index 3d021ec411..c4c8c722e3 100644 --- a/packages/codev/src/commands/consult/index.ts +++ b/packages/codev/src/commands/consult/index.ts @@ -195,52 +195,6 @@ function loadDotenv(workspaceRoot: string): void { } } -/** - * Find a spec file by ID. Returns null if not found. - * Errors if multiple matches found. - */ -function findSpec(workspaceRoot: string, id: string): string | null { - const specsDir = path.join(workspaceRoot, 'codev', 'specs'); - const unpadded = id.replace(/^0+/, '') || '0'; - const padded = /^\d+$/.test(id) ? id.padStart(4, '0') : id; - - if (fs.existsSync(specsDir)) { - const files = fs.readdirSync(specsDir).filter(f => f.endsWith('.md')); - const matches = files.filter(f => f.startsWith(`${unpadded}-`) || f.startsWith(`${padded}-`) || f.startsWith(`${id}-`)); - if (matches.length > 1) { - const list = matches.map(f => ` - codev/specs/${f}`).join('\n'); - throw new Error(`Multiple spec files match '${id}':\n${list}`); - } - if (matches.length === 1) { - return path.join(specsDir, matches[0]); - } - } - return null; -} - -/** - * Find a plan file by ID. Returns null if not found. - * Errors if multiple matches found. - */ -function findPlan(workspaceRoot: string, id: string): string | null { - const plansDir = path.join(workspaceRoot, 'codev', 'plans'); - const unpadded = id.replace(/^0+/, '') || '0'; - const padded = /^\d+$/.test(id) ? id.padStart(4, '0') : id; - - if (fs.existsSync(plansDir)) { - const files = fs.readdirSync(plansDir).filter(f => f.endsWith('.md')); - const matches = files.filter(f => f.startsWith(`${unpadded}-`) || f.startsWith(`${padded}-`) || f.startsWith(`${id}-`)); - if (matches.length > 1) { - const list = matches.map(f => ` - codev/plans/${f}`).join('\n'); - throw new Error(`Multiple plan files match '${id}':\n${list}`); - } - if (matches.length === 1) { - return path.join(plansDir, matches[0]); - } - } - return null; -} - /** * Find spec content via artifact resolver. * Uses the configured resolver (cli or local). Config errors propagate @@ -1191,7 +1145,6 @@ function resolveArchitectQuery(workspaceRoot: string, type: string, options: Con const issueId = options.issue; - switch (type) { case 'spec': { const spec = findSpecContent(workspaceRoot, issueId); diff --git a/packages/codev/src/commands/porch/index.ts b/packages/codev/src/commands/porch/index.ts index a70a4fb4fe..981d47d054 100644 --- a/packages/codev/src/commands/porch/index.ts +++ b/packages/codev/src/commands/porch/index.ts @@ -32,12 +32,12 @@ import { getVerifyConfig, } from './protocol.js'; import { - findPlanFile, - extractPhasesFromFile, getCurrentPlanPhase, getPhaseContent, allPlanPhasesComplete, isPlanPhaseComplete, + getPlanContent, + extractPlanPhasesResolved, } from './plan.js'; import { runPhaseChecks, @@ -46,7 +46,7 @@ import { type CheckEnv, } from './checks.js'; import { loadCheckOverrides } from './config.js'; -import { getResolver } from './artifacts.js'; +import { getResolver, type ArtifactResolver } from './artifacts.js'; // ============================================================================ // Output Helpers @@ -159,10 +159,9 @@ export async function status(workspaceRoot: string, projectId: string): Promise< console.log(chalk.bold(`CURRENT: ${currentPlanPhase.id} - ${currentPlanPhase.title}`)); // Show phase content from plan - const planPath = findPlanFile(workspaceRoot, state.id, state.title); - if (planPath) { - const content = fs.readFileSync(planPath, 'utf-8'); - const phaseContent = getPhaseContent(content, currentPlanPhase.id); + const planContent = getPlanContent(workspaceRoot, state.id, state.title, getResolver(workspaceRoot)); + if (planContent) { + const phaseContent = getPhaseContent(planContent, currentPlanPhase.id); if (phaseContent) { console.log(section('FROM THE PLAN', phaseContent.slice(0, 500))); } @@ -372,10 +371,10 @@ export async function done(workspaceRoot: string, projectId: string): Promise 0) { - state.current_plan_phase = state.plan_phases[0].id; - } + const phases = extractPlanPhasesResolved(workspaceRoot, state.id, state.title, resolver); + if (phases.length > 0) { + state.plan_phases = phases; + state.current_plan_phase = phases[0].id; } } @@ -417,10 +413,9 @@ function advanceProtocolPhase(workspaceRoot: string, state: ProjectState, protoc console.log(chalk.bold(`YOUR TASK: ${firstPhase.id} - "${firstPhase.title}"`)); // Show phase content from plan - const planPath = findPlanFile(workspaceRoot, state.id, state.title); - if (planPath) { - const content = fs.readFileSync(planPath, 'utf-8'); - const phaseContent = getPhaseContent(content, firstPhase.id); + const planContent = getPlanContent(workspaceRoot, state.id, state.title, resolver); + if (planContent) { + const phaseContent = getPhaseContent(planContent, firstPhase.id); if (phaseContent) { console.log(section('FROM THE PLAN', phaseContent.slice(0, 800))); } @@ -641,18 +636,9 @@ export async function rollback( // If rolling back to a phased phase, re-extract plan phases from plan file if (isPhased(protocol, targetPhase)) { - const planPath = findPlanFile(workspaceRoot, state.id, state.title); - if (planPath) { - state.plan_phases = extractPhasesFromFile(planPath); - if (state.plan_phases.length > 0) { - state.current_plan_phase = state.plan_phases[0].id; - } else { - state.current_plan_phase = null; - } - } else { - state.plan_phases = []; - state.current_plan_phase = null; - } + const phases = extractPlanPhasesResolved(workspaceRoot, state.id, state.title, getResolver(workspaceRoot)); + state.plan_phases = phases; + state.current_plan_phase = phases.length > 0 ? phases[0].id : null; } else { state.plan_phases = []; state.current_plan_phase = null; diff --git a/packages/codev/src/commands/porch/prompts.ts b/packages/codev/src/commands/porch/prompts.ts index 9d4a4bca7e..f94715c970 100644 --- a/packages/codev/src/commands/porch/prompts.ts +++ b/packages/codev/src/commands/porch/prompts.ts @@ -41,7 +41,6 @@ export async function getProjectSummary(workspaceRoot: string, projectId: string if (issue?.title) { return issue.title; } - console.error(`[porch] project summary: GitHub issue #${issueNumber} not found, trying spec`); } // 2. Fallback: read first heading from spec via resolver @@ -52,9 +51,6 @@ export async function getProjectSummary(workspaceRoot: string, projectId: string if (headingMatch) { return headingMatch[1].trim(); } - console.error(`[porch] project summary: spec found but no heading, using title`); - } else { - console.error(`[porch] project summary: spec not found via resolver, using title`); } // 3. Last resort: project title from status.yaml From e912ccc744f18ceafe3aee07f535ecb137855df6 Mon Sep 17 00:00:00 2001 From: "Younes (Machine Wisdom)" Date: Thu, 26 Mar 2026 19:29:09 -0400 Subject: [PATCH 10/10] [Bugfix #612] Fix remaining hardcoded path in spawn.ts fatal message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fatal() error message when no spec is found still referenced codev/specs/ as the expected location, which is confusing for users with a CLI artifact backend. Remove the local path hint — the spec ID is sufficient to identify what's missing. Co-Authored-By: Claude Sonnet 4.6 --- packages/codev/src/agent-farm/commands/spawn.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/codev/src/agent-farm/commands/spawn.ts b/packages/codev/src/agent-farm/commands/spawn.ts index 7a3e09d247..7b5b96776f 100644 --- a/packages/codev/src/agent-farm/commands/spawn.ts +++ b/packages/codev/src/agent-farm/commands/spawn.ts @@ -313,7 +313,7 @@ async function spawnSpec(options: SpawnOptions, config: Config): Promise { // Protocol allows no-spec spawn — will derive naming from GitHub issue title logger.info('No spec file found. Protocol allows spawning without one (Specify phase will create it).'); } else { - fatal(`Spec not found for ${protocol === 'tick' ? `amends #${options.amends}` : `issue #${issueNumber}`}. Expected: codev/specs/${specLookupId}-*.md`); + fatal(`Spec not found for ${protocol === 'tick' ? `amends #${options.amends}` : `issue #${issueNumber}`}. Expected spec ID: ${specLookupId}`); } }