Skip to content

feat: inject config.yaml context and rules into apply/verify phases #887

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
k1r3zz wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: inject config.yaml context and rules into apply/verify phases #887

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6610014
feat: inject config.yaml context and rules into apply/verify phases
k1r3zz Mar 28, 2026
317838c
refactor: address CodeRabbit review feedback
Mar 28, 2026
d192f08
fix: case-insensitive check for reserved phase IDs
Mar 28, 2026
afb54d9
fix(config): normalize YAML objects back to strings in rules parsing
k1r3zz Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion src/cli/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ import {
statusCommand,
instructionsCommand,
applyInstructionsCommand,
verifyInstructionsCommand,
templatesCommand,
schemasCommand,
newChangeCommand,
DEFAULT_SCHEMA,
type StatusOptions,
type InstructionsOptions,
type VerifyInstructionsOptions,
type TemplatesOptions,
type SchemasOptions,
type NewChangeOptions,
Expand Down Expand Up @@ -445,9 +447,11 @@ program
.option('--json', 'Output as JSON')
.action(async (artifactId: string | undefined, options: InstructionsOptions) => {
try {
// Special case: "apply" is not an artifact, but a command to get apply instructions
// Special case: "apply" and "verify" are not artifacts, but phase commands
if (artifactId === 'apply') {
await applyInstructionsCommand(options);
} else if (artifactId === 'verify') {
await verifyInstructionsCommand(options as VerifyInstructionsOptions);
} else {
await instructionsCommand(artifactId, options);
}
Expand Down
4 changes: 2 additions & 2 deletions src/commands/workflow/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
export { statusCommand } from './status.js';
export type { StatusOptions } from './status.js';

export { instructionsCommand, applyInstructionsCommand } from './instructions.js';
export type { InstructionsOptions } from './instructions.js';
export { instructionsCommand, applyInstructionsCommand, verifyInstructionsCommand } from './instructions.js';
export type { InstructionsOptions, VerifyInstructionsOptions } from './instructions.js';

export { templatesCommand } from './templates.js';
export type { TemplatesOptions } from './templates.js';
Expand Down
168 changes: 167 additions & 1 deletion src/commands/workflow/instructions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ import {
import {
validateChangeExists,
validateSchemaExists,
readPhaseConfig,
type TaskItem,
type ApplyInstructions,
type VerifyInstructions,
} from './shared.js';

// -----------------------------------------------------------------------------
Expand All @@ -37,6 +39,12 @@ export interface ApplyInstructionsOptions {
json?: boolean;
}

export interface VerifyInstructionsOptions {
change?: string;
schema?: string;
json?: boolean;
}

// -----------------------------------------------------------------------------
// Artifact Instructions Command
// -----------------------------------------------------------------------------
Expand Down Expand Up @@ -386,6 +394,9 @@ export async function generateApplyInstructions(
instruction = schemaInstruction?.trim() ?? 'Read context files, work through pending tasks, mark complete as you go.\nPause if you hit blockers or need clarification.';
}

// Read project config for context and rules injection
const phaseConfig = readPhaseConfig(projectRoot, 'apply');

return {
changeName,
changeDir,
Expand All @@ -396,6 +407,8 @@ export async function generateApplyInstructions(
state,
missingArtifacts: missingArtifacts.length > 0 ? missingArtifacts : undefined,
instruction,
context: phaseConfig.context,
rules: phaseConfig.rules,
};
}

Expand Down Expand Up @@ -429,7 +442,7 @@ export async function applyInstructionsCommand(options: ApplyInstructionsOptions
}

export function printApplyInstructionsText(instructions: ApplyInstructions): void {
const { changeName, schemaName, contextFiles, progress, tasks, state, missingArtifacts, instruction } = instructions;
const { changeName, schemaName, contextFiles, progress, tasks, state, missingArtifacts, instruction, context, rules } = instructions;

console.log(`## Apply: ${changeName}`);
console.log(`Schema: ${schemaName}`);
Expand Down Expand Up @@ -475,7 +488,160 @@ export function printApplyInstructionsText(instructions: ApplyInstructions): voi
console.log();
}

// Project context (if present)
if (context) {
console.log('### Project Context');
console.log(context);
console.log();
}

// Rules (if present)
if (rules && rules.length > 0) {
console.log('### Rules');
for (const rule of rules) {
console.log(`- ${rule}`);
}
console.log();
}

// Instruction
console.log('### Instruction');
console.log(instruction);
}

// -----------------------------------------------------------------------------
// Verify Instructions Command
// -----------------------------------------------------------------------------

/**
* Generates verify instructions for validating implementation against artifacts.
* Reads config.yaml context and rules.verify for custom verification strategies.
*/
export async function generateVerifyInstructions(
projectRoot: string,
changeName: string,
schemaName?: string
): Promise<VerifyInstructions> {
const context = loadChangeContext(projectRoot, changeName, schemaName);
const changeDir = path.join(projectRoot, 'openspec', 'changes', changeName);

const schema = resolveSchema(context.schemaName, projectRoot);

// Build context files from all existing artifacts in schema
const contextFiles: Record<string, string> = {};
for (const artifact of schema.artifacts) {
if (artifactOutputExists(changeDir, artifact.generates)) {
contextFiles[artifact.id] = path.join(changeDir, artifact.generates);
}
Comment on lines +530 to +535
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Resolve globbed artifact outputs before returning contextFiles.

artifactOutputExists() already treats generates as a glob, but this branch stores path.join(changeDir, artifact.generates) verbatim. For multi-file artifacts like specs/*.md, verify output will hand consumers a wildcard string instead of the concrete files to read, so required context can be skipped. Please expand matches here, or change the field shape to hold arrays of resolved paths.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/commands/workflow/instructions.ts` around lines 542 - 547, The loop that
builds contextFiles stores path.join(changeDir, artifact.generates) verbatim
even though artifact.generates may be a glob; update the logic in the block that
iterates schema.artifacts (the contextFiles map, the artifactOutputExists check,
and the artifact.generates usage) to expand the glob into concrete file paths
before assigning: use the same glob resolution used by artifactOutputExists to
collect all matching files under changeDir and either (a) set
contextFiles[artifact.id] to an array of resolved absolute/relative file paths,
or (b) if you must keep a string shape, join the resolved paths into a
delimiter-separated string—ensure consumers are updated accordingly so
multi-file artifacts like "specs/*.md" return the actual matched files rather
than the wildcard.

}

// Parse tasks if tracking file exists (reuse apply logic)
const applyConfig = schema.apply;
const tracksFile = applyConfig?.tracks ?? null;
let tasks: TaskItem[] = [];
if (tracksFile) {
const tracksPath = path.join(changeDir, tracksFile);
if (fs.existsSync(tracksPath)) {
const tasksContent = await fs.promises.readFile(tracksPath, 'utf-8');
tasks = parseTasksFile(tasksContent);
}
}

const total = tasks.length;
const complete = tasks.filter((t) => t.done).length;
const remaining = total - complete;

// Read project config for context and rules injection
const phaseConfig = readPhaseConfig(projectRoot, 'verify');

return {
changeName,
changeDir,
schemaName: context.schemaName,
contextFiles,
progress: { total, complete, remaining },
tasks,
context: phaseConfig.context,
rules: phaseConfig.rules,
};
}

export async function verifyInstructionsCommand(options: VerifyInstructionsOptions): Promise<void> {
const spinner = ora('Generating verify instructions...').start();

try {
const projectRoot = process.cwd();
const changeName = await validateChangeExists(options.change, projectRoot);

if (options.schema) {
validateSchemaExists(options.schema, projectRoot);
}

const instructions = await generateVerifyInstructions(projectRoot, changeName, options.schema);

spinner.stop();

if (options.json) {
console.log(JSON.stringify(instructions, null, 2));
return;
}

printVerifyInstructionsText(instructions);
} catch (error) {
spinner.stop();
throw error;
}
}

export function printVerifyInstructionsText(instructions: VerifyInstructions): void {
const { changeName, schemaName, contextFiles, progress, tasks, context, rules } = instructions;

console.log(`## Verify: ${changeName}`);
console.log(`Schema: ${schemaName}`);
console.log();

// Context files
const contextFileEntries = Object.entries(contextFiles);
if (contextFileEntries.length > 0) {
console.log('### Context Files');
for (const [artifactId, filePath] of contextFileEntries) {
console.log(`- ${artifactId}: ${filePath}`);
}
console.log();
}

// Progress
if (progress.total > 0) {
console.log('### Progress');
console.log(`${progress.complete}/${progress.total} tasks complete`);
console.log();
}

// Project context (if present)
if (context) {
console.log('### Project Context');
console.log(context);
console.log();
}

// Rules (if present)
if (rules && rules.length > 0) {
console.log('### Rules');
for (const rule of rules) {
console.log(`- ${rule}`);
}
console.log();
}

// Incomplete tasks
if (tasks.length > 0) {
const incomplete = tasks.filter(t => !t.done);
if (incomplete.length > 0) {
console.log('### Incomplete Tasks');
for (const task of incomplete) {
console.log(`- [ ] ${task.description}`);
}
console.log();
}
}
}
51 changes: 51 additions & 0 deletions src/commands/workflow/shared.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import path from 'path';
import * as fs from 'fs';
import { getSchemaDir, listSchemas } from '../../core/artifact-graph/index.js';
import { validateChangeName } from '../../utils/change-utils.js';
import { readProjectConfig, RESERVED_PHASE_IDS } from '../../core/project-config.js';

// -----------------------------------------------------------------------------
// Types
Expand All @@ -35,6 +36,56 @@ export interface ApplyInstructions {
state: 'blocked' | 'all_done' | 'ready';
missingArtifacts?: string[];
instruction: string;
context?: string;
rules?: string[];
}

export interface VerifyInstructions {
changeName: string;
changeDir: string;
schemaName: string;
contextFiles: Record<string, string>;
progress: {
total: number;
complete: number;
remaining: number;
};
tasks: TaskItem[];
context?: string;
rules?: string[];
}

// Re-export RESERVED_PHASE_IDS for convenience
export { RESERVED_PHASE_IDS } from '../../core/project-config.js';

/**
* Reads project config and extracts context + phase-specific rules.
* Centralizes the config-reading logic used by both apply and verify phases.
*
* @param projectRoot - The root directory of the project
* @param phase - The phase to read rules for ('apply' | 'verify')
* @returns Object with optional context and rules, or empty object on failure
*/
export function readPhaseConfig(
projectRoot: string,
phase: 'apply' | 'verify'
): { context?: string; rules?: string[] } {
try {
const projectConfig = readProjectConfig(projectRoot);
const result: { context?: string; rules?: string[] } = {};

if (projectConfig?.context?.trim()) {
result.context = projectConfig.context.trim();
}
if (projectConfig?.rules?.[phase] && projectConfig.rules[phase].length > 0) {
result.rules = projectConfig.rules[phase];
}

return result;
} catch {
// If config read fails, continue without config
return {};
}
}

// -----------------------------------------------------------------------------
Expand Down
19 changes: 19 additions & 0 deletions src/core/artifact-graph/schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as fs from 'node:fs';
import { parse as parseYaml } from 'yaml';
import { SchemaYamlSchema, type SchemaYaml, type Artifact } from './types.js';
import { RESERVED_PHASE_IDS } from '../project-config.js';

export class SchemaValidationError extends Error {
constructor(message: string) {
Expand Down Expand Up @@ -32,6 +33,9 @@ export function parseSchema(yamlContent: string): SchemaYaml {

const schema = result.data;

// Check for reserved phase IDs used as artifact IDs
validateNoReservedIds(schema.artifacts);

// Check for duplicate artifact IDs
validateNoDuplicateIds(schema.artifacts);

Expand All @@ -44,6 +48,21 @@ export function parseSchema(yamlContent: string): SchemaYaml {
return schema;
}

/**
* Validates that no artifact uses a reserved phase ID ('apply', 'verify').
* These names are reserved for phase-level commands in the CLI.
*/
function validateNoReservedIds(artifacts: Artifact[]): void {
for (const artifact of artifacts) {
if (RESERVED_PHASE_IDS.has(artifact.id.toLowerCase())) {
throw new SchemaValidationError(
`Artifact ID '${artifact.id}' is reserved for phase commands. ` +
`Reserved IDs: ${Array.from(RESERVED_PHASE_IDS).join(', ')}`
);
}
}
Comment on lines +55 to +63
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the definition of RESERVED_PHASE_IDS
echo "=== RESERVED_PHASE_IDS definition ==="
rg -n -A5 'RESERVED_PHASE_IDS' src/core/project-config.ts

# Check how CLI routes the instructions command to see if it's case-sensitive
echo -e "\n=== CLI routing for instructions command ==="
rg -n -B2 -A5 'instructions' src/cli/index.ts | head -50

Repository: Fission-AI/OpenSpec

Length of output: 1595

🏁 Script executed:

#!/bin/bash
# Check if artifact IDs are normalized to lowercase anywhere
echo "=== Artifact ID normalization ==="
rg -n -i 'toLowerCase|normalize' src/core/ | grep -i artifact | head -20

# Check the full schema validation file to understand artifact ID handling
echo -e "\n=== Artifact ID schema validation ==="
rg -n 'artifact.*id|id.*artifact' src/core/artifact-graph/schema.ts -i | head -20

# Check if there's any case-insensitive comparison or normalization
echo -e "\n=== Case-insensitive comparisons ==="
rg -n 'toLowerCase\|toUpperCase' src/ | head -20

# Look at the verifyInstructionsCommand and instructionsCommand handlers
echo -e "\n=== Verify instructions command handler ==="
rg -n -A10 'verifyInstructionsCommand' src/cli/ | head -40

Repository: Fission-AI/OpenSpec

Length of output: 3150

🏁 Script executed:

#!/bin/bash
# Check the Artifact interface definition to see if case is preserved
echo "=== Artifact interface definition ==="
rg -n -B2 -A5 'interface Artifact\|type Artifact' src/ | head -40

# Check how instructionsCommand handler processes the artifactId
echo -e "\n=== Instructions command handler implementation ==="
rg -n -A15 'export.*instructionsCommand|async.*instructionsCommand' src/cli/ | head -50

# Check how the artifact ID is retrieved/validated in the commands
echo -e "\n=== Artifact retrieval in commands ==="
rg -n 'getArtifact|findArtifact' src/cli/ -A3 | head -40

# Check if there's any example schema with mixed-case artifact IDs
echo -e "\n=== Sample artifact definitions ==="
find . -name "*.yaml" -o -name "*.yml" | xargs grep -l "artifacts:" 2>/dev/null | head -5 | xargs head -100

Repository: Fission-AI/OpenSpec

Length of output: 5139

🏁 Script executed:

#!/bin/bash
# Search more broadly for Artifact type
echo "=== Artifact type/interface definition ==="
rg -n 'Artifact' src/core/artifact-graph/schema.ts | head -20

# Check the imports and types in schema.ts
echo -e "\n=== Schema.ts imports and types ==="
head -30 src/core/artifact-graph/schema.ts

# Search for the actual type definition in artifact-graph directory
echo -e "\n=== Types in artifact-graph ==="
fd '\.ts' src/core/artifact-graph/ | xargs grep -l 'interface Artifact\|type Artifact'

# Find the commands implementation file
echo -e "\n=== Find commands file ==="
find src -name '*command*' -type f

# Check the instructions command more deeply
echo -e "\n=== Instructions command in commands file ==="
rg -n 'instructionsCommand' src/ -l

Repository: Fission-AI/OpenSpec

Length of output: 1928

🏁 Script executed:

#!/bin/bash
# Read the Artifact type definition
echo "=== Artifact type definition in types.ts ==="
cat src/core/artifact-graph/types.ts

# Read the instructions command handler
echo -e "\n=== Instructions command handler ==="
cat src/commands/workflow/instructions.ts

# Check if there's any validation of artifact IDs in the resolver
echo -e "\n=== Artifact resolution/retrieval ==="
rg -n 'artifact\.id|artifactId' src/commands/workflow/ -A2 -B2 | head -80

Repository: Fission-AI/OpenSpec

Length of output: 28155

🏁 Script executed:

#!/bin/bash
# Find and examine the getArtifact implementation
echo "=== getArtifact implementation ==="
rg -n 'getArtifact' src/core/artifact-graph/ -A5 | head -50

# Check if artifact IDs are used in case-sensitive lookups
echo -e "\n=== Artifact ID lookups in graph ==="
rg -n 'class.*Graph|getArtifact|find.*artifact' src/core/artifact-graph/ -B2 -A8 | head -100

Repository: Fission-AI/OpenSpec

Length of output: 4805

Normalize artifact IDs to lowercase when validating reserved IDs.

The check uses exact string matching (RESERVED_PHASE_IDS.has(artifact.id)), but the schema allows artifact IDs with any casing. An artifact ID like Apply or VERIFY would bypass this validation since only lowercase apply and verify are checked. Normalize to lowercase during validation:

if (RESERVED_PHASE_IDS.has(artifact.id.toLowerCase())) {
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/core/artifact-graph/schema.ts` around lines 55 - 63, The reserved-ID
check in validateNoReservedIds currently does a case-sensitive lookup against
RESERVED_PHASE_IDS using artifact.id; change the check to normalize the artifact
id to lowercase (e.g., use artifact.id.toLowerCase() when calling
RESERVED_PHASE_IDS.has) so IDs like "Apply" or "VERIFY" are caught, and update
the thrown SchemaValidationError message to reference the original artifact.id
while still checking the lowercase form; key symbols: validateNoReservedIds,
RESERVED_PHASE_IDS, Artifact.id, SchemaValidationError.

}

/**
* Validates that there are no duplicate artifact IDs.
*/
Expand Down
Loading