Support annotate command and add SKILL.md by hejingkan2005 · Pull Request #122 · MicrosoftDocs/mcp

hejingkan2005 · 2026-03-12T08:28:56Z

No description provided.

Copilot

Pull request overview

This PR adds a new mslearn annotate command to persist local notes per Microsoft Learn URL, introduces a filesystem-backed annotation store, and includes a new SKILL.md intended to guide usage.

Changes:

Added annotate CLI command with support for saving, reading, clearing, and listing URL-based annotations.
Implemented a file-based annotation store persisted under the user’s data directory.
Added unit tests covering both the CLI command behavior and the annotation store, plus a new SKILL.md for the workflow.

Reviewed changes

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

Show a summary per file

File	Description
cli/src/commands/annotate.ts	Adds the `annotate` command behavior (save/read/clear/list).
cli/src/utils/annotations.ts	Introduces the on-disk annotation store implementation and default storage location.
cli/src/context.ts	Extends CLI context to provide an annotation-store factory.
cli/src/index.ts	Registers the new `annotate` command with the CLI program.
cli/test/unit/cli.test.ts	Adds CLI-level tests for `annotate` and wires a temp annotation store into the test context.
cli/test/unit/annotations.test.ts	Adds unit tests for the annotation store implementation.
cli/skills/get-mslearn-docs/SKILL.md	Documents a workflow using `mslearn` commands, including `annotate`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-12T08:32:37Z

cli/src/utils/annotations.ts

