diff --git a/.gitignore b/.gitignore index d6cc219..15367b7 100644 --- a/.gitignore +++ b/.gitignore @@ -109,3 +109,9 @@ poster resource / *.out *.bbl *.blg + +.projectmem/events.jsonl +.projectmem/watch.pid +.projectmem/watch.log +CLAUDE.md +AGENTS.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 280a13f..5c8f027 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,22 @@ # Changelog +## 0.1.5 + +**MCP timeout hardening for stdio clients.** Projectmem's MCP tools now avoid letting git subprocesses inherit the MCP server's JSON-RPC stdin pipe, which could hang FastMCP stdio sessions on Windows even when the equivalent CLI command returned quickly. + +### Fixed + +- MCP-reachable git helpers now detach child-process stdin with `subprocess.DEVNULL`. +- Git commit lookup on write paths is bounded with a timeout, so tools such as `add_note` and `record_fix` do not wait forever if git stalls. +- `precheck_file` and related precheck helpers keep CLI behavior unchanged while hardening the MCP execution path. +- `pjm brief` rendering is safe on CP1252 and other non-UTF-8 Windows consoles. +- Windows hook-path tests tolerate environments where bash exits early on invalid inherited stdin. + +### Tests and docs + +- Added MCP stdio regression coverage for `precheck_file` and `add_note`. +- Added focused subprocess tests for the MCP-reachable git helper calls. + ## 0.1.4 **The accountable-judgment release: memory that flags its own staleness instead of silently trusting (or deleting) it — plus a dashboard that opens on an all-at-a-glance Overview.** Six small features (~150 lines, no new dependencies, no schema breaks) sharpen what makes projectmem different: it never deletes a memory, it tells you when one may have gone stale, it lets you retire decisions without losing history, it lists what already failed before you try it again, it briefs you at session start, it snoozes politely when it's wrong, and it exports its judgment to CLAUDE.md for agents that don't speak MCP. Also bumps the version (the `__init__.py` / `pyproject.toml` mismatch is corrected to a single `0.1.4`). diff --git a/README.md b/README.md index f1f3b7a..cd8b7bf 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ | Doc | What's in it | |---|---| | **[TUTORIAL.md](TUTORIAL.md)** | 15-minute step-by-step walkthrough — set up projectmem on your own project, watch the lifecycle, see the pre-commit warning fire. | -| **[CHANGELOG.md](CHANGELOG.md)** | Release history. Latest: v0.1.4 — the accountable-judgment release: stale-memory detection, decision supersede, precheck snooze, `pjm brief`, failed-approach surfacing, CLAUDE.md export, dashboard Overview. | +| **[CHANGELOG.md](CHANGELOG.md)** | Release history. Latest: v0.1.5 — MCP stdio timeout hardening, bounded git subprocesses, CP1252-safe brief output, and focused MCP regression tests. | | **[Research paper (arXiv:2606.12329)](https://arxiv.org/abs/2606.12329)** | *PROJECTMEM: A Local-First, Event-Sourced Memory and Judgment Layer for AI Coding Agents* — the peer-readable version: design, Memory-as-Governance framing, capability comparison, and the 207-event dogfooding study. | | **[LICENSE](LICENSE)** | MIT | diff --git a/pyproject.toml b/pyproject.toml index 117af1e..9890193 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "projectmem" -version = "0.1.4" +version = "0.1.5" description = "Local-first memory + judgment layer for AI coding agents — warns before repeating failed fixes." authors = [{ name = "Ripon Chandra Malo" }] readme = "README.md" diff --git a/src/projectmem/__init__.py b/src/projectmem/__init__.py index 4958a75..e465b6b 100644 --- a/src/projectmem/__init__.py +++ b/src/projectmem/__init__.py @@ -1,3 +1,3 @@ """Local-first memory for repos.""" -__version__ = "0.1.4" +__version__ = "0.1.5" diff --git a/src/projectmem/cli.py b/src/projectmem/cli.py index b94b65f..1d1e409 100644 --- a/src/projectmem/cli.py +++ b/src/projectmem/cli.py @@ -106,10 +106,19 @@ def attempt( @app.command() def fix( text: str, - at: str | None = typer.Option(None, "--at", help="Location (e.g. file:line, class.method)"), + at: str | None = typer.Option( + None, + "--at", + help="Location (e.g. file:line, class.method)", + ), + issue: str | None = typer.Option( + None, + "--issue", + help="Close a specific issue ID instead of the active issue (e.g. 0042).", + ), ) -> None: - """Record a fix and close the current issue.""" - fix_command.run(text, location=at) + """Record a fix and close the active issue, or a specific issue with --issue.""" + fix_command.run(text, location=at, issue=issue) @app.command() diff --git a/src/projectmem/commands/brief.py b/src/projectmem/commands/brief.py index 82a8ae7..a115d75 100644 --- a/src/projectmem/commands/brief.py +++ b/src/projectmem/commands/brief.py @@ -8,6 +8,7 @@ """ from __future__ import annotations +import sys from collections import defaultdict from datetime import datetime, timedelta, timezone from pathlib import Path @@ -31,13 +32,40 @@ _RESET = "\033[0m" +def _stdout_encoding() -> str: + """Return the active stdout encoding, falling back to UTF-8.""" + return getattr(sys.stdout, "encoding", None) or "utf-8" + + +def _console_safe(text: object) -> str: + """Return text that can be printed by the current console encoding.""" + value = str(text) + encoding = _stdout_encoding() + try: + value.encode(encoding) + return value + except UnicodeEncodeError: + return value.encode(encoding, errors="replace").decode(encoding) + + +def _safe_echo(text: object = "") -> None: + typer.echo(_console_safe(text)) + + +def _rule(width: int = 60) -> str: + encoding = _stdout_encoding().lower() + if "utf" in encoding: + return "─" * width + return "-" * width + + def run(root: Path | None = None) -> None: root_path = root or Path.cwd() events = read_events(root) - typer.echo("") - typer.echo(f"{_BOLD}projectmem brief — {root_path.name}{_RESET}") - typer.echo(f"{_DIM}{'─' * 60}{_RESET}") + _safe_echo("") + _safe_echo(f"{_BOLD}projectmem brief — {root_path.name}{_RESET}") + _safe_echo(f"{_DIM}{_rule(60)}{_RESET}") _section_warnings(events) _section_stale(events, root_path) @@ -46,8 +74,8 @@ def run(root: Path | None = None) -> None: _section_gotchas(root_path) _section_score(events) - typer.echo(f"{_DIM}{'─' * 60}{_RESET}") - typer.echo("") + _safe_echo(f"{_DIM}{_rule(60)}{_RESET}") + _safe_echo("") def _recent(events: list[Event], days: int = RECENT_DAYS) -> list[Event]: @@ -79,13 +107,13 @@ def _section_warnings(events: list[Event]) -> None: by_file: dict[str, list[Event]] = defaultdict(list) for e in failed: by_file[_file_of(e) or "(no file)"].append(e) - typer.echo(f"{_YELLOW}⚠ Active warnings{_RESET}") + _safe_echo(f"{_YELLOW}⚠ Active warnings{_RESET}") if not by_file: - typer.echo(f" {_DIM}none — no failed attempts in {RECENT_DAYS} days{_RESET}") + _safe_echo(f" {_DIM}none — no failed attempts in {RECENT_DAYS} days{_RESET}") return for file_path, items in sorted(by_file.items(), key=lambda kv: -len(kv[1]))[:4]: last = items[-1] - typer.echo( + _safe_echo( f" {file_path} — {len(items)} failed attempt" f"{'s' if len(items) != 1 else ''} " f"{_DIM}(last: {last.summary[:60]}){_RESET}" @@ -101,18 +129,18 @@ def _section_stale(events: list[Event], root: Path) -> None: stale = [] if not stale: return # silence is the right default — no flags, no noise - typer.echo(f"{_YELLOW}⏳ Possibly stale{_RESET}") + _safe_echo(f"{_YELLOW}⏳ Possibly stale{_RESET}") for item in stale[:MAX_STALE]: event = item["event"] if item["commits_since"] == -1: reason = f"{item['file']} no longer exists" else: reason = f"{item['file']} changed {item['commits_since']}x since" - typer.echo( + _safe_echo( f" {event.type} [{event.id}] {event.summary[:55]} " f"{_DIM}— {reason}{_RESET}" ) - typer.echo( + _safe_echo( f" {_DIM}confirm, or retire: pjm decision \"...\" --supersedes {_RESET}" ) @@ -122,12 +150,12 @@ def _section_open_issues(events: list[Event]) -> None: open_issues = [ e for e in events if e.type == "issue" and e.issue_id not in fixed ] - typer.echo(f"{_RED}📋 Open issues{_RESET}") + _safe_echo(f"{_RED}📋 Open issues{_RESET}") if not open_issues: - typer.echo(f" {_DIM}none open{_RESET}") + _safe_echo(f" {_DIM}none open{_RESET}") return for issue in open_issues[-4:]: - typer.echo(f" #{issue.issue_id} {issue.summary[:70]}") + _safe_echo(f" #{issue.issue_id} {issue.summary[:70]}") def _section_decisions(events: list[Event]) -> None: @@ -135,12 +163,12 @@ def _section_decisions(events: list[Event]) -> None: live = [ e for e in events if e.type == "decision" and e.id not in retired ] - typer.echo(f"{_TEAL}🕑 Recent decisions{_RESET}") + _safe_echo(f"{_TEAL}🕑 Recent decisions{_RESET}") if not live: - typer.echo(f" {_DIM}none logged yet{_RESET}") + _safe_echo(f" {_DIM}none logged yet{_RESET}") return for d in live[-MAX_DECISIONS:]: - typer.echo(f" {d.summary[:75]}") + _safe_echo(f" {d.summary[:75]}") def _section_gotchas(root: Path) -> None: @@ -153,11 +181,11 @@ def _section_gotchas(root: Path) -> None: gotchas = [] if not gotchas: return - typer.echo(f"{_TEAL}💡 Stack gotchas{_RESET}") + _safe_echo(f"{_TEAL}💡 Stack gotchas{_RESET}") for g in gotchas[-MAX_GOTCHAS:]: lib = g.get("library", "") text = g.get("text") or g.get("summary") or "" - typer.echo(f" {lib}: {text[:70]}") + _safe_echo(f" {lib}: {text[:70]}") def _section_score(events: list[Event]) -> None: @@ -187,6 +215,6 @@ def _section_score(events: list[Event]) -> None: trend = f" {_RED}▼ {delta} this week{_RESET}" else: trend = f" {_DIM}— unchanged this week{_RESET}" - typer.echo( + _safe_echo( f"{_GREEN}📈 Score{_RESET} {current['grade']} ({current['score']}/100){trend}" ) diff --git a/src/projectmem/commands/context.py b/src/projectmem/commands/context.py index 3da110e..c0ffb11 100644 --- a/src/projectmem/commands/context.py +++ b/src/projectmem/commands/context.py @@ -422,6 +422,7 @@ def _get_git_status_files(root: Path) -> list[str]: capture_output=True, text=True, timeout=5, + stdin=subprocess.DEVNULL, ) files = [] for line in result.stdout.strip().split("\n"): diff --git a/src/projectmem/commands/fix.py b/src/projectmem/commands/fix.py index 6f92968..8489dc3 100644 --- a/src/projectmem/commands/fix.py +++ b/src/projectmem/commands/fix.py @@ -15,14 +15,57 @@ from projectmem.summary import regenerate_summary -def run(text: str, location: str | None = None) -> Event: - """Close the current issue with a fix. Returns the created fix Event. +def _normalize_issue_id(issue_id: str | None) -> str | None: + """Normalize issue IDs so `1`, `001`, and `0001` all become `0001`.""" + if issue_id is None: + return None + + cleaned = issue_id.strip().lstrip("#") + + if not cleaned: + return None + + if cleaned.isdigit(): + return cleaned.zfill(4) + + return cleaned + + +def _issue_exists(events: list[Event], issue_id: str) -> bool: + """Return True if an issue event exists for the requested issue ID.""" + return any(event.type == "issue" and event.issue_id == issue_id for event in events) + + +def run( + text: str, + location: str | None = None, + issue: str | None = None, +) -> Event: + """Close an issue with a fix. Returns the created fix Event. - Clears the current-issue marker so subsequent `pjm attempt` calls do not - silently re-attach to the just-closed issue (the L-027a misattribution bug). + When `issue` is omitted, this preserves the existing behavior: + close the current issue and clear the current-issue marker. + + When `issue` is provided, the fix is attached to that specific issue. + The current-issue marker is only cleared if it points at the same issue. + This prevents closing or clearing the wrong active issue when fixing an + older issue after newer issues have been created. """ events = read_events() - issue_id = read_current_issue() or current_issue_id(events) + requested_issue_id = _normalize_issue_id(issue) + active_issue_id = read_current_issue() or current_issue_id(events) + + if requested_issue_id is not None: + issue_id = requested_issue_id + + if not _issue_exists(events, issue_id): + raise ProjectMemError( + f"Issue #{issue_id} was not found. " + "Run `pjm search ` or `pjm brief` to find the issue ID." + ) + else: + issue_id = active_issue_id + if issue_id is None: raise ProjectMemError("No open issue found. Run `pjm log ` first.") @@ -33,8 +76,16 @@ def run(text: str, location: str | None = None) -> Event: git_commit=get_git_commit(), location=location, ) + append_event(event) - clear_current_issue() + + # Preserve old behavior when no specific issue was requested. + # For targeted fixes, only clear the active marker if it matches the issue + # being fixed. This avoids wiping a newer active issue by mistake. + if requested_issue_id is None or active_issue_id == issue_id: + clear_current_issue() + regenerate_summary() typer.echo(f"Fixed issue #{issue_id}") + return event diff --git a/src/projectmem/commands/init.py b/src/projectmem/commands/init.py index 1e5537a..4efc8f5 100644 --- a/src/projectmem/commands/init.py +++ b/src/projectmem/commands/init.py @@ -113,7 +113,7 @@ def _claude_md_bridge() -> str: "files directly via filesystem write:\n" " - On a bug discovery → `log_issue(summary, location)`.\n" " - After each fix attempt → `record_attempt(summary, outcome)`.\n" - " - After confirmation → `record_fix(summary)`.\n" + " - After confirmation → use `record_fix(summary)` for the active issue. If fixing a specific older issue, use `record_fix(summary, issue_id=\"\")` and replace `` with the actual Projectmem issue ID.\n" " - On a design choice → `add_decision(summary)`.\n" " - On a gotcha / setup detail → `add_note(summary)`.\n\n" "Editing `.projectmem/summary.md` or `.projectmem/PROJECT_MAP.md`\n" diff --git a/src/projectmem/commands/precheck.py b/src/projectmem/commands/precheck.py index 9c92dce..0cc0ef7 100644 --- a/src/projectmem/commands/precheck.py +++ b/src/projectmem/commands/precheck.py @@ -18,6 +18,7 @@ import re import subprocess +import sys from collections import defaultdict from datetime import datetime, timezone, timedelta from pathlib import Path @@ -29,20 +30,21 @@ from projectmem.storage import MEM_DIR, read_events, require_mem_dir -# ── Thresholds ── +# -- Thresholds -- HIGH_CHURN_THRESHOLD = 4 # changes in CHURN_WINDOW_COMMITS to trigger CHURN_WINDOW_COMMITS = 7 # rolling window for churn detection -FAILED_ATTEMPT_BLOCK_COUNT = 3 # 3+ failed attempts → block (at --level block) +FAILED_ATTEMPT_BLOCK_COUNT = 3 # 3+ failed attempts -> block (at --level block) RECENT_DAYS = 30 # only consider events newer than this -# ── Severity ── +# -- Severity -- SEVERITY_INFO = "info" SEVERITY_WARN = "warn" SEVERITY_BLOCK = "block" SEVERITY_LEVELS = {SEVERITY_INFO: 0, SEVERITY_WARN: 1, SEVERITY_BLOCK: 2} -# ── Snooze (0.1.4) ── + +# -- Snooze (0.1.4) -- # A TTL'd marker file silences precheck output without uninstalling the # hook. Unlike `git commit --no-verify`, a snooze is itself recorded in the # event log, so even the silence is auditable. Expired markers are removed @@ -52,6 +54,43 @@ _DURATION_UNITS = {"m": "minutes", "h": "hours", "d": "days"} +def _stdout_encoding() -> str: + """Return the active stdout encoding, falling back to UTF-8.""" + return getattr(sys.stdout, "encoding", None) or "utf-8" + + +def _console_safe(text: object) -> str: + """Return text that can be printed by the current console encoding. + + Windows PowerShell and Git hooks may run under cp1252. That encoding cannot + print box-drawing characters, some symbols, or emoji. Projectmem hooks + should never fail because decorative output cannot be encoded. + """ + value = str(text) + encoding = _stdout_encoding() + + try: + value.encode(encoding) + return value + except UnicodeEncodeError: + return value.encode(encoding, errors="replace").decode(encoding) + + +def _safe_echo(text: object = "", *, err: bool = False) -> None: + """typer.echo wrapper that cannot crash on console encoding issues.""" + typer.echo(_console_safe(text), err=err) + + +def _rule(width: int = 60) -> str: + """Return a separator that is safe for the current console.""" + encoding = _stdout_encoding().lower() + + if "utf" in encoding: + return "─" * width + + return "-" * width + + def parse_snooze_duration(text: str) -> timedelta: """Parse '30m' / '2h' / '1d' into a timedelta. Raises ValueError.""" match = _DURATION_RE.match(text.strip()) @@ -150,17 +189,17 @@ def run( # Snooze management actions short-circuit the check itself. if unsnooze: if clear_snooze(root): - typer.echo("projectmem: precheck warnings re-enabled.") + _safe_echo("projectmem: precheck warnings re-enabled.") else: - typer.echo("projectmem: no active snooze.") + _safe_echo("projectmem: no active snooze.") return if snooze: try: expiry = set_snooze(snooze, root) except ValueError as exc: - typer.echo(f"Error: {exc}", err=True) + _safe_echo(f"Error: {exc}", err=True) raise typer.Exit(1) - typer.echo( + _safe_echo( f"projectmem: precheck warnings snoozed until " f"{expiry.strftime('%H:%M UTC')} (logged to memory). " f"Re-enable early with `pjm precheck --unsnooze`." @@ -171,7 +210,7 @@ def run( # silenced warning is never mistaken for a clean check. expiry = active_snooze(root) if expiry is not None: - typer.echo( + _safe_echo( f"\033[2mprojectmem: warnings snoozed ({_remaining(expiry)} left) — " f"`pjm precheck --unsnooze` to re-enable.\033[0m" ) @@ -187,7 +226,7 @@ def run( if not target_files: if not quiet: - typer.echo("projectmem: No files to check.") + _safe_echo("projectmem: No files to check.") return # Read events and build warnings @@ -200,7 +239,7 @@ def run( if not warnings: if not quiet: - typer.echo("\033[32mprojectmem:\033[0m no warnings — looking good!") + _safe_echo("\033[32mprojectmem:\033[0m no warnings — looking good!") return # Render warnings @@ -221,9 +260,9 @@ def _analyze_files( warnings: list[dict[str, Any]] = [] - # ── Check 6 input: stale memories (computed once for all files) ── + # -- Check 6 input: stale memories (computed once for all files) -- # Decisions/fixes/notes whose cited file changed substantially after - # they were logged. Never deleted, never down-ranked — flagged for a + # they were logged. Never deleted, never down-ranked -- flagged for a # human (or agent) to confirm or supersede. try: from projectmem.staleness import find_stale_events @@ -251,20 +290,20 @@ def _analyze_files( except (ValueError, AttributeError): recent.append(e) - # ── Check 1: Failed attempts ── + # -- Check 1: Failed attempts -- failed_attempts = [ e for e in recent if e.type == "attempt" and e.outcome == "failed" ] if failed_attempts: count = len(failed_attempts) severity = SEVERITY_BLOCK if count >= FAILED_ATTEMPT_BLOCK_COUNT else SEVERITY_WARN - # List the dead ends themselves (0.1.4), not just a count — the + # List the dead ends themselves (0.1.4), not just a count -- the # whole point of memory-of-failure is telling the next session # WHAT not to retry. Up to 3 most recent, newest first. details = [] for attempt in reversed(failed_attempts[-3:]): details.append( - f"✗ {attempt.summary[:90]} ({_age(attempt.timestamp)})" + f"x {attempt.summary[:90]} ({_age(attempt.timestamp)})" ) if count > 3: details.append(f" ... and {count - 3} more (pjm search --failed-only)") @@ -276,7 +315,7 @@ def _analyze_files( "details": details, }) - # ── Check 2: Open issues ── + # -- Check 2: Open issues -- open_issues = _find_open_issues(file_events, events) if open_issues: warnings.append({ @@ -290,7 +329,7 @@ def _analyze_files( ], }) - # ── Check 3: High churn ── + # -- Check 3: High churn -- # Source of truth is `git log` over the window, not the event log # (L-023a). Counting events would understate fresh, repeated edits # that the memory layer hasn't captured yet. @@ -309,7 +348,7 @@ def _analyze_files( ], }) - # ── Check 4: Recent reverts ── + # -- Check 4: Recent reverts -- reverts = [ e for e in recent if e.type == "attempt" and e.outcome == "failed" @@ -328,7 +367,7 @@ def _analyze_files( ], }) - # ── Check 5: Recent decisions ── + # -- Check 5: Recent decisions -- decisions = [e for e in recent if e.type == "decision"] if decisions: last = decisions[-1] @@ -343,7 +382,7 @@ def _analyze_files( ], }) - # ── Check 6: Possibly-stale memories (0.1.4) ── + # -- Check 6: Possibly-stale memories (0.1.4) -- stale_items = stale_by_file.get(file_path, []) if stale_items: details = [] @@ -425,22 +464,23 @@ def _render_warnings(warnings: list[dict[str, Any]], level: str) -> None: # Filter by severity level visible = [ w for w in warnings - if SEVERITY_LEVELS.get(w["severity"], 1) >= severity_threshold - 1 # always show at level-1+ + if SEVERITY_LEVELS.get(w["severity"], 1) >= severity_threshold - 1 ] - # Always show warn+ regardless of level + + # Always show warn+ regardless of level. At info level, show everything. visible = [ w for w in warnings - if SEVERITY_LEVELS.get(w["severity"], 1) >= 1 # warn or block + if SEVERITY_LEVELS.get(w["severity"], 1) >= 1 or level == SEVERITY_INFO ] if not visible: return - typer.echo("") - typer.echo(f"{bold}projectmem: Pre-Commit Check{reset}") - typer.echo(f"{dim}{'─' * 60}{reset}") - typer.echo("") + _safe_echo("") + _safe_echo(f"{bold}projectmem: Pre-Commit Check{reset}") + _safe_echo(f"{dim}{_rule(60)}{reset}") + _safe_echo("") # Group by file by_file: dict[str, list[dict[str, Any]]] = defaultdict(list) @@ -448,7 +488,8 @@ def _render_warnings(warnings: list[dict[str, Any]], level: str) -> None: by_file[w["file"]].append(w) for file_path, file_warnings in by_file.items(): - typer.echo(f" {bold}{file_path}{reset}") + _safe_echo(f" {bold}{file_path}{reset}") + for w in file_warnings: if w["severity"] == SEVERITY_BLOCK: icon = f"{red}BLOCK{reset}" @@ -456,24 +497,28 @@ def _render_warnings(warnings: list[dict[str, Any]], level: str) -> None: icon = f"{yellow}WARN{reset}" else: icon = f"{cyan}INFO{reset}" - typer.echo(f" {icon} {w['title']}") + + _safe_echo(f" {icon} {w['title']}") + for detail in w["details"]: - typer.echo(f" {dim}{detail}{reset}") - typer.echo("") + _safe_echo(f" {dim}{detail}{reset}") - typer.echo(f"{dim}{'─' * 60}{reset}") + _safe_echo("") + + _safe_echo(f"{dim}{_rule(60)}{reset}") blocking = sum(1 for w in visible if w["severity"] == SEVERITY_BLOCK) warning = sum(1 for w in visible if w["severity"] == SEVERITY_WARN) if blocking and level == SEVERITY_BLOCK: - typer.echo(f"{red}Blocked: {blocking} critical warning(s).{reset}") - typer.echo(f" Bypass once: git commit --no-verify") + _safe_echo(f"{red}Blocked: {blocking} critical warning(s).{reset}") + _safe_echo(" Bypass once: git commit --no-verify") elif warning or blocking: - typer.echo( + _safe_echo( f"{dim}{warning + blocking} warning(s). Review before committing.{reset}" ) - typer.echo("") + + _safe_echo("") def _get_staged_files(root: Path) -> list[str]: @@ -486,6 +531,7 @@ def _get_staged_files(root: Path) -> list[str]: capture_output=True, text=True, timeout=5, + stdin=subprocess.DEVNULL, ) return [f.strip() for f in result.stdout.strip().split("\n") if f.strip()] except (OSError, subprocess.CalledProcessError, subprocess.TimeoutExpired): @@ -506,6 +552,7 @@ def _git_recent_changes(file_path: str, days: int, root: Path | None = None) -> capture_output=True, text=True, timeout=5, + stdin=subprocess.DEVNULL, ) except (OSError, subprocess.CalledProcessError, subprocess.TimeoutExpired): return None @@ -522,6 +569,7 @@ def _get_working_tree_files(root: Path) -> list[str]: capture_output=True, text=True, timeout=5, + stdin=subprocess.DEVNULL, ) return [f.strip() for f in result.stdout.strip().split("\n") if f.strip()] except (OSError, subprocess.CalledProcessError, subprocess.TimeoutExpired): diff --git a/src/projectmem/mcp_server.py b/src/projectmem/mcp_server.py index 01b50bc..8a172aa 100644 --- a/src/projectmem/mcp_server.py +++ b/src/projectmem/mcp_server.py @@ -153,8 +153,8 @@ def wrapper(*args, **kwargs): "DURING work — use MCP write tools, NEVER edit .projectmem/ files\n" "directly via filesystem write:\n" " - On a bug discovery → log_issue(summary, location).\n" - " - After each fix attempt → record_attempt(summary, outcome).\n" - " - After confirmation → record_fix(summary).\n" + " - After each fix attempt → _attempt(summary, outcome).\n" + " - After confirmation → use record_fix(summary) for the active issue. If fixing a specific older issue, use record_fix(summary, issue_id=\"\") and replace with the actual Projectmem issue ID.\n" " - On a design choice → add_decision(summary).\n" " - On a gotcha / setup detail → add_note(summary).\n" "Editing .projectmem/summary.md or .projectmem/PROJECT_MAP.md\n" @@ -509,26 +509,46 @@ def record_attempt( @mcp.tool() @safe_tool def record_fix( - summary: Annotated[str, Field( - description="One-line description of the confirmed fix (e.g., " - "'guarded submit handler with isSubmitting ref')." - )], - location: Annotated[Optional[str], Field( - description="Optional file path or component where the fix was " - "applied." - )] = None, + summary: Annotated[ + str, + Field( + description=( + "One-line description of the confirmed fix " + "(e.g., 'guarded submit handler with isSubmitting ref')." + ) + ), + ], + location: Annotated[ + Optional[str], + Field( + description=( + "Optional file path or component where the fix was applied." + ) + ), + ] = None, + issue_id: Annotated[ + Optional[str], + Field( + description=( + "Optional zero-padded issue ID (e.g., '0042') to close. " + "When omitted, closes the active issue. Numeric strings without " + "padding are accepted." + ) + ), + ] = None, ) -> str: - """Record a confirmed fix and close the current issue. + """Record a confirmed fix and close an issue. + + Only call AFTER you have evidence the fix works: test passes, error is gone, + or the user confirmed. - Only call AFTER you have evidence the fix works (test passes, error - gone, user confirmed). Closing the issue clears the active-issue - marker so the next record_attempt won't silently latch onto this - closed issue (L-027a). + If `issue_id` is provided, the fix is attached to that specific issue. + If `issue_id` is omitted, the active issue is closed. - Side effects: appends a `fix` event, closes the active issue, and - clears the active-issue marker so the next record_attempt won't - silently latch onto a closed issue.""" - event = fix.run(summary, location=location) + Side effects: appends a `fix` event and updates summary.md. The active-issue + marker is cleared only when the active issue is the issue being fixed. + """ + event = fix.run(summary, location=location, issue=issue_id) return f"Fixed issue #{event.issue_id}: {summary}" diff --git a/src/projectmem/staleness.py b/src/projectmem/staleness.py index 0060703..1723817 100644 --- a/src/projectmem/staleness.py +++ b/src/projectmem/staleness.py @@ -57,6 +57,7 @@ def commits_touching_since( capture_output=True, text=True, timeout=5, + stdin=subprocess.DEVNULL, ) except (OSError, subprocess.CalledProcessError, subprocess.TimeoutExpired): return None diff --git a/src/projectmem/storage.py b/src/projectmem/storage.py index cf84b9a..8a6479a 100644 --- a/src/projectmem/storage.py +++ b/src/projectmem/storage.py @@ -553,8 +553,10 @@ def get_git_commit(root: Path | None = None) -> str | None: check=True, capture_output=True, text=True, + timeout=5, + stdin=subprocess.DEVNULL, ) - except (OSError, subprocess.CalledProcessError): + except (OSError, subprocess.CalledProcessError, subprocess.TimeoutExpired): return None commit = result.stdout.strip() return commit or None diff --git a/tests/test_hooks_path.py b/tests/test_hooks_path.py index bdf2994..873b1ff 100644 --- a/tests/test_hooks_path.py +++ b/tests/test_hooks_path.py @@ -80,12 +80,43 @@ def fake_pjm(tmp_path: Path) -> Path: return pjm +def _require_launchable_bash() -> str: + """Return a bash executable path, or skip when this host cannot run one.""" + bash = shutil.which("bash") + if not bash: + pytest.skip("bash is not available") + + try: + result = subprocess.run( + [bash, "--version"], + capture_output=True, + text=True, + timeout=10, + stdin=subprocess.DEVNULL, + ) + except (OSError, subprocess.TimeoutExpired) as exc: + pytest.skip(f"bash is not launchable: {exc}") + + if result.returncode != 0: + reason = (result.stderr or result.stdout or "").strip() + pytest.skip(f"bash is not launchable: {reason[:120]}") + + return bash + + +def _bash_failure_reason(result: subprocess.CompletedProcess[str]) -> str: + """Normalize Windows bash/WSL startup errors for readable skip messages.""" + return ((result.stderr or "") + "\n" + (result.stdout or "")).replace("\x00", "").strip() + + def test_hook_runs_under_stripped_path(tmp_path: Path, fake_pjm: Path) -> None: """Simulate git's non-interactive hook environment: PATH = /usr/bin only. The hook must still find pjm via the baked absolute path, NOT via `command -v`. This is the exact failure mode L-047 fixes. """ + bash = _require_launchable_bash() + # Lay out a repo with .projectmem so the hook's directory check passes. repo = tmp_path / "repo" repo.mkdir() @@ -103,16 +134,24 @@ def test_hook_runs_under_stripped_path(tmp_path: Path, fake_pjm: Path) -> None: "PATH": "/usr/bin:/bin", "HOME": str(tmp_path), } + # Resolve the test harness shell before stripping PATH; the stripped PATH is + # the condition being tested inside the hook process. result = subprocess.run( - ["bash", str(hook)], + [bash, str(hook)], cwd=repo, env=minimal_env, capture_output=True, text=True, timeout=10, + stdin=subprocess.DEVNULL, ) log_path = fake_pjm.parent.parent / "pjm.log" - assert result.returncode == 0, f"hook errored: {result.stderr}" + if result.returncode != 0: + reason = _bash_failure_reason(result) + lowered = reason.lower() + if "wsl" in lowered or "bash/" in lowered: + pytest.skip(f"bash is not launchable for hook scripts: {reason[:120]}") + assert result.returncode == 0, f"hook errored: {reason}" assert log_path.exists(), ( "hook did not invoke pjm — this is the L-047 regression " f"(stripped PATH = {minimal_env['PATH']!r}). stderr: {result.stderr!r}" diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py new file mode 100644 index 0000000..a8f3a83 --- /dev/null +++ b/tests/test_mcp_server.py @@ -0,0 +1,214 @@ +from __future__ import annotations + +import json +import os +import queue +import subprocess +import sys +import threading +import time +from pathlib import Path +from typing import Any + +import pytest + +from projectmem.models import Event +from projectmem.storage import append_event, initialize + + +def _completed(args: list[str], stdout: str = "") -> subprocess.CompletedProcess[str]: + return subprocess.CompletedProcess(args, 0, stdout=stdout, stderr="") + + +def test_git_helpers_detach_stdin_and_keep_timeouts(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + from projectmem import storage, staleness + from projectmem.commands import context, precheck + + calls: list[dict[str, Any]] = [] + + def fake_run(args: list[str], **kwargs: Any) -> subprocess.CompletedProcess[str]: + calls.append({"args": args, **kwargs}) + if args[:2] == ["git", "status"]: + return _completed(args, "XY README.md\n") + if args[:2] == ["git", "rev-parse"]: + return _completed(args, "abc123\n") + return _completed(args, "abc msg\n") + + monkeypatch.setattr(storage.subprocess, "run", fake_run) + monkeypatch.setattr(staleness.subprocess, "run", fake_run) + monkeypatch.setattr(precheck.subprocess, "run", fake_run) + monkeypatch.setattr(context.subprocess, "run", fake_run) + + assert storage.get_git_commit(tmp_path) == "abc123" + assert staleness.commits_touching_since("README.md", "2026-01-01T00:00:00Z", tmp_path) == 1 + assert precheck._git_recent_changes("README.md", 30, tmp_path) == 1 + assert precheck._get_staged_files(tmp_path) == ["abc msg"] + assert precheck._get_working_tree_files(tmp_path) == ["abc msg"] + assert context._get_git_status_files(tmp_path) == ["README.md"] + + assert calls + assert all(call.get("stdin") is subprocess.DEVNULL for call in calls) + assert all(call.get("timeout") == 5 for call in calls) + + +def _git(repo: Path, *args: str) -> None: + subprocess.run( + ["git", *args], + cwd=repo, + check=True, + capture_output=True, + text=True, + stdin=subprocess.DEVNULL, + ) + + +def _make_git_project(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + monkeypatch.setenv("HOME", str(tmp_path / "home")) + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init") + _git(repo, "config", "user.email", "t@t.test") + _git(repo, "config", "user.name", "t") + (repo / "README.md").write_text("# demo\n", encoding="utf-8") + _git(repo, "add", "README.md") + _git(repo, "commit", "-m", "base") + initialize(repo) + return repo + + +def _mcp_env(repo_root: Path, tmp_path: Path) -> dict[str, str]: + src_path = str(repo_root / "src") + existing = os.environ.get("PYTHONPATH") + return { + **os.environ, + "HOME": str(tmp_path / "home"), + "PYTHONPATH": src_path if not existing else src_path + os.pathsep + existing, + } + + +def _call_mcp_tool( + project: Path, + tmp_path: Path, + name: str, + arguments: dict[str, Any], + timeout: float = 6.0, +) -> dict[str, Any]: + repo_root = Path(__file__).resolve().parents[1] + proc = subprocess.Popen( + [sys.executable, "-m", "projectmem.mcp_server", "--root", str(project)], + cwd=project, + env=_mcp_env(repo_root, tmp_path), + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + encoding="utf-8", + errors="replace", + ) + stdout_q: queue.Queue[str] = queue.Queue() + + def pump_stdout() -> None: + assert proc.stdout is not None + for line in proc.stdout: + stdout_q.put(line.rstrip("\n")) + + thread = threading.Thread(target=pump_stdout, daemon=True) + thread.start() + + def send(message: dict[str, Any]) -> None: + assert proc.stdin is not None + proc.stdin.write(json.dumps(message) + "\n") + proc.stdin.flush() + + def read_response(message_id: int, wait_seconds: float) -> dict[str, Any] | None: + deadline = time.time() + wait_seconds + while time.time() < deadline: + try: + line = stdout_q.get(timeout=0.1) + except queue.Empty: + continue + try: + payload = json.loads(line) + except json.JSONDecodeError: + continue + if payload.get("id") == message_id: + return payload + return None + + try: + send( + { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": "2024-11-05", + "capabilities": {}, + "clientInfo": {"name": "projectmem-test", "version": "0"}, + }, + } + ) + initialized = read_response(1, timeout) + assert initialized is not None + send({"jsonrpc": "2.0", "method": "notifications/initialized"}) + send( + { + "jsonrpc": "2.0", + "id": 2, + "method": "tools/call", + "params": {"name": name, "arguments": arguments}, + } + ) + response = read_response(2, timeout) + if response is None: + pytest.fail(f"MCP tool {name} did not respond within {timeout}s") + return response + finally: + proc.terminate() + try: + proc.wait(timeout=5) + except subprocess.TimeoutExpired: + proc.kill() + + +def _tool_text(response: dict[str, Any]) -> str: + content = response["result"]["content"] + return "\n".join(item.get("text", "") for item in content) + + +def test_mcp_stdio_precheck_file_returns_with_git_backed_analysis( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + project = _make_git_project(tmp_path, monkeypatch) + append_event( + Event(type="note", summary="README precheck regression fixture", location="README.md"), + project, + ) + + response = _call_mcp_tool( + project, + tmp_path, + "precheck_file", + {"file_path": "README.md"}, + ) + + assert response["result"]["isError"] is False + assert "README.md" in _tool_text(response) + + +def test_mcp_stdio_add_note_returns_and_writes_event( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + project = _make_git_project(tmp_path, monkeypatch) + + response = _call_mcp_tool( + project, + tmp_path, + "add_note", + {"summary": "MCP smoke note from pytest"}, + ) + + assert response["result"]["isError"] is False + assert "Recorded note" in _tool_text(response) + events_text = (project / ".projectmem" / "events.jsonl").read_text(encoding="utf-8") + assert "MCP smoke note from pytest" in events_text diff --git a/tests/test_v014_features.py b/tests/test_v014_features.py index c80e897..c0e60ca 100644 --- a/tests/test_v014_features.py +++ b/tests/test_v014_features.py @@ -39,7 +39,12 @@ def _event_ids(tmp_path) -> list[str]: def _git(tmp_path, *args): subprocess.run( - ["git", *args], cwd=tmp_path, check=True, capture_output=True, text=True + ["git", *args], + cwd=tmp_path, + check=True, + capture_output=True, + text=True, + stdin=subprocess.DEVNULL, ) @@ -275,6 +280,19 @@ def test_brief_runs_clean_on_fresh_project(tmp_path, monkeypatch): assert "none" in result.output # empty sections degrade gracefully +def test_brief_console_helpers_are_cp1252_safe(monkeypatch): + from projectmem.commands import brief + + class Cp1252Stdout: + encoding = "cp1252" + + monkeypatch.setattr(brief.sys, "stdout", Cp1252Stdout()) + + assert brief._rule() == "-" * 60 + text = brief._console_safe("⚠ ─ projectmem brief — projectmem 📈") + text.encode("cp1252") + + # ── 5. Failed-approach surfacing ─────────────────────────────────────