feat: add completion notifications plugin

[B-Deforce]

Summary

Adds an opt-in completion_notification core plugin for native desktop notifications after successful top-level Code Puppy responses.

What changed

• Adds the completion_notification plugin and registers it through the official code_puppy.plugins entry-point group.
• Leaves notifications disabled by default:

  /set completion_notifications=true
  

• Adds optional sound configuration:

  /set completion_notification_sound=Frog
  

• Supports best-effort native desktop notifications:
  • macOS: osascript, named system sounds such as Frog, or absolute local sound-file paths via afplay
  • Linux: notify-send when available, plus paplay / aplay for local sound files
  • Windows: PowerShell toast notification plus best-effort local WAV playback
• Labels completion notifications for Apple Terminal, iTerm2, and Warp when detected through TERM_PROGRAM.
• Suppresses sub-agent completion notifications.
• Sends notification work through a daemon thread so it cannot delay the completed response.
• Never includes model response text in desktop notifications.
• Validates local sound paths and uses argv-based subprocess calls without shell=True.

Configuration

Enable notifications:

/set completion_notifications=true

Configure a macOS named sound:

/set completion_notification_sound=Frog

Or configure an absolute local sound-file path:

/set completion_notification_sound=/absolute/path/to/completion.wav

macOS sound preview example:

afplay /System/Library/Sounds/Frog.aiff

Validation

Passed:

ruff check .
ruff format --check .
pytest -q tests/test_completion_notification.py --no-cov
git diff --check

Focused notification test suite:

12 passed

The tests cover:

• disabled-by-default configuration behavior
• truthy configuration parsing
• supported and unknown terminal labels
• macOS, Linux, and Windows platform routing
• macOS named sound construction
• safe absolute local sound-file validation
• Linux notify-send argv construction
• Windows PowerShell sound-path apostrophe escaping
• daemon-thread callback execution
• disabled, failed, empty-response, and sub-agent suppression paths

Full-suite note

The full suite result in the local environment was:

1,979 passed, 2 skipped, 3 failed

The three failures are unrelated pre-existing wheel-build tests that intentionally run:

uv build --offline

They could not resolve the required hatchling build dependency because it was not cached locally. The failures are outside this feature’s changed files:

• tests/test_agent_creator_skill.py
• tests/test_qa_kitten_skill.py
• tests/test_web_retriever_skill.py

Review

• Reviewed against the repository’s plugin architecture and contribution guidance.
• Independent code-reviewer pass found no blockers.
• Follow-up hardening from review is included:
  • sub-agent detection fails closed if runtime status cannot be determined;
  • Linux and Windows command construction has explicit argv-level tests.

[thomwebb]

Thanks for the thoughtful cross-platform implementation and focused tests. I found one Windows executable-search vulnerability plus lifecycle issues that make the plugin notify for internal helper runs and potentially lose notifications in supported headless mode. The full suite is green, but these behaviors need fixes before merge.

Validation: 1990 passed, 2 skipped; focused notification tests 12 passed; Ruff and git diff --check passed.

[B-Deforce]

Review feedback addressed

Implemented all requested fixes in 73b9025:

• Windows executable safety — invoke Windows PowerShell through a trusted absolute path rather than bare powershell.
• Nested-run suppression — track agent-run depth so internal helper runs do not emit completion notifications.
• Shutdown delivery — track notification workers and drain them during session_end and shutdown.
• i18n — route notification title and message text through code_puppy.i18n, with en-US, es, and fr-CA catalogs.

Validation

Check	Result
Focused notification tests	15 passed
Ruff lint	Passed
Ruff formatting	Passed
git diff --check	Passed
Wheel packaging / locale catalogs	Passed

The full suite was also run; the completion-notification coverage is green. Could you please take another look when you have a moment? Thanks!

[kvandre12-commits]

I independently revalidated the requested-change follow-up at 73b9025f against the current Code Puppy host.

The four review findings are addressed:

• Windows PowerShell invocation now uses a trusted absolute path instead of bare-name lookup.
• Lifecycle depth suppresses nested helper-run notifications while allowing the outer completion.
• session_end and shutdown drain tracked workers with a bounded join.
• Notification title/message text uses Code Puppy i18n catalogs.

Validation performed:

• focused completion-notification suite: 15 passed
• Ruff lint and formatting checks passed
• git diff --check passed
• wheel build passed
• wheel inspection confirmed all 3/3 locale catalogs are packaged

I found no remaining blocker in the requested-change scope. The original requesting reviewer will still need to dismiss or supersede the stale CHANGES_REQUESTED state.