+    } catch {
+      return [];


list() returns an empty array on any error reading the directory, which can cause mslearn annotate --list to print "No annotations." even when annotations exist but the directory is unreadable/corrupt. Similar to clear(), consider treating ENOENT as "no annotations" but surfacing other I/O errors so users can diagnose permission/path issues.

Suggested change

} catch {

return [];

} catch (error: unknown) {

const err = error as NodeJS.ErrnoException;

if (err && err.code === 'ENOENT') {

// Treat missing annotations directory as "no annotations".

return [];

}

// Surface other I/O errors so callers can diagnose issues.

throw error;

Copilot · 2026-03-12T08:32:37Z

cli/test/unit/cli.test.ts

  const stdout: string[] = [];
  const stderr: string[] = [];
+  const annotationsDir = mkdtempSync(join(tmpdir(), 'mslearn-test-annotations-'));



The test creates a temporary annotations directory via mkdtempSync but never removes it. Over time (especially when running tests locally) this can clutter the system temp directory. Consider adding an afterEach/afterAll cleanup (e.g., rmSync(dir, { recursive: true, force: true })).

Copilot · 2026-03-12T08:32:38Z

cli/test/unit/annotations.test.ts

+import { mkdtempSync, readFileSync, readdirSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+
+import { describe, expect, it } from 'vitest';
+
+import { createFileAnnotationStore } from '../../src/utils/annotations.js';
+
+function createTempStore(now?: () => number) {
+  const dir = mkdtempSync(join(tmpdir(), 'mslearn-annotations-'));
+  return {
+    store: createFileAnnotationStore({ annotationsDir: dir, now }),
+    dir,
+  };
+}
+


These tests create temp directories via mkdtempSync but never remove them, which can clutter the OS temp folder across repeated local test runs. Consider tracking created dirs and cleaning them up in afterEach/afterAll (e.g., rmSync(dir, { recursive: true, force: true })).

Suggested change

import { mkdtempSync, readFileSync, readdirSync } from 'node:fs';

import { tmpdir } from 'node:os';

import { join } from 'node:path';

import { describe, expect, it } from 'vitest';

import { createFileAnnotationStore } from '../../src/utils/annotations.js';

function createTempStore(now?: () => number) {

const dir = mkdtempSync(join(tmpdir(), 'mslearn-annotations-'));

return {

store: createFileAnnotationStore({ annotationsDir: dir, now }),

dir,

};

}

import { mkdtempSync, readFileSync, readdirSync, rmSync } from 'node:fs';

import { tmpdir } from 'node:os';

import { join } from 'node:path';

import { afterEach, describe, expect, it } from 'vitest';

import { createFileAnnotationStore } from '../../src/utils/annotations.js';

const tempDirs: string[] = [];

function createTempStore(now?: () => number) {

const dir = mkdtempSync(join(tmpdir(), 'mslearn-annotations-'));

tempDirs.push(dir);

return {

store: createFileAnnotationStore({ annotationsDir: dir, now }),

dir,

};

}

afterEach(() => {

for (const dir of tempDirs) {

rmSync(dir, { recursive: true, force: true });

}

tempDirs.length = 0;

});

Copilot · 2026-03-12T08:32:38Z

cli/skills/get-mslearn-docs/SKILL.md

+  and you need current reference material. Fetch the docs with mslearn before
+  answering, rather than relying on training knowledge.
+---
+


This SKILL.md lives under cli/skills/..., but the repo validation script (and existing convention) expects skills under the root skills/<skill>/SKILL.md. As-is, this skill likely won’t be discovered/validated by tooling. Consider moving it to skills/get-mslearn-docs/SKILL.md (or updating the validation/discovery logic if cli/skills is intentional).

Suggested change

> NOTE: For repository tooling and validation to recognize this skill, the

> canonical location for this file is `skills/get-mslearn-docs/SKILL.md`.

> Move or copy this content there and update any references accordingly.

Copilot · 2026-03-12T08:32:38Z

cli/skills/get-mslearn-docs/SKILL.md

+Review the search results to find the most relevant document `contentUrl`. If nothing matches, try a broader term.
+
+Pick the best-matching `id` from the results (e.g. `openai/chat`, `anthropic/sdk`,
+`stripe/api`). If nothing matches, try a broader term.
+


Step 1 says to review results for contentUrl, but then it instructs the reader to pick an id like openai/chat / stripe/api, which appears unrelated to this CLI and inconsistent with the mslearn search payload shape. Consider removing the id guidance and focusing on selecting the appropriate contentUrl from the search results.

Suggested change

Review the search results to find the most relevant document `contentUrl`. If nothing matches, try a broader term.

Pick the best-matching `id` from the results (e.g. `openai/chat`, `anthropic/sdk`,

`stripe/api`). If nothing matches, try a broader term.

Review the search results to find the most relevant document. Use its `contentUrl` in the next step. If nothing matches, try a broader or alternative search term.

Copilot · 2026-03-12T08:32:39Z

cli/skills/get-mslearn-docs/SKILL.md

+| Goal | Command |
+
+|------|---------|
+
+| Search docs | `mslearn search "<microsoft product or topic>"` |
+
+| Fetch a doc | `mslearn fetch "<contentUrl>"` |


The “Quick reference” table is not valid Markdown table syntax (it uses || prefixes and has extra blank | lines), so it likely won’t render as a table. Consider rewriting it using standard Markdown table formatting (| Goal | Command | header + separator row + data rows).

Copilot · 2026-03-12T08:32:39Z

cli/src/utils/annotations.ts

+function urlToFilename(url: string): string {
+  return url.replace(/[^a-zA-Z0-9.-]/g, '_') + '.json';
+}


urlToFilename() derives the filename by replacing non-alphanumerics with _, which can cause collisions (different URLs mapping to the same filename) and can exceed filesystem filename limits for long URLs/query strings. This can lead to overwriting the wrong annotation or write failures. Consider using a stable hash of the normalized URL (optionally with a short readable prefix) as the filename instead of (or in addition to) a fully-sanitized URL string.

Copilot · 2026-03-12T08:32:39Z

cli/src/utils/annotations.ts

+    } catch {
+      return false;


clear() returns false for any error (including permission errors, EISDIR, etc.), which will make the CLI report "No annotation found" even when the file exists but couldn't be removed. Consider only returning false on ENOENT and surfacing other filesystem errors (e.g., by throwing an OperationError or letting the exception propagate) so the user isn't misled.

Suggested change

} catch {

return false;

} catch (err) {

const error = err as NodeJS.ErrnoException;

if (error && error.code === 'ENOENT') {

// File does not exist: report as "not cleared" without treating as an error.

return false;

}

// Surface other filesystem errors so callers aren't misled.

throw err;

TianqiZhang · 2026-03-12T08:50:49Z

cli/skills/get-mslearn-docs/SKILL.md

@@ -0,0 +1,185 @@
+---
+name: get-mslearn-docs


we should update our existing skills instead of adding new skill

TianqiZhang · 2026-03-12T09:18:23Z

cli/src/utils/annotations.ts

+  return new FileAnnotationStore(options);
+}
+
+function urlToFilename(url: string): string {


URL-to-filename collisions — urlToFilename replaces all non-alphanumeric chars (except . and -) with _, so distinct URLs can silently map to the same file. For example, https://learn.microsoft.com/a-b and https://learn.microsoft.com/a_b both produce https___learn.microsoft.com_a_b.json. This causes one annotation to overwrite another with no warning. Consider using a hash-based key (e.g. crypto.createHash('sha256').update(url).digest('hex')) to avoid collisions.

TianqiZhang · 2026-03-12T09:18:23Z

cli/src/utils/annotations.ts

+  return new FileAnnotationStore(options);
+}
+
+function urlToFilename(url: string): string {


Filename length limits — Long Microsoft Learn URLs can easily produce filenames exceeding the 255-character OS limit (Windows and most Linux filesystems), which would throw an ENAMETOOLONG error. Hashing the URL (as suggested above) would also solve this since the output length is fixed.

TianqiZhang · 2026-03-12T09:18:23Z

cli/test/unit/annotations.test.ts

+function createTempStore(now?: () => number) {
+  const dir = mkdtempSync(join(tmpdir(), 'mslearn-annotations-'));
+  return {
+    store: createFileAnnotationStore({ annotationsDir: dir, now }),


Temp directories never cleaned up — mkdtempSync creates a new directory per test helper call, but none are removed after the test suite finishes. These accumulate in the OS temp dir over repeated test runs. Consider adding an afterAll or afterEach hook with rmSync(dir, { recursive: true }) to clean up.

- add SKILL.md and annotate command

c7dadc3

Copilot AI review requested due to automatic review settings March 12, 2026 08:28

Copilot started reviewing on behalf of hejingkan2005 March 12, 2026 08:29 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

TianqiZhang reviewed Mar 12, 2026

View reviewed changes

-    } catch {
-      return [];
+    } catch (error: unknown) {
+      const err = error as NodeJS.ErrnoException;
+      if (err && err.code === 'ENOENT') {
+        // Treat missing annotations directory as "no annotations".
+        return [];
+      }
+      // Surface other I/O errors so callers can diagnose issues.
+      throw error;

+> NOTE: For repository tooling and validation to recognize this skill, the
+> canonical location for this file is `skills/get-mslearn-docs/SKILL.md`.
+> Move or copy this content there and update any references accordingly.

-    } catch {
-      return false;
+    } catch (err) {
+      const error = err as NodeJS.ErrnoException;
+      if (error && error.code === 'ENOENT') {
+        // File does not exist: report as "not cleared" without treating as an error.
+        return false;
+      }
+      // Surface other filesystem errors so callers aren't misled.
+      throw err;

Conversation

hejingkan2005 commented Mar 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

TianqiZhang Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

TianqiZhang Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

TianqiZhang Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

TianqiZhang Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants