Skip to content

feat: add nested instruction strategy with hub + detail files#22

Merged
digitarald merged 2 commits intomainfrom
feat/nested-instruction-strategy
Feb 28, 2026
Merged

feat: add nested instruction strategy with hub + detail files#22
digitarald merged 2 commits intomainfrom
feat/nested-instruction-strategy

Conversation

@digitarald
Copy link
Collaborator

Summary

Adds a nested instruction strategy that generates a lean AGENTS.md hub with deep-dive detail files in a configurable directory (.agents/ by default), plus optional CLAUDE.md transclusion.

Changes

  • Config & TypesInstructionStrategy type, parentArea, detailDir, claudeMd in AgentrcConfig with validation
  • CLI--strategy <mode> and --claude-md flags. CLI > config > default flat
  • Nested Generation — Single CopilotClient, hub extracts topic JSON, per-topic detail sessions, slug sanitization
  • File WritingwriteInstructionFile() with traversal validation, writeNestedInstructions() orchestrator
  • DetectiondetectExistingInstructions() finds .agents/*.md detail files
  • VS Code Extension — Strategy and CLAUDE.md quick picks, nested generation flow
  • Tests — 20+ new tests for parsing, writing, detection, and config validation

Verification

  • Typecheck: clean
  • Lint: clean
  • Tests: 503/503 passed
  • Build: CLI + extension clean

Copilot AI review requested due to automatic review settings February 28, 2026 00:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new nested instruction generation strategy (hub AGENTS.md + per-topic detail files in a configurable directory, plus optional CLAUDE.md transclusion) and wires it through the CLI and VS Code extension, with accompanying config parsing and expanded test coverage.

Changes:

  • Introduces InstructionStrategy (flat | nested) and config support for strategy, detailDir, claudeMd, and parentArea (with validation).
  • Implements nested instruction generation + writing (generateNested*, writeInstructionFile, writeNestedInstructions) and detection of existing nested detail files.
  • Updates CLI/VS Code flows to select strategy and execute nested generation; adds tests for parsing/writing/detection/config.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
vscode-extension/src/services.ts Re-exports new analyzer/config and nested instruction services for the extension.
vscode-extension/src/commands/instructions.ts Adds strategy quick pick + nested generation flow in VS Code.
src/services/instructions.ts Implements nested hub/detail generation, writing helpers, and nested detail detection.
src/services/analyzer.ts Extends config/types to include strategy/detailDir/claudeMd/parentArea with validation.
src/services/tests/instructions.test.ts Adds tests for nested parsing/writing/detection and new writer helper.
src/services/tests/analyzer.test.ts Adds tests for new config fields and validation.
src/commands/instructions.ts Adds --strategy / --claude-md behavior and nested generation/write flow in CLI.
src/commands/generate.ts Plumbs --strategy through primer generate ... to instructions generation.
src/cli.ts Adds CLI flags for --strategy and --claude-md.
Comments suppressed due to low confidence (2)

src/commands/instructions.ts:83

  • Nested generation logs Skipped ... (exists, use --force) for every non-"wrote" action, but writeInstructionFile() can also skip due to symlink or empty content. With the current writeNestedInstructions() return shape, this message will be misleading in those cases. Either propagate a skip reason in the actions or use a more generic skip message here.
          for (const action of actions) {
            const relPath = path.relative(process.cwd(), action.path);
            if (action.action === "wrote") {
              if (shouldLog(options)) progress.succeed(`Wrote ${relPath}`);
            } else if (shouldLog(options)) {
              progress.update(`Skipped ${relPath} (exists, use --force)`);
            }

src/services/instructions.ts:782

  • Same as above: this process.stderr.write warning is emitted from the service regardless of --quiet / UI context. Consider routing warnings through onProgress or returning them so the CLI/VS Code layer controls how (or whether) they are displayed.
      } catch (err) {
        const msg = err instanceof Error ? err.message : String(err);
        process.stderr.write(`Warning: Failed to generate detail for "${topic.title}": ${msg}\n`);
      }

Comment on lines +104 to +116
async function findDetailFiles(repoPath: string, detailDirName: string): Promise<string[]> {
const detailFiles: string[] = [];
const excludeDirs = new Set([".git", "node_modules", "apm_modules", ".apm"]);

async function walk(dir: string, relPath: string): Promise<void> {
const entries = await fs.readdir(dir, { withFileTypes: true }).catch(() => []);
for (const entry of entries) {
if (excludeDirs.has(entry.name)) continue;
if (entry.isSymbolicLink()) continue;
if (entry.isDirectory()) {
if (entry.name === detailDirName) {
// Found a detail directory — list .md files inside
const detailDir = path.join(dir, entry.name);
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

detailDir is treated as a single directory name (entry.name === detailDirName). However, loadAgentrcConfig() allows values containing / (e.g. docs/ai). In that case, existing detail files won’t be detected. Either constrain detailDir to a single path segment during config validation, or update findDetailFiles() to handle multi-segment relative paths.

Copilot uses AI. Check for mistakes.
Comment on lines 1067 to 1072
areas.push({
name: e.name as string,
applyTo,
description: typeof e.description === "string" ? e.description : undefined
description: typeof e.description === "string" ? e.description : undefined,
parentArea: typeof e.parentArea === "string" ? e.parentArea : undefined
});
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

loadAgentrcConfig() now parses parentArea, but the analyzed Area objects used by CLI/extension appear to never receive this field (searching src/services/analyzer.ts shows no other parentArea assignments). As a result, downstream logic like areas.filter((a) => a.parentArea === area.name) and hub parentContext will never work. Ensure detectAreas()/the config-area→Area mapping propagates parentArea onto Area objects.

Copilot uses AI. Check for mistakes.
const progress = options.onProgress ?? (() => {});
const model = options.model ?? DEFAULT_MODEL;

const existingCtx = await detectExistingInstructions(options.repoPath);
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

generateNestedHub() calls detectExistingInstructions(options.repoPath) without passing the configured detailDir. If detailDir is customized (e.g. docs-ai), existing nested detail files won’t be detected and the hub prompt can duplicate content. Pass options.detailDir as the detailDirName argument so detection matches the strategy output directory.

Suggested change
const existingCtx = await detectExistingInstructions(options.repoPath);
const existingCtx = await detectExistingInstructions(options.repoPath, options.detailDir);

Copilot uses AI. Check for mistakes.
@digitarald digitarald marked this pull request as ready for review February 28, 2026 00:42
@digitarald digitarald merged commit df1e5fe into main Feb 28, 2026
9 checks passed
@digitarald digitarald deleted the feat/nested-instruction-strategy branch February 28, 2026 02:14
@github-actions github-actions bot mentioned this pull request Mar 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants