Fixes as per PR Review from Copilot

This commit is contained in:
danielmeppiel
2025-09-16 16:08:42 +02:00
parent 794515d242
commit a9512e00fc
13 changed files with 211 additions and 80 deletions

View File

@@ -144,8 +144,8 @@ Spec Kit includes full APM (Agent Package Manager) functionality for managing mo
### Unified Initialization
```bash
# The --with-apm flag creates both SDD and APM structures
specify init my-project --ai claude --with-apm
# The --use-apm flag creates both SDD and APM structures
specify init my-project --ai claude --use-apm
```
### APM Commands

View File

@@ -34,7 +34,7 @@ Context_Efficiency = Relevant_Instructions / Total_Instructions_Loaded
## Quick Start
```bash
specify init my-project --with-apm --ai copilot
specify init my-project --use-apm --ai copilot
specify apm install company/design-system
specify apm compile # Mathematical optimization generates distributed AGENTS.md files
```

View File

@@ -1587,6 +1587,48 @@ def compile(ctx, output, dry_run, no_links, chatmode, watch, validate, with_cons
• --clean: Remove orphaned AGENTS.md files that are no longer generated
"""
try:
# Check if this is an APM project first
from pathlib import Path
if not Path('apm.yml').exists():
_rich_error("❌ Not an APM project - no apm.yml found")
_rich_info("💡 To initialize an APM project, run:")
_rich_info(" specify init --use-apm")
_rich_info(" # or")
_rich_info(" specify apm init")
sys.exit(1)
# Check if there are any instruction files to compile
from .compilation.constitution import find_constitution
apm_modules_exists = Path("apm_modules").exists()
constitution_exists = find_constitution(Path(".")).exists()
# Check if .apm directory has actual content
apm_dir = Path(".apm")
local_apm_has_content = (apm_dir.exists() and
(any(apm_dir.rglob("*.instructions.md")) or
any(apm_dir.rglob("*.chatmode.md"))))
# If no primitive sources exist, check deeper to provide better feedback
if not apm_modules_exists and not local_apm_has_content and not constitution_exists:
# Check if .apm directories exist but are empty
has_empty_apm = apm_dir.exists() and not any(apm_dir.rglob("*.instructions.md")) and not any(apm_dir.rglob("*.chatmode.md"))
if has_empty_apm:
_rich_error("❌ No instruction files found in .apm/ directory")
_rich_info("💡 To add instructions, create files like:")
_rich_info(" .apm/instructions/coding-standards.instructions.md")
_rich_info(" .apm/chatmodes/backend-engineer.chatmode.md")
else:
_rich_error("❌ No APM content found to compile")
_rich_info("💡 To get started:")
_rich_info(" 1. Install APM dependencies: specify apm install <owner>/<repo>")
_rich_info(" 2. Or create local instructions: mkdir -p .apm/instructions")
_rich_info(" 3. Then create .instructions.md or .chatmode.md files")
if not dry_run: # Don't exit on dry-run to allow testing
sys.exit(1)
# Validation-only mode
if validate:
_rich_info("Validating APM context...", symbol="gear")
@@ -1640,25 +1682,6 @@ def compile(ctx, output, dry_run, no_links, chatmode, watch, validate, with_cons
else:
_rich_info("Using single-file compilation (legacy mode)", symbol="page")
# Check if there are any primitives to compile
try:
from pathlib import Path
from .compilation.constitution import find_constitution
apm_modules_exists = Path("apm_modules").exists()
local_apm_exists = Path(".apm").exists()
constitution_exists = find_constitution(Path(".")).exists()
if not apm_modules_exists and not local_apm_exists and not constitution_exists:
_rich_warning("No APM dependencies, local .apm/ directory, or constitution found")
_rich_info("💡 Nothing to compile. To get started:")
_rich_info(" 1. Install APM dependencies: specify apm install")
_rich_info(" 2. Or initialize APM project: specify apm init")
_rich_info(" 3. Then run: specify apm compile")
return
except Exception:
pass # Continue with compilation if check fails
# Perform compilation
compiler = AgentsCompiler(".")
result = compiler.compile(config)

View File

@@ -8,7 +8,7 @@ deterministic Build ID (content hash) is substituted post-generation.
# Constitution injection markers
CONSTITUTION_MARKER_BEGIN = "<!-- SPEC-KIT CONSTITUTION: BEGIN -->"
CONSTITUTION_MARKER_END = "<!-- SPEC-KIT CONSTITUTION: END -->"
CONSTITUTION_RELATIVE_PATH = "memory/constitution.md" # repo-root relative
CONSTITUTION_RELATIVE_PATH = ".specify/memory/constitution.md" # repo-root relative
# Build ID placeholder & regex pattern (line-level). The placeholder line is
# inserted during initial template generation; after all transformations

View File

@@ -317,32 +317,57 @@ class ContextOptimizer:
file_types.update(analysis.file_types)
total_files += analysis.total_files
# Check for constitution
from .constitution import find_constitution
constitution_path = find_constitution(Path(self.base_dir))
constitution_detected = constitution_path.exists()
project_analysis = ProjectAnalysis(
directories_scanned=len(self._directory_cache),
files_analyzed=total_files,
file_types_detected=file_types,
instruction_patterns_detected=len(self._optimization_decisions),
max_depth=max((a.depth for a in self._directory_cache.values()), default=0)
max_depth=max((a.depth for a in self._directory_cache.values()), default=0),
constitution_detected=constitution_detected,
constitution_path=str(constitution_path.relative_to(self.base_dir)) if constitution_detected else None
)
# Create placement summaries
placement_summaries = []
for directory, instructions in placement_map.items():
# Count unique sources
sources = set()
for instruction in instructions:
if hasattr(instruction, 'source_file') and instruction.source_file:
sources.add(instruction.source_file)
elif hasattr(instruction, 'source') and instruction.source:
sources.add(str(instruction.source))
# Special case: if no instructions but constitution exists, create root placement
if not placement_map and constitution_detected:
# Create a root placement for constitution-only projects
root_sources = {"constitution.md"}
summary = PlacementSummary(
path=directory,
instruction_count=len(instructions),
source_count=len(sources),
sources=list(sources)
path=Path(self.base_dir),
instruction_count=0,
source_count=len(root_sources),
sources=list(root_sources)
)
placement_summaries.append(summary)
else:
# Normal case: create summaries for each placement in the map
for directory, instructions in placement_map.items():
# Count unique sources
sources = set()
for instruction in instructions:
if hasattr(instruction, 'source_file') and instruction.source_file:
sources.add(instruction.source_file)
elif hasattr(instruction, 'source') and instruction.source:
sources.add(str(instruction.source))
# Add constitution as a source if it exists and will be injected
if constitution_detected:
sources.add("constitution.md")
summary = PlacementSummary(
path=directory,
instruction_count=len(instructions),
source_count=len(sources),
sources=list(sources)
)
placement_summaries.append(summary)
# Get optimization statistics
optimization_stats = self.get_optimization_stats(placement_map)

View File

@@ -245,6 +245,14 @@ class DistributedAgentsCompiler:
enable_timing=debug # Enable timing when debug mode is on
)
# Special case: if no instructions but constitution exists, create root placement
if not optimized_placement:
from .constitution import find_constitution
constitution_path = find_constitution(Path(self.base_dir))
if constitution_path.exists():
# Create an empty placement for the root directory to enable verbose output
optimized_placement = {Path(self.base_dir): []}
# Store optimization results for output formatting later
# Update with proper dry run status in the final result
self._placement_map = optimized_placement
@@ -286,30 +294,49 @@ class DistributedAgentsCompiler:
"""
placements = []
for dir_path, instructions in placement_map.items():
agents_path = dir_path / "AGENTS.md"
# Build source attribution map if enabled
source_map = {}
if source_attribution:
# Special case: if no instructions but constitution exists, create root placement
if not placement_map:
from .constitution import find_constitution
constitution_path = find_constitution(Path(self.base_dir))
if constitution_path.exists():
# Create a root placement for constitution-only projects
root_path = Path(self.base_dir)
agents_path = root_path / "AGENTS.md"
placement = PlacementResult(
agents_path=agents_path,
instructions=[], # No instructions, just constitution
coverage_patterns=set(), # No patterns since no instructions
source_attribution={"constitution": "constitution.md"} if source_attribution else {}
)
placements.append(placement)
else:
# Normal case: create placements for each entry in placement_map
for dir_path, instructions in placement_map.items():
agents_path = dir_path / "AGENTS.md"
# Build source attribution map if enabled
source_map = {}
if source_attribution:
for instruction in instructions:
source_info = getattr(instruction, 'source', 'local')
source_map[str(instruction.file_path)] = source_info
# Extract coverage patterns
patterns = set()
for instruction in instructions:
source_info = getattr(instruction, 'source', 'local')
source_map[str(instruction.file_path)] = source_info
# Extract coverage patterns
patterns = set()
for instruction in instructions:
if instruction.apply_to:
patterns.add(instruction.apply_to)
placement = PlacementResult(
agents_path=agents_path,
instructions=instructions,
coverage_patterns=patterns,
source_attribution=source_map
)
placements.append(placement)
if instruction.apply_to:
patterns.add(instruction.apply_to)
placement = PlacementResult(
agents_path=agents_path,
instructions=instructions,
coverage_patterns=patterns,
source_attribution=source_map
)
placements.append(placement)
return placements
@@ -479,7 +506,7 @@ class DistributedAgentsCompiler:
# Footer
sections.append("---")
sections.append("*This file was generated by APM CLI. Do not edit manually.*")
sections.append("*To regenerate: `apm compile`*")
sections.append("*To regenerate: `specify apm compile`*")
sections.append("")
return "\n".join(sections)

View File

@@ -132,7 +132,7 @@ def generate_agents_md_template(template_data: TemplateData) -> str:
# Footer
sections.append("---")
sections.append("*This file was generated by APM CLI. Do not edit manually.*")
sections.append("*To regenerate: `apm compile`*")
sections.append("*To regenerate: `specify apm compile`*")
sections.append("")
return "\n".join(sections)

View File

@@ -333,8 +333,9 @@ class ScriptRunner:
if '=' in arg and not actual_command_args:
# This looks like an environment variable and we haven't started the actual command yet
key, value = arg.split('=', 1)
# Validate this looks like a valid environment variable name
if key.isidentifier() or key.replace('_', '').isalnum():
# Validate environment variable name with restrictive pattern
# Only allow uppercase letters, numbers, and underscores, starting with letter or underscore
if re.match(r'^[A-Z_][A-Z0-9_]*$', key):
env_vars[key] = value
continue
# Once we hit a non-env-var argument, everything else is part of the command

View File

@@ -153,7 +153,8 @@ class GitHubPackageDownloader:
error_msg += "Please check repository access permissions and authentication setup."
if last_error:
error_msg += f" Last error: {last_error}"
sanitized_error = self._sanitize_git_error(str(last_error))
error_msg += f" Last error: {sanitized_error}"
raise RuntimeError(error_msg)
@@ -197,7 +198,8 @@ class GitHubPackageDownloader:
resolved_commit = commit.hexsha
ref_name = ref
except Exception as e:
raise ValueError(f"Could not resolve commit '{ref}' in repository {dep_ref.repo_url}: {e}")
sanitized_error = self._sanitize_git_error(str(e))
raise ValueError(f"Could not resolve commit '{ref}' in repository {dep_ref.repo_url}: {sanitized_error}")
else:
# For branches and tags, try shallow clone first
try:
@@ -236,7 +238,8 @@ class GitHubPackageDownloader:
raise ValueError(f"Reference '{ref}' not found in repository {dep_ref.repo_url}")
except Exception as e:
raise ValueError(f"Could not resolve reference '{ref}' in repository {dep_ref.repo_url}: {e}")
sanitized_error = self._sanitize_git_error(str(e))
raise ValueError(f"Could not resolve reference '{ref}' in repository {dep_ref.repo_url}: {sanitized_error}")
except GitCommandError as e:
# Check if this might be a private repository access issue
@@ -249,7 +252,8 @@ class GitHubPackageDownloader:
error_msg += "Authentication failed. Please check your GitHub token permissions."
raise RuntimeError(error_msg)
else:
raise RuntimeError(f"Failed to clone repository {dep_ref.repo_url}: {e}")
sanitized_error = self._sanitize_git_error(str(e))
raise RuntimeError(f"Failed to clone repository {dep_ref.repo_url}: {sanitized_error}")
finally:
# Clean up temporary directory
@@ -326,7 +330,8 @@ class GitHubPackageDownloader:
error_msg += "Authentication failed. Please check your GitHub token permissions."
raise RuntimeError(error_msg)
else:
raise RuntimeError(f"Failed to clone repository {dep_ref.repo_url}: {e}")
sanitized_error = self._sanitize_git_error(str(e))
raise RuntimeError(f"Failed to clone repository {dep_ref.repo_url}: {sanitized_error}")
except RuntimeError:
# Re-raise RuntimeError from _clone_with_fallback
raise

View File

@@ -47,7 +47,7 @@ class CompilationFormatter:
lines.append("")
# Phase 2: Optimization Progress
lines.extend(self._format_optimization_progress(results.optimization_decisions))
lines.extend(self._format_optimization_progress(results.optimization_decisions, results.project_analysis))
lines.append("")
# Phase 3: Results Summary
@@ -76,7 +76,7 @@ class CompilationFormatter:
lines.append("")
# Phase 2: Optimization Progress
lines.extend(self._format_optimization_progress(results.optimization_decisions))
lines.extend(self._format_optimization_progress(results.optimization_decisions, results.project_analysis))
lines.append("")
# Phase 3: Mathematical Analysis Section (verbose only)
@@ -163,12 +163,12 @@ class CompilationFormatter:
# Show distribution of AGENTS.md files
for summary in results.placement_summaries:
rel_path = str(summary.get_relative_path(Path.cwd()))
instruction_text = f"{summary.instruction_count} instruction{'s' if summary.instruction_count != 1 else ''}"
content_text = self._get_placement_description(summary)
source_text = f"{summary.source_count} source{'s' if summary.source_count != 1 else ''}"
# Use proper tree formatting
prefix = "├─" if summary != results.placement_summaries[-1] else "└─"
line = f"{prefix} {rel_path:<30} {instruction_text} from {source_text}"
line = f"{prefix} {rel_path:<30} {content_text} from {source_text}"
if self.use_color:
lines.append(self._styled(line, "dim"))
@@ -191,7 +191,7 @@ class CompilationFormatter:
# Standard analysis
lines.extend(self._format_project_discovery(results.project_analysis))
lines.append("")
lines.extend(self._format_optimization_progress(results.optimization_decisions))
lines.extend(self._format_optimization_progress(results.optimization_decisions, results.project_analysis))
lines.append("")
# Dry run specific output
@@ -213,6 +213,14 @@ class CompilationFormatter:
else:
lines.append("Analyzing project structure...")
# Constitution detection (first priority)
if analysis.constitution_detected:
constitution_line = f"├─ Constitution detected: {analysis.constitution_path}"
if self.use_color:
lines.append(self._styled(constitution_line, "dim"))
else:
lines.append(constitution_line)
# Structure tree with more detailed information
file_types_summary = analysis.get_file_types_summary() if hasattr(analysis, 'get_file_types_summary') else "various"
tree_lines = [
@@ -229,7 +237,7 @@ class CompilationFormatter:
return lines
def _format_optimization_progress(self, decisions: List[OptimizationDecision]) -> List[str]:
def _format_optimization_progress(self, decisions: List[OptimizationDecision], analysis=None) -> List[str]:
"""Format optimization progress display using Rich table for better readability."""
lines = []
@@ -247,6 +255,16 @@ class CompilationFormatter:
table.add_column("Placement", style="green", width=25)
table.add_column("Metrics", style="dim", width=20)
# Add constitution row first if detected
if analysis and analysis.constitution_detected:
table.add_row(
"**",
"constitution.md",
"ALL",
"./AGENTS.md",
"rel: 100%"
)
for decision in decisions:
pattern_display = decision.pattern if decision.pattern else "(global)"
@@ -288,6 +306,10 @@ class CompilationFormatter:
lines.extend(table_output.split('\n'))
else:
# Fallback to simplified text display for non-Rich environments
# Add constitution first if detected
if analysis and analysis.constitution_detected:
lines.append("** constitution.md ALL → ./AGENTS.md (rel: 100%)")
for decision in decisions:
pattern_display = decision.pattern if decision.pattern else "(global)"
@@ -376,12 +398,12 @@ class CompilationFormatter:
# Show distribution of AGENTS.md files
for summary in results.placement_summaries:
rel_path = str(summary.get_relative_path(Path.cwd()))
instruction_text = f"{summary.instruction_count} instruction{'s' if summary.instruction_count != 1 else ''}"
content_text = self._get_placement_description(summary)
source_text = f"{summary.source_count} source{'s' if summary.source_count != 1 else ''}"
# Use proper tree formatting
prefix = "├─" if summary != results.placement_summaries[-1] else "└─"
line = f"{prefix} {rel_path:<30} {instruction_text} from {source_text}"
line = f"{prefix} {rel_path:<30} {content_text} from {source_text}"
if self.use_color:
lines.append(self._styled(line, "dim"))
@@ -851,6 +873,32 @@ Pollution Level:
return lines
def _get_placement_description(self, summary) -> str:
"""Get description of what's included in a placement summary.
Args:
summary: PlacementSummary object
Returns:
str: Description like "Constitution and 1 instruction" or "Constitution"
"""
# Check if constitution is included
has_constitution = any("constitution.md" in source for source in summary.sources)
# Build the description based on what's included
parts = []
if has_constitution:
parts.append("Constitution")
if summary.instruction_count > 0:
instruction_text = f"{summary.instruction_count} instruction{'s' if summary.instruction_count != 1 else ''}"
parts.append(instruction_text)
if parts:
return " and ".join(parts)
else:
return "content"
def _styled(self, text: str, style: str) -> str:
"""Apply styling to text with rich fallback."""
if self.use_color and RICH_AVAILABLE:

View File

@@ -23,6 +23,8 @@ class ProjectAnalysis:
file_types_detected: Set[str]
instruction_patterns_detected: int
max_depth: int
constitution_detected: bool = False
constitution_path: Optional[str] = None
def get_file_types_summary(self) -> str:
"""Get a concise summary of detected file types."""

View File

@@ -113,8 +113,8 @@ def run_workflow(workflow_name, params=None, base_dir=None):
params = params or {}
# Extract runtime and model information
runtime_name = params.pop('_runtime', None)
fallback_llm = params.pop('_llm', None)
runtime_name = params.get('_runtime', None)
fallback_llm = params.get('_llm', None)
# Find the workflow
workflow = find_workflow_by_name(workflow_name, base_dir)

View File

@@ -327,7 +327,7 @@ apm_click.add_command(apm_deps, name="deps")
# Create APM subcommands as Typer commands
apm_app = typer.Typer(
name="apm",
help="APM - Agent Package Manager commands. Package Agentic workflows and Agent context as code.",
help="APM - Agent Package Manager commands for context management.",
add_completion=False,
)