feat(copilot): add Nowledge Mem plugin for Copilot CLI#201
Open
HamsteRider-m wants to merge 2 commits intonowledge-co:mainfrom
Open
feat(copilot): add Nowledge Mem plugin for Copilot CLI#201HamsteRider-m wants to merge 2 commits intonowledge-co:mainfrom
HamsteRider-m wants to merge 2 commits intonowledge-co:mainfrom
Conversation
Full-featured plugin with auto-capture, working memory, search, and distill. - Plugin manifest (.claude-plugin/plugin.json) v0.1.0 - 4 lifecycle hooks (SessionStart×2, UserPromptSubmit, Stop) - 4 skills (read-working-memory, search-memory, distill-memory, save-thread) - 4 commands (/save, /search, /sum, /status) - Session capture script (copilot-stop-save.py) with: - Incremental append via nmem t import + nmem t append fallback - Secret filtering (6 patterns) and sensitive content detection - Auto-distill with cooldown, content hash dedup, min thresholds - Cross-platform locking (fcntl/msvcrt) - Incomplete turn detection (background tasks, ask_user, questions) - Idempotent installer (install-hooks.sh) - Registry entry in integrations.json (plugin-capture transport) - Full test suite (14 tests covering all capture script logic) - Documentation: README, CHANGELOG, RELEASING, AGENTS.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 27 minutes and 3 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR introduces a new Copilot CLI plugin integration for Nowledge Mem, adding plugin registration, lifecycle hook handlers, CLI command documentation, Python-based session transcript capture with secret redaction, and comprehensive test coverage to enable persistent knowledge management within Copilot CLI sessions. Changes
Sequence DiagramsequenceDiagram
participant COP as Copilot CLI
participant HOOK as Hook System
participant PYSC as Python Capture Script
participant NMEM as nmem CLI
participant FS as Filesystem
COP->>HOOK: SessionStart (startup/resume)
HOOK->>NMEM: nmem --json wm read
NMEM->>HOOK: Return working memory JSON
HOOK->>COP: Inject working memory context
COP->>HOOK: UserPromptSubmit (per-turn)
HOOK->>NMEM: nmem --json m search "query"
NMEM->>HOOK: Return memory results
HOOK->>COP: Inject search suggestion
COP->>HOOK: Stop (end_turn)
HOOK->>PYSC: exec(copilot-stop-save.py) via stdin
PYSC->>FS: Read/write state lock at ~/.copilot/nowledge-mem-hooks/state/
PYSC->>PYSC: Parse transcript, redact secrets, extract turn
PYSC->>NMEM: nmem t import (create thread)
alt Thread exists
PYSC->>NMEM: nmem t append (add turn)
end
NMEM->>FS: Store thread in Nowledge Mem
PYSC->>NMEM: nmem t triage + distill (auto-extract insights)
NMEM->>FS: Store distilled memories
PYSC->>FS: Log action to hook-log.jsonl
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eee5555b54
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return [json.loads(line) for line in fh] | ||
|
|
||
|
|
||
| def find_index(events: list[dict], event_id: str | None) -> int | None: |
There was a problem hiding this comment.
This script uses 3.10-only type syntax (str | None / -> int | None), but the installer only checks for python3 and does not require 3.10+, so users on Python 3.8/3.9 will hit a SyntaxError before the Stop hook can run. In those environments, session capture silently never works (the hook command ends with || true). Either enforce a minimum Python version during install or replace these annotations with older-compatible typing syntax.
Useful? React with 👍 / 👎.
Comment on lines
+574
to
+577
| triage = run_json( | ||
| build_nmem_command( | ||
| nmem_bin, "--json", "t", "triage", thread_id | ||
| ) |
There was a problem hiding this comment.
A failure in nmem t triage (for example, older nmem versions without this subcommand or transient backend errors) raises out of main() before last_saved_turn_end_id is updated, because state is saved only later. After that, each Stop hook reprocesses the same turn and keeps trying to import/append it again, causing repeated duplicate appends and noisy errors. Triage/distill should be best-effort and must not block state advancement once thread import/append succeeded.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (10)
nowledge-mem-copilot-cli-plugin/skills/save-thread/SKILL.md (1)
10-12: Include/savein explicit trigger examples.Please add
/saveto the explicit “when to save” trigger list so the skill aligns with command-driven usage.As per coding guidelines: Use slash commands (
/save,/search <query>,/sum,/status) to manage Nowledge Mem integration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-copilot-cli-plugin/skills/save-thread/SKILL.md` around lines 10 - 12, Update the explicit trigger examples in SKILL.md's "Only when user explicitly says:" list to include the slash command `/save` alongside the existing phrases ("Save this session", "Checkpoint this", "Record conversation") so the skill supports command-driven usage; edit the trigger list in SKILL.md to add `/save` and ensure consistency with other command examples (`/search <query>`, `/sum`, `/status`) mentioned in the guidelines.nowledge-mem-copilot-cli-plugin/commands/status.md (1)
9-13: Prefer/statusas the command shown to users.Please make
/statusthe top-level invocation in this page, and keepnmem statusas implementation/troubleshooting detail.As per coding guidelines: Use slash commands (
/save,/search <query>,/sum,/status) to manage Nowledge Mem integration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-copilot-cli-plugin/commands/status.md` around lines 9 - 13, Update the example in status.md to present "/status" as the primary invocation and move "nmem status" into a secondary implementation/troubleshooting note; specifically replace the top-level code block showing "nmem status" with "/status" and add a small following paragraph or code block that notes the equivalent CLI command "nmem status" for debugging or local runs so both usages appear but "/status" is shown as the canonical user-facing command.nowledge-mem-copilot-cli-plugin/commands/search.md (1)
6-14: Add/search <query>as the primary command example.The current section is technically correct, but this command page should lead with the slash command and then optionally show the underlying
nmem --json ...call.As per coding guidelines: Use slash commands (
/save,/search <query>,/sum,/status) to manage Nowledge Mem integration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-copilot-cli-plugin/commands/search.md` around lines 6 - 14, The documentation should lead with the slash command example; update the Search Memory section to present "/search <query>" as the primary command example (e.g., show a code block with "/search <query>") and then optionally include the underlying CLI invocation "nmem --json m search \"$ARGUMENTS\"" afterwards; ensure you keep the existing heading "Search Memory" and the explanatory sentence, and follow the guideline to use slash commands (`/save`, `/search <query>`, `/sum`, `/status`) as the primary examples.nowledge-mem-copilot-cli-plugin/commands/sum.md (1)
7-30: Make/sumthe primary invocation in this command page.This page currently teaches direct
nmemusage first; for consistency with plugin UX, show/sumas the primary action and keep raw CLI as implementation detail/reference.As per coding guidelines: Use slash commands (
/save,/search <query>,/sum,/status) to manage Nowledge Mem integration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-copilot-cli-plugin/commands/sum.md` around lines 7 - 30, The page should present the slash command /sum as the primary invocation and move the raw nmem CLI example (nmem m add ...) to a secondary "Implementation/CLI reference" section; update the introduction and the top-level example to show "/sum" (and related slash commands /save, /search <query>, /status) as the recommended workflow, then retain the existing nmem CLI snippet only as an optional reference and label it accordingly; modify headings and order in sum.md and adjust any examples or usage steps that currently teach direct nmem usage first so they now demonstrate slash-command usage first.nowledge-mem-copilot-cli-plugin/commands/save.md (1)
9-17: Show/savefirst in the Usage section.Since this is the
/savecommand doc, lead with slash-command usage and keepnmem t create ...as the underlying execution detail.As per coding guidelines: Use slash commands (
/save,/search <query>,/sum,/status) to manage Nowledge Mem integration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-copilot-cli-plugin/commands/save.md` around lines 9 - 17, Reorder the Usage section to lead with the slash command `/save` (showing the exact slash-command usage first) and then present the `nmem t create "..." --title "..." -s copilot-cli` example as the underlying execution detail; update the text to state that `/save` triggers an immediate save and that `nmem t create` is the equivalent lower-level invocation, and ensure the slash-command conforms to the established style used elsewhere (`/save`, `/search <query>`, `/sum`, `/status`) so readers see the slash command first and the `nmem t create` form as supplemental.nowledge-mem-copilot-cli-plugin/tests/test_copilot_plugin.py (3)
324-345: Test isolation: mutating module-levelSTATE_DIRcan cause flaky tests.Directly assigning
copilot_stop_save.STATE_DIR = Path(tmpdir)mutates global state that persists across tests. If test ordering changes or tests run in parallel, this can cause failures. Also, the unpackedpathandpath2variables are unused (per static analysis).♻️ Proposed fix using `patch` and underscore prefix
class TestStateManagement: def test_load_state_new_session(self): with tempfile.TemporaryDirectory() as tmpdir: - copilot_stop_save.STATE_DIR = Path(tmpdir) - path, state = copilot_stop_save.load_state("new-session") + with patch.object(copilot_stop_save, "STATE_DIR", Path(tmpdir)): + _path, state = copilot_stop_save.load_state("new-session") assert state["active_start_event_id"] is None assert state["last_saved_turn_end_id"] is None assert state["last_distill_ts"] == 0 assert state["last_content_hash"] is None def test_save_and_load_state(self): with tempfile.TemporaryDirectory() as tmpdir: - copilot_stop_save.STATE_DIR = Path(tmpdir) - path, state = copilot_stop_save.load_state("test-session") - state["last_saved_turn_end_id"] = "e4" - state["last_distill_ts"] = 1000 - state["last_content_hash"] = "abc123" - copilot_stop_save.save_state(path, state) - - path2, state2 = copilot_stop_save.load_state("test-session") - assert state2["last_saved_turn_end_id"] == "e4" - assert state2["last_distill_ts"] == 1000 - assert state2["last_content_hash"] == "abc123" + with patch.object(copilot_stop_save, "STATE_DIR", Path(tmpdir)): + path, state = copilot_stop_save.load_state("test-session") + state["last_saved_turn_end_id"] = "e4" + state["last_distill_ts"] = 1000 + state["last_content_hash"] = "abc123" + copilot_stop_save.save_state(path, state) + + _path2, state2 = copilot_stop_save.load_state("test-session") + assert state2["last_saved_turn_end_id"] == "e4" + assert state2["last_distill_ts"] == 1000 + assert state2["last_content_hash"] == "abc123"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-copilot-cli-plugin/tests/test_copilot_plugin.py` around lines 324 - 345, The tests mutate the module-level copilot_stop_save.STATE_DIR directly which can leak between tests; update both test_load_state_new_session and test_save_and_load_state to patch STATE_DIR only for the test scope (use unittest.mock.patch or pytest monkeypatch to set copilot_stop_save.STATE_DIR = Path(tmpdir) inside the context) and avoid assigning unused variables by renaming path and path2 to _path/_path2 or not capturing them when calling load_state; keep calls to load_state and save_state unchanged.
383-392: Test leaves temporary file behind on assertion failure.If an assertion fails before
os.unlink(f.name), the temp file remains. Using a context manager withdelete_on_close=False(Python 3.12+) or atry/finallyblock ensures cleanup:♻️ Proposed fix for reliable cleanup
def test_load_events_from_jsonl(self): events = basic_transcript() - with tempfile.NamedTemporaryFile("w", suffix=".jsonl", delete=False) as f: - for event in events: - f.write(json.dumps(event) + "\n") - f.flush() - loaded = copilot_stop_save.load_events(f.name) - assert len(loaded) == len(events) - assert loaded[0]["type"] == "user.message" - os.unlink(f.name) + with tempfile.TemporaryDirectory() as tmpdir: + fpath = Path(tmpdir) / "transcript.jsonl" + with fpath.open("w") as f: + for event in events: + f.write(json.dumps(event) + "\n") + loaded = copilot_stop_save.load_events(str(fpath)) + assert len(loaded) == len(events) + assert loaded[0]["type"] == "user.message"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-copilot-cli-plugin/tests/test_copilot_plugin.py` around lines 383 - 392, The test test_load_events_from_jsonl leaves the temp file behind if an assertion fails; wrap the body that writes, calls copilot_stop_save.load_events and asserts in a try/finally so os.unlink(f.name) is always called (or use a context manager that ensures deletion), locating the tempfile usage in test_load_events_from_jsonl and the call to copilot_stop_save.load_events to add the try/finally cleanup around those operations.
151-174: Secret redaction tests use weak assertions.The assertions like
assert "ghp_" not in result or "[REDACTED]" in resultpass even if the token is partially present. A stricter check ensures the secret is fully redacted:♻️ Proposed stricter assertions
def test_redacts_github_token(self): text = "Use token ghp_1234567890abcdefghijklmn" result = copilot_stop_save.redact(text) - assert "ghp_" not in result or "[REDACTED]" in result + assert "ghp_1234567890abcdefghijklmn" not in result + assert "[REDACTED]" in result def test_redacts_github_pat(self): text = "Token: github_pat_1234567890abcdefghijklmn" result = copilot_stop_save.redact(text) - assert "github_pat_" not in result or "[REDACTED]" in result + assert "github_pat_1234567890abcdefghijklmn" not in result + assert "[REDACTED]" in result def test_redacts_openai_key(self): text = "API key is sk-1234567890abcdefgh" result = copilot_stop_save.redact(text) - assert "sk-" not in result or "[REDACTED]" in result + assert "sk-1234567890abcdefgh" not in result + assert "[REDACTED]" in result🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-copilot-cli-plugin/tests/test_copilot_plugin.py` around lines 151 - 174, The tests (test_redacts_github_token, test_redacts_github_pat, test_redacts_openai_key, test_redacts_bearer_token) use weak OR assertions that allow partial secrets to remain; update each to assert the secret prefix is absent AND that "[REDACTED]" appears (e.g. replace `assert "ghp_" not in result or "[REDACTED]" in result` with `assert "ghp_" not in result and "[REDACTED]" in result`), and do the same for "github_pat_", "sk-" and the bearer case when calling copilot_stop_save.redact to ensure full redaction while keeping test_preserves_normal_text unchanged.nowledge-mem-copilot-cli-plugin/README.md (1)
32-37: WSL shim may break on arguments containing spaces or special characters.The quoting approach
q="$q \"$a\""doesn't handle arguments with embedded quotes, backslashes, or other special characters. Consider usingprintf '%q 'for proper shell escaping:♻️ Proposed fix for robust argument handling
mkdir -p ~/.local/bin && cat > ~/.local/bin/nmem << 'SHIMEOF' #!/bin/bash -q=""; for a in "$@"; do q="$q \"$a\""; done -cmd.exe /s /c "\"nmem.cmd\"$q" +args=() +for a in "$@"; do args+=("$(printf '%q' "$a")"); done +cmd.exe /s /c "nmem.cmd ${args[*]}" SHIMEOF chmod +x ~/.local/bin/nmem🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-copilot-cli-plugin/README.md` around lines 32 - 37, The WSL shim in the nmem script builds q with q="$q \"$a\"" which fails on embedded quotes/backslashes; update the argument-quoting loop in the nmem shim to use a robust shell-escaping approach (e.g., build q with printf '%q' for each "$a" or otherwise use an array and proper quoting) so arguments containing spaces/special chars are escaped correctly; look for the q variable and the for a in "$@" loop in the nmem shim and replace the naive concatenation with printf '%q' "$a" (or an equivalent safe-escaping strategy) before invoking cmd.exe.nowledge-mem-copilot-cli-plugin/scripts/copilot-stop-save.py (1)
314-321: File locking on Windows locks only 1 byte of an empty file.
msvcrt.locking(lock_fh.fileno(), msvcrt.LK_LOCK, 1)locks 1 byte, but the file is empty (just opened with"w"). While this works as a mutex (any concurrent lock attempt will block), the lock is never released explicitly—relying on file handle close. Also, if the file grows beyond 1 byte from another process writing, the lock scope is ambiguous.Consider writing a sentinel byte before locking, or document this is intentional mutex-style locking:
♻️ Proposed improvement
with lock_path.open("w", encoding="utf-8") as lock_fh: + lock_fh.write("L") # Sentinel byte for msvcrt locking + lock_fh.flush() if fcntl: fcntl.flock(lock_fh, fcntl.LOCK_EX) else: import msvcrt msvcrt.locking(lock_fh.fileno(), msvcrt.LK_LOCK, 1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-copilot-cli-plugin/scripts/copilot-stop-save.py` around lines 314 - 321, The Windows branch currently calls msvcrt.locking(lock_fh.fileno(), msvcrt.LK_LOCK, 1) on an empty file which leads to ambiguous locking; change the Windows path (around lock_path, lock_fh, session_id, and the fcntl/ mscrt branch) to first write a sentinel byte (e.g., b"\0") to the opened file, flush() and seek(0) so the file actually contains that byte, then call msvcrt.locking(..., msvcrt.LK_LOCK, 1) to lock that byte; keep the existing behavior for fcntl, and rely on closing lock_fh to release the lock as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nowledge-mem-copilot-cli-plugin/CHANGELOG.md`:
- Line 8: The release heading "## [0.1.0] - 2026-07-14" uses a future date;
update that heading in CHANGELOG.md (the "## [0.1.0] - 2026-07-14" line) to
either the actual release date (e.g., "2026-04-16") or replace the date with
"Unreleased" until the real release date is known, ensuring the header stays
exactly "## [0.1.0] - <date or Unreleased>".
In `@nowledge-mem-copilot-cli-plugin/RELEASING.md`:
- Around line 5-7: Reorder the three release steps so the `integrations.json`
update happens before the `.claude-plugin/plugin.json` update: first bump the
`copilot-cli` version in integrations.json (the canonical source), then update
the `version` field in `.claude-plugin/plugin.json`, and finally add the new
section to `CHANGELOG.md`; ensure the RELEASING.md steps reflect that exact
sequence and phrasing.
In `@nowledge-mem-copilot-cli-plugin/scripts/copilot-stop-save.py`:
- Around line 240-254: The load_state function (and any code that builds
lock_path or other filesystem paths from session_id) currently embeds raw
session_id into STATE_DIR / f"{session_id}.json" which allows path traversal;
add a sanitization/normalization step (e.g., a sanitize_session_id(session_id)
helper called from main() and before any use in load_state or when creating
lock_path) that either (a) validates allowed characters and rejects or
canonicalizes input (strip path separators, dots, and directory components), or
(b) replaces the session_id with a safe deterministic token (e.g., hex of
sha256(session_id) or a UUID derived from it) and use that token for filenames;
update load_state, any lock_path construction, and callers in main() to use the
sanitized token instead of the raw session_id.
- Around line 125-134: The redact function currently discards group 1 prefixes
by replacing the entire match when pattern.groups == 1; update the logic in
redact (which loops over SECRET_PATTERNS and checks pattern.groups) so that for
pattern.groups == 1 you preserve the first capture and append “[REDACTED]”
(i.e., perform the same substitution used for 2+ groups such as using the
"\1[REDACTED]" replacement), keep 0-group behavior unchanged, and ensure
SECRET_PATTERNS and pattern.groups are referenced as-is to locate the change.
In `@nowledge-mem-copilot-cli-plugin/skills/read-working-memory/SKILL.md`:
- Around line 34-36: Replace the plain-shell example command "nmem wm read" with
the JSON-mode command "nmem --json wm read" in SKILL.md (update the code block
that contains the command), and scan the same MD file for any other occurrences
of "nmem wm read" to change them to the "--json" variant so recall examples
follow the structured parsing convention.
In `@nowledge-mem-copilot-cli-plugin/skills/save-thread/SKILL.md`:
- Around line 33-37: Update the fenced code block that contains the "✓ Thread
saved" message so it declares a language identifier (e.g., change the opening
``` to ```text) to satisfy markdown linting; locate the fenced block that wraps
"✓ Thread saved / Summary: {summary} / Thread ID: {thread_id from nmem output}"
in SKILL.md and add the language token immediately after the opening backticks.
---
Nitpick comments:
In `@nowledge-mem-copilot-cli-plugin/commands/save.md`:
- Around line 9-17: Reorder the Usage section to lead with the slash command
`/save` (showing the exact slash-command usage first) and then present the `nmem
t create "..." --title "..." -s copilot-cli` example as the underlying execution
detail; update the text to state that `/save` triggers an immediate save and
that `nmem t create` is the equivalent lower-level invocation, and ensure the
slash-command conforms to the established style used elsewhere (`/save`,
`/search <query>`, `/sum`, `/status`) so readers see the slash command first and
the `nmem t create` form as supplemental.
In `@nowledge-mem-copilot-cli-plugin/commands/search.md`:
- Around line 6-14: The documentation should lead with the slash command
example; update the Search Memory section to present "/search <query>" as the
primary command example (e.g., show a code block with "/search <query>") and
then optionally include the underlying CLI invocation "nmem --json m search
\"$ARGUMENTS\"" afterwards; ensure you keep the existing heading "Search Memory"
and the explanatory sentence, and follow the guideline to use slash commands
(`/save`, `/search <query>`, `/sum`, `/status`) as the primary examples.
In `@nowledge-mem-copilot-cli-plugin/commands/status.md`:
- Around line 9-13: Update the example in status.md to present "/status" as the
primary invocation and move "nmem status" into a secondary
implementation/troubleshooting note; specifically replace the top-level code
block showing "nmem status" with "/status" and add a small following paragraph
or code block that notes the equivalent CLI command "nmem status" for debugging
or local runs so both usages appear but "/status" is shown as the canonical
user-facing command.
In `@nowledge-mem-copilot-cli-plugin/commands/sum.md`:
- Around line 7-30: The page should present the slash command /sum as the
primary invocation and move the raw nmem CLI example (nmem m add ...) to a
secondary "Implementation/CLI reference" section; update the introduction and
the top-level example to show "/sum" (and related slash commands /save, /search
<query>, /status) as the recommended workflow, then retain the existing nmem CLI
snippet only as an optional reference and label it accordingly; modify headings
and order in sum.md and adjust any examples or usage steps that currently teach
direct nmem usage first so they now demonstrate slash-command usage first.
In `@nowledge-mem-copilot-cli-plugin/README.md`:
- Around line 32-37: The WSL shim in the nmem script builds q with q="$q \"$a\""
which fails on embedded quotes/backslashes; update the argument-quoting loop in
the nmem shim to use a robust shell-escaping approach (e.g., build q with printf
'%q' for each "$a" or otherwise use an array and proper quoting) so arguments
containing spaces/special chars are escaped correctly; look for the q variable
and the for a in "$@" loop in the nmem shim and replace the naive concatenation
with printf '%q' "$a" (or an equivalent safe-escaping strategy) before invoking
cmd.exe.
In `@nowledge-mem-copilot-cli-plugin/scripts/copilot-stop-save.py`:
- Around line 314-321: The Windows branch currently calls
msvcrt.locking(lock_fh.fileno(), msvcrt.LK_LOCK, 1) on an empty file which leads
to ambiguous locking; change the Windows path (around lock_path, lock_fh,
session_id, and the fcntl/ mscrt branch) to first write a sentinel byte (e.g.,
b"\0") to the opened file, flush() and seek(0) so the file actually contains
that byte, then call msvcrt.locking(..., msvcrt.LK_LOCK, 1) to lock that byte;
keep the existing behavior for fcntl, and rely on closing lock_fh to release the
lock as before.
In `@nowledge-mem-copilot-cli-plugin/skills/save-thread/SKILL.md`:
- Around line 10-12: Update the explicit trigger examples in SKILL.md's "Only
when user explicitly says:" list to include the slash command `/save` alongside
the existing phrases ("Save this session", "Checkpoint this", "Record
conversation") so the skill supports command-driven usage; edit the trigger list
in SKILL.md to add `/save` and ensure consistency with other command examples
(`/search <query>`, `/sum`, `/status`) mentioned in the guidelines.
In `@nowledge-mem-copilot-cli-plugin/tests/test_copilot_plugin.py`:
- Around line 324-345: The tests mutate the module-level
copilot_stop_save.STATE_DIR directly which can leak between tests; update both
test_load_state_new_session and test_save_and_load_state to patch STATE_DIR only
for the test scope (use unittest.mock.patch or pytest monkeypatch to set
copilot_stop_save.STATE_DIR = Path(tmpdir) inside the context) and avoid
assigning unused variables by renaming path and path2 to _path/_path2 or not
capturing them when calling load_state; keep calls to load_state and save_state
unchanged.
- Around line 383-392: The test test_load_events_from_jsonl leaves the temp file
behind if an assertion fails; wrap the body that writes, calls
copilot_stop_save.load_events and asserts in a try/finally so os.unlink(f.name)
is always called (or use a context manager that ensures deletion), locating the
tempfile usage in test_load_events_from_jsonl and the call to
copilot_stop_save.load_events to add the try/finally cleanup around those
operations.
- Around line 151-174: The tests (test_redacts_github_token,
test_redacts_github_pat, test_redacts_openai_key, test_redacts_bearer_token) use
weak OR assertions that allow partial secrets to remain; update each to assert
the secret prefix is absent AND that "[REDACTED]" appears (e.g. replace `assert
"ghp_" not in result or "[REDACTED]" in result` with `assert "ghp_" not in
result and "[REDACTED]" in result`), and do the same for "github_pat_", "sk-"
and the bearer case when calling copilot_stop_save.redact to ensure full
redaction while keeping test_preserves_normal_text unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: afb203e7-f806-40ad-87b3-00520c2d7682
⛔ Files ignored due to path filters (1)
nowledge-mem-copilot-cli-plugin/icon.pngis excluded by!**/*.png
📒 Files selected for processing (19)
integrations.jsonnowledge-mem-copilot-cli-plugin/.claude-plugin/plugin.jsonnowledge-mem-copilot-cli-plugin/AGENTS.mdnowledge-mem-copilot-cli-plugin/CHANGELOG.mdnowledge-mem-copilot-cli-plugin/README.mdnowledge-mem-copilot-cli-plugin/RELEASING.mdnowledge-mem-copilot-cli-plugin/commands/save.mdnowledge-mem-copilot-cli-plugin/commands/search.mdnowledge-mem-copilot-cli-plugin/commands/status.mdnowledge-mem-copilot-cli-plugin/commands/sum.mdnowledge-mem-copilot-cli-plugin/hooks/hooks.jsonnowledge-mem-copilot-cli-plugin/scripts/copilot-stop-save.pynowledge-mem-copilot-cli-plugin/scripts/copilot-stop-save.shnowledge-mem-copilot-cli-plugin/scripts/install-hooks.shnowledge-mem-copilot-cli-plugin/skills/distill-memory/SKILL.mdnowledge-mem-copilot-cli-plugin/skills/read-working-memory/SKILL.mdnowledge-mem-copilot-cli-plugin/skills/save-thread/SKILL.mdnowledge-mem-copilot-cli-plugin/skills/search-memory/SKILL.mdnowledge-mem-copilot-cli-plugin/tests/test_copilot_plugin.py
| The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), | ||
| and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | ||
|
|
||
| ## [0.1.0] - 2026-07-14 |
There was a problem hiding this comment.
Release date is set in the future.
Line 8 uses 2026-07-14, which is after today (April 16, 2026). Please switch to the actual release date or use an Unreleased section until release day.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nowledge-mem-copilot-cli-plugin/CHANGELOG.md` at line 8, The release heading
"## [0.1.0] - 2026-07-14" uses a future date; update that heading in
CHANGELOG.md (the "## [0.1.0] - 2026-07-14" line) to either the actual release
date (e.g., "2026-04-16") or replace the date with "Unreleased" until the real
release date is known, ensuring the header stays exactly "## [0.1.0] - <date or
Unreleased>".
Comment on lines
+5
to
+7
| 1. Update `version` in `.claude-plugin/plugin.json` | ||
| 2. Update `version` for the `copilot-cli` entry in `integrations.json` (repo root) | ||
| 3. Add a new section to `CHANGELOG.md` |
There was a problem hiding this comment.
Reorder release steps to update integrations.json first
Line 5-Line 7 currently instruct updating .claude-plugin/plugin.json before integrations.json. Swap these so integrations.json is updated first to match the registry workflow contract.
As per coding guidelines: integrations.json is the single source of truth and should be updated first when adding/modifying an integration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nowledge-mem-copilot-cli-plugin/RELEASING.md` around lines 5 - 7, Reorder the
three release steps so the `integrations.json` update happens before the
`.claude-plugin/plugin.json` update: first bump the `copilot-cli` version in
integrations.json (the canonical source), then update the `version` field in
`.claude-plugin/plugin.json`, and finally add the new section to `CHANGELOG.md`;
ensure the RELEASING.md steps reflect that exact sequence and phrasing.
Comment on lines
+125
to
+134
| def redact(text: str) -> str: | ||
| redacted = text or "" | ||
| for pattern in SECRET_PATTERNS: | ||
| if pattern.groups == 0: | ||
| redacted = pattern.sub("[REDACTED]", redacted) | ||
| elif pattern.groups == 1: | ||
| redacted = pattern.sub("[REDACTED]", redacted) | ||
| else: | ||
| redacted = pattern.sub(r"\1[REDACTED]", redacted) | ||
| return redacted |
There was a problem hiding this comment.
Redaction logic discards intended prefixes for single-group patterns.
When pattern.groups == 1, the current code replaces the entire match with [REDACTED], losing the prefix captured in group 1. Patterns like the Bearer token pattern (line 56) capture "Bearer " in group 1 specifically to preserve it. The logic should be:
- 0 groups → replace whole match
- 1 group → preserve group 1 as prefix
- 2+ groups → already handled with
\1[REDACTED]
🐛 Proposed fix
def redact(text: str) -> str:
redacted = text or ""
for pattern in SECRET_PATTERNS:
if pattern.groups == 0:
redacted = pattern.sub("[REDACTED]", redacted)
elif pattern.groups == 1:
- redacted = pattern.sub("[REDACTED]", redacted)
+ redacted = pattern.sub(r"\1[REDACTED]", redacted)
else:
redacted = pattern.sub(r"\1[REDACTED]", redacted)
return redactedOr simplify since groups >= 1 now have the same behavior:
def redact(text: str) -> str:
redacted = text or ""
for pattern in SECRET_PATTERNS:
if pattern.groups == 0:
redacted = pattern.sub("[REDACTED]", redacted)
else:
redacted = pattern.sub(r"\1[REDACTED]", redacted)
return redacted📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def redact(text: str) -> str: | |
| redacted = text or "" | |
| for pattern in SECRET_PATTERNS: | |
| if pattern.groups == 0: | |
| redacted = pattern.sub("[REDACTED]", redacted) | |
| elif pattern.groups == 1: | |
| redacted = pattern.sub("[REDACTED]", redacted) | |
| else: | |
| redacted = pattern.sub(r"\1[REDACTED]", redacted) | |
| return redacted | |
| def redact(text: str) -> str: | |
| redacted = text or "" | |
| for pattern in SECRET_PATTERNS: | |
| if pattern.groups == 0: | |
| redacted = pattern.sub("[REDACTED]", redacted) | |
| elif pattern.groups == 1: | |
| redacted = pattern.sub(r"\1[REDACTED]", redacted) | |
| else: | |
| redacted = pattern.sub(r"\1[REDACTED]", redacted) | |
| return redacted |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nowledge-mem-copilot-cli-plugin/scripts/copilot-stop-save.py` around lines
125 - 134, The redact function currently discards group 1 prefixes by replacing
the entire match when pattern.groups == 1; update the logic in redact (which
loops over SECRET_PATTERNS and checks pattern.groups) so that for pattern.groups
== 1 you preserve the first capture and append “[REDACTED]” (i.e., perform the
same substitution used for 2+ groups such as using the "\1[REDACTED]"
replacement), keep 0-group behavior unchanged, and ensure SECRET_PATTERNS and
pattern.groups are referenced as-is to locate the change.
Comment on lines
+240
to
+254
| def load_state(session_id: str) -> tuple[Path, dict]: | ||
| STATE_DIR.mkdir(parents=True, exist_ok=True) | ||
| path = STATE_DIR / f"{session_id}.json" | ||
| if not path.exists(): | ||
| return path, { | ||
| "active_start_event_id": None, | ||
| "last_saved_turn_end_id": None, | ||
| "last_distill_ts": 0, | ||
| "last_content_hash": None, | ||
| } | ||
| with path.open(encoding="utf-8") as fh: | ||
| state = json.load(fh) | ||
| state.setdefault("last_distill_ts", 0) | ||
| state.setdefault("last_content_hash", None) | ||
| return path, state |
There was a problem hiding this comment.
Session ID used directly in filesystem path without sanitization.
session_id comes from external input (stdin payload) and is used directly to construct file paths (lines 242, 314). A malicious session_id like ../../../etc/passwd could cause path traversal. Sanitize or validate:
🛡️ Proposed fix to sanitize session_id
+import re as _re
+
+_SAFE_SESSION_ID_RE = _re.compile(r"^[A-Za-z0-9_-]{1,128}$")
+
+def _sanitize_session_id(session_id: str) -> str:
+ """Ensure session_id is safe for use in filenames."""
+ if not session_id or not _SAFE_SESSION_ID_RE.match(session_id):
+ # Fall back to hash if invalid
+ return hashlib.sha256(session_id.encode()).hexdigest()[:32]
+ return session_id
+
def load_state(session_id: str) -> tuple[Path, dict]:
+ session_id = _sanitize_session_id(session_id)
STATE_DIR.mkdir(parents=True, exist_ok=True)
path = STATE_DIR / f"{session_id}.json"And apply similar sanitization at line 314 for lock_path, or sanitize once in main() before any use.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nowledge-mem-copilot-cli-plugin/scripts/copilot-stop-save.py` around lines
240 - 254, The load_state function (and any code that builds lock_path or other
filesystem paths from session_id) currently embeds raw session_id into STATE_DIR
/ f"{session_id}.json" which allows path traversal; add a
sanitization/normalization step (e.g., a sanitize_session_id(session_id) helper
called from main() and before any use in load_state or when creating lock_path)
that either (a) validates allowed characters and rejects or canonicalizes input
(strip path separators, dots, and directory components), or (b) replaces the
session_id with a safe deterministic token (e.g., hex of sha256(session_id) or a
UUID derived from it) and use that token for filenames; update load_state, any
lock_path construction, and callers in main() to use the sanitized token instead
of the raw session_id.
Comment on lines
+34
to
+36
| ```bash | ||
| nmem wm read | ||
| ``` |
There was a problem hiding this comment.
Use JSON mode for Working Memory recall command
Line 35 should use nmem --json wm read to keep recall flows consistent with structured parsing expectations.
Suggested doc fix
- nmem wm read
+ nmem --json wm readAs per coding guidelines: for nowledge-mem-copilot-cli-plugin/**/*.{md,sh,bash,py,yml,yaml}, use the nmem CLI with --json flag to search and recall persistent knowledge.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ```bash | |
| nmem wm read | |
| ``` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nowledge-mem-copilot-cli-plugin/skills/read-working-memory/SKILL.md` around
lines 34 - 36, Replace the plain-shell example command "nmem wm read" with the
JSON-mode command "nmem --json wm read" in SKILL.md (update the code block that
contains the command), and scan the same MD file for any other occurrences of
"nmem wm read" to change them to the "--json" variant so recall examples follow
the structured parsing convention.
Comment on lines
+33
to
+37
| ``` | ||
| ✓ Thread saved | ||
| Summary: {summary} | ||
| Thread ID: {thread_id from nmem output} | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the fenced response block.
The fenced block at Line 33 should declare a language (for example, text) to satisfy markdown lint expectations.
Suggested patch
-```
+```text
✓ Thread saved
Summary: {summary}
Thread ID: {thread_id from nmem output}</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nowledge-mem-copilot-cli-plugin/skills/save-thread/SKILL.md` around lines 33
- 37, Update the fenced code block that contains the "✓ Thread saved" message so
it declares a language identifier (e.g., change the opening ``` to ```text) to
satisfy markdown linting; locate the fenced block that wraps "✓ Thread saved /
Summary: {summary} / Thread ID: {thread_id from nmem output}" in SKILL.md and
add the language token immediately after the opening backticks.
Clarify that Copilot CLI currently ships bundled command docs without exposing them as interactive slash commands, and update the install path examples accordingly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
nowledge-mem-copilot-cli-pluginpackage with Copilot-native hooks, commands, skills, installer, docs, and testsintegrations.jsonso it can ship through the Nowledge community marketplaceWhy this exists
GitHub Copilot CLI can load Claude Code-style plugins, but using the Claude Code integration directly is brittle in practice because some lifecycle assumptions, command surfaces, and config paths are Claude-specific.
This PR therefore treats the Copilot plugin as a real adaptation, not a straight copy:
Implementation approach
The recorded build session shows an orchestrator-worker approach rather than a role-play pipeline:
The captured planning artifacts also show three design corrections made before/during implementation:
copilot-{session_id}What the reviewer should look at first
If you want to review from the source of the design rather than only from the final diff, start here:
nowledge-mem-copilot-cli-plugin/scripts/copilot-stop-save.py— core Copilot transcript capture and incremental thread save logicnowledge-mem-copilot-cli-plugin/hooks/hooks.json— lifecycle wiring forSessionStart,UserPromptSubmit, and asyncStopnowledge-mem-copilot-cli-plugin/README.mdandAGENTS.md— user-facing and agent-facing behavior modelintegrations.json— registry entry for the newcopilot-cliintegrationSession evidence and validation notes
The Copilot session artifacts I reviewed show:
claude-opus-4.6claude-haiku-4.5gpt-5.4I did not find reliable evidence for a specific reasoning-effort tier such as
high, so I am intentionally not claiming that here.The session artifacts clearly preserve the planning and review flow. They are less consistent on which validation steps were fully completed versus merely planned, so I would treat the recorded validation narrative as implementation context rather than as the authoritative release checklist.
Test plan
scripts/install-hooks.sh🤖 Generated with Claude Code