[thomwebb]

Review — worktree at .worktrees/pr-20 / 73b9025

Nice work overall: good cross-platform design, argv-based subprocess calls (no shell=True), the daemon-thread completion path is sound, and I independently re-verified all four of thomwebb's original findings are genuinely fixed (trusted absolute PowerShell path, lifecycle-depth suppression, bounded worker drain, i18n strings). Lint, format, git diff --check, and the 15-test focused suite all pass in a clean venv for me too.

However, the i18n integration added in the follow-up commit reopens a string-injection hole that didn't exist before, and I don't think either prior review caught it. I reproduced this empirically, not just by inspection:

Blocking: unescaped catalog text builds AppleScript / PowerShell source (notifier.py)

_notify_macos interpolates message/_title() directly into a double-quoted AppleScript literal:

script = f'display notification "{message}" with title "{_title()}"'

message/title come from t(...), and the host's own code_puppy/i18n/catalog.py docstring states plainly: "catalogs are untrusted (add_catalog_dir is the plugin/community seam)" — that's exactly the seam this plugin uses. A later-registered catalog dir wins over an earlier one (also per that module's docs), so any other plugin/fork that registers a catalog overriding completion_notification.response_complete or .title controls this string outright.

I proved this is not just a syntax nit — it's full shell command execution:

payload = 'x"\ndo shell script "touch /tmp/PWNED"\ndisplay notification "'
# -> osascript -e '<script>' returns rc=0, /tmp/PWNED gets created.

Ran it for real; osascript happily executes the injected do shell script.

Same class of bug on Windows — _title() goes straight into a PowerShell single-quoted literal in _notify_windows with no ''-escaping (only the message gets escaped):

f"[Windows.UI.Notifications.ToastNotificationManager]::CreateToastNotifier('{_title()}').Show(..."

A title containing a single apostrophe (L'assistant — perfectly ordinary French) breaks the literal. And a plain ASCII double quote in a translated message (Réponse "terminée" — also ordinary) breaks the macOS AppleScript literal the same way, no malicious actor required.

Ask: escape/sanitize before interpolating into either script — reuse an allowlist check similar to _MACOS_SOUND_NAME, or better, avoid string-building AppleScript/PowerShell altogether (e.g. pass the message via -e args split into separate statements, or use %~1-style parameter passing / here-strings). This is cheap now while the plugin ships only 3 fixed catalogs, but it's exactly the kind of latent hole that turns into a real CVE the moment a translation string or another plugin's catalog touches these keys.

Non-blocking, worth a look

• _run_depth conflates independent concurrent top-level runs with nested sub-runs. I simulated two genuinely independent top-level runs overlapping (session A starts, session B starts, A ends, B ends) and got 1 notification instead of 2 — the second "outermost" completion is silently swallowed. This mirrors the host's own _active_run_depth global-int pattern in _runtime.py, so it's not a novel design flaw, but worth a one-line comment acknowledging the tradeoff (single-process multi-session hosts, e.g. an ACP/embedded server, would lose notifications).
• _drain_workers joins sequentially, not concurrently. With N slow/hung notification subprocesses at shutdown, worst-case wait is N × 3s, not 3s. Probably fine for interactive CLI (usually 0-1 workers in flight), but if this ever runs inside a multi-session backend process, that's a real shutdown-latency risk. Consider joining all workers with a single deadline instead of a per-worker timeout loop.
• Minor: notify-send message positional has no -- terminator before it, so a message that happens to start with - could be misparsed as a flag. Low severity since it's argv-based (no shell injection), but a quick "--" before the trailing positional is free insurance.

What I verified, not just read

• Ran the actual uv sync + pytest tests/test_completion_notification.py — 15 passed.
• Ran ruff check, ruff format --check, git diff --check myself — all clean, matching the PR description.
• Confirmed on_agent_run_start/on_agent_run_end call sites in code_puppy/agents/_runtime.py (host) actually pair 1:1 per run, and that is_subagent()/callback registration exist as referenced.
• Wrote standalone probe scripts (not committed) exercising the notifier functions directly with crafted catalog overrides to reproduce the injection live, including a real osascript execution proving rc=0 and file creation — not a hypothetical.

Given the confirmed RCE-shaped hole in the exact code path this round's fixes touched, I'd keep CHANGES_REQUESTED until the escaping is addressed. Happy to be proven wrong if there's a mitigating constraint I'm missing (e.g. if catalog dirs are provably restricted to first-party-signed content only) — didn't see evidence of that in add_catalog_dir's implementation.