Skip to content

Add instruction recommendation feature via session menu, repo menu, and /instructions command#115

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/add-instruction-recommendations
Draft

Add instruction recommendation feature via session menu, repo menu, and /instructions command#115
Copilot wants to merge 2 commits intomainfrom
copilot/add-instruction-recommendations

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 15, 2026

Adds the ability to ask the AI to analyze a project and recommend improvements to copilot instructions, skills, and agent definitions.

Entry points

  • Session 3-dot menu → "💡 Recommend Instructions"
  • Repo group context menu → "💡 Recommend Instructions"
  • Slash command/instructions

Implementation

  • InstructionRecommendationHelper — Builds a context-aware prompt that includes discovered existing skills/agents so the AI suggests improvements rather than duplicates
  • CopilotService.OnInstructionRecommendationRequested — Event bridge between sidebar and dashboard (sibling components communicate via service events)
  • SessionListItem.razor / SessionSidebar.razor — Menu items wired through EventCallbackCopilotService.RequestInstructionRecommendation()
  • Dashboard.razor — Handles the event, resolves target session, gathers project context (working directory, repo info, existing skills/agents), and sends the prompt

The prompt asks the AI to recommend concrete file contents for .github/copilot-instructions.md, skills (SKILL.md with frontmatter), and agent definitions scoped to the project's structure and patterns.

Tests

9 unit tests covering BuildRecommendationPrompt — parameter combinations, empty/null handling, skill/agent listing with and without descriptions.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…and /instructions slash command

Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com>
Copilot AI changed the title [WIP] Add ability to generate instruction recommendations Add instruction recommendation feature via session menu, repo menu, and /instructions command Feb 15, 2026
Copilot AI requested a review from PureWeen February 15, 2026 05:55
@PureWeen
Copy link
Copy Markdown
Owner

PureWeen commented Apr 3, 2026

🔍 R1 Review — PR #115 (Add instruction recommendation feature)

Reviewer: PP PR REVIEWER-worker-5 (3-model consensus)
Models: Claude Opus 4.6 · Claude Sonnet 4.6 · GPT-5.3-Codex
CI: No checks configured
Prior reviews: None


Consensus Findings

# Finding Severity Models
1 Duplicate /instructions message in chat history 🔴 CRITICAL 2/3 (Opus + Codex)
2 Synchronous disk I/O on Blazor UI thread 🟡 MODERATE 3/3 (all)
3 Fire-and-forget SendPromptAsync without error handling 🟡 MODERATE 3/3 (all)
4 Repo menu may target unrelated session 🟡 MODERATE 2/3 (Sonnet + Codex)

Finding Details

1. 🔴 CRITICAL — Duplicate /instructions message in chat history

File: Dashboard.razorHandleSlashCommand + SendInstructionRecommendation

HandleSlashCommand adds the user message for ALL slash commands before the switch statement:

session.History.Add(ChatMessage.UserMessage(input));  // runs for all commands

Then case "instructions" calls SendInstructionRecommendation, which adds another:

session.History.Add(ChatMessage.UserMessage("/instructions"));

Result: duplicate /instructions entry in chat history via the slash command path. Additionally, SendPromptAsync may also add a user message internally, potentially creating a triple entry.

Furthermore, if the session is already processing (IsProcessing), the history entry is added but SendPromptAsync may fail silently — leaving an orphaned ghost message with no response.

Fix: Remove the session.History.Add from SendInstructionRecommendation, or add a bool addToHistory = true parameter and pass false from HandleSlashCommand. Also add an IsProcessing guard at the top.


2. 🟡 MODERATE — Synchronous disk I/O on UI thread

File: Dashboard.razorSendInstructionRecommendation, lines calling DiscoverAvailableSkills/DiscoverAvailableAgents

Both methods perform blocking Directory.GetDirectories + File.ReadAllText (scanning .claude/skills/, .copilot/skills/, .github/skills/, ~/.copilot/installed-plugins/*/skills/, etc.). This runs on the Blazor UI synchronization context, freezing rendering during the scan.

Fix: Wrap in await Task.Run(() => { ... }):

var (skills, agents) = await Task.Run(() => (
    CopilotService.DiscoverAvailableSkills(workingDirectory)
        .Select(s => (s.Name, s.Description)).ToList(),
    CopilotService.DiscoverAvailableAgents(workingDirectory)
        .Select(a => (a.Name, a.Description)).ToList()
));

3. 🟡 MODERATE — Fire-and-forget without error handling

File: Dashboard.razorSendInstructionRecommendation

_ = CopilotService.SendPromptAsync(targetSession, prompt);

Every other significant send site in Dashboard.razor attaches a .ContinueWith() fault handler to surface errors. This bare _ = silently swallows exceptions. If the session is disposed, reconnecting, or in an error state, the user gets no feedback.

Fix: Add .ContinueWith() error handling consistent with the primary send path.


4. 🟡 MODERATE — Repo menu targets unrelated session

File: Dashboard.razorSendInstructionRecommendation (repo menu path)

When triggered from the repo context menu (sessionName is null, only repoId provided), the code falls back to expandedSession or sessions[0] — neither guaranteed to be associated with the given repo. The skill/agent discovery uses the repo's worktree path, but the prompt is sent to a session that may be working in a completely different repository. The AI gets contradictory context.

Additionally, RepoManager.Worktrees.FirstOrDefault(w => w.RepoId == repoId) is non-deterministic when multiple worktrees exist for the same repo.

Fix: Use WorktreeInfo.SessionName to find the session associated with this repo's worktree, or create a new session scoped to the repo.


Non-consensus observations (1/3, informational)

  • Sonnet flagged prompt injection via skill/agent names — malicious SKILL.md descriptions could inject into the LLM prompt. Low practical risk since the prompt targets the same AI that already has access to those files, but worth noting.
  • Sonnet flagged sessions[0] fallback as surprising UX (silently hijacks session focus)
  • Opus noted redundant TOCTOU double-scan in session resolution (Any then FirstOrDefault)

What's clean ✅

  • Event subscribe/unsubscribe correctly paired in OnInitializedAsync and DisposeAsync
  • Thread marshaling via InvokeAsync in HandleInstructionRecommendation is correct
  • InstructionRecommendationHelper is well-structured static class with good test coverage (9 tests)
  • RepoManager injection is valid (already registered as a service)
  • Compile Include link correctly added to test csproj

Test coverage assessment

The 9 InstructionRecommendationHelperTests cover the prompt builder well. Missing: tests for the duplicate-message bug, session resolution edge cases, IsProcessing guard, and error paths in SendInstructionRecommendation.


Recommended Action: ⚠️ Request Changes

  1. Fix duplicate history entry (Critical) — the /instructions slash command path double-adds the user message
  2. Offload disk I/O to Task.Run for DiscoverAvailableSkills/DiscoverAvailableAgents
  3. Add error handling to the fire-and-forget SendPromptAsync call
  4. Fix repo-menu session targeting to prefer sessions associated with the repo's worktree

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