From 38a56f2bc922fc7fd618575a615e3659b6921bd6 Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 07:29:38 +0530 Subject: [PATCH 01/21] =?UTF-8?q?feat(github):=20GraphQL=20thread=20API=20?= =?UTF-8?q?=E2=80=94=20list/resolve/unresolve/reply=20+=20thread=5Fid=20on?= =?UTF-8?q?=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/canopy/integrations/github.py | 216 +++++++++++++++++++++++++++++- tests/test_thread_graphql.py | 79 +++++++++++ 2 files changed, 290 insertions(+), 5 deletions(-) create mode 100644 tests/test_thread_graphql.py diff --git a/src/canopy/integrations/github.py b/src/canopy/integrations/github.py index 53ae33e..af3963a 100644 --- a/src/canopy/integrations/github.py +++ b/src/canopy/integrations/github.py @@ -666,22 +666,222 @@ def _normalize_pr(data: dict) -> dict: } +def _graphql_via_mcp(workspace_root: Path, query: str, vars: dict) -> dict: + """Run a GraphQL call via the configured GitHub MCP server. + + Most GitHub MCP servers don't expose raw GraphQL, so this is best-effort. + Raises McpClientError (or any exception) on failure so callers fall through. + """ + config = _get_github_config(workspace_root) + result = call_tool( + config, "graphql", + {"query": query, "variables": vars}, + timeout=15.0, server_name="github", + ) + parsed = _parse_mcp_result(result) + if parsed is None: + raise McpClientError("graphql tool returned no data") + return parsed + + +def _graphql_via_gh_cli(query: str, vars: dict) -> dict: + """Run a GraphQL call via ``gh api graphql``. + + Variables are passed as ``-F name=value`` (typed integers stay integers). + Returns the parsed JSON response body. + """ + args = ["gh", "api", "graphql", "-f", f"query={query}"] + for k, v in vars.items(): + args.extend(["-F", f"{k}={v}"]) + proc = subprocess.run(args, capture_output=True, text=True) + if proc.returncode != 0: + raise GitHubNotConfiguredError( + f"gh api graphql failed: {proc.stderr.strip()}" + ) + return json.loads(proc.stdout) + + +def _graphql(workspace_root: Path, query: str, **vars) -> dict: + """Run a GraphQL call via MCP if configured, otherwise via gh CLI. + + Returns the parsed JSON response body. + MCP failure falls through silently to gh CLI. + """ + if is_mcp_configured(workspace_root, "github"): + try: + return _graphql_via_mcp(workspace_root, query, vars) + except Exception: + pass # fall through to gh CLI + return _graphql_via_gh_cli(query, vars) + + +def list_review_threads( + workspace_root: Path, + owner: str, + repo: str, + pr_number: int, +) -> list[dict]: + """Return every review thread on the PR with node IDs and resolution state. + + GraphQL because REST /pulls//comments doesn't surface thread IDs. + + Returns: + [{thread_id, is_resolved, resolved_at, comments: [{ + comment_id, path, line, body, author, created_at, url + }]}, ...] + """ + query = """ + query($owner: String!, $repo: String!, $number: Int!) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { + reviewThreads(first: 100) { + nodes { + id + isResolved + resolvedAt + comments(first: 20) { + nodes { + databaseId path line body createdAt url + author { login } + } + } + } + } + } + } + } + """ + data = _graphql(workspace_root, query, owner=owner, repo=repo, number=pr_number) + nodes = ( + data.get("data", {}) + .get("repository", {}) + .get("pullRequest", {}) + .get("reviewThreads", {}) + .get("nodes") or [] + ) + out: list[dict] = [] + for n in nodes: + comments = [ + { + "comment_id": c.get("databaseId"), + "path": c.get("path"), + "line": c.get("line"), + "body": c.get("body"), + "created_at": c.get("createdAt"), + "url": c.get("url"), + "author": (c.get("author") or {}).get("login", ""), + } + for c in (n.get("comments") or {}).get("nodes", []) + ] + out.append({ + "thread_id": n["id"], + "is_resolved": bool(n.get("isResolved")), + "resolved_at": n.get("resolvedAt"), + "comments": comments, + }) + return out + + +def resolve_thread(workspace_root: Path, thread_id: str) -> dict: + """Resolve a GitHub PR review thread by its node ID.""" + query = ( + "mutation($id: ID!) { resolveReviewThread(input: {threadId: $id})" + " { thread { id isResolved } } }" + ) + data = _graphql(workspace_root, query, id=thread_id) + thread = data.get("data", {}).get("resolveReviewThread", {}).get("thread") or {} + return {"thread_id": thread.get("id"), "is_resolved": bool(thread.get("isResolved"))} + + +def unresolve_thread(workspace_root: Path, thread_id: str) -> dict: + """Unresolve a GitHub PR review thread by its node ID.""" + query = ( + "mutation($id: ID!) { unresolveReviewThread(input: {threadId: $id})" + " { thread { id isResolved } } }" + ) + data = _graphql(workspace_root, query, id=thread_id) + thread = data.get("data", {}).get("unresolveReviewThread", {}).get("thread") or {} + return {"thread_id": thread.get("id"), "is_resolved": bool(thread.get("isResolved"))} + + +def reply_to_thread(workspace_root: Path, thread_id: str, body: str) -> dict: + """Post a reply to a GitHub PR review thread.""" + query = ( + "mutation($id: ID!, $body: String!) {" + " addPullRequestReviewThreadReply(" + " input: {pullRequestReviewThreadId: $id, body: $body}) {" + " comment { id url } } }" + ) + data = _graphql(workspace_root, query, id=thread_id, body=body) + comment = ( + data.get("data", {}) + .get("addPullRequestReviewThreadReply", {}) + .get("comment") or {} + ) + return {"comment_id": comment.get("id"), "url": comment.get("url", "")} + + +def _build_comments_from_threads(threads: list[dict]) -> tuple[list[dict], int]: + """Build a normalized comment list from list_review_threads output. + + Threads where is_resolved is True contribute to resolved_count and their + comments are excluded (matching the existing _normalize_comments behavior). + Each comment dict carries thread_id plus the standard normalized fields. + """ + comments: list[dict] = [] + resolved_count = 0 + for t in threads: + if t["is_resolved"]: + resolved_count += len(t["comments"]) + continue + for c in t["comments"]: + comments.append({ + "id": c["comment_id"], + "path": c["path"] or "", + "line": c["line"] or 0, + "body": c["body"] or "", + "author": c["author"], + "author_type": "", + "state": "", + "created_at": c["created_at"] or "", + "url": c["url"] or "", + "in_reply_to_id": None, + "commit_id": "", + "thread_id": t["thread_id"], + }) + return comments, resolved_count + + def get_review_comments( workspace_root: Path, owner: str, repo: str, pr_number: int, ) -> tuple[list[dict], int]: - """Fetch review comments for a PR. MCP first; gh CLI fallback. + """Fetch review comments for a PR. GraphQL first; MCP/gh CLI fallback. Returns ``(comments, resolved_count)``: comments are normalized with fields ``path, line, body, author, author_type, state, created_at, - url, in_reply_to_id``. ``resolved_count`` is the number of threads - excluded because GitHub flagged them resolved. + url, in_reply_to_id, commit_id, thread_id``. ``resolved_count`` is the + number of threads excluded because GitHub flagged them resolved. + + When GraphQL succeeds it is the sole source of truth (one round-trip, + thread_id on every comment). When GraphQL fails, falls back to the MCP + three-tool ladder and gh CLI REST path; those comments get thread_id="". Bot threads are kept (the temporal classifier downstream handles staleness). If neither path is available, returns ``([], 0)``. """ + # Try GraphQL first — gets threads + comments + thread_id in one shot. + if is_github_configured(workspace_root): + try: + threads = list_review_threads(workspace_root, owner, repo, pr_number) + return _build_comments_from_threads(threads) + except Exception: + pass # fall through to legacy path — never crash on GraphQL failure + + # Legacy fallback: MCP three-tool ladder then gh CLI REST. + # Decorate each comment with thread_id="" so callers always have the key. if is_mcp_configured(workspace_root, "github"): config = _get_github_config(workspace_root) for tool_name, args in [ @@ -693,7 +893,10 @@ def get_review_comments( result = call_tool(config, tool_name, args, timeout=15.0, server_name="github") parsed = _parse_mcp_result(result) if parsed is not None: - return _normalize_comments(parsed) + comments, resolved_count = _normalize_comments(parsed) + for c in comments: + c.setdefault("thread_id", "") + return comments, resolved_count except McpClientError: continue return [], 0 @@ -705,7 +908,10 @@ def get_review_comments( "--paginate", ]) data = json.loads(output) if output.strip() else [] - return _normalize_comments(data) + comments, resolved_count = _normalize_comments(data) + for c in comments: + c.setdefault("thread_id", "") + return comments, resolved_count except (GitHubNotConfiguredError, json.JSONDecodeError): return [], 0 diff --git a/tests/test_thread_graphql.py b/tests/test_thread_graphql.py new file mode 100644 index 0000000..0ee086f --- /dev/null +++ b/tests/test_thread_graphql.py @@ -0,0 +1,79 @@ +""" +Tests for the GraphQL thread API added in T1. + +Four tests covering: +1. list_review_threads — returns structured thread + comment data with thread IDs +2. resolve_thread mutation — gh CLI invoked with correct shape +3. reply_to_thread mutation — returns comment url +4. get_review_comments — includes thread_id on each comment dict +""" +from unittest.mock import patch, MagicMock +import json +import pytest + + +def _mock_graphql_response(response_body): + """Patch the subprocess call gh api graphql makes.""" + proc = MagicMock(returncode=0, stdout=json.dumps(response_body), stderr="") + return patch("subprocess.run", return_value=proc) + + +def test_list_review_threads_returns_thread_ids(tmp_path): + fake = { + "data": {"repository": {"pullRequest": {"reviewThreads": {"nodes": [ + {"id": "PRRT_abc", "isResolved": False, "resolvedAt": None, + "comments": {"nodes": [{ + "databaseId": 1, "path": "a.py", "line": 1, + "body": "fix this", "author": {"login": "cursor"}, + "createdAt": "2026-05-20T10:00:00Z", + "url": "https://github.com/o/r/pull/1#x"}]}}, + {"id": "PRRT_def", "isResolved": True, "resolvedAt": "2026-05-25T11:30:00Z", + "comments": {"nodes": [{ + "databaseId": 2, "path": "b.py", "line": 2, + "body": "done", "author": {"login": "human"}, + "createdAt": "2026-05-21T10:00:00Z", + "url": "https://github.com/o/r/pull/1#y"}]}}, + ]}}}} + } + with _mock_graphql_response(fake): + from canopy.integrations.github import list_review_threads + threads = list_review_threads(tmp_path, "o", "r", 1) + assert len(threads) == 2 + assert threads[0]["thread_id"] == "PRRT_abc" + assert threads[0]["is_resolved"] is False + assert threads[1]["resolved_at"] == "2026-05-25T11:30:00Z" + + +def test_resolve_thread_mutation_called(tmp_path): + fake = {"data": {"resolveReviewThread": {"thread": {"id": "PRRT_abc", "isResolved": True}}}} + with _mock_graphql_response(fake) as m: + from canopy.integrations.github import resolve_thread + result = resolve_thread(tmp_path, "PRRT_abc") + assert result["is_resolved"] is True + # Verify the gh CLI was invoked with the right shape + args = m.call_args.args[0] + assert "graphql" in args + assert "resolveReviewThread" in " ".join(args) + + +def test_reply_to_thread_mutation_called(tmp_path): + fake = {"data": {"addPullRequestReviewThreadReply": {"comment": { + "id": "C_1", "url": "https://github.com/o/r/pull/1#r"}}}} + with _mock_graphql_response(fake): + from canopy.integrations.github import reply_to_thread + result = reply_to_thread(tmp_path, "PRRT_abc", "Tracking in DOC-1.") + assert result["url"].endswith("#r") + + +def test_get_review_comments_includes_thread_id(tmp_path): + """After T1, every comment dict has a thread_id field.""" + fake_threads = {"data": {"repository": {"pullRequest": {"reviewThreads": {"nodes": [ + {"id": "PRRT_abc", "isResolved": False, "resolvedAt": None, "comments": {"nodes": [ + {"databaseId": 1, "path": "a.py", "line": 1, "body": "b", + "author": {"login": "u"}, "createdAt": "2026-05-20T10:00:00Z", + "url": "https://github.com/o/r/pull/1#x"}]}}, + ]}}}}} + with _mock_graphql_response(fake_threads): + from canopy.integrations.github import get_review_comments + comments, resolved_count = get_review_comments(tmp_path, "o", "r", 1) + assert comments[0]["thread_id"] == "PRRT_abc" From 6ceed0d56c64cfd6e7d7c24034ccfda00156a92a Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 07:37:10 +0530 Subject: [PATCH 02/21] =?UTF-8?q?feat(actions):=20canopy=20resolve=20=20=E2=80=94=20close=20+=20log?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds actions/thread_resolutions.py (atomic JSON log), actions/thread_actions.py (resolve_thread + reply_to_thread wrappers calling GH + recording locally), `canopy resolve ` CLI subcommand, and mcp__canopy__resolve_thread MCP tool. 12 new tests, full suite 770 passed. --- src/canopy/actions/thread_actions.py | 85 +++++++++ src/canopy/actions/thread_resolutions.py | 101 +++++++++++ src/canopy/cli/main.py | 61 +++++++ src/canopy/mcp/server.py | 29 ++++ tests/test_thread_actions.py | 208 +++++++++++++++++++++++ 5 files changed, 484 insertions(+) create mode 100644 src/canopy/actions/thread_actions.py create mode 100644 src/canopy/actions/thread_resolutions.py create mode 100644 tests/test_thread_actions.py diff --git a/src/canopy/actions/thread_actions.py b/src/canopy/actions/thread_actions.py new file mode 100644 index 0000000..1d8dac9 --- /dev/null +++ b/src/canopy/actions/thread_actions.py @@ -0,0 +1,85 @@ +"""Thread action wrappers — resolve, reply, unresolve a GitHub review thread. + +Each wrapper calls the GitHub integration and records the event locally in +``.canopy/state/thread_resolutions.json`` so the resume brief can attribute +"resolved by canopy" vs "resolved on GitHub directly". +""" +from __future__ import annotations + +from ..workspace.workspace import Workspace +from .errors import BlockerError +from . import thread_resolutions as tr + + +def _validate_thread_id(thread_id: str) -> None: + if not thread_id.startswith("PRRT_"): + raise BlockerError( + code="invalid_thread_id", + what=f"thread_id must start with 'PRRT_'; got {thread_id!r}", + ) + + +def resolve_thread( + workspace: Workspace, + thread_id: str, + *, + feature: str, + via_command: str = "resolve", + via_commit_sha: str | None = None, +) -> dict: + """Resolve a GitHub PR review thread and record it locally. + + Steps: + 1. Validate thread_id format. + 2. Call the GitHub GraphQL mutation. + 3. Log the resolution to ``.canopy/state/thread_resolutions.json``. + 4. Return the combined result. + + Raises: + BlockerError: if ``thread_id`` does not start with ``PRRT_``. + """ + from ..integrations import github as gh + + _validate_thread_id(thread_id) + gh_result = gh.resolve_thread(workspace.config.root, thread_id) + log_entry = tr.record( + workspace.config.root, + thread_id=thread_id, + feature=feature, + via_command=via_command, + via_commit_sha=via_commit_sha, + ) + return {**gh_result, "logged": log_entry} + + +def reply_to_thread( + workspace: Workspace, + thread_id: str, + body: str, + *, + feature: str, + resolve_after: bool = False, +) -> dict: + """Post a reply to a GitHub PR review thread, optionally resolving it. + + Steps: + 1. Validate thread_id format. + 2. Post the reply via the GitHub GraphQL mutation. + 3. If ``resolve_after`` is True, resolve the thread and include the result. + + Returns a dict with ``posted`` and optionally ``resolved`` keys. + + Raises: + BlockerError: if ``thread_id`` does not start with ``PRRT_``. + """ + from ..integrations import github as gh + + _validate_thread_id(thread_id) + posted = gh.reply_to_thread(workspace.config.root, thread_id, body) + result: dict = {"posted": posted} + if resolve_after: + resolved = resolve_thread( + workspace, thread_id, feature=feature, via_command="reply_resolve", + ) + result["resolved"] = resolved + return result diff --git a/src/canopy/actions/thread_resolutions.py b/src/canopy/actions/thread_resolutions.py new file mode 100644 index 0000000..e6a89ce --- /dev/null +++ b/src/canopy/actions/thread_resolutions.py @@ -0,0 +1,101 @@ +"""Thread resolution log — persistent record of threads resolved via canopy. + +State file: .canopy/state/thread_resolutions.json (atomic temp+rename writes). + +Schema:: + + { + "PRRT_abc": { + "resolved_by_canopy_at": "2026-05-29T12:00:00Z", + "feature": "auth-flow", + "via_command": "resolve" | "commit_address" | "reply_resolve", + "via_commit_sha": "abc1234" | null + } + } + +Used by the resume brief to attribute "resolved by canopy" vs "resolved on +GitHub directly" — only canopy-initiated resolutions appear here. +""" +from __future__ import annotations + +import json +import os +import tempfile +from datetime import datetime, timezone +from pathlib import Path + + +def _state_path(workspace_root: Path) -> Path: + return workspace_root / ".canopy" / "state" / "thread_resolutions.json" + + +def _now_iso() -> str: + return datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + + +def load(workspace_root: Path) -> dict: + """Return the full log dict. Missing file returns an empty dict.""" + path = _state_path(workspace_root) + if not path.exists(): + return {} + try: + data = json.loads(path.read_text()) + if isinstance(data, dict): + return data + except (OSError, json.JSONDecodeError): + pass + return {} + + +def record( + workspace_root: Path, + *, + thread_id: str, + feature: str, + via_command: str, + via_commit_sha: str | None = None, +) -> dict: + """Append or overwrite a thread resolution entry. Returns the entry written. + + Atomic write: mkstemp in same dir → write → os.replace. + """ + log = load(workspace_root) + entry = { + "resolved_by_canopy_at": _now_iso(), + "feature": feature, + "via_command": via_command, + "via_commit_sha": via_commit_sha, + } + log[thread_id] = entry + + path = _state_path(workspace_root) + path.parent.mkdir(parents=True, exist_ok=True) + + fd, tmp_path = tempfile.mkstemp(dir=path.parent, suffix=".json.tmp") + try: + with os.fdopen(fd, "w") as fh: + json.dump(log, fh, indent=2) + os.replace(tmp_path, path) + except Exception: + try: + os.unlink(tmp_path) + except OSError: + pass + raise + + return entry + + +def filter_since(workspace_root: Path, since_iso: str) -> dict: + """Return only entries with ``resolved_by_canopy_at`` >= ``since_iso``. + + Comparison is lexicographic on the ISO 8601 Z strings (safe because they + are always produced in the same zero-padded format by ``_now_iso``). + """ + log = load(workspace_root) + return { + tid: entry + for tid, entry in log.items() + if isinstance(entry, dict) + and entry.get("resolved_by_canopy_at", "") >= since_iso + } diff --git a/src/canopy/cli/main.py b/src/canopy/cli/main.py index ff9ac5c..13b86df 100644 --- a/src/canopy/cli/main.py +++ b/src/canopy/cli/main.py @@ -3115,6 +3115,57 @@ def cmd_doctor(args: argparse.Namespace) -> None: console.print() +def _resolve_thread_feature(workspace, feature: str | None) -> str: + """Return a feature name for thread commands, falling back to canonical.""" + from ..actions import slots as slots_mod + from ..actions.aliases import resolve_feature + from ..actions.errors import BlockerError + if feature: + return resolve_feature(workspace, feature) + state = slots_mod.read_state(workspace) + if state is None or state.canonical is None: + raise BlockerError( + code="no_canonical_feature", + what="no active feature; pass --feature or run `canopy switch ` first", + ) + return state.canonical.feature + + +def cmd_resolve_thread(args: argparse.Namespace) -> None: + """Resolve a GitHub PR review thread and record the resolution locally.""" + from ..actions.thread_actions import resolve_thread + from ..actions.errors import ActionError + from .render import render_blocker + from .ui import console + + workspace = _load_workspace() + try: + feature = _resolve_thread_feature(workspace, getattr(args, "feature", None)) + except ActionError as err: + if args.json: + _print_json(err.to_dict()) + else: + render_blocker(err, action="resolve") + sys.exit(1) + + try: + result = resolve_thread(workspace, args.thread_id, feature=feature) + except ActionError as err: + if args.json: + _print_json(err.to_dict()) + else: + render_blocker(err, action="resolve") + sys.exit(1) + + if args.json: + _print_json(result) + return + + console.print() + console.print(f" Resolved [info]{args.thread_id}[/]") + console.print() + + def cmd_context(args: argparse.Namespace) -> None: """Show detected canopy context for current directory (debug).""" from ..workspace.context import detect_context @@ -3695,6 +3746,15 @@ def main() -> None: ) doctor_p.add_argument("--json", action="store_true", help="Output as JSON") + resolve_p = subparsers.add_parser( + "resolve", + help="Resolve a GitHub PR review thread and record it locally", + ) + resolve_p.add_argument("thread_id", help="Thread node ID (starts with PRRT_)") + resolve_p.add_argument("--feature", default=None, + help="Feature alias; defaults to canonical feature") + resolve_p.add_argument("--json", action="store_true", help="Output as JSON") + args = parser.parse_args() if not args.command: @@ -3740,6 +3800,7 @@ def main() -> None: "ship": cmd_ship, "worktree-bootstrap": cmd_worktree_bootstrap, "pr-checks": cmd_pr_checks, + "resolve": cmd_resolve_thread, } if args.command == "feature": diff --git a/src/canopy/mcp/server.py b/src/canopy/mcp/server.py index a82a75d..807aa52 100644 --- a/src/canopy/mcp/server.py +++ b/src/canopy/mcp/server.py @@ -788,6 +788,35 @@ def conflicts( ) +@mcp.tool() +def resolve_thread(thread_id: str, feature: str | None = None) -> dict: + """Resolve a GitHub PR review thread and record the resolution locally. + + Calls the GitHub GraphQL ``resolveReviewThread`` mutation and appends + an entry to ``.canopy/state/thread_resolutions.json`` so the resume + brief can distinguish threads closed by canopy from those resolved + directly on GitHub. + + Args: + thread_id: The GitHub review thread node ID (must start with + ``PRRT_``). + feature: Feature to attribute the resolution to. Defaults to the + canonical feature if not supplied. + """ + from ..actions.thread_actions import resolve_thread as _impl + from ..actions.errors import ActionError + + ws = _get_workspace() + try: + feat = _historian_feature(feature)[1] + except ActionError as e: + return e.to_dict() + try: + return _impl(ws, thread_id, feature=feat) + except ActionError as e: + return e.to_dict() + + @mcp.tool() def drift(feature: str | None = None) -> dict: """Compare recorded HEAD state vs feature lane expectations. diff --git a/tests/test_thread_actions.py b/tests/test_thread_actions.py new file mode 100644 index 0000000..94ef5bf --- /dev/null +++ b/tests/test_thread_actions.py @@ -0,0 +1,208 @@ +"""Tests for actions/thread_actions.py and actions/thread_resolutions.py.""" +import json +from pathlib import Path + +import pytest + +from canopy.actions import thread_resolutions as tr +from canopy.actions.errors import BlockerError +from canopy.workspace.config import load_config +from canopy.workspace.workspace import Workspace + + +# ── thread_resolutions helpers ─────────────────────────────────────────── + + +def test_load_returns_empty_when_missing(canopy_toml): + result = tr.load(canopy_toml) + assert result == {} + + +def test_record_and_load_roundtrip(canopy_toml): + entry = tr.record( + canopy_toml, + thread_id="PRRT_abc123", + feature="auth-flow", + via_command="resolve", + ) + assert entry["feature"] == "auth-flow" + assert entry["via_command"] == "resolve" + assert entry["via_commit_sha"] is None + assert "resolved_by_canopy_at" in entry + + log = tr.load(canopy_toml) + assert "PRRT_abc123" in log + assert log["PRRT_abc123"]["feature"] == "auth-flow" + + +def test_record_with_commit_sha(canopy_toml): + entry = tr.record( + canopy_toml, + thread_id="PRRT_xyz", + feature="my-feat", + via_command="commit_address", + via_commit_sha="deadbeef", + ) + assert entry["via_commit_sha"] == "deadbeef" + log = tr.load(canopy_toml) + assert log["PRRT_xyz"]["via_commit_sha"] == "deadbeef" + + +def test_record_is_last_write_wins(canopy_toml): + tr.record(canopy_toml, thread_id="PRRT_abc", feature="feat-a", via_command="resolve") + tr.record(canopy_toml, thread_id="PRRT_abc", feature="feat-b", via_command="reply_resolve") + log = tr.load(canopy_toml) + assert log["PRRT_abc"]["feature"] == "feat-b" + assert log["PRRT_abc"]["via_command"] == "reply_resolve" + + +def test_thread_resolutions_filter_since(canopy_toml): + tr.record(canopy_toml, thread_id="PRRT_old", feature="f", via_command="resolve") + tr.record(canopy_toml, thread_id="PRRT_new", feature="f", via_command="resolve") + + log = tr.load(canopy_toml) + # Both entries have timestamps >= epoch + filtered = tr.filter_since(canopy_toml, "2000-01-01T00:00:00Z") + assert "PRRT_old" in filtered + assert "PRRT_new" in filtered + + # Filtering with a far-future cutoff should return nothing + empty = tr.filter_since(canopy_toml, "2099-01-01T00:00:00Z") + assert empty == {} + + +def test_filter_since_partial(canopy_toml): + tr.record(canopy_toml, thread_id="PRRT_early", feature="f", via_command="resolve") + # Manually write a past timestamp for one entry + path = canopy_toml / ".canopy" / "state" / "thread_resolutions.json" + data = json.loads(path.read_text()) + data["PRRT_early"]["resolved_by_canopy_at"] = "2020-01-01T00:00:00Z" + path.write_text(json.dumps(data)) + + tr.record(canopy_toml, thread_id="PRRT_recent", feature="f", via_command="resolve") + + filtered = tr.filter_since(canopy_toml, "2025-01-01T00:00:00Z") + assert "PRRT_early" not in filtered + assert "PRRT_recent" in filtered + + +# ── thread_actions wrappers ────────────────────────────────────────────── + + +def _make_workspace(canopy_toml): + return Workspace(load_config(canopy_toml)) + + +def test_resolve_thread_writes_log(canopy_toml, monkeypatch): + """resolve_thread calls gh integration and writes thread_resolutions.json.""" + from canopy.actions import thread_actions + + gh_result = {"thread_id": "PRRT_abc", "is_resolved": True} + monkeypatch.setattr( + "canopy.integrations.github.resolve_thread", + lambda root, tid: gh_result, + ) + + ws = _make_workspace(canopy_toml) + result = thread_actions.resolve_thread(ws, "PRRT_abc", feature="auth-flow") + + # Return value carries both gh result and log + assert result["is_resolved"] is True + assert "logged" in result + assert result["logged"]["feature"] == "auth-flow" + assert result["logged"]["via_command"] == "resolve" + + # State file written + log = tr.load(canopy_toml) + assert "PRRT_abc" in log + assert log["PRRT_abc"]["feature"] == "auth-flow" + assert log["PRRT_abc"]["via_command"] == "resolve" + + +def test_resolve_thread_idempotent(canopy_toml, monkeypatch): + """Calling resolve_thread twice is safe (last-write-wins, no crash).""" + from canopy.actions import thread_actions + + monkeypatch.setattr( + "canopy.integrations.github.resolve_thread", + lambda root, tid: {"thread_id": tid, "is_resolved": True}, + ) + + ws = _make_workspace(canopy_toml) + thread_actions.resolve_thread(ws, "PRRT_dup", feature="feat-a") + thread_actions.resolve_thread(ws, "PRRT_dup", feature="feat-a") + + log = tr.load(canopy_toml) + assert "PRRT_dup" in log + assert log["PRRT_dup"]["feature"] == "feat-a" + + +def test_resolve_thread_invalid_id(canopy_toml): + """thread_id that doesn't start with PRRT_ raises BlockerError.""" + from canopy.actions import thread_actions + + ws = _make_workspace(canopy_toml) + with pytest.raises(BlockerError) as exc_info: + thread_actions.resolve_thread(ws, "IC_badinput", feature="feat") + + err = exc_info.value + assert err.code == "invalid_thread_id" + assert err.STATUS == "blocked" + + +def test_reply_to_thread_invalid_id(canopy_toml): + """reply_to_thread also validates the thread_id.""" + from canopy.actions import thread_actions + + ws = _make_workspace(canopy_toml) + with pytest.raises(BlockerError) as exc_info: + thread_actions.reply_to_thread(ws, "BAD_ID", "hello", feature="feat") + + assert exc_info.value.code == "invalid_thread_id" + + +def test_reply_to_thread_posts_without_resolve(canopy_toml, monkeypatch): + """reply_to_thread posts a comment but does not resolve by default.""" + from canopy.actions import thread_actions + + posted_result = {"comment_id": "IC_xyz", "url": "https://github.com/..."} + monkeypatch.setattr( + "canopy.integrations.github.reply_to_thread", + lambda root, tid, body: posted_result, + ) + + ws = _make_workspace(canopy_toml) + result = thread_actions.reply_to_thread(ws, "PRRT_abc", "LGTM", feature="feat") + + assert result["posted"] == posted_result + assert "resolved" not in result + # Should NOT have written a resolution log + log = tr.load(canopy_toml) + assert "PRRT_abc" not in log + + +def test_reply_to_thread_resolve_after(canopy_toml, monkeypatch): + """reply_to_thread with resolve_after=True also resolves and logs.""" + from canopy.actions import thread_actions + + monkeypatch.setattr( + "canopy.integrations.github.reply_to_thread", + lambda root, tid, body: {"comment_id": "IC_1", "url": ""}, + ) + monkeypatch.setattr( + "canopy.integrations.github.resolve_thread", + lambda root, tid: {"thread_id": tid, "is_resolved": True}, + ) + + ws = _make_workspace(canopy_toml) + result = thread_actions.reply_to_thread( + ws, "PRRT_abc", "Done!", feature="my-feat", resolve_after=True, + ) + + assert "posted" in result + assert "resolved" in result + assert result["resolved"]["is_resolved"] is True + + log = tr.load(canopy_toml) + assert "PRRT_abc" in log + assert log["PRRT_abc"]["via_command"] == "reply_resolve" From 1b1034ebc0c080a454dd9553fcf60babe6d1cd0e Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 07:46:03 +0530 Subject: [PATCH 03/21] =?UTF-8?q?feat(actions):=20canopy=20reply=20=20--body=20<=E2=80=A6>=20[--resolve]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/canopy/cli/main.py | 71 ++++++++++++++++++++++++++++++++++++ src/canopy/mcp/server.py | 32 ++++++++++++++++ tests/test_thread_actions.py | 35 ++++++++++++++++++ 3 files changed, 138 insertions(+) diff --git a/src/canopy/cli/main.py b/src/canopy/cli/main.py index 13b86df..bd8a2bb 100644 --- a/src/canopy/cli/main.py +++ b/src/canopy/cli/main.py @@ -3131,6 +3131,60 @@ def _resolve_thread_feature(workspace, feature: str | None) -> str: return state.canonical.feature +def cmd_reply_thread(args: argparse.Namespace) -> None: + """Post a reply to a GitHub PR review thread, optionally resolving it.""" + from ..actions.thread_actions import reply_to_thread + from ..actions.errors import ActionError, BlockerError + from .render import render_blocker + from .ui import console + + workspace = _load_workspace() + try: + feature = _resolve_thread_feature(workspace, getattr(args, "feature", None)) + except ActionError as err: + if args.json: + _print_json(err.to_dict()) + else: + render_blocker(err, action="reply") + sys.exit(1) + + if args.body is not None: + body = args.body + elif args.body_file is not None: + from pathlib import Path + body = Path(args.body_file).read_text() + else: + body = sys.stdin.read() + + if not body.strip(): + err = BlockerError(code="empty_reply_body", what="reply body is empty") + if args.json: + _print_json(err.to_dict()) + else: + render_blocker(err, action="reply") + sys.exit(1) + + try: + result = reply_to_thread(workspace, args.thread_id, body, + feature=feature, resolve_after=args.resolve) + except ActionError as err: + if args.json: + _print_json(err.to_dict()) + else: + render_blocker(err, action="reply") + sys.exit(1) + + if args.json: + _print_json(result) + return + + console.print() + console.print(f" Posted reply: [info]{result['posted']['url']}[/]") + if args.resolve and "resolved" in result: + console.print(f" Resolved [info]{args.thread_id}[/]") + console.print() + + def cmd_resolve_thread(args: argparse.Namespace) -> None: """Resolve a GitHub PR review thread and record the resolution locally.""" from ..actions.thread_actions import resolve_thread @@ -3746,6 +3800,22 @@ def main() -> None: ) doctor_p.add_argument("--json", action="store_true", help="Output as JSON") + reply_p = subparsers.add_parser( + "reply", + help="Post a reply to a GH review thread", + ) + reply_p.add_argument("thread_id", help="Thread node ID (starts with PRRT_)") + _reply_body_g = reply_p.add_mutually_exclusive_group() + _reply_body_g.add_argument("--body", default=None, + help="Reply body text") + _reply_body_g.add_argument("--body-file", default=None, + help="Path to file containing the reply body") + reply_p.add_argument("--resolve", action="store_true", + help="Resolve the thread after posting") + reply_p.add_argument("--feature", default=None, + help="Feature alias; defaults to canonical feature") + reply_p.add_argument("--json", action="store_true", help="Output as JSON") + resolve_p = subparsers.add_parser( "resolve", help="Resolve a GitHub PR review thread and record it locally", @@ -3801,6 +3871,7 @@ def main() -> None: "worktree-bootstrap": cmd_worktree_bootstrap, "pr-checks": cmd_pr_checks, "resolve": cmd_resolve_thread, + "reply": cmd_reply_thread, } if args.command == "feature": diff --git a/src/canopy/mcp/server.py b/src/canopy/mcp/server.py index 807aa52..b44b5c6 100644 --- a/src/canopy/mcp/server.py +++ b/src/canopy/mcp/server.py @@ -788,6 +788,38 @@ def conflicts( ) +@mcp.tool() +def reply_to_thread( + thread_id: str, + body: str, + feature: str | None = None, + resolve_after: bool = False, +) -> dict: + """Post a reply to a GH review thread; optionally resolve after. + + Args: + thread_id: The GitHub review thread node ID (must start with + ``PRRT_``). + body: The reply text to post. + feature: Feature to attribute the reply to. Defaults to the + canonical feature if not supplied. + resolve_after: If True, resolve the thread after posting the reply + and record the resolution in the canopy log. + """ + from ..actions.thread_actions import reply_to_thread as _impl + from ..actions.errors import ActionError + + ws = _get_workspace() + try: + feat = _historian_feature(feature)[1] + except ActionError as e: + return e.to_dict() + try: + return _impl(ws, thread_id, body, feature=feat, resolve_after=resolve_after) + except ActionError as e: + return e.to_dict() + + @mcp.tool() def resolve_thread(thread_id: str, feature: str | None = None) -> dict: """Resolve a GitHub PR review thread and record the resolution locally. diff --git a/tests/test_thread_actions.py b/tests/test_thread_actions.py index 94ef5bf..107c391 100644 --- a/tests/test_thread_actions.py +++ b/tests/test_thread_actions.py @@ -206,3 +206,38 @@ def test_reply_to_thread_resolve_after(canopy_toml, monkeypatch): log = tr.load(canopy_toml) assert "PRRT_abc" in log assert log["PRRT_abc"]["via_command"] == "reply_resolve" + + +# ── CLI wiring smoke test ──────────────────────────────────────────────── + + +def test_reply_command_json_smoke(canopy_toml, monkeypatch, capsys): + """cmd_reply_thread wires body → action → JSON output (no GH call).""" + import argparse + import json as _json + + posted = {"comment_id": "IC_1", "url": "https://github.com/org/repo/pull/1#r999"} + monkeypatch.setattr( + "canopy.integrations.github.reply_to_thread", + lambda root, tid, body: posted, + ) + + ws = _make_workspace(canopy_toml) + monkeypatch.setattr("canopy.cli.main._load_workspace", lambda: ws) + monkeypatch.setattr("canopy.cli.main._resolve_thread_feature", + lambda workspace, feature: "feat") + + from canopy.cli.main import cmd_reply_thread + args = argparse.Namespace( + thread_id="PRRT_abc", + body="Looks good to me.", + body_file=None, + resolve=False, + feature="feat", + json=True, + ) + cmd_reply_thread(args) + + data = _json.loads(capsys.readouterr().out) + assert "posted" in data + assert data["posted"]["url"] == posted["url"] From b65b92d35dd7f98db97b9e5bac3f54b067e09469 Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 07:54:57 +0530 Subject: [PATCH 04/21] feat(commit): --address optionally resolves the GH thread --- src/canopy/actions/augments.py | 6 +- src/canopy/actions/commit.py | 123 ++++++++++++++++-- src/canopy/cli/main.py | 25 ++++ src/canopy/mcp/server.py | 9 +- tests/test_commit.py | 219 +++++++++++++++++++++++++++++++++ 5 files changed, 372 insertions(+), 10 deletions(-) diff --git a/src/canopy/actions/augments.py b/src/canopy/actions/augments.py index 83851d4..986d54f 100644 --- a/src/canopy/actions/augments.py +++ b/src/canopy/actions/augments.py @@ -6,8 +6,12 @@ Consumed by: - ``integrations/precommit.py`` — ``preflight_cmd`` overrides auto-detection. -- (planned) ``actions/feature_state.py`` — ``review_bots`` filters bot-vs-human +- ``actions/feature_state.py`` — ``review_bots`` filters bot-vs-human comment classification (M3 bot-tracking). +- ``actions/commit.py`` — ``auto_resolve_threads_on_address`` (T4): when + ``true``, ``commit --address `` automatically resolves the corresponding + GitHub review thread after a successful push. Overridden by CLI flags + ``--resolve-thread`` / ``--no-resolve-thread``. - (planned) future ``canopy test`` command — ``test_cmd`` per-repo. The resolver is intentionally lenient: missing keys return ``None`` / empty diff --git a/src/canopy/actions/commit.py b/src/canopy/actions/commit.py index d6bbf63..8b26c2b 100644 --- a/src/canopy/actions/commit.py +++ b/src/canopy/actions/commit.py @@ -179,6 +179,7 @@ def commit( no_hooks: bool = False, amend: bool = False, address: str | None = None, + resolve_thread: bool | None = None, ) -> dict[str, Any]: """Commit across every repo in a feature lane. @@ -202,13 +203,23 @@ def commit( ``.canopy/state/bot_resolutions.json``. Comment must belong to one of the feature's actionable bot threads; a non-bot comment raises ``BlockerError(code='not_a_bot_comment')``. + resolve_thread: when ``address`` is set, controls whether the + corresponding GitHub review thread is resolved after a + successful commit. ``True`` forces resolve; ``False`` forces + skip; ``None`` (default) defers to the workspace augment + ``auto_resolve_threads_on_address`` (which defaults to + ``False`` when absent). Thread resolution is a best-effort + step — failures are captured in ``result["thread_resolved"]`` + as ``{"skipped": ""}`` rather than raising. Returns ``{feature, results: {: {...}}, addressed?}``. The per-repo dict has shape ``{status, sha?, files_changed?, reason?, hook_output?, amended?}`` where ``status`` is one of ``ok | nothing | hooks_failed | failed``. When ``--address`` is given and a resolution was recorded, ``addressed`` carries - ``{comment_id, repo, sha, title, url}``. + ``{comment_id, repo, sha, title, url}``. When thread resolution was + attempted, ``addressed`` also carries ``thread_resolved`` (a dict + with the GH result or a ``{"skipped": ""}`` entry). """ feature_name = _resolve_feature_name(workspace, feature) repo_branches = repos_for_feature(workspace, feature_name) @@ -237,8 +248,10 @@ def commit( addressed_info: dict[str, Any] | None = None if address is not None: comment_id = _parse_comment_id(address) - bot_comment, owning_repo = _find_actionable_bot_comment( - workspace, feature_name, repo_branches, repo_paths, comment_id, + bot_comment, owning_repo, _owner, _repo_slug, _pr_number = ( + _find_actionable_bot_comment( + workspace, feature_name, repo_branches, repo_paths, comment_id, + ) ) if bot_comment is None: raise BlockerError( @@ -265,6 +278,9 @@ def commit( "repo": owning_repo, "title": title, "url": url, + "_owner": _owner, + "_repo_slug": _repo_slug, + "_pr_number": _pr_number, } if not message and not amend: @@ -326,11 +342,93 @@ def commit( pass addressed_info["sha"] = sha addressed_info["recorded"] = True + + # ── Optional: resolve the GH review thread (T4) ───────────── + # Determine effective resolve flag: explicit flag > augment default. + from . import augments as augments_mod + from ..integrations import github as gh + from .thread_actions import resolve_thread as _resolve_thread_action + + augment_default = bool( + (workspace.config.augments or {}).get( + "auto_resolve_threads_on_address", False, + ) + ) + effective_resolve = ( + resolve_thread if resolve_thread is not None else augment_default + ) + + if effective_resolve: + owner = addressed_info.get("_owner") or "" + repo_slug = addressed_info.get("_repo_slug") or "" + pr_number = addressed_info.get("_pr_number") + comment_id_val = addressed_info["comment_id"] + + if not (owner and repo_slug and pr_number): + addressed_info["thread_resolved"] = { + "skipped": "pr_not_found", + "comment_id": comment_id_val, + } + else: + try: + threads = gh.list_review_threads( + workspace.config.root, owner, repo_slug, pr_number, + ) + except Exception as exc: + addressed_info["thread_resolved"] = { + "skipped": "gh_unreachable", + "error": str(exc), + "comment_id": comment_id_val, + } + else: + # comment_id may be str or int; normalise to int for comparison. + try: + cid_int = int(comment_id_val) + except (ValueError, TypeError): + cid_int = None + + target_tid = next( + ( + t["thread_id"] + for t in threads + if any( + c.get("comment_id") == cid_int + for c in t.get("comments", []) + ) + ), + None, + ) + if target_tid is None: + addressed_info["thread_resolved"] = { + "skipped": "thread_not_found", + "comment_id": comment_id_val, + } + else: + try: + from .errors import ActionError + addressed_info["thread_resolved"] = _resolve_thread_action( + workspace, + target_tid, + feature=feature_name, + via_command="commit_address", + via_commit_sha=sha, + ) + except ActionError as exc: + addressed_info["thread_resolved"] = { + "skipped": "resolve_failed", + "error": exc.to_dict(), + "comment_id": comment_id_val, + } else: addressed_info["recorded"] = False addressed_info["reason"] = ( f"owning repo '{owning}' commit status: {owning_result.get('status', 'unknown')}" ) + + # Strip internal PR-coordinate keys before returning. + addressed_info.pop("_owner", None) + addressed_info.pop("_repo_slug", None) + addressed_info.pop("_pr_number", None) out["addressed"] = addressed_info return out @@ -365,18 +463,27 @@ def _find_actionable_bot_comment( repo_branches: dict[str, str], repo_paths: dict[str, Path], comment_id: str, -) -> tuple[dict | None, str | None]: +) -> tuple[dict | None, str | None, str | None, str | None, int | None]: """Walk per-repo bot threads for a matching comment id. - Returns ``(comment_dict, owning_repo)`` or ``(None, None)`` when no - actionable bot thread carries the requested id. + Returns ``(comment_dict, owning_repo, owner, repo_slug, pr_number)`` or + ``(None, None, None, None, None)`` when no actionable bot thread carries + the requested id. The extra fields come from the same ``_per_repo_facts`` + call so no second network round-trip is needed. """ facts = _per_repo_facts(workspace, feature_name, repo_branches, repo_paths) for repo_name, repo_facts in facts.items(): for thread in repo_facts.get("actionable_bot_threads", []): if str(thread.get("id", "")) == comment_id: - return thread, repo_name - return None, None + pr = repo_facts.get("pr") or {} + return ( + thread, + repo_name, + repo_facts.get("owner"), + repo_facts.get("repo_slug"), + pr.get("number"), + ) + return None, None, None, None, None def _comment_title(body: str, max_len: int = 80) -> str: diff --git a/src/canopy/cli/main.py b/src/canopy/cli/main.py index bd8a2bb..e58896a 100644 --- a/src/canopy/cli/main.py +++ b/src/canopy/cli/main.py @@ -2398,6 +2398,7 @@ def cmd_commit(args: argparse.Namespace) -> None: no_hooks=args.no_hooks, amend=args.amend, address=getattr(args, "address", None), + resolve_thread=getattr(args, "resolve_thread", None), ) except ActionError as err: if args.json: @@ -2441,6 +2442,21 @@ def cmd_commit(args: argparse.Namespace) -> None: f" [warning]·[/] bot comment [muted]{cid}[/] not recorded " f"({addressed.get('reason', 'no successful commit in owning repo')})", ) + tr = addressed.get("thread_resolved") + if tr is not None: + if tr.get("skipped"): + console.print( + f" [muted]·[/] thread resolve skipped " + f"([muted]{tr['skipped']}[/])", + ) + elif tr.get("is_resolved"): + tid = tr.get("thread_id") or tr.get("logged", {}).get("thread_id", "") + console.print( + f" [success]✓[/] GH review thread resolved " + f"([muted]{tid}[/])", + ) + else: + console.print(" [warning]·[/] GH thread resolve returned unexpected result") console.print() @@ -3482,6 +3498,15 @@ def main() -> None: help="Address a bot review comment (numeric id or GitHub URL); " "auto-suffixes the message with the comment title + URL " "and records the resolution in .canopy/state/bot_resolutions.json") + _resolve_grp = commit_p.add_mutually_exclusive_group() + _resolve_grp.add_argument("--resolve-thread", action="store_true", default=None, + dest="resolve_thread", + help="After --address commit succeeds, resolve the GH review " + "thread (overrides augment auto_resolve_threads_on_address)") + _resolve_grp.add_argument("--no-resolve-thread", action="store_false", + dest="resolve_thread", + help="Do NOT resolve the GH review thread even if the augment " + "auto_resolve_threads_on_address is true") commit_p.add_argument("--json", action="store_true", help="Output as JSON") # bot-status — per-feature bot-comment rollup (M3) diff --git a/src/canopy/mcp/server.py b/src/canopy/mcp/server.py index b44b5c6..0411e1a 100644 --- a/src/canopy/mcp/server.py +++ b/src/canopy/mcp/server.py @@ -299,7 +299,8 @@ def slot_swap(slot_a: str, slot_b: str) -> dict: def commit(message: str = "", feature: str | None = None, repos: list[str] | None = None, paths: list[str] | None = None, no_hooks: bool = False, amend: bool = False, - address: str | None = None) -> dict: + address: str | None = None, + resolve_thread: bool | None = None) -> dict: """Commit across every repo in a feature lane with a single message (Wave 2.3). Defaults to the canonical feature when ``feature`` is omitted (reads @@ -316,6 +317,11 @@ def commit(message: str = "", feature: str | None = None, against the matching repo's commit SHA. Non-bot comments raise ``BlockerError(code='not_a_bot_comment')``. + ``resolve_thread`` (T4): when ``address`` is set, controls whether the + corresponding GitHub review thread is resolved after a successful commit. + ``True`` forces resolve; ``False`` forces skip; ``None`` (default) defers + to the workspace augment ``auto_resolve_threads_on_address``. + Per-repo result statuses: - ``ok`` — committed; carries ``sha``, ``files_changed``. - ``nothing`` — no changes staged. @@ -334,6 +340,7 @@ def commit(message: str = "", feature: str | None = None, ws, message, feature=feature, repos=repos, paths=paths, no_hooks=no_hooks, amend=amend, address=address, + resolve_thread=resolve_thread, ) except ActionError as e: return e.to_dict() diff --git a/tests/test_commit.py b/tests/test_commit.py index 930866b..2ef3920 100644 --- a/tests/test_commit.py +++ b/tests/test_commit.py @@ -436,3 +436,222 @@ def test_commit_with_address_uses_only_auto_message_when_no_message( check=True, capture_output=True, text=True, ).stdout assert msg.strip().startswith('Addresses bot comment: "please rename hit_rate"') + + +# ── T4: --resolve-thread / --no-resolve-thread / augment ──────────────── + + +def _thread(thread_id, comment_id): + """Minimal review thread dict matching list_review_threads output.""" + return { + "thread_id": thread_id, + "is_resolved": False, + "resolved_at": None, + "comments": [ + { + "comment_id": int(comment_id), + "path": "src/auth.py", + "line": 1, + "body": "rename foo to bar", + "author": "coderabbit", + "created_at": "2030-01-01T00:00:00Z", + "url": f"https://github.com/o/r/pull/1#discussion_r{comment_id}", + } + ], + } + + +def _setup_address_workspace(workspace_with_feature, comment_id): + """Common setup for --address resolve-thread tests. + + Returns workspace with repo-a feature and a dirty file to commit. + """ + _features_file(workspace_with_feature, { + "auth-flow": {"repos": ["repo-a"], "status": "active"}, + }) + _set_remote(workspace_with_feature / "repo-a", "git@github.com:owner/repo-a.git") + ws = _make_workspace(workspace_with_feature, repos=("repo-a",)) + (workspace_with_feature / "repo-a" / "src" / "models.py").write_text("fixed\n") + return ws + + +def test_commit_address_with_resolve_thread_flag(workspace_with_feature): + """--resolve-thread resolves the GH thread and logs to thread_resolutions.json.""" + from canopy.actions import thread_resolutions as tr + + comment_id = 12345 + thread_id = "PRRT_x" + ws = _setup_address_workspace(workspace_with_feature, comment_id) + + gh_resolve_result = {"thread_id": thread_id, "is_resolved": True} + + with patch("canopy.actions.feature_state.gh.find_pull_request", + return_value=_open_pr()), \ + patch("canopy.actions.feature_state.gh.get_review_comments", + return_value=([_bot_comment(comment_id)], 0)), \ + patch("canopy.integrations.github.list_review_threads", + return_value=[_thread(thread_id, comment_id)]), \ + patch("canopy.integrations.github.resolve_thread", + return_value=gh_resolve_result): + result = commit( + ws, "fix it", + feature="auth-flow", + address=str(comment_id), + resolve_thread=True, + ) + + assert result["results"]["repo-a"]["status"] == "ok" + addressed = result["addressed"] + assert addressed["recorded"] is True + tr_result = addressed["thread_resolved"] + assert tr_result.get("is_resolved") is True + + # thread_resolutions.json should carry via_command + via_commit_sha + log = tr.load(workspace_with_feature) + assert thread_id in log + entry = log[thread_id] + assert entry["via_command"] == "commit_address" + assert entry["via_commit_sha"] == addressed["sha"] + + # bot_resolutions.json still written (unchanged behaviour) + assert is_resolved(workspace_with_feature, comment_id) is True + + +def test_commit_address_no_resolve_default(workspace_with_feature): + """Without --resolve-thread and no augment, GH thread resolution is skipped.""" + comment_id = 99991 + ws = _setup_address_workspace(workspace_with_feature, comment_id) + + # resolve_thread must NOT be called — crash if it is. + def _crash(*_a, **_kw): + raise AssertionError("resolve_thread should not have been called") + + with patch("canopy.actions.feature_state.gh.find_pull_request", + return_value=_open_pr()), \ + patch("canopy.actions.feature_state.gh.get_review_comments", + return_value=([_bot_comment(comment_id)], 0)), \ + patch("canopy.integrations.github.list_review_threads", side_effect=_crash), \ + patch("canopy.integrations.github.resolve_thread", side_effect=_crash): + result = commit( + ws, "fix it", + feature="auth-flow", + address=str(comment_id), + # resolve_thread not passed → defaults to None → augment default (False) + ) + + assert result["results"]["repo-a"]["status"] == "ok" + # No thread_resolved key when resolution was not attempted + assert "thread_resolved" not in result.get("addressed", {}) + + +def test_commit_address_augment_default_true(workspace_with_feature): + """augment auto_resolve_threads_on_address=true fires resolve without CLI flag.""" + from canopy.actions import thread_resolutions as tr + from canopy.workspace.config import load_config + + comment_id = 77777 + thread_id = "PRRT_augment" + + _features_file(workspace_with_feature, { + "auth-flow": {"repos": ["repo-a"], "status": "active"}, + }) + _set_remote(workspace_with_feature / "repo-a", "git@github.com:owner/repo-a.git") + (workspace_with_feature / "canopy.toml").write_text( + "[workspace]\nname = \"test\"\n\n" + "[augments]\nauto_resolve_threads_on_address = true\n\n" + "[[repos]]\nname = \"repo-a\"\npath = \"./repo-a\"\nrole = \"x\"\nlang = \"x\"\n" + ) + ws = Workspace(load_config(workspace_with_feature)) + (workspace_with_feature / "repo-a" / "src" / "models.py").write_text("augment\n") + + with patch("canopy.actions.feature_state.gh.find_pull_request", + return_value=_open_pr()), \ + patch("canopy.actions.feature_state.gh.get_review_comments", + return_value=([_bot_comment(comment_id)], 0)), \ + patch("canopy.integrations.github.list_review_threads", + return_value=[_thread(thread_id, comment_id)]), \ + patch("canopy.integrations.github.resolve_thread", + return_value={"thread_id": thread_id, "is_resolved": True}): + result = commit( + ws, "augment driven", + feature="auth-flow", + address=str(comment_id), + # resolve_thread not passed → None → augment kicks in + ) + + assert result["addressed"]["thread_resolved"].get("is_resolved") is True + log = tr.load(workspace_with_feature) + assert thread_id in log + + +def test_commit_address_no_resolve_thread_overrides_augment(workspace_with_feature): + """--no-resolve-thread prevents resolution even if augment is true.""" + from canopy.workspace.config import load_config + + comment_id = 55555 + + _features_file(workspace_with_feature, { + "auth-flow": {"repos": ["repo-a"], "status": "active"}, + }) + _set_remote(workspace_with_feature / "repo-a", "git@github.com:owner/repo-a.git") + (workspace_with_feature / "canopy.toml").write_text( + "[workspace]\nname = \"test\"\n\n" + "[augments]\nauto_resolve_threads_on_address = true\n\n" + "[[repos]]\nname = \"repo-a\"\npath = \"./repo-a\"\nrole = \"x\"\nlang = \"x\"\n" + ) + ws = Workspace(load_config(workspace_with_feature)) + (workspace_with_feature / "repo-a" / "src" / "models.py").write_text("noreso\n") + + def _crash(*_a, **_kw): + raise AssertionError("resolve_thread should not have been called") + + with patch("canopy.actions.feature_state.gh.find_pull_request", + return_value=_open_pr()), \ + patch("canopy.actions.feature_state.gh.get_review_comments", + return_value=([_bot_comment(comment_id)], 0)), \ + patch("canopy.integrations.github.list_review_threads", side_effect=_crash), \ + patch("canopy.integrations.github.resolve_thread", side_effect=_crash): + result = commit( + ws, "explicit skip", + feature="auth-flow", + address=str(comment_id), + resolve_thread=False, # explicit False overrides augment=true + ) + + assert result["results"]["repo-a"]["status"] == "ok" + assert "thread_resolved" not in result.get("addressed", {}) + + +def test_commit_address_resolve_thread_skipped_when_thread_not_found(workspace_with_feature): + """When list_review_threads returns no matching comment_id, skipped=thread_not_found.""" + from canopy.actions import thread_resolutions as tr + + comment_id = 12345 + ws = _setup_address_workspace(workspace_with_feature, comment_id) + + # Thread exists but its comments have a *different* comment_id + unrelated_thread = _thread("PRRT_other", 99999) + + with patch("canopy.actions.feature_state.gh.find_pull_request", + return_value=_open_pr()), \ + patch("canopy.actions.feature_state.gh.get_review_comments", + return_value=([_bot_comment(comment_id)], 0)), \ + patch("canopy.integrations.github.list_review_threads", + return_value=[unrelated_thread]), \ + patch("canopy.integrations.github.resolve_thread", + side_effect=AssertionError("should not be called")): + result = commit( + ws, "not found", + feature="auth-flow", + address=str(comment_id), + resolve_thread=True, + ) + + assert result["results"]["repo-a"]["status"] == "ok" + assert result["addressed"]["thread_resolved"] == { + "skipped": "thread_not_found", + "comment_id": str(comment_id), + } + # thread_resolutions.json should NOT have an entry + log = tr.load(workspace_with_feature) + assert log == {} From c2ad0c38cf00c105fc2d2f4152119a3ef2283953 Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 08:06:38 +0530 Subject: [PATCH 05/21] fix(milestone-1): author_type from __typename + resolve_after failure capture + gh -f for strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MUST #1: Add author { __typename } to list_review_threads GraphQL query so _build_comments_from_threads sets author_type="Bot"/"User" correctly. Without this, _is_bot_comment never matched and bot detection was broken. MUST #2 (scenario b): _commit_one only commits, does not push. The thread resolve fires on local commit success, before the commit is on GitHub. Added a code comment at the resolve guard to document this — no new push step added; canopy separates commit and push by design. MUST #3: reply_to_thread with resolve_after=True now catches ActionError from the inner resolve_thread call and returns {"error": e.to_dict()} instead of propagating — so the caller always sees posted + resolved. SHOULD #4: _graphql_via_gh_cli now uses -f for string vars (body, id) and -F only for int/bool vars, preventing String! args from being coerced to the wrong GraphQL type. SHOULD #5: resolved_count in _build_comments_from_threads now increments by 1 per resolved thread (not per comment), matching _normalize_comments. SHOULD #7: cmd_reply_thread guards sys.stdin.read() with isatty() check and exits with missing_reply_body BlockerError rather than blocking on stdin. Cleanup: removed dead `from . import augments as augments_mod` in commit.py. New tests: test_author_type_from_graphql_typename, test_resolved_count_counts_threads_not_comments, test_get_review_comments_fallback_path_carries_thread_id (test_thread_graphql.py), test_reply_to_thread_resolve_after_failure_keeps_posted (test_thread_actions.py). 780 tests, 0 failures. --- src/canopy/actions/commit.py | 4 +- src/canopy/actions/thread_actions.py | 13 +++-- src/canopy/cli/main.py | 10 ++++ src/canopy/integrations/github.py | 14 +++-- tests/test_thread_actions.py | 28 ++++++++++ tests/test_thread_graphql.py | 81 ++++++++++++++++++++++++++++ 6 files changed, 140 insertions(+), 10 deletions(-) diff --git a/src/canopy/actions/commit.py b/src/canopy/actions/commit.py index 8b26c2b..4afc0ef 100644 --- a/src/canopy/actions/commit.py +++ b/src/canopy/actions/commit.py @@ -345,7 +345,9 @@ def commit( # ── Optional: resolve the GH review thread (T4) ───────────── # Determine effective resolve flag: explicit flag > augment default. - from . import augments as augments_mod + # Note: this fires on local commit success. The thread will be + # resolved before the commit reaches GitHub — push your branch to + # make the linkage live on the remote. from ..integrations import github as gh from .thread_actions import resolve_thread as _resolve_thread_action diff --git a/src/canopy/actions/thread_actions.py b/src/canopy/actions/thread_actions.py index 1d8dac9..9c6f53a 100644 --- a/src/canopy/actions/thread_actions.py +++ b/src/canopy/actions/thread_actions.py @@ -7,7 +7,7 @@ from __future__ import annotations from ..workspace.workspace import Workspace -from .errors import BlockerError +from .errors import ActionError, BlockerError from . import thread_resolutions as tr @@ -78,8 +78,11 @@ def reply_to_thread( posted = gh.reply_to_thread(workspace.config.root, thread_id, body) result: dict = {"posted": posted} if resolve_after: - resolved = resolve_thread( - workspace, thread_id, feature=feature, via_command="reply_resolve", - ) - result["resolved"] = resolved + try: + res = resolve_thread( + workspace, thread_id, feature=feature, via_command="reply_resolve", + ) + result["resolved"] = res + except ActionError as e: + result["resolved"] = {"error": e.to_dict()} return result diff --git a/src/canopy/cli/main.py b/src/canopy/cli/main.py index e58896a..66cbde6 100644 --- a/src/canopy/cli/main.py +++ b/src/canopy/cli/main.py @@ -3170,6 +3170,16 @@ def cmd_reply_thread(args: argparse.Namespace) -> None: from pathlib import Path body = Path(args.body_file).read_text() else: + if sys.stdin.isatty(): + err = BlockerError( + code="missing_reply_body", + what="reply body required: pass --body, --body-file, or pipe stdin", + ) + if args.json: + _print_json(err.to_dict()) + else: + render_blocker(err, action="reply") + sys.exit(1) body = sys.stdin.read() if not body.strip(): diff --git a/src/canopy/integrations/github.py b/src/canopy/integrations/github.py index af3963a..8cb6da6 100644 --- a/src/canopy/integrations/github.py +++ b/src/canopy/integrations/github.py @@ -692,7 +692,12 @@ def _graphql_via_gh_cli(query: str, vars: dict) -> dict: """ args = ["gh", "api", "graphql", "-f", f"query={query}"] for k, v in vars.items(): - args.extend(["-F", f"{k}={v}"]) + if isinstance(v, bool): + args.extend(["-F", f"{k}={'true' if v else 'false'}"]) + elif isinstance(v, int): + args.extend(["-F", f"{k}={v}"]) + else: + args.extend(["-f", f"{k}={v}"]) proc = subprocess.run(args, capture_output=True, text=True) if proc.returncode != 0: raise GitHubNotConfiguredError( @@ -742,7 +747,7 @@ def list_review_threads( comments(first: 20) { nodes { databaseId path line body createdAt url - author { login } + author { login __typename } } } } @@ -770,6 +775,7 @@ def list_review_threads( "created_at": c.get("createdAt"), "url": c.get("url"), "author": (c.get("author") or {}).get("login", ""), + "author_type": (c.get("author") or {}).get("__typename", ""), } for c in (n.get("comments") or {}).get("nodes", []) ] @@ -832,7 +838,7 @@ def _build_comments_from_threads(threads: list[dict]) -> tuple[list[dict], int]: resolved_count = 0 for t in threads: if t["is_resolved"]: - resolved_count += len(t["comments"]) + resolved_count += 1 continue for c in t["comments"]: comments.append({ @@ -841,7 +847,7 @@ def _build_comments_from_threads(threads: list[dict]) -> tuple[list[dict], int]: "line": c["line"] or 0, "body": c["body"] or "", "author": c["author"], - "author_type": "", + "author_type": c.get("author_type", ""), "state": "", "created_at": c["created_at"] or "", "url": c["url"] or "", diff --git a/tests/test_thread_actions.py b/tests/test_thread_actions.py index 107c391..41c9155 100644 --- a/tests/test_thread_actions.py +++ b/tests/test_thread_actions.py @@ -208,6 +208,34 @@ def test_reply_to_thread_resolve_after(canopy_toml, monkeypatch): assert log["PRRT_abc"]["via_command"] == "reply_resolve" +def test_reply_to_thread_resolve_after_failure_keeps_posted(canopy_toml, monkeypatch): + """When resolve fails after successful post, both states are reported.""" + from canopy.actions import thread_actions + from canopy.actions.errors import BlockerError + + monkeypatch.setattr( + "canopy.integrations.github.reply_to_thread", + lambda root, tid, body: {"comment_id": "C_1", "url": "https://github.com/o/r/pull/1#r"}, + ) + + def boom(root, tid): + raise BlockerError(code="gh_error", what="fake failure") + + monkeypatch.setattr("canopy.integrations.github.resolve_thread", boom) + + ws = _make_workspace(canopy_toml) + result = thread_actions.reply_to_thread( + ws, "PRRT_abc", "Looks good", feature="feat", resolve_after=True, + ) + + # The successful reply must be present + assert result["posted"]["url"] == "https://github.com/o/r/pull/1#r" + # The failed resolve must be captured, not raised + assert "resolved" in result + assert "error" in result["resolved"] + assert result["resolved"]["error"]["code"] == "gh_error" + + # ── CLI wiring smoke test ──────────────────────────────────────────────── diff --git a/tests/test_thread_graphql.py b/tests/test_thread_graphql.py index 0ee086f..c2d24e7 100644 --- a/tests/test_thread_graphql.py +++ b/tests/test_thread_graphql.py @@ -77,3 +77,84 @@ def test_get_review_comments_includes_thread_id(tmp_path): from canopy.integrations.github import get_review_comments comments, resolved_count = get_review_comments(tmp_path, "o", "r", 1) assert comments[0]["thread_id"] == "PRRT_abc" + + +def test_author_type_from_graphql_typename(tmp_path): + """Bot and User author __typename propagates to author_type on each comment.""" + fake = {"data": {"repository": {"pullRequest": {"reviewThreads": {"nodes": [ + {"id": "PRRT_bot", "isResolved": False, "resolvedAt": None, + "comments": {"nodes": [{ + "databaseId": 10, "path": "a.py", "line": 1, + "body": "bot comment", "createdAt": "2026-05-20T10:00:00Z", + "url": "https://github.com/o/r/pull/1#a", + "author": {"login": "cursor[bot]", "__typename": "Bot"}, + }]}}, + {"id": "PRRT_human", "isResolved": False, "resolvedAt": None, + "comments": {"nodes": [{ + "databaseId": 11, "path": "b.py", "line": 2, + "body": "human comment", "createdAt": "2026-05-21T10:00:00Z", + "url": "https://github.com/o/r/pull/1#b", + "author": {"login": "alice", "__typename": "User"}, + }]}}, + ]}}}}} + with _mock_graphql_response(fake): + from canopy.integrations.github import get_review_comments + comments, _ = get_review_comments(tmp_path, "o", "r", 1) + + by_path = {c["path"]: c for c in comments} + assert by_path["a.py"]["author_type"] == "Bot" + assert by_path["b.py"]["author_type"] == "User" + + +def test_resolved_count_counts_threads_not_comments(tmp_path): + """resolved_count should be 1 for a resolved thread with 3 comments.""" + fake = {"data": {"repository": {"pullRequest": {"reviewThreads": {"nodes": [ + {"id": "PRRT_resolved", "isResolved": True, "resolvedAt": "2026-05-25T00:00:00Z", + "comments": {"nodes": [ + {"databaseId": 1, "path": "a.py", "line": 1, "body": "c1", + "createdAt": "2026-05-20T00:00:00Z", "url": "u1", + "author": {"login": "alice", "__typename": "User"}}, + {"databaseId": 2, "path": "a.py", "line": 2, "body": "c2", + "createdAt": "2026-05-21T00:00:00Z", "url": "u2", + "author": {"login": "alice", "__typename": "User"}}, + {"databaseId": 3, "path": "a.py", "line": 3, "body": "c3", + "createdAt": "2026-05-22T00:00:00Z", "url": "u3", + "author": {"login": "alice", "__typename": "User"}}, + ]}, + }, + ]}}}}} + with _mock_graphql_response(fake): + from canopy.integrations.github import get_review_comments + comments, resolved_count = get_review_comments(tmp_path, "o", "r", 1) + + assert resolved_count == 1 # one thread resolved, not 3 comments + assert comments == [] + + +def test_get_review_comments_fallback_path_carries_thread_id(tmp_path, monkeypatch): + """When GraphQL path fails, REST fallback comments must still have thread_id=''.""" + import json as _json + from canopy.integrations import github as ghmod + + # Make GraphQL path fail + monkeypatch.setattr(ghmod, "list_review_threads", + lambda *a, **k: (_ for _ in ()).throw(RuntimeError("simulated"))) + # Disable MCP path + monkeypatch.setattr(ghmod, "is_mcp_configured", lambda *a, **k: False) + # Simulate gh CLI available + monkeypatch.setattr(ghmod, "have_gh_cli", lambda: True) + fake_rest = _json.dumps([{ + "id": 1, "path": "a.py", "line": 1, "body": "b", + "user": {"login": "u", "type": "User"}, + "created_at": "2026-05-20T00:00:00Z", + "url": "https://github.com/o/r/pull/1#r1", + }]) + monkeypatch.setattr(ghmod, "_gh", lambda *a, **k: fake_rest) + + # GraphQL path is gated on is_github_configured — stub that too + monkeypatch.setattr(ghmod, "is_github_configured", lambda *a, **k: False) + + from canopy.integrations.github import get_review_comments + comments, _ = get_review_comments(tmp_path, "o", "r", 1) + assert len(comments) == 1 + assert comments[0]["thread_id"] == "" From 422c5981dde9bd1be9d8e3283936cb9a591635ee Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 08:12:41 +0530 Subject: [PATCH 06/21] =?UTF-8?q?feat(actions):=20last=5Fvisit=20state=20?= =?UTF-8?q?=E2=80=94=20per-feature=20visit=20anchor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/canopy/actions/last_visit.py | 83 ++++++++++++++++++++++++++++++++ tests/test_last_visit.py | 68 ++++++++++++++++++++++++++ 2 files changed, 151 insertions(+) create mode 100644 src/canopy/actions/last_visit.py create mode 100644 tests/test_last_visit.py diff --git a/src/canopy/actions/last_visit.py b/src/canopy/actions/last_visit.py new file mode 100644 index 0000000..bc11734 --- /dev/null +++ b/src/canopy/actions/last_visit.py @@ -0,0 +1,83 @@ +"""Per-feature last-visit anchor for the feature-resume brief. + +State file: .canopy/state/visits.json +Schema: {"": {"last_visit": "ISO", "previous_visit": "ISO|null"}} + +Bumped by switch (T13). Read by resume (T6+). Atomic temp+replace writes. +""" +from __future__ import annotations + +import json +import os +import tempfile +from datetime import datetime, timezone +from pathlib import Path +from typing import Any + +from ..workspace.workspace import Workspace + + +_STATE = ".canopy/state/visits.json" + + +def _path(workspace: Workspace) -> Path: + return workspace.config.root / _STATE + + +def _load(workspace: Workspace) -> dict[str, dict[str, Any]]: + p = _path(workspace) + if not p.exists(): + return {} + try: + return json.loads(p.read_text()) + except (OSError, json.JSONDecodeError): + return {} + + +def _save(workspace: Workspace, data: dict) -> None: + p = _path(workspace) + p.parent.mkdir(parents=True, exist_ok=True) + fd, tmp = tempfile.mkstemp( + prefix=f".{p.name}.", suffix=".tmp", dir=str(p.parent) + ) + try: + with os.fdopen(fd, "w") as f: + json.dump(data, f, indent=2, sort_keys=True) + os.replace(tmp, p) + except Exception: + try: + os.unlink(tmp) + except FileNotFoundError: + pass + raise + + +def get_last_visit(workspace: Workspace, feature: str) -> dict[str, Any] | None: + """Return the visit entry for a feature, or None if not visited.""" + return _load(workspace).get(feature) + + +def mark_visited(workspace: Workspace, feature: str) -> str: + """Bump last_visit to now; carry old value to previous_visit. + + Returns the new timestamp (ISO 8601 Z format). + """ + now = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + data = _load(workspace) + prev = data.get(feature, {}).get("last_visit") + data[feature] = {"last_visit": now, "previous_visit": prev} + _save(workspace, data) + return now + + +def reset_anchor(workspace: Workspace, feature: str) -> bool: + """Drop the feature entry from the visits log. + + Returns True if the entry existed and was removed, False otherwise. + """ + data = _load(workspace) + if feature not in data: + return False + del data[feature] + _save(workspace, data) + return True diff --git a/tests/test_last_visit.py b/tests/test_last_visit.py new file mode 100644 index 0000000..24c96da --- /dev/null +++ b/tests/test_last_visit.py @@ -0,0 +1,68 @@ +"""Tests for actions/last_visit.py.""" +import time + +from canopy.actions.last_visit import ( + get_last_visit, + mark_visited, + reset_anchor, +) +from canopy.workspace.config import load_config +from canopy.workspace.workspace import Workspace + + +def _load_workspace(canopy_toml): + """Load a workspace from the canopy.toml fixture root.""" + return Workspace(load_config(canopy_toml)) + + +def test_get_last_visit_returns_none_when_missing(canopy_toml): + """get_last_visit returns None for a feature that hasn't been visited.""" + ws = _load_workspace(canopy_toml) + assert get_last_visit(ws, "auth-flow") is None + + +def test_mark_visited_roundtrip(canopy_toml): + """mark_visited stores and retrieves a visit entry.""" + ws = _load_workspace(canopy_toml) + ts = mark_visited(ws, "auth-flow") + got = get_last_visit(ws, "auth-flow") + assert got["last_visit"] == ts + assert got["previous_visit"] is None + + +def test_mark_visited_bumps_previous(canopy_toml): + """mark_visited carries previous last_visit to previous_visit.""" + ws = _load_workspace(canopy_toml) + ts1 = mark_visited(ws, "auth-flow") + ts2 = mark_visited(ws, "auth-flow") + got = get_last_visit(ws, "auth-flow") + assert got["last_visit"] == ts2 + assert got["previous_visit"] == ts1 + + +def test_reset_anchor(canopy_toml): + """reset_anchor removes a feature entry and returns True if it existed.""" + ws = _load_workspace(canopy_toml) + mark_visited(ws, "auth-flow") + result = reset_anchor(ws, "auth-flow") + assert result is True + assert get_last_visit(ws, "auth-flow") is None + + +def test_reset_anchor_returns_false_when_missing(canopy_toml): + """reset_anchor returns False if the feature doesn't exist.""" + ws = _load_workspace(canopy_toml) + result = reset_anchor(ws, "nonexistent") + assert result is False + + +def test_mark_visited_increases_monotonically(canopy_toml): + """Two consecutive mark_visited calls produce strictly increasing timestamps.""" + ws = _load_workspace(canopy_toml) + ts1 = mark_visited(ws, "auth-flow") + time.sleep(1.1) # Sleep 1.1s to ensure second boundary crossed + ts2 = mark_visited(ws, "auth-flow") + assert ts1 < ts2 + got = get_last_visit(ws, "auth-flow") + assert got["last_visit"] == ts2 + assert got["previous_visit"] == ts1 From 9a66523ce9e99d6e2a5c3f7e6780d8b207a9c418 Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 08:23:05 +0530 Subject: [PATCH 07/21] feat(resume): switch-aware compound action + intent hints --- src/canopy/actions/resume.py | 221 ++++++++++++++++++++++++++++ tests/test_resume.py | 277 +++++++++++++++++++++++++++++++++++ 2 files changed, 498 insertions(+) create mode 100644 src/canopy/actions/resume.py create mode 100644 tests/test_resume.py diff --git a/src/canopy/actions/resume.py b/src/canopy/actions/resume.py new file mode 100644 index 0000000..c757818 --- /dev/null +++ b/src/canopy/actions/resume.py @@ -0,0 +1,221 @@ +"""feature_resume — switch-aware compound action. + +Single command takes an alias and gets the user back in business: + 1. Resolve alias to feature name. + 2. If feature isn't canonical, switch to it (which will bump last_visit once T13 lands). + 3. Compute "since prior anchor" sections (commits, threads, drafts...). + 4. Compute current_state (feature_state, CI, bot rollup, branch position). + 5. Build intent_hints from the deltas + current state. + 6. If no switch happened, bump last_visit at the end. + 7. Return the complete brief. + +Refreshes from GitHub/Linear on every call. No caching at this layer. + +Single-bump invariant: last_visit moves exactly once per feature_resume call. + - switch ran → switch bumps (T13). Resume does NOT bump again. + - no switch → resume bumps at the end, after delta is computed. +""" +from __future__ import annotations + +from datetime import datetime, timezone +from typing import Any + +from ..workspace.workspace import Workspace +from . import last_visit as lv +from . import slots as slots_mod +from .aliases import resolve_feature +from .switch import switch + + +def feature_resume(workspace: Workspace, alias: str) -> dict[str, Any]: + """Resolve alias, switch-if-needed, build and return the resume brief.""" + feature = resolve_feature(workspace, alias) + now_iso = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + + # 1. Capture prior anchor BEFORE any switch can move it. + prior_visit = lv.get_last_visit(workspace, feature) + prior_iso: str | None = prior_visit["last_visit"] if prior_visit else None + + # 2. Switch-if-needed. Read slot state to decide. + switch_summary: dict | None = None + state = slots_mod.read_state(workspace) + is_canonical = ( + state is not None + and state.canonical is not None + and state.canonical.feature == feature + ) + if not is_canonical: + switch_summary = switch(workspace, feature) + # T13 will make switch bump last_visit internally. Until then, the + # single-bump invariant is: resume does NOT bump when switch ran. + + # 3. Empty containers — T7–T12 expand _populate_since / _populate_current. + since: dict[str, Any] = { + "commits": {}, + "threads_new": [], + "threads_resolved_on_github": [], + "threads_resolved_by_canopy": [], + "ci_status_delta": {}, + "draft_replies_pending": 0, + "historian_excerpt": "", + } + current: dict[str, Any] = { + "feature_state": None, + "open_thread_count": 0, + "ci_summary_per_repo": {}, + "bot_unresolved_total": 0, + "draft_replies_summary": {"addressed_total": 0, "unaddressed_total": 0}, + "branch_position_per_repo": {}, + "linear_issue": None, + "linear_url": None, + } + + # 4. Populate sections (prior_iso may be None on first visit). + if prior_iso is not None: + since = _populate_since(workspace, feature, prior_iso, since) + current = _populate_current(workspace, feature, current) + + # 5. Build intent hints from populated shapes. + intent_hints = _intent_hints(since, current, prior_iso is None) + + # 6. Single-bump: only bump when switch didn't already run. + if switch_summary is None: + lv.mark_visited(workspace, feature) + + # 7. Window duration (None on first visit). + window_hours = _hours_between(prior_iso, now_iso) if prior_iso is not None else None + + # Strip transport-only internal key before returning. + current.pop("__feature_name__", None) + + return { + "version": 1, + "feature": feature, + "now": now_iso, + "last_visit": prior_iso, # the PRIOR anchor, not freshly bumped + "first_visit": prior_iso is None, + "window_hours": window_hours, + "switch_performed": switch_summary is not None, + "switch_summary": switch_summary, + "intent_hints": intent_hints, + "since_last_visit": since, + "current_state": current, + } + + +# ── Section populators (stubs) ──────────────────────────────────────────────── +# T7–T12 fill these in. T6 leaves the shapes as-is. + +def _populate_since( + workspace: Workspace, + feature: str, + last_visit_iso: str, + since: dict[str, Any], +) -> dict[str, Any]: + """T7-T12 fill this. T6 leaves the shape as-is.""" + return since + + +def _populate_current( + workspace: Workspace, + feature: str, + current: dict[str, Any], +) -> dict[str, Any]: + """T9-T11 fill this. T6 only seeds the internal __feature_name__ key.""" + current["__feature_name__"] = feature + return current + + +# ── Intent hints ────────────────────────────────────────────────────────────── + +def _intent_hints( + since: dict[str, Any], + current: dict[str, Any], + first_visit: bool, +) -> list[dict]: + """Build prioritized next-action suggestions from the brief data. + + Hints are derived, not stored — recomputed on every call. Ordering by + ``priority`` field; the agent typically reads top 3. + """ + hints: list[dict] = [] + + # Address new comments (highest priority — reviewer activity is most + # actionable thing after returning). + new_threads = since.get("threads_new") or [] + if new_threads: + hints.append({ + "kind": "address_comments", + "summary": f"{len(new_threads)} new PR comment(s) since last visit", + "suggested_tool": "review_comments", + "suggested_args": {"alias": current.get("__feature_name__")}, + "priority": 1, + }) + + # Align with default branch (the user's "align with dev" intent path). + behind_per_repo = { + r: info.get("behind", 0) + for r, info in (current.get("branch_position_per_repo") or {}).items() + if info.get("behind", 0) > 0 + } + if behind_per_repo: + worst = max(behind_per_repo.items(), key=lambda kv: kv[1]) + hints.append({ + "kind": "align_with_default", + "summary": ( + f"behind default by {worst[1]} commits in {worst[0]}" + + ( + f" (+ {len(behind_per_repo) - 1} other repos)" + if len(behind_per_repo) > 1 + else "" + ) + ), + "suggested_tool": "log", + "suggested_args": {"repo": worst[0]}, + "priority": 2, + }) + + # Post drafted replies. + drafts = current.get("draft_replies_summary") or {} + if drafts.get("addressed_total", 0) > 0: + hints.append({ + "kind": "post_drafts", + "summary": f"{drafts['addressed_total']} draft replies ready", + "suggested_tool": "draft_replies", + "suggested_args": {"alias": current.get("__feature_name__")}, + "priority": 3, + }) + + # CI failing. + ci = current.get("ci_summary_per_repo") or {} + failing = [r for r, status in ci.items() if status == "failing"] + if failing: + hints.append({ + "kind": "investigate_ci", + "summary": f"CI failing in {', '.join(failing)}", + "suggested_tool": "pr_checks", + "suggested_args": {"alias": current.get("__feature_name__")}, + "priority": 1, # ties with comments — both are blockers + }) + + # First-visit special case: hint to read the linear issue. + if first_visit and current.get("linear_issue"): + hints.append({ + "kind": "read_issue", + "summary": f"first visit — read {current['linear_issue']}", + "suggested_tool": "linear_get_issue", + "suggested_args": {"alias": current.get("linear_issue")}, + "priority": 1, + }) + + hints.sort(key=lambda h: h["priority"]) + return hints + + +# ── Helpers ─────────────────────────────────────────────────────────────────── + +def _hours_between(start_iso: str, end_iso: str) -> float: + """Return elapsed hours between two ISO-Z timestamps.""" + def _parse(s: str) -> datetime: + return datetime.fromisoformat(s.replace("Z", "+00:00")) + return (_parse(end_iso) - _parse(start_iso)).total_seconds() / 3600.0 diff --git a/tests/test_resume.py b/tests/test_resume.py new file mode 100644 index 0000000..c435c1c --- /dev/null +++ b/tests/test_resume.py @@ -0,0 +1,277 @@ +"""Tests for actions/resume.py — feature_resume compound orchestrator. + +Fixture strategy +---------------- +- ``canopy_toml_for_workspace`` for all tests — this fixture has both + repo-a and repo-b with an ``auth-flow`` branch already checked out, + which means ``resolve_feature`` can resolve "auth-flow" without an + explicit features.json entry (implicit multi-repo feature). +- ``_make_canonical`` writes slots.json directly so switch doesn't need + to run in test setup (avoids worktree overhead). +- Monkeypatch for the switch-when-not-canonical path (faster and + avoids real worktree ops in tests). + +Single-bump invariant (plan lines 131-146): + - switch ran → resume does NOT call mark_visited. + - no switch → resume calls mark_visited once at the end. +""" +import pytest + +from canopy.actions import last_visit as lv +from canopy.actions import slots as slots_mod +from canopy.actions.resume import feature_resume +from canopy.workspace.config import load_config +from canopy.workspace.workspace import Workspace + + +# ── Helpers ─────────────────────────────────────────────────────────────────── + +def _load_workspace(root) -> Workspace: + """Load a Workspace from the fixture root path.""" + return Workspace(load_config(root)) + + +def _make_canonical(ws: Workspace, feature: str) -> None: + """Write slots.json so ``feature`` is recorded as canonical. + + Uses direct state write (no switch) to avoid worktree overhead in tests. + ``per_repo_paths`` points at the real repo dirs on disk so the staleness + check in ``read_state`` keeps the entry valid. + """ + per_repo = { + repo.name: str(ws.config.root / repo.path) + for repo in ws.config.repos + } + slots_mod.write_state( + ws, + slots_mod.SlotState( + slot_count=2, + canonical=slots_mod.CanonicalEntry( + feature=feature, + activated_at=slots_mod.now_iso(), + per_repo_paths=per_repo, + ), + ), + ) + + +# ── Tests ───────────────────────────────────────────────────────────────────── + +class TestFirstVisitAndShape: + """Tests that don't require switch (feature already canonical or mocked).""" + + def test_resume_first_visit_marks_flag(self, canopy_toml_for_workspace): + """No prior anchor → first_visit=True, last_visit=None.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + brief = feature_resume(ws, "auth-flow") + + assert brief["feature"] == "auth-flow" + assert brief["first_visit"] is True + assert brief["last_visit"] is None + assert "since_last_visit" in brief + assert "current_state" in brief + assert "intent_hints" in brief + assert isinstance(brief["intent_hints"], list) + + def test_resume_no_switch_when_already_canonical(self, canopy_toml_for_workspace): + """Feature already canonical → switch_performed=False, switch_summary=None.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + brief = feature_resume(ws, "auth-flow") + + assert brief["switch_performed"] is False + assert brief["switch_summary"] is None + + def test_resume_brief_shape_has_all_keys(self, canopy_toml_for_workspace): + """Regression guard: all required top-level keys must be present.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + brief = feature_resume(ws, "auth-flow") + + required_top = { + "version", "feature", "now", "last_visit", "first_visit", + "window_hours", "switch_performed", "switch_summary", + "intent_hints", "since_last_visit", "current_state", + } + assert required_top <= set(brief.keys()), ( + f"Missing keys: {required_top - set(brief.keys())}" + ) + + required_since = { + "commits", "threads_new", "threads_resolved_on_github", + "threads_resolved_by_canopy", "ci_status_delta", + "draft_replies_pending", "historian_excerpt", + } + assert required_since <= set(brief["since_last_visit"].keys()), ( + f"Missing since_last_visit keys: " + f"{required_since - set(brief['since_last_visit'].keys())}" + ) + + required_current = { + "feature_state", "open_thread_count", "ci_summary_per_repo", + "bot_unresolved_total", "draft_replies_summary", + "branch_position_per_repo", "linear_issue", "linear_url", + } + assert required_current <= set(brief["current_state"].keys()), ( + f"Missing current_state keys: " + f"{required_current - set(brief['current_state'].keys())}" + ) + + def test_resume_strips_internal_keys(self, canopy_toml_for_workspace): + """__feature_name__ transport key must NOT appear in the returned brief.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + brief = feature_resume(ws, "auth-flow") + + assert "__feature_name__" not in brief["current_state"] + + def test_resume_since_containers_are_empty_stubs(self, canopy_toml_for_workspace): + """T6 stubs: all since_last_visit containers are empty (T7+ fills them).""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + # Pre-seed a visit so _populate_since is invoked. + lv.mark_visited(ws, "auth-flow") + brief = feature_resume(ws, "auth-flow") + + s = brief["since_last_visit"] + assert s["commits"] == {} + assert s["threads_new"] == [] + assert s["threads_resolved_on_github"] == [] + assert s["threads_resolved_by_canopy"] == [] + assert s["ci_status_delta"] == {} + assert s["draft_replies_pending"] == 0 + assert s["historian_excerpt"] == "" + + +class TestSwitchBehavior: + """Tests for the switch-if-not-canonical path (monkeypatched).""" + + def test_resume_switches_when_not_canonical( + self, canopy_toml_for_workspace, monkeypatch + ): + """Feature not canonical → switch() is called with (workspace, feature).""" + ws = _load_workspace(canopy_toml_for_workspace) + # No slots.json → read_state returns None → not canonical → switch runs. + calls = [] + + def fake_switch(workspace, feature, **kwargs): + calls.append((workspace, feature)) + return {"feature": feature, "switch_performed": True} + + # Patch the module-level `switch` name in resume so feature_resume uses fake. + import canopy.actions.resume as resume_mod + monkeypatch.setattr(resume_mod, "switch", fake_switch) + + brief = feature_resume(ws, "auth-flow") + + assert brief["switch_performed"] is True + assert brief["switch_summary"] is not None + assert len(calls) == 1 + assert calls[0][1] == "auth-flow" + + def test_resume_does_not_double_bump_when_switch_ran( + self, canopy_toml_for_workspace, monkeypatch + ): + """Single-bump invariant: resume does NOT call mark_visited when switch ran. + + The mock switch does NOT bump last_visit (T13 hasn't landed). We verify + that resume also skips the bump, leaving the anchor at the pre-seeded value. + """ + ws = _load_workspace(canopy_toml_for_workspace) + # Pre-seed a visit. + t0 = lv.mark_visited(ws, "auth-flow") + + def fake_switch(workspace, feature, **kwargs): + # Deliberately does NOT bump last_visit (simulating pre-T13 state). + return {"feature": feature, "switch_performed": True} + + import canopy.actions.resume as resume_mod + monkeypatch.setattr(resume_mod, "switch", fake_switch) + # Report NOT canonical so the switch branch fires. + monkeypatch.setattr(resume_mod.slots_mod, "read_state", lambda ws: None) + + feature_resume(ws, "auth-flow") + + # last_visit should still be t0 — neither mock-switch nor resume bumped. + after = lv.get_last_visit(ws, "auth-flow") + assert after["last_visit"] == t0, ( + "resume must not bump last_visit when switch_summary is truthy " + "(single-bump invariant)" + ) + + +class TestAnchorBumping: + """Tests for the last_visit bumping logic (no-switch path).""" + + def test_resume_bumps_anchor_once_when_no_switch(self, canopy_toml_for_workspace): + """No switch ran → resume bumps last_visit at the end.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + ts1 = lv.mark_visited(ws, "auth-flow") + + feature_resume(ws, "auth-flow") + + after = lv.get_last_visit(ws, "auth-flow") + assert after["last_visit"] >= ts1, "last_visit must have advanced" + assert after["previous_visit"] == ts1, ( + "previous_visit must equal the pre-resume anchor" + ) + + def test_resume_diffs_against_prior_anchor(self, canopy_toml_for_workspace): + """The brief's last_visit field must show the anchor BEFORE the bump. + + Ensures the orchestrator captures prior_iso before calling mark_visited + so T7+ populators diff against the correct window. + """ + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + t0 = lv.mark_visited(ws, "auth-flow") + + brief = feature_resume(ws, "auth-flow") + + # The brief must report t0 (the prior anchor), not the freshly-bumped value. + assert brief["last_visit"] == t0, ( + "brief['last_visit'] must be the anchor captured BEFORE the bump" + ) + + def test_resume_returns_window_hours_when_visited(self, canopy_toml_for_workspace): + """window_hours >= 0 when a prior anchor exists; first_visit=False.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + lv.mark_visited(ws, "auth-flow") + + brief = feature_resume(ws, "auth-flow") + + assert brief["first_visit"] is False + assert brief["window_hours"] is not None + assert brief["window_hours"] >= 0.0 + + def test_resume_first_visit_has_no_window(self, canopy_toml_for_workspace): + """On first visit, window_hours must be None.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + brief = feature_resume(ws, "auth-flow") + + assert brief["first_visit"] is True + assert brief["window_hours"] is None + + +class TestIntentHints: + """Tests for _intent_hints with empty stubs (T6 state: all hints []).""" + + def test_intent_hints_empty_when_no_data(self, canopy_toml_for_workspace): + """With T6 stubs, no populators fill since/current, so no hints fire.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + brief = feature_resume(ws, "auth-flow") + + assert brief["intent_hints"] == [] + + def test_intent_hints_is_list(self, canopy_toml_for_workspace): + """intent_hints must always be a list, never None.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + brief = feature_resume(ws, "auth-flow") + + assert isinstance(brief["intent_hints"], list) From b5597b6996d69dd57d4adc099728458970dbad06 Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 08:32:04 +0530 Subject: [PATCH 08/21] feat(resume): populate commits-since-last-visit per repo (T7) Add log_since() to git/repo.py for ISO-timestamp-based commit filtering. Implement _commits_since() and _populate_since() in actions/resume.py to populate per-repo commits authored after the last-visit anchor. Each repo in the feature maps to a list of {sha, short_sha, at, author, subject}. Per-repo git errors default to empty list; no crash on missing branch/repo. Add 3 tests: - Happy path: commit made after anchor appears in resume brief - First visit: commits not populated (no prior anchor) - No-new-commits: all repos map to empty lists when no commits made after anchor 802 tests passing (799 + 3 new). --- src/canopy/actions/resume.py | 27 +++++++++- src/canopy/git/repo.py | 30 +++++++++++ tests/test_resume.py | 97 +++++++++++++++++++++++++++++++++++- 3 files changed, 151 insertions(+), 3 deletions(-) diff --git a/src/canopy/actions/resume.py b/src/canopy/actions/resume.py index c757818..464af35 100644 --- a/src/canopy/actions/resume.py +++ b/src/canopy/actions/resume.py @@ -112,10 +112,35 @@ def _populate_since( last_visit_iso: str, since: dict[str, Any], ) -> dict[str, Any]: - """T7-T12 fill this. T6 leaves the shape as-is.""" + """T7-T12 fill this. T7 populates commits.""" + since["commits"] = _commits_since(workspace, feature, last_visit_iso) return since +def _commits_since(workspace: Workspace, feature: str, since_iso: str) -> dict[str, list]: + """Populate per-repo commits authored after since_iso on the feature branch. + + Returns {repo_name: [commit dicts]} where each commit has + {sha, short_sha, at, author, subject}. Per-repo errors (missing branch, + git failures) silently default to empty list; exceptions don't crash the brief. + """ + from ..git import repo as git + from .aliases import repos_for_feature + + out: dict[str, list] = {} + repos_map = repos_for_feature(workspace, feature) + + for repo_name, branch in repos_map.items(): + try: + state = workspace.get_repo(repo_name) + out[repo_name] = git.log_since(state.abs_path, branch, since_iso) + except Exception: + # Missing repo in workspace, or git error — default to empty list. + out[repo_name] = [] + + return out + + def _populate_current( workspace: Workspace, feature: str, diff --git a/src/canopy/git/repo.py b/src/canopy/git/repo.py index 062fa71..8b7f1b7 100644 --- a/src/canopy/git/repo.py +++ b/src/canopy/git/repo.py @@ -827,3 +827,33 @@ def log_structured( "subject": parts[4], }) return entries + + +def log_since(repo_path: Path, branch: str, since_iso: str) -> list[dict]: + """Return commits on ``branch`` authored after ``since_iso`` (ISO 8601). + + Used by feature_resume to populate the commits-since-last-visit section. + Returns a list of {sha, short_sha, at, author, subject} or [] on error. + """ + sep = "\x1f" # unit separator + fmt = f"%H{sep}%h{sep}%aI{sep}%an{sep}%s" + output = _run_ok( + ["log", branch, f"--since={since_iso}", f"--format={fmt}"], + cwd=repo_path, + ) + if not output: + return [] + + entries = [] + for line in output.splitlines(): + parts = line.split(sep) + if len(parts) < 5: + continue + entries.append({ + "sha": parts[0], + "short_sha": parts[1], + "at": parts[2], + "author": parts[3], + "subject": parts[4], + }) + return entries diff --git a/tests/test_resume.py b/tests/test_resume.py index c435c1c..5c9ad63 100644 --- a/tests/test_resume.py +++ b/tests/test_resume.py @@ -127,7 +127,10 @@ def test_resume_strips_internal_keys(self, canopy_toml_for_workspace): assert "__feature_name__" not in brief["current_state"] def test_resume_since_containers_are_empty_stubs(self, canopy_toml_for_workspace): - """T6 stubs: all since_last_visit containers are empty (T7+ fills them).""" + """T6-T7 stubs: most since_last_visit containers are empty (T8+ fills them). + + T7 fills commits; T8-T12 fill the rest. + """ ws = _load_workspace(canopy_toml_for_workspace) _make_canonical(ws, "auth-flow") # Pre-seed a visit so _populate_since is invoked. @@ -135,7 +138,9 @@ def test_resume_since_containers_are_empty_stubs(self, canopy_toml_for_workspace brief = feature_resume(ws, "auth-flow") s = brief["since_last_visit"] - assert s["commits"] == {} + # T7 fills commits (so it's now a populated dict, not empty {}). + assert isinstance(s["commits"], dict), "commits must be populated by T7" + # T8+ fill the remaining containers. assert s["threads_new"] == [] assert s["threads_resolved_on_github"] == [] assert s["threads_resolved_by_canopy"] == [] @@ -275,3 +280,91 @@ def test_intent_hints_is_list(self, canopy_toml_for_workspace): brief = feature_resume(ws, "auth-flow") assert isinstance(brief["intent_hints"], list) + + +class TestCommitsSinceLastVisit: + """Tests for T7: commits-since-last-visit population (T7).""" + + def test_resume_includes_commits_per_repo(self, canopy_toml_for_workspace): + """Commits authored after last_visit, on the feature branch.""" + import subprocess + import os + import time + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + # Mark first visit. + lv.mark_visited(ws, "auth-flow") + time.sleep(1.1) # ensure --since granularity passes + + # Make a commit on auth-flow in repo-a (repo_a is the first repo in canopy_toml). + repo_a = ws.config.root / ws.config.repos[0].path + env = { + **os.environ, + "GIT_AUTHOR_NAME": "Test", + "GIT_AUTHOR_EMAIL": "test@test.com", + "GIT_COMMITTER_NAME": "Test", + "GIT_COMMITTER_EMAIL": "test@test.com", + } + # Ensure we're on auth-flow branch. + subprocess.run(["git", "checkout", "auth-flow"], cwd=repo_a, env=env, check=True) + # Make an empty commit with a recognizable message. + subprocess.run( + ["git", "commit", "--allow-empty", "-m", "tweak: update auth module"], + cwd=repo_a, + env=env, + check=True, + ) + + # Resume and check commits are included. + brief = feature_resume(ws, "auth-flow") + repo_a_name = ws.config.repos[0].name + repo_a_commits = brief["since_last_visit"]["commits"].get(repo_a_name, []) + + assert len(repo_a_commits) >= 1, "Expected at least one commit in repo_a since last visit" + assert "tweak" in repo_a_commits[0]["subject"], "Expected 'tweak' in commit subject" + assert "update auth module" in repo_a_commits[0]["subject"] + + def test_resume_commits_empty_when_no_anchor(self, canopy_toml_for_workspace): + """First visit (no prior anchor) → commits not populated.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + # No mark_visited() call — first visit. + + brief = feature_resume(ws, "auth-flow") + + # On first visit, _populate_since is not called (prior_iso is None). + # commits should remain as the empty stub from initialization. + assert brief["since_last_visit"]["commits"] == {} + + def test_resume_commits_empty_when_no_new_commits(self, canopy_toml_for_workspace): + """Anchor set, no new commits → repos map to []. + + Fixture creates commits at setup time. We set anchor, sleep, then resume. + Since no commits are made AFTER the anchor, all repos should be empty. + """ + import time + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + # Sleep to ensure fixture commits are older than the anchor. + time.sleep(2) + # Mark visit — this captures the current second as the anchor. + lv.mark_visited(ws, "auth-flow") + # Sleep past the second to verify absence of new commits. + time.sleep(1.1) + + # Resume WITHOUT making new commits. + brief = feature_resume(ws, "auth-flow") + + commits = brief["since_last_visit"]["commits"] + assert isinstance(commits, dict), "commits must be a dict" + + # All repos should have empty lists (no commits AFTER the anchor). + for repo_name, commit_list in commits.items(): + assert commit_list == [], ( + f"Expected empty commit list for {repo_name}, " + f"but got {commit_list}" + ) From 78c4c8e0dcb5b3547ed40aeca93c95132f337c71 Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 08:39:00 +0530 Subject: [PATCH 09/21] =?UTF-8?q?feat(resume):=20thread=20delta=20?= =?UTF-8?q?=E2=80=94=20new=20+=20resolved-on-GH=20+=20resolved-by-canopy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Populates three since_last_visit sub-sections: - threads_new: unresolved threads whose first comment arrived after last_visit - threads_resolved_on_github: resolved threads since last_visit with by_canopy flag (joined against thread_resolutions.json) - threads_resolved_by_canopy: bot_resolutions entries for this feature since anchor All GH calls are swallowed on error; brief always completes. 5 new tests added. --- src/canopy/actions/resume.py | 131 +++++++++++++++++++- tests/test_resume.py | 230 ++++++++++++++++++++++++++++++++++- 2 files changed, 358 insertions(+), 3 deletions(-) diff --git a/src/canopy/actions/resume.py b/src/canopy/actions/resume.py index 464af35..b984ab3 100644 --- a/src/canopy/actions/resume.py +++ b/src/canopy/actions/resume.py @@ -112,8 +112,14 @@ def _populate_since( last_visit_iso: str, since: dict[str, Any], ) -> dict[str, Any]: - """T7-T12 fill this. T7 populates commits.""" + """T7-T12 fill this. T7 populates commits. T8 populates thread deltas.""" since["commits"] = _commits_since(workspace, feature, last_visit_iso) + threads = _threads_delta(workspace, feature, last_visit_iso) + since["threads_new"] = threads["new"] + since["threads_resolved_on_github"] = threads["resolved_gh"] + since["threads_resolved_by_canopy"] = _resolutions_by_canopy_since( + workspace, feature, last_visit_iso, + ) return since @@ -141,6 +147,129 @@ def _commits_since(workspace: Workspace, feature: str, since_iso: str) -> dict[s return out +def _pr_coords_per_repo( + workspace: Workspace, feature: str, +) -> dict[str, dict | None]: + """Return {repo_name: {"owner": str, "repo_slug": str, "pr_number": int} | None}. + + Uses the same pattern as FeatureCoordinator.review_status: iterates repos + in the feature lane, resolves remote URL → owner/slug, finds the open PR. + On any per-repo error (no remote, unparseable URL, no PR) returns None for + that repo. Propagates only hard exceptions (feature not found, etc.). + """ + from ..git import repo as git + from ..integrations.github import _extract_owner_repo, find_pull_request + from .aliases import repos_for_feature + + repos_map = repos_for_feature(workspace, feature) + out: dict[str, dict | None] = {} + + for repo_name, branch in repos_map.items(): + try: + state = workspace.get_repo(repo_name) + remote = git.remote_url(state.abs_path) + if not remote: + out[repo_name] = None + continue + parsed = _extract_owner_repo(remote) + if not parsed: + out[repo_name] = None + continue + owner, repo_slug = parsed + pr = find_pull_request(workspace.config.root, owner, repo_slug, branch) + if pr is None: + out[repo_name] = None + else: + out[repo_name] = { + "owner": owner, + "repo_slug": repo_slug, + "pr_number": pr["number"], + } + except Exception: + out[repo_name] = None + + return out + + +def _threads_delta( + workspace: Workspace, feature: str, since_iso: str, +) -> dict[str, list]: + """Return {"new": [...], "resolved_gh": [...]}. + + Calls list_review_threads per-repo+PR. On ANY exception (no PR yet, + GH unreachable, etc.), returns {"new": [], "resolved_gh": []} and + swallows. Never crashes the brief. + """ + from ..integrations import github as gh + from . import thread_resolutions as tr + + try: + pr_coords = _pr_coords_per_repo(workspace, feature) + except Exception: + return {"new": [], "resolved_gh": []} + + canopy_log = tr.load(workspace.config.root) + new_threads: list[dict] = [] + resolved_gh: list[dict] = [] + + for repo_name, coords in pr_coords.items(): + if not coords: + continue + owner = coords["owner"] + repo_slug = coords["repo_slug"] + pr_number = coords["pr_number"] + try: + threads = gh.list_review_threads( + workspace.config.root, owner, repo_slug, pr_number, + ) + except Exception: + continue + for t in threads: + first = (t.get("comments") or [None])[0] + created_at = (first or {}).get("created_at", "") + if (not t["is_resolved"]) and created_at > since_iso: + new_threads.append({ + "thread_id": t["thread_id"], + "comment_id": (first or {}).get("comment_id"), + "author": (first or {}).get("author", ""), + "path": (first or {}).get("path", ""), + "line": (first or {}).get("line", 0), + "body_excerpt": ((first or {}).get("body") or "")[:200], + "created_at": created_at, + "url": (first or {}).get("url", ""), + "repo": repo_name, + "pr_number": pr_number, + }) + elif t["is_resolved"] and (t.get("resolved_at") or "") > since_iso: + resolved_gh.append({ + "thread_id": t["thread_id"], + "resolved_at": t["resolved_at"], + "by_canopy": t["thread_id"] in canopy_log, + "repo": repo_name, + "pr_number": pr_number, + "summary_excerpt": ((first or {}).get("body") or "")[:200], + }) + + return {"new": new_threads, "resolved_gh": resolved_gh} + + +def _resolutions_by_canopy_since( + workspace: Workspace, feature: str, since_iso: str, +) -> list[dict]: + """Bot_resolutions entries for this feature with addressed_at > since_iso.""" + from . import bot_resolutions as br + + out: list[dict] = [] + try: + entries = br.resolutions_for_feature(workspace.config.root, feature) + except Exception: + return [] + for cid, e in entries.items(): + if e.get("addressed_at", "") > since_iso: + out.append({"comment_id": cid, **e}) + return out + + def _populate_current( workspace: Workspace, feature: str, diff --git a/tests/test_resume.py b/tests/test_resume.py index 5c9ad63..a162fc7 100644 --- a/tests/test_resume.py +++ b/tests/test_resume.py @@ -15,6 +15,9 @@ - switch ran → resume does NOT call mark_visited. - no switch → resume calls mark_visited once at the end. """ +import json +import time + import pytest from canopy.actions import last_visit as lv @@ -344,8 +347,6 @@ def test_resume_commits_empty_when_no_new_commits(self, canopy_toml_for_workspac Fixture creates commits at setup time. We set anchor, sleep, then resume. Since no commits are made AFTER the anchor, all repos should be empty. """ - import time - ws = _load_workspace(canopy_toml_for_workspace) _make_canonical(ws, "auth-flow") @@ -368,3 +369,228 @@ def test_resume_commits_empty_when_no_new_commits(self, canopy_toml_for_workspac f"Expected empty commit list for {repo_name}, " f"but got {commit_list}" ) + + +class TestThreadDeltaSinceLastVisit: + """Tests for T8: threads_new, threads_resolved_on_github, threads_resolved_by_canopy.""" + + # Shared fake threads used across several tests. + _FAKE_THREADS = [ + { + "thread_id": "PRRT_old", + "is_resolved": False, + "resolved_at": None, + "comments": [{ + "comment_id": 1, + "created_at": "1900-01-01T00:00:00Z", + "author": "alice", + "path": "a.py", + "line": 1, + "body": "old comment", + "url": "https://github.com/o/r/pull/1#discussion_r1", + }], + }, + { + "thread_id": "PRRT_new", + "is_resolved": False, + "resolved_at": None, + "comments": [{ + "comment_id": 2, + "created_at": "2999-01-01T00:00:00Z", + "author": "bob", + "path": "b.py", + "line": 2, + "body": "new comment", + "url": "https://github.com/o/r/pull/1#discussion_r2", + }], + }, + ] + + def _patch_pr_coords(self, monkeypatch): + """Patch _pr_coords_per_repo to return one fake repo+PR.""" + import canopy.actions.resume as resume_mod + monkeypatch.setattr( + resume_mod, + "_pr_coords_per_repo", + lambda ws, f: {"repo-a": {"owner": "o", "repo_slug": "r", "pr_number": 1}}, + ) + + def test_resume_threads_new_only_after_last_visit( + self, canopy_toml_for_workspace, monkeypatch + ): + """Only threads with created_at > last_visit and not resolved land in threads_new.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + lv.mark_visited(ws, "auth-flow") + time.sleep(1.1) + + self._patch_pr_coords(monkeypatch) + monkeypatch.setattr( + "canopy.integrations.github.list_review_threads", + lambda *a, **k: self._FAKE_THREADS, + ) + + brief = feature_resume(ws, "auth-flow") + new_ids = [t["thread_id"] for t in brief["since_last_visit"]["threads_new"]] + assert new_ids == ["PRRT_new"], ( + f"Expected only PRRT_new in threads_new, got {new_ids}" + ) + # PRRT_old predates the anchor and must be absent. + assert "PRRT_old" not in new_ids + + def test_resume_threads_resolved_gh_attributed_to_canopy( + self, canopy_toml_for_workspace, monkeypatch + ): + """A resolved thread whose thread_id is in thread_resolutions.json gets by_canopy=True.""" + from canopy.actions import thread_resolutions as tr + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + lv.mark_visited(ws, "auth-flow") + time.sleep(1.1) + + # Pre-write a canopy resolution for PRRT_resolved. + tr.record( + ws.config.root, + thread_id="PRRT_resolved", + feature="auth-flow", + via_command="resolve", + ) + + resolved_thread = { + "thread_id": "PRRT_resolved", + "is_resolved": True, + "resolved_at": "2999-06-01T00:00:00Z", + "comments": [{ + "comment_id": 10, + "created_at": "2999-01-01T00:00:00Z", + "author": "carol", + "path": "x.py", + "line": 5, + "body": "please fix this", + "url": "https://github.com/o/r/pull/1#discussion_r10", + }], + } + + self._patch_pr_coords(monkeypatch) + monkeypatch.setattr( + "canopy.integrations.github.list_review_threads", + lambda *a, **k: [resolved_thread], + ) + + brief = feature_resume(ws, "auth-flow") + resolved = brief["since_last_visit"]["threads_resolved_on_github"] + assert len(resolved) == 1 + assert resolved[0]["thread_id"] == "PRRT_resolved" + assert resolved[0]["by_canopy"] is True + + def test_resume_threads_resolved_gh_not_attributed_when_external( + self, canopy_toml_for_workspace, monkeypatch + ): + """A resolved thread with no canopy log entry gets by_canopy=False.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + lv.mark_visited(ws, "auth-flow") + time.sleep(1.1) + + resolved_thread = { + "thread_id": "PRRT_external", + "is_resolved": True, + "resolved_at": "2999-06-01T00:00:00Z", + "comments": [{ + "comment_id": 20, + "created_at": "2999-01-01T00:00:00Z", + "author": "dave", + "path": "y.py", + "line": 7, + "body": "fix this too", + "url": "https://github.com/o/r/pull/1#discussion_r20", + }], + } + + self._patch_pr_coords(monkeypatch) + monkeypatch.setattr( + "canopy.integrations.github.list_review_threads", + lambda *a, **k: [resolved_thread], + ) + + brief = feature_resume(ws, "auth-flow") + resolved = brief["since_last_visit"]["threads_resolved_on_github"] + assert len(resolved) == 1 + assert resolved[0]["thread_id"] == "PRRT_external" + assert resolved[0]["by_canopy"] is False + + def test_resume_threads_by_canopy_filtered_by_feature_and_since( + self, canopy_toml_for_workspace, monkeypatch + ): + """bot_resolutions entries: only matching feature AND addressed_at > anchor appear.""" + from canopy.actions.bot_resolutions import record_resolution + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + lv.mark_visited(ws, "auth-flow") + time.sleep(1.1) + + # Record one entry for auth-flow AFTER anchor, one BEFORE, one for wrong feature. + record_resolution( + ws.config.root, + comment_id="111", + feature="auth-flow", + repo="repo-a", + commit_sha="abc", + comment_title="fix cache", + addressed_at="2999-01-01T00:00:00Z", # after anchor → should appear + ) + record_resolution( + ws.config.root, + comment_id="222", + feature="auth-flow", + repo="repo-a", + commit_sha="def", + comment_title="old fix", + addressed_at="1900-01-01T00:00:00Z", # before anchor → must be excluded + ) + record_resolution( + ws.config.root, + comment_id="333", + feature="other-feature", + repo="repo-a", + commit_sha="ghi", + comment_title="unrelated", + addressed_at="2999-06-01T00:00:00Z", # after anchor but wrong feature + ) + + # Suppress GH calls — threads_by_canopy doesn't need them. + self._patch_pr_coords(monkeypatch) + monkeypatch.setattr( + "canopy.integrations.github.list_review_threads", + lambda *a, **k: [], + ) + + brief = feature_resume(ws, "auth-flow") + by_canopy = brief["since_last_visit"]["threads_resolved_by_canopy"] + cids = [e["comment_id"] for e in by_canopy] + assert "111" in cids, "entry 111 (auth-flow, after anchor) must appear" + assert "222" not in cids, "entry 222 (auth-flow, before anchor) must be excluded" + assert "333" not in cids, "entry 333 (other-feature) must be excluded" + + def test_resume_threads_delta_empty_when_no_pr( + self, canopy_toml_for_workspace, monkeypatch + ): + """When _pr_coords_per_repo returns {}, all three thread fields are empty arrays.""" + import canopy.actions.resume as resume_mod + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + lv.mark_visited(ws, "auth-flow") + time.sleep(1.1) + + monkeypatch.setattr( + resume_mod, "_pr_coords_per_repo", lambda ws, f: {}, + ) + + brief = feature_resume(ws, "auth-flow") + s = brief["since_last_visit"] + assert s["threads_new"] == [] + assert s["threads_resolved_on_github"] == [] + assert s["threads_resolved_by_canopy"] == [] From 1fef8a09eb8e1f922a8cf79a0b0455792bd6c451 Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 08:45:51 +0530 Subject: [PATCH 10/21] feat(resume): current_state.feature_state + ci_summary + branch_position Implements T9: _populate_current now fills feature_state (or "unknown" on failure), ci_summary_per_repo (lifted from summary.ci_per_repo), and branch_position_per_repo (ahead/behind/last_sync_at per repo vs default branch). The align_with_default intent hint fires correctly when any repo has behind > 0. Adds 4 tests; suite at 811 passed. --- src/canopy/actions/resume.py | 47 ++++++++++++- tests/test_resume.py | 127 +++++++++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 1 deletion(-) diff --git a/src/canopy/actions/resume.py b/src/canopy/actions/resume.py index b984ab3..1e06af2 100644 --- a/src/canopy/actions/resume.py +++ b/src/canopy/actions/resume.py @@ -275,8 +275,53 @@ def _populate_current( feature: str, current: dict[str, Any], ) -> dict[str, Any]: - """T9-T11 fill this. T6 only seeds the internal __feature_name__ key.""" + """T9: feature_state, ci_summary_per_repo, branch_position_per_repo. + + T10-T11 fill the remaining sections. Errors in any sub-section are + swallowed so the brief always returns with reasonable defaults. + """ current["__feature_name__"] = feature + + from . import feature_state as fs + from ..git import repo as git + from .aliases import repos_for_feature + + # feature_state + ci_summary_per_repo ───────────────────────────────── + try: + st = fs.feature_state(workspace, feature) + except Exception: + st = {} + + current["feature_state"] = st.get("state", "unknown") + + # CI lives in summary["ci_per_repo"] → {repo: {"status": ...}} + ci_per_repo = (st.get("summary") or {}).get("ci_per_repo") or {} + current["ci_summary_per_repo"] = { + r: (info.get("status") or "no_checks") + for r, info in ci_per_repo.items() + } + + # branch_position_per_repo ──────────────────────────────────────────── + pos: dict[str, dict] = {} + for repo_name, branch in repos_for_feature(workspace, feature).items(): + try: + repo_state = workspace.get_repo(repo_name) + default = repo_state.config.default_branch + ahead, behind = git.divergence(repo_state.abs_path, branch, default) + last_sync_at = git.commit_iso_date( + repo_state.abs_path, f"{branch}...{default}", + ) + pos[repo_name] = { + "branch": branch, + "default_branch": default, + "ahead": ahead, + "behind": behind, + "last_sync_at": last_sync_at or "", + } + except Exception: + continue + current["branch_position_per_repo"] = pos + return current diff --git a/tests/test_resume.py b/tests/test_resume.py index a162fc7..fddeafe 100644 --- a/tests/test_resume.py +++ b/tests/test_resume.py @@ -594,3 +594,130 @@ def test_resume_threads_delta_empty_when_no_pr( assert s["threads_new"] == [] assert s["threads_resolved_on_github"] == [] assert s["threads_resolved_by_canopy"] == [] + + +class TestCurrentStatePopulation: + """Tests for T9: feature_state, ci_summary_per_repo, branch_position_per_repo.""" + + def test_resume_branch_position_per_repo(self, canopy_toml_for_workspace): + """branch_position_per_repo has correct shape for each repo in the feature.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + brief = feature_resume(ws, "auth-flow") + pos = brief["current_state"]["branch_position_per_repo"] + + assert isinstance(pos, dict), "branch_position_per_repo must be a dict" + # The fixture creates auth-flow in both repo-a and repo-b. + assert len(pos) >= 1, "At least one repo should appear in branch_position" + + for repo_name, entry in pos.items(): + assert "branch" in entry, f"{repo_name}: missing 'branch'" + assert "default_branch" in entry, f"{repo_name}: missing 'default_branch'" + assert "ahead" in entry, f"{repo_name}: missing 'ahead'" + assert "behind" in entry, f"{repo_name}: missing 'behind'" + assert "last_sync_at" in entry, f"{repo_name}: missing 'last_sync_at'" + assert entry["ahead"] >= 0, f"{repo_name}: ahead must be >= 0" + assert entry["behind"] >= 0, f"{repo_name}: behind must be >= 0" + assert entry["branch"] == "auth-flow" + assert isinstance(entry["last_sync_at"], str) + + def test_resume_ci_summary_per_repo(self, canopy_toml_for_workspace, monkeypatch): + """ci_summary_per_repo lifts CI status strings from feature_state summary.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + def fake_feature_state(workspace, feature): + return { + "state": "awaiting_ci", + "summary": { + "ci_per_repo": { + "repo-a": {"status": "passing"}, + "repo-b": {"status": "failing"}, + }, + }, + } + + monkeypatch.setattr( + "canopy.actions.feature_state.feature_state", + fake_feature_state, + ) + + brief = feature_resume(ws, "auth-flow") + ci = brief["current_state"]["ci_summary_per_repo"] + + assert ci.get("repo-a") == "passing" + assert ci.get("repo-b") == "failing" + assert brief["current_state"]["feature_state"] == "awaiting_ci" + + def test_resume_align_with_default_hint_surfaces( + self, canopy_toml_for_workspace, monkeypatch + ): + """align_with_default intent hint fires when any repo has behind > 0.""" + import os + import subprocess + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + env = { + **os.environ, + "GIT_AUTHOR_NAME": "Test", + "GIT_AUTHOR_EMAIL": "test@test.com", + "GIT_COMMITTER_NAME": "Test", + "GIT_COMMITTER_EMAIL": "test@test.com", + } + + # Add a commit to main in repo-a AFTER the auth-flow branch was created, + # so auth-flow ends up behind main by 1. + repo_a = canopy_toml_for_workspace / "repo-a" + subprocess.run(["git", "checkout", "main"], cwd=repo_a, env=env, check=True) + subprocess.run( + ["git", "commit", "--allow-empty", "-m", "main: advance default"], + cwd=repo_a, env=env, check=True, + ) + subprocess.run(["git", "checkout", "auth-flow"], cwd=repo_a, env=env, check=True) + + # Suppress feature_state (avoids GH calls) — we only need branch_position. + monkeypatch.setattr( + "canopy.actions.feature_state.feature_state", + lambda ws, f: {}, + ) + + brief = feature_resume(ws, "auth-flow") + + pos = brief["current_state"]["branch_position_per_repo"] + assert pos.get("repo-a", {}).get("behind", 0) > 0, ( + "repo-a should be behind main after the commit on main" + ) + + hint_kinds = [h["kind"] for h in brief["intent_hints"]] + assert "align_with_default" in hint_kinds, ( + f"align_with_default hint must fire when behind > 0; hints={brief['intent_hints']}" + ) + + align_hint = next(h for h in brief["intent_hints"] if h["kind"] == "align_with_default") + assert align_hint["priority"] == 2 + + def test_resume_feature_state_unknown_when_lookup_fails( + self, canopy_toml_for_workspace, monkeypatch + ): + """feature_state lookup raises → current_state.feature_state == 'unknown', brief intact.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + def boom(workspace, feature): + raise RuntimeError("simulated failure") + + monkeypatch.setattr( + "canopy.actions.feature_state.feature_state", + boom, + ) + + brief = feature_resume(ws, "auth-flow") + + assert brief["current_state"]["feature_state"] == "unknown" + # The rest of the brief should still be populated (not crash). + assert "branch_position_per_repo" in brief["current_state"] + assert "ci_summary_per_repo" in brief["current_state"] + assert isinstance(brief["intent_hints"], list) From fb7c41e0fec507cc145d2a5833f2d6726c163d0e Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 08:50:56 +0530 Subject: [PATCH 11/21] feat(resume): current_state.bot_unresolved_total --- src/canopy/actions/resume.py | 10 +++++++ tests/test_resume.py | 52 ++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/src/canopy/actions/resume.py b/src/canopy/actions/resume.py index 1e06af2..bf9d96b 100644 --- a/src/canopy/actions/resume.py +++ b/src/canopy/actions/resume.py @@ -283,6 +283,7 @@ def _populate_current( current["__feature_name__"] = feature from . import feature_state as fs + from . import bot_status as bs from ..git import repo as git from .aliases import repos_for_feature @@ -301,6 +302,15 @@ def _populate_current( for r, info in ci_per_repo.items() } + # bot_unresolved_total ──────────────────────────────────────────────── + try: + roll = bs.bot_comments_status(workspace, feature) + except Exception: + roll = {"repos": {}} + current["bot_unresolved_total"] = sum( + r.get("unresolved", 0) for r in (roll.get("repos") or {}).values() + ) + # branch_position_per_repo ──────────────────────────────────────────── pos: dict[str, dict] = {} for repo_name, branch in repos_for_feature(workspace, feature).items(): diff --git a/tests/test_resume.py b/tests/test_resume.py index fddeafe..19056d6 100644 --- a/tests/test_resume.py +++ b/tests/test_resume.py @@ -721,3 +721,55 @@ def boom(workspace, feature): assert "branch_position_per_repo" in brief["current_state"] assert "ci_summary_per_repo" in brief["current_state"] assert isinstance(brief["intent_hints"], list) + + def test_resume_bot_unresolved_total_from_status( + self, canopy_toml_for_workspace, monkeypatch + ): + """bot_unresolved_total sums unresolved counts per repo from bot_comments_status.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + def fake_bot_comments_status(workspace, feature): + return { + "feature": feature, + "repos": { + "repo-a": {"unresolved": 2, "resolved": 1, "total": 3}, + "repo-b": {"unresolved": 1, "resolved": 0, "total": 1}, + }, + "all_resolved": False, + "any_bot_comments": True, + } + + monkeypatch.setattr( + "canopy.actions.bot_status.bot_comments_status", + fake_bot_comments_status, + ) + + brief = feature_resume(ws, "auth-flow") + + # Sum of unresolved: repo-a(2) + repo-b(1) = 3 + assert brief["current_state"]["bot_unresolved_total"] == 3 + + def test_resume_bot_unresolved_total_zero_on_failure( + self, canopy_toml_for_workspace, monkeypatch + ): + """bot_comments_status raises → bot_unresolved_total == 0, brief intact.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + def boom(workspace, feature): + raise RuntimeError("simulated bot_status failure") + + monkeypatch.setattr( + "canopy.actions.bot_status.bot_comments_status", + boom, + ) + + brief = feature_resume(ws, "auth-flow") + + # Error swallowed, defaults to 0. + assert brief["current_state"]["bot_unresolved_total"] == 0 + # Rest of brief intact. + assert brief["feature"] == "auth-flow" + assert "branch_position_per_repo" in brief["current_state"] + assert isinstance(brief["intent_hints"], list) From 6705995ce2febd7185e3d87a69d998d6d7cb327c Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 08:57:56 +0530 Subject: [PATCH 12/21] feat(resume): draft_replies_summary + draft_replies_pending (T11) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Populate current_state.draft_replies_summary from draft_replies() call, capturing addressed_total and unaddressed_total. Populate since_last_visit.draft_replies_pending with count of addressed-but-not-yet-posted drafts only when there is a prior anchor (not on first visit). Both populate with graceful error handling — failures leave defaults in place. The existing post_drafts intent hint already fires when addressed_total > 0, now backed by real data. Add 4 tests: - draft_replies_summary populated from call - post_drafts hint fires when addressed_total > 0 - draft_replies_pending only when anchor exists (0 on first visit) - error handling preserves defaults All 817 tests passing. --- src/canopy/actions/resume.py | 25 ++++++ tests/test_resume.py | 153 +++++++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+) diff --git a/src/canopy/actions/resume.py b/src/canopy/actions/resume.py index bf9d96b..7b6a3fa 100644 --- a/src/canopy/actions/resume.py +++ b/src/canopy/actions/resume.py @@ -120,6 +120,19 @@ def _populate_since( since["threads_resolved_by_canopy"] = _resolutions_by_canopy_since( workspace, feature, last_visit_iso, ) + + # T11: draft_replies_pending — count of addressed-but-not-yet-posted drafts. + # Only populate when there's a prior anchor (not first visit). + from . import draft_replies as dr + try: + drafts = dr.draft_replies(workspace, feature) + since["draft_replies_pending"] = sum( + len(r.get("addressed") or []) + for r in (drafts.get("repos") or {}).values() + ) + except Exception: + pass # leaves the default 0 + return since @@ -311,6 +324,18 @@ def _populate_current( r.get("unresolved", 0) for r in (roll.get("repos") or {}).values() ) + # draft_replies_summary ─────────────────────────────────────────────── + # T11: Populate from draft_replies; swallow errors, use defaults. + from . import draft_replies as dr + try: + drafts = dr.draft_replies(workspace, feature) + current["draft_replies_summary"] = { + "addressed_total": drafts.get("addressed_total", 0), + "unaddressed_total": drafts.get("unaddressed_total", 0), + } + except Exception: + pass # leaves the T6 default {addressed_total: 0, unaddressed_total: 0} + # branch_position_per_repo ──────────────────────────────────────────── pos: dict[str, dict] = {} for repo_name, branch in repos_for_feature(workspace, feature).items(): diff --git a/tests/test_resume.py b/tests/test_resume.py index 19056d6..6ed10bc 100644 --- a/tests/test_resume.py +++ b/tests/test_resume.py @@ -773,3 +773,156 @@ def boom(workspace, feature): assert brief["feature"] == "auth-flow" assert "branch_position_per_repo" in brief["current_state"] assert isinstance(brief["intent_hints"], list) + + +class TestDraftRepliesSummary: + """Tests for T11: draft_replies_summary and draft_replies_pending.""" + + def test_resume_draft_replies_summary_in_current( + self, canopy_toml_for_workspace, monkeypatch + ): + """T11: draft_replies_summary populated from draft_replies call.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + def fake_drafts(workspace, feature, **kwargs): + return { + "alias": feature, + "addressed_total": 3, + "unaddressed_total": 1, + "repos": { + "repo-a": { + "pr_number": 42, + "pr_url": "https://github.com/a/pr/42", + "addressed": [{"comment_id": 1}, {"comment_id": 2}], + "unaddressed": [{"comment_id": 3}], + }, + "repo-b": { + "pr_number": 43, + "pr_url": "https://github.com/b/pr/43", + "addressed": [{"comment_id": 4}], + "unaddressed": [], + }, + }, + } + + monkeypatch.setattr( + "canopy.actions.draft_replies.draft_replies", + fake_drafts, + ) + + brief = feature_resume(ws, "auth-flow") + + assert brief["current_state"]["draft_replies_summary"] == { + "addressed_total": 3, + "unaddressed_total": 1, + } + + def test_resume_post_drafts_hint_fires_when_addressed_total_gt_0( + self, canopy_toml_for_workspace, monkeypatch + ): + """T11: post_drafts intent hint fires when addressed_total > 0.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + def fake_drafts(workspace, feature, **kwargs): + return { + "alias": feature, + "addressed_total": 2, + "unaddressed_total": 0, + "repos": { + "repo-a": { + "pr_number": 42, + "pr_url": "https://github.com/a/pr/42", + "addressed": [{"comment_id": 1}, {"comment_id": 2}], + "unaddressed": [], + }, + }, + } + + monkeypatch.setattr( + "canopy.actions.draft_replies.draft_replies", + fake_drafts, + ) + + brief = feature_resume(ws, "auth-flow") + + # Check that post_drafts hint is in the list. + post_drafts_hints = [h for h in brief["intent_hints"] if h["kind"] == "post_drafts"] + assert len(post_drafts_hints) == 1 + hint = post_drafts_hints[0] + assert hint["summary"] == "2 draft replies ready" + assert hint["suggested_tool"] == "draft_replies" + assert hint["priority"] == 3 + + def test_resume_draft_replies_pending_only_when_anchor( + self, canopy_toml_for_workspace, monkeypatch + ): + """T11: draft_replies_pending only when prior anchor exists (not first visit).""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + def fake_drafts(workspace, feature, **kwargs): + return { + "alias": feature, + "addressed_total": 2, + "unaddressed_total": 0, + "repos": { + "repo-a": { + "pr_number": 42, + "pr_url": "https://github.com/a/pr/42", + "addressed": [{"comment_id": 1}, {"comment_id": 2}], + "unaddressed": [], + }, + "repo-b": { + "pr_number": 43, + "pr_url": "https://github.com/b/pr/43", + "addressed": [], + "unaddressed": [], + }, + }, + } + + monkeypatch.setattr( + "canopy.actions.draft_replies.draft_replies", + fake_drafts, + ) + + # First visit (no anchor): draft_replies_pending should be 0. + brief1 = feature_resume(ws, "auth-flow") + assert brief1["first_visit"] is True + assert brief1["since_last_visit"]["draft_replies_pending"] == 0 + + # After seeding a visit, resume again (now has anchor). + lv.mark_visited(ws, "auth-flow") + brief2 = feature_resume(ws, "auth-flow") + assert brief2["first_visit"] is False + # Sum of addressed per repo: repo-a(2) + repo-b(0) = 2. + assert brief2["since_last_visit"]["draft_replies_pending"] == 2 + + def test_resume_draft_replies_summary_zero_on_failure( + self, canopy_toml_for_workspace, monkeypatch + ): + """T11: draft_replies failure → summary defaults, brief intact.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + def boom(workspace, feature, **kwargs): + raise RuntimeError("simulated draft_replies failure") + + monkeypatch.setattr( + "canopy.actions.draft_replies.draft_replies", + boom, + ) + + brief = feature_resume(ws, "auth-flow") + + # Error swallowed, defaults to zero totals. + assert brief["current_state"]["draft_replies_summary"] == { + "addressed_total": 0, + "unaddressed_total": 0, + } + # Rest of brief intact. + assert brief["feature"] == "auth-flow" + assert "branch_position_per_repo" in brief["current_state"] + assert isinstance(brief["intent_hints"], list) From dc7864371a40a5b559130b0c4aec0f2f5a961afb Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 09:09:10 +0530 Subject: [PATCH 13/21] fix(milestone-2): linear from lane + open_thread_count rollup + hint coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Populate current_state.linear_issue/linear_url from FeatureLane via FeatureCoordinator.status(); empty string normalized to None; lane lookup failures swallowed and default to None. - Add _open_thread_count helper that rolls up unresolved threads per-PR using _pr_coords_per_repo + list_review_threads; TODO comment marks the 2x round-trip as a milestone-3 cache opportunity. - Tests: investigate_ci hint, read_issue hint (first visit + linear_issue), read_issue absent on second visit, _pr_coords_per_repo direct (file:// remotes → None), future-anchor edge case (2099 timestamp → empty since sections, no crash), open_thread_count rollup + zero paths, linear populated/none/failure-swallowed. --- src/canopy/actions/resume.py | 48 ++++++ tests/test_resume.py | 278 +++++++++++++++++++++++++++++++++++ 2 files changed, 326 insertions(+) diff --git a/src/canopy/actions/resume.py b/src/canopy/actions/resume.py index 7b6a3fa..a455e2f 100644 --- a/src/canopy/actions/resume.py +++ b/src/canopy/actions/resume.py @@ -357,9 +357,57 @@ def _populate_current( continue current["branch_position_per_repo"] = pos + # linear_issue / linear_url — lifted from FeatureLane ───────────────── + try: + from ..features.coordinator import FeatureCoordinator + coord = FeatureCoordinator(workspace) + lane = coord.status(feature) + current["linear_issue"] = getattr(lane, "linear_issue", None) or None + current["linear_url"] = getattr(lane, "linear_url", None) or None + except Exception: + pass # leaves the defaults (None) from initialization + + # open_thread_count — rolled up from list_review_threads per PR ─────── + current["open_thread_count"] = _open_thread_count(workspace, feature) + return current +# ── Helpers for current_state ───────────────────────────────────────────────── + +def _open_thread_count(workspace: Workspace, feature: str) -> int: + """Total unresolved review threads across all repos+PRs for the feature. + + Calls list_review_threads per-repo+PR using the same coords that + _threads_delta uses. On any exception, returns 0 and swallows. + + # TODO: cache list_review_threads per resume call to avoid 2x round-trips + # when _threads_delta already ran in _populate_since (milestone-3 item). + """ + from ..integrations import github as gh + + try: + pr_coords = _pr_coords_per_repo(workspace, feature) + except Exception: + return 0 + + total = 0 + for repo_name, coords in pr_coords.items(): + if not coords: + continue + try: + threads = gh.list_review_threads( + workspace.config.root, + coords["owner"], + coords["repo_slug"], + coords["pr_number"], + ) + except Exception: + continue + total += sum(1 for t in threads if not t.get("is_resolved", False)) + return total + + # ── Intent hints ────────────────────────────────────────────────────────────── def _intent_hints( diff --git a/tests/test_resume.py b/tests/test_resume.py index 6ed10bc..b767f55 100644 --- a/tests/test_resume.py +++ b/tests/test_resume.py @@ -926,3 +926,281 @@ def boom(workspace, feature, **kwargs): assert brief["feature"] == "auth-flow" assert "branch_position_per_repo" in brief["current_state"] assert isinstance(brief["intent_hints"], list) + + +class TestLinearFromLane: + """Tests for Fix #1: linear_issue + linear_url populated from FeatureLane.""" + + def test_resume_populates_linear_from_lane(self, canopy_toml_for_workspace, monkeypatch): + """Brief surfaces lane.linear_issue and lane.linear_url.""" + import canopy.actions.resume as resume_mod + from canopy.features.coordinator import FeatureLane + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + fake_lane = FeatureLane( + name="auth-flow", + repos=["repo-a", "repo-b"], + linear_issue="DOC-42", + linear_url="https://linear.app/team/issue/DOC-42", + ) + + import canopy.features.coordinator as coord_mod + original_status = coord_mod.FeatureCoordinator.status + + def fake_status(self, name): + return fake_lane + + monkeypatch.setattr(coord_mod.FeatureCoordinator, "status", fake_status) + + brief = feature_resume(ws, "auth-flow") + + assert brief["current_state"]["linear_issue"] == "DOC-42" + assert brief["current_state"]["linear_url"] == "https://linear.app/team/issue/DOC-42" + + def test_resume_linear_none_when_lane_has_no_issue(self, canopy_toml_for_workspace): + """Implicit feature (no features.json entry) → linear_issue/url are None.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + # auth-flow is an implicit feature in the fixture (no features.json entry), + # so linear_issue defaults to "" on the lane → normalized to None. + brief = feature_resume(ws, "auth-flow") + + assert brief["current_state"]["linear_issue"] is None + assert brief["current_state"]["linear_url"] is None + + def test_resume_linear_none_when_lane_lookup_fails( + self, canopy_toml_for_workspace, monkeypatch + ): + """If FeatureLane lookup raises, linear_issue/url default to None and brief is intact.""" + import canopy.features.coordinator as coord_mod + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + def boom(self, name): + raise RuntimeError("simulated coordinator failure") + + monkeypatch.setattr(coord_mod.FeatureCoordinator, "status", boom) + + brief = feature_resume(ws, "auth-flow") + + assert brief["current_state"]["linear_issue"] is None + assert brief["current_state"]["linear_url"] is None + # Rest of brief intact. + assert brief["feature"] == "auth-flow" + assert isinstance(brief["intent_hints"], list) + + +class TestOpenThreadCount: + """Tests for Fix #2: open_thread_count rolled up from list_review_threads.""" + + def test_resume_open_thread_count_rolled_up( + self, canopy_toml_for_workspace, monkeypatch + ): + """open_thread_count sums unresolved threads across repos.""" + import canopy.actions.resume as resume_mod + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + monkeypatch.setattr( + resume_mod, + "_pr_coords_per_repo", + lambda ws, f: {"repo-a": {"owner": "o", "repo_slug": "r", "pr_number": 1}}, + ) + monkeypatch.setattr( + "canopy.integrations.github.list_review_threads", + lambda *a, **k: [ + {"thread_id": "PRRT_1", "is_resolved": False, "comments": []}, + {"thread_id": "PRRT_2", "is_resolved": True, "comments": []}, + {"thread_id": "PRRT_3", "is_resolved": False, "comments": []}, + ], + ) + + brief = feature_resume(ws, "auth-flow") + assert brief["current_state"]["open_thread_count"] == 2 + + def test_resume_open_thread_count_zero_when_no_prs( + self, canopy_toml_for_workspace, monkeypatch + ): + """When all PR coords are None, open_thread_count is 0.""" + import canopy.actions.resume as resume_mod + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + monkeypatch.setattr( + resume_mod, + "_pr_coords_per_repo", + lambda ws, f: {"repo-a": None}, + ) + + brief = feature_resume(ws, "auth-flow") + assert brief["current_state"]["open_thread_count"] == 0 + + def test_resume_open_thread_count_zero_on_exception( + self, canopy_toml_for_workspace, monkeypatch + ): + """If _pr_coords_per_repo raises, open_thread_count defaults to 0.""" + import canopy.actions.resume as resume_mod + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + def boom(ws, f): + raise RuntimeError("simulated failure") + + monkeypatch.setattr(resume_mod, "_pr_coords_per_repo", boom) + + brief = feature_resume(ws, "auth-flow") + assert brief["current_state"]["open_thread_count"] == 0 + + +class TestHintCoverage: + """Tests for SHOULD FIX items: investigate_ci and read_issue hints.""" + + def test_resume_investigate_ci_hint_surfaces( + self, canopy_toml_for_workspace, monkeypatch + ): + """When any repo CI is failing, investigate_ci hint fires at priority 1.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + monkeypatch.setattr( + "canopy.actions.feature_state.feature_state", + lambda ws, f: { + "state": "awaiting_ci", + "summary": { + "ci_per_repo": { + "repo-a": {"status": "failing"}, + "repo-b": {"status": "passing"}, + }, + }, + }, + ) + + brief = feature_resume(ws, "auth-flow") + + ci_hints = [h for h in brief["intent_hints"] if h["kind"] == "investigate_ci"] + assert len(ci_hints) == 1, f"Expected 1 investigate_ci hint, got {ci_hints}" + assert ci_hints[0]["priority"] == 1 + assert "repo-a" in ci_hints[0]["summary"] + + def test_resume_read_issue_hint_on_first_visit( + self, canopy_toml_for_workspace, monkeypatch + ): + """First visit AND linear_issue set → read_issue hint fires at priority 1.""" + import canopy.features.coordinator as coord_mod + from canopy.features.coordinator import FeatureLane + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + # No mark_visited() → first_visit=True + + fake_lane = FeatureLane( + name="auth-flow", + repos=["repo-a", "repo-b"], + linear_issue="DOC-42", + linear_url="https://linear.app/team/issue/DOC-42", + ) + + monkeypatch.setattr( + coord_mod.FeatureCoordinator, "status", lambda self, name: fake_lane + ) + + brief = feature_resume(ws, "auth-flow") + + assert brief["first_visit"] is True + read_hints = [h for h in brief["intent_hints"] if h["kind"] == "read_issue"] + assert len(read_hints) == 1, ( + f"Expected 1 read_issue hint on first visit with linear_issue set; " + f"hints={brief['intent_hints']}" + ) + assert read_hints[0]["priority"] == 1 + assert "DOC-42" in read_hints[0]["summary"] + + def test_resume_read_issue_hint_not_on_second_visit( + self, canopy_toml_for_workspace, monkeypatch + ): + """read_issue hint must NOT fire when first_visit=False, even with linear_issue set.""" + import canopy.features.coordinator as coord_mod + from canopy.features.coordinator import FeatureLane + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + lv.mark_visited(ws, "auth-flow") # second visit now + + fake_lane = FeatureLane( + name="auth-flow", + repos=["repo-a", "repo-b"], + linear_issue="DOC-42", + linear_url="https://linear.app/team/issue/DOC-42", + ) + monkeypatch.setattr( + coord_mod.FeatureCoordinator, "status", lambda self, name: fake_lane + ) + + brief = feature_resume(ws, "auth-flow") + + assert brief["first_visit"] is False + read_hints = [h for h in brief["intent_hints"] if h["kind"] == "read_issue"] + assert read_hints == [], "read_issue hint must not fire on second visit" + + +class TestPrCoordsPerRepo: + """Direct tests for _pr_coords_per_repo.""" + + def test_pr_coords_per_repo_handles_unresolvable_remote( + self, canopy_toml_for_workspace + ): + """_pr_coords_per_repo returns repo -> None for unparseable (file://) remotes.""" + from canopy.actions.resume import _pr_coords_per_repo + + ws = _load_workspace(canopy_toml_for_workspace) + # The fixture uses file:// remotes which _extract_owner_repo cannot parse. + result = _pr_coords_per_repo(ws, "auth-flow") + + assert isinstance(result, dict) + assert len(result) >= 1, "Expected at least one repo in result" + for repo_name, coords in result.items(): + assert coords is None, ( + f"{repo_name}: expected None for file:// remote, got {coords}" + ) + + +class TestFutureAnchor: + """Edge case: last_visit set to a future timestamp.""" + + def test_resume_future_anchor_returns_empty_sections( + self, canopy_toml_for_workspace + ): + """Anchor in the future → since_last_visit sections are empty, brief doesn't crash.""" + from canopy.actions import last_visit as lv_mod + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + # Write a future timestamp directly. + lv_mod._save( + ws, + {"auth-flow": {"last_visit": "2099-01-01T00:00:00Z", "previous_visit": None}}, + ) + + brief = feature_resume(ws, "auth-flow") + + assert brief["first_visit"] is False + assert brief["last_visit"] == "2099-01-01T00:00:00Z" + + # commits: all repos should have empty lists (no commits after 2099). + commits = brief["since_last_visit"]["commits"] + assert isinstance(commits, dict) + for repo_name, commit_list in commits.items(): + assert commit_list == [], ( + f"{repo_name}: expected no commits before 2099 anchor, got {commit_list}" + ) + + # thread sections should also be empty. + assert brief["since_last_visit"]["threads_new"] == [] + assert brief["since_last_visit"]["threads_resolved_on_github"] == [] From 27983ef31723520acfcd344cdc93ea48381ea600 Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 09:19:12 +0530 Subject: [PATCH 14/21] feat(resume): historian session excerpts since last visit Add format_for_agent_since(workspace_root, feature, since_iso) to historian.py to render only entries with timestamp > since_iso. This enables windowed session summaries in the feature-resume brief. Wire the new helper into _populate_since to populate since_last_visit[ historian_excerpt] when an anchor exists. Timestamps filtered lexicographically using ISO 8601 Z format (e.g. "2026-05-26T15:30:00Z"). Tests added: - historian: 5 tests for format_for_agent_since (filter, empty cases, headers, boundaries) - resume: 4 tests for historian_excerpt integration (anchor, first visit, errors, filtering) All 837 tests pass (9 new tests added). --- src/canopy/actions/historian.py | 18 ++++ src/canopy/actions/resume.py | 9 ++ tests/test_historian.py | 82 ++++++++++++++++ tests/test_resume.py | 159 ++++++++++++++++++++++++++++++++ 4 files changed, 268 insertions(+) diff --git a/src/canopy/actions/historian.py b/src/canopy/actions/historian.py index b36b762..e4d75b0 100644 --- a/src/canopy/actions/historian.py +++ b/src/canopy/actions/historian.py @@ -316,6 +316,24 @@ def format_for_agent(workspace_root: Path, feature: str) -> str: return _render(feature, entries) +def format_for_agent_since( + workspace_root: Path, feature: str, since_iso: str, +) -> str: + """Render only entries with timestamp > since_iso. + + Returns an empty string when no entries match the filter or the feature + has no memory yet. Timestamps are compared lexicographically, so + since_iso must be in ISO 8601 Z format (e.g. "2026-05-26T15:30:00Z"). + """ + entries = _load_entries(workspace_root, feature) + if not entries: + return "" + filtered = [e for e in entries if e.get("at", "") > since_iso] + if not filtered: + return "" + return _render(feature, filtered) + + # ── Compaction ────────────────────────────────────────────────────────── diff --git a/src/canopy/actions/resume.py b/src/canopy/actions/resume.py index a455e2f..ed60a8e 100644 --- a/src/canopy/actions/resume.py +++ b/src/canopy/actions/resume.py @@ -133,6 +133,15 @@ def _populate_since( except Exception: pass # leaves the default 0 + # T12: historian_excerpt — sessions/events/decisions since last_visit. + from . import historian + try: + since["historian_excerpt"] = historian.format_for_agent_since( + workspace.config.root, feature, last_visit_iso, + ) + except Exception: + pass # leaves the default "" + return since diff --git a/tests/test_historian.py b/tests/test_historian.py index c65b79e..f8347f2 100644 --- a/tests/test_historian.py +++ b/tests/test_historian.py @@ -238,3 +238,85 @@ def test_compact_drops_old_sessions_keeps_structural(tmp_path, monkeypatch): def test_compact_noop_when_no_memory(tmp_path): assert compact(tmp_path, "ghost")["action"] == "noop" + + +# ── format_for_agent_since ────────────────────────────────────────────────── + + +def test_format_for_agent_since_filters_by_timestamp(tmp_path): + """Only entries with at > since_iso appear in the output.""" + # Write entries with specific timestamps. + record_event(tmp_path, "auth-flow", summary="old event", + at="2026-01-01T00:00:00Z") + record_event(tmp_path, "auth-flow", summary="new event", + at="2026-05-26T00:00:00Z") + record_decision(tmp_path, "auth-flow", title="new decision", + at="2026-05-25T12:00:00Z") + + # Import format_for_agent_since + from canopy.actions.historian import format_for_agent_since + + # Filter with a threshold between old and new. + md = format_for_agent_since(tmp_path, "auth-flow", "2026-05-01T00:00:00Z") + + # Should include both entries newer than the threshold. + assert "new event" in md + assert "new decision" in md + # Should exclude the old entry. + assert "old event" not in md + + +def test_format_for_agent_since_empty_when_all_older(tmp_path): + """Returns empty string when no entry beats the since_iso threshold.""" + from canopy.actions.historian import format_for_agent_since + + record_event(tmp_path, "auth-flow", summary="ancient", + at="2026-01-01T00:00:00Z") + record_event(tmp_path, "auth-flow", summary="old", + at="2026-02-01T00:00:00Z") + + md = format_for_agent_since(tmp_path, "auth-flow", "2026-05-01T00:00:00Z") + assert md == "" + + +def test_format_for_agent_since_empty_when_no_feature(tmp_path): + """Returns empty string when the feature has no history.""" + from canopy.actions.historian import format_for_agent_since + + md = format_for_agent_since(tmp_path, "ghost-feature", "2026-05-01T00:00:00Z") + assert md == "" + + +def test_format_for_agent_since_includes_section_headers(tmp_path): + """Output is properly structured markdown with section headers.""" + from canopy.actions.historian import format_for_agent_since + + record_decision(tmp_path, "feat-1", title="library choice", + at="2026-05-26T10:00:00Z") + record_comment_resolved(tmp_path, "feat-1", comment_id=5, commit_sha="abc123", + at="2026-05-26T11:00:00Z") + + md = format_for_agent_since(tmp_path, "feat-1", "2026-05-20T00:00:00Z") + + # Should have feature header and all standard sections. + assert "# Feature: feat-1" in md + assert "## Resolutions log" in md + assert "## PR context" in md + assert "## Sessions (newest first)" in md + + +def test_format_for_agent_since_boundary_exact_match(tmp_path): + """Entry at exactly since_iso is excluded (> comparison, not >=).""" + from canopy.actions.historian import format_for_agent_since + + record_event(tmp_path, "feat-1", summary="at boundary", + at="2026-05-26T12:00:00Z") + record_event(tmp_path, "feat-1", summary="after boundary", + at="2026-05-26T12:00:01Z") + + md = format_for_agent_since(tmp_path, "feat-1", "2026-05-26T12:00:00Z") + + # Exact match should be excluded. + assert "at boundary" not in md + # After should be included. + assert "after boundary" in md diff --git a/tests/test_resume.py b/tests/test_resume.py index b767f55..e5e56fd 100644 --- a/tests/test_resume.py +++ b/tests/test_resume.py @@ -1204,3 +1204,162 @@ def test_resume_future_anchor_returns_empty_sections( # thread sections should also be empty. assert brief["since_last_visit"]["threads_new"] == [] assert brief["since_last_visit"]["threads_resolved_on_github"] == [] + + +class TestHistorianExcerpt: + """Tests for T12: historian_excerpt in since_last_visit.""" + + def test_resume_historian_excerpt_populated_when_anchor_exists( + self, canopy_toml_for_workspace, monkeypatch + ): + """When historian has entries past last_visit, the brief carries the excerpt.""" + from canopy.actions import historian + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + # Mark first visit. + t_anchor = lv.mark_visited(ws, "auth-flow") + time.sleep(1.1) + + # Record a historian entry after the anchor. + historian.record_event( + ws.config.root, + "auth-flow", + summary="reviewed PR comments", + at="2026-05-26T15:00:00Z", + ) + + # Mock format_for_agent_since to verify it's called with correct args. + calls = [] + original_fn = historian.format_for_agent_since + + def tracked_format(root, feature, since_iso): + calls.append((root, feature, since_iso)) + return original_fn(root, feature, since_iso) + + monkeypatch.setattr( + "canopy.actions.historian.format_for_agent_since", + tracked_format, + ) + + brief = feature_resume(ws, "auth-flow") + + # Verify the function was called with correct arguments. + assert len(calls) == 1, f"Expected 1 call, got {len(calls)}" + assert calls[0][0] == ws.config.root + assert calls[0][1] == "auth-flow" + assert calls[0][2] == t_anchor, "Must pass the prior anchor timestamp" + + # The excerpt should be populated (non-empty when entry exists after anchor). + assert isinstance(brief["since_last_visit"]["historian_excerpt"], str) + + def test_resume_historian_excerpt_empty_on_first_visit( + self, canopy_toml_for_workspace, monkeypatch + ): + """On first visit (no prior anchor), historian_excerpt is '' (populator not called).""" + from canopy.actions import historian + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + # Pre-populate some historian entries (but we won't have an anchor). + historian.record_event( + ws.config.root, + "auth-flow", + summary="old event", + at="2026-01-01T00:00:00Z", + ) + + # Mock to ensure format_for_agent_since is NOT called on first visit. + calls = [] + + def tracked_format(root, feature, since_iso): + calls.append((root, feature, since_iso)) + return "" + + monkeypatch.setattr( + "canopy.actions.historian.format_for_agent_since", + tracked_format, + ) + + brief = feature_resume(ws, "auth-flow") + + # On first visit, _populate_since is NOT called (prior_iso is None). + # So format_for_agent_since should not be called. + assert len(calls) == 0, ( + f"format_for_agent_since must not be called on first visit; " + f"got {len(calls)} calls" + ) + + # historian_excerpt should remain the default empty string. + assert brief["since_last_visit"]["historian_excerpt"] == "" + assert brief["first_visit"] is True + + def test_resume_historian_excerpt_empty_string_on_failure( + self, canopy_toml_for_workspace, monkeypatch + ): + """If format_for_agent_since raises, historian_excerpt defaults to ''.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + lv.mark_visited(ws, "auth-flow") + time.sleep(1.1) + + def boom(root, feature, since_iso): + raise RuntimeError("simulated historian failure") + + monkeypatch.setattr( + "canopy.actions.historian.format_for_agent_since", + boom, + ) + + brief = feature_resume(ws, "auth-flow") + + # Error swallowed, defaults to empty string. + assert brief["since_last_visit"]["historian_excerpt"] == "" + # Rest of brief intact. + assert brief["feature"] == "auth-flow" + assert "commits" in brief["since_last_visit"] + assert isinstance(brief["intent_hints"], list) + + def test_resume_historian_excerpt_filters_old_sessions( + self, canopy_toml_for_workspace, monkeypatch + ): + """historian_excerpt includes only entries newer than last_visit.""" + from canopy.actions import historian + from datetime import datetime, timezone, timedelta + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + # Record old entry before anchor (1 hour ago). + now = datetime.now(timezone.utc) + old_time = (now - timedelta(hours=1)).strftime("%Y-%m-%dT%H:%M:%SZ") + historian.record_event( + ws.config.root, + "auth-flow", + summary="old session work", + at=old_time, + ) + + t_anchor = lv.mark_visited(ws, "auth-flow") + time.sleep(1.1) + + # Record new entry after anchor (current time, which is after the anchor). + new_time = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + historian.record_decision( + ws.config.root, + "auth-flow", + title="new library choice", + at=new_time, + ) + + brief = feature_resume(ws, "auth-flow") + + excerpt = brief["since_last_visit"]["historian_excerpt"] + # New entry should appear. + assert "new library choice" in excerpt + # Old entry should not appear. + assert "old session work" not in excerpt + # Should still have markdown structure. + assert "# Feature: auth-flow" in excerpt From c5c3ea80a421de7bce6002c093432ba7531a2a7a Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 09:26:06 +0530 Subject: [PATCH 15/21] feat(switch): bump last_visit on every switch-into-feature T13: Call lv.mark_visited(workspace, feature_name) in _post_switch_persist after slots_mod.write_state completes. Every successful switch into a feature counts as a conscious look per the single-bump invariant. This ensures the resume/switch separation of concerns: switch bumps once after all state is committed, and resume does not bump again when switch ran. Test coverage: - test_switch_bumps_last_visit: basic bump on first switch - test_switch_bumps_previous_visit_on_reswitch: previous_visit carries old anchor - test_resume_single_bump_when_real_switch_runs: single-bump invariant with real switch Acceptance criteria met: - Every successful switch bumps last_visit - No bump on failed switch (mark_visited after write_state) - Single-bump invariant holds: resume skips bump when switch ran - All tests passing (57 in test_switch/resume, full suite pending) --- src/canopy/actions/switch.py | 6 +++++ tests/test_resume.py | 40 ++++++++++++++++++++++++++++++ tests/test_switch.py | 48 ++++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+) diff --git a/src/canopy/actions/switch.py b/src/canopy/actions/switch.py index e886af7..660cb81 100644 --- a/src/canopy/actions/switch.py +++ b/src/canopy/actions/switch.py @@ -478,6 +478,12 @@ def _post_switch_persist( state.in_flight = None slots_mod.write_state(workspace, state) + + # T13: bump last_visit after slots.json is committed — every successful + # switch into a feature counts as a "conscious look" per the plan. + from . import last_visit as lv + lv.mark_visited(workspace, feature_name) + out["activated_at"] = now if state.previous_canonical: out["previous_feature_in_state"] = state.previous_canonical diff --git a/tests/test_resume.py b/tests/test_resume.py index e5e56fd..1afed38 100644 --- a/tests/test_resume.py +++ b/tests/test_resume.py @@ -208,6 +208,46 @@ def fake_switch(workspace, feature, **kwargs): "(single-bump invariant)" ) + def test_resume_single_bump_when_real_switch_runs( + self, canopy_toml_for_workspace + ): + """feature_resume with real switch (T13 on): switch bumps once, resume doesn't bump again. + + This test verifies the single-bump invariant with the real switch.switch() + implementation, which now bumps last_visit after write_state. + """ + from canopy.actions.switch import switch + ws = _load_workspace(canopy_toml_for_workspace) + + # Make auth-flow canonical, leave Y as a cold branch. + _make_canonical(ws, "auth-flow") + + # Pre-seed an anchor for auth-flow. + t0 = lv.mark_visited(ws, "auth-flow") + time.sleep(1.1) + + # Create Y branch in both repos for the second switch. + import subprocess + for repo in ("repo-a", "repo-b"): + subprocess.run( + ["git", "branch", "Y"], + cwd=ws.config.root / repo, + check=True, + ) + + # Now resume on Y (not canonical, so switch will run). + brief = feature_resume(ws, "Y") + + # Verify switch ran. + assert brief["switch_performed"] is True + + # Verify Y's last_visit was bumped by switch (and not again by resume). + visit_after = lv.get_last_visit(ws, "Y") + assert visit_after is not None + assert visit_after["last_visit"] > t0, ( + "Y's last_visit must have been bumped by switch" + ) + class TestAnchorBumping: """Tests for the last_visit bumping logic (no-switch path).""" diff --git a/tests/test_switch.py b/tests/test_switch.py index 670f2f2..fb7effa 100644 --- a/tests/test_switch.py +++ b/tests/test_switch.py @@ -187,3 +187,51 @@ def test_switch_blocked_when_in_flight_set(workspace_with_canonical_only): with pytest.raises(BlockerError) as e: switch(ws, "Y") assert e.value.code == "slot_state_inconsistent" + + +# ── T13: last_visit bumping ──────────────────────────────────────────────── + + +def test_switch_bumps_last_visit(workspace_with_canonical_only): + """Successful switch advances last_visit for the target feature.""" + ws = workspace_with_canonical_only + from canopy.actions.switch import switch + from canopy.actions import last_visit as lv + + # Pre-condition: no anchor for Y + assert lv.get_last_visit(ws, "Y") is None + + # Switch to Y + result = switch(ws, "Y") + + # Verify switch succeeded + assert result["feature"] == "Y" + + # Verify last_visit was bumped + visit = lv.get_last_visit(ws, "Y") + assert visit is not None + assert "last_visit" in visit + assert visit["last_visit"] is not None + + +def test_switch_bumps_previous_visit_on_reswitch(workspace_with_canonical_only): + """Subsequent switches roll the prior anchor into previous_visit.""" + import time + ws = workspace_with_canonical_only + from canopy.actions.switch import switch + from canopy.actions import last_visit as lv + + # First switch to Y + switch(ws, "Y") + ts1 = lv.get_last_visit(ws, "Y")["last_visit"] + + # Wait to ensure timestamp advances + time.sleep(1.1) + + # Re-switch to Y (no-op on branches, but anchor still bumps) + switch(ws, "Y") + + # Verify last_visit advanced and previous_visit holds the old value + after = lv.get_last_visit(ws, "Y") + assert after["last_visit"] > ts1, "last_visit must have advanced" + assert after["previous_visit"] == ts1, "previous_visit must hold the old anchor" From 7863a519c9ce18f9d5ee297fb55712302d65d3df Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 09:39:43 +0530 Subject: [PATCH 16/21] feat(switch): embed since_last_visit_summary in switch return --- src/canopy/actions/resume.py | 70 ++++++++++++++++ src/canopy/actions/switch.py | 26 +++++- tests/test_resume.py | 149 +++++++++++++++++++++++++++++++++++ tests/test_switch.py | 96 ++++++++++++++++++++++ 4 files changed, 340 insertions(+), 1 deletion(-) diff --git a/src/canopy/actions/resume.py b/src/canopy/actions/resume.py index ed60a8e..cce49ba 100644 --- a/src/canopy/actions/resume.py +++ b/src/canopy/actions/resume.py @@ -27,6 +27,76 @@ from .switch import switch +_UNSET = object() # sentinel to distinguish "not passed" from "explicitly None" + + +def resume_summary( + workspace: Workspace, feature: str, prior_iso: str | None = _UNSET, # type: ignore[assignment] +) -> dict[str, Any]: + """Counts-only view embedded in switch return. + + No GH fetch on failure — falls back to local state and sets degraded: True. + + ``prior_iso`` should be the last_visit anchor captured BEFORE any + mark_visited call so the diff window is meaningful. + + - Pass the prior anchor explicitly (including ``None`` for first visit) when + you've already captured it (e.g. switch.py). + - Omit it (leave as default) to let resume_summary read the current + last_visit itself. Only do this when mark_visited has NOT yet run. + """ + from . import historian + + # Resolve anchor: explicit wins; fallback reads live state. + if prior_iso is _UNSET: + visit = lv.get_last_visit(workspace, feature) + anchor: str | None = visit["last_visit"] if visit else None + else: + anchor = prior_iso # type: ignore[assignment] + + memory_present = bool( + historian.format_for_agent(workspace.config.root, feature) + ) + + if anchor is None: + return { + "last_visit": None, + "first_visit": True, + "new_commit_count": 0, + "new_thread_count": 0, + "github_resolved_count": 0, + "ci_changed": False, + "draft_replies_pending": 0, + "memory_present": memory_present, + "degraded": False, + } + + new_commit_count = sum( + len(c) for c in _commits_since(workspace, feature, anchor).values() + ) + degraded = False + new_thread_count = 0 + github_resolved_count = 0 + try: + td = _threads_delta(workspace, feature, anchor) + new_thread_count = len(td["new"]) + github_resolved_count = len(td["resolved_gh"]) + except Exception: + degraded = True + + return { + "last_visit": anchor, + "first_visit": False, + "new_commit_count": new_commit_count, + "new_thread_count": new_thread_count, + "github_resolved_count": github_resolved_count, + "ci_changed": False, # v1: no persisted snapshot to diff + "draft_replies_pending": 0, # v1: skip for switch tail + "memory_present": memory_present, + "degraded": degraded, + } + + def feature_resume(workspace: Workspace, alias: str) -> dict[str, Any]: """Resolve alias, switch-if-needed, build and return the resume brief.""" feature = resolve_feature(workspace, alias) diff --git a/src/canopy/actions/switch.py b/src/canopy/actions/switch.py index 660cb81..3c844ff 100644 --- a/src/canopy/actions/switch.py +++ b/src/canopy/actions/switch.py @@ -477,13 +477,37 @@ def _post_switch_persist( # Clear any in_flight marker — this switch completed cleanly. state.in_flight = None + # T14: capture prior anchor BEFORE the T13 bump so the summary's + # "since when" window reflects what the user last saw, not this visit. + from . import last_visit as lv + _prior = lv.get_last_visit(workspace, feature_name) + prior_iso: str | None = _prior["last_visit"] if _prior else None + slots_mod.write_state(workspace, state) # T13: bump last_visit after slots.json is committed — every successful # switch into a feature counts as a "conscious look" per the plan. - from . import last_visit as lv lv.mark_visited(workspace, feature_name) + # T14: embed since-last-visit summary using the PRIOR anchor. + try: + from . import resume + out["since_last_visit_summary"] = resume.resume_summary( + workspace, feature_name, prior_iso=prior_iso, + ) + except Exception: + out["since_last_visit_summary"] = { + "last_visit": prior_iso, + "first_visit": prior_iso is None, + "new_commit_count": 0, + "new_thread_count": 0, + "github_resolved_count": 0, + "ci_changed": False, + "draft_replies_pending": 0, + "memory_present": False, + "degraded": True, + } + out["activated_at"] = now if state.previous_canonical: out["previous_feature_in_state"] = state.previous_canonical diff --git a/tests/test_resume.py b/tests/test_resume.py index 1afed38..5c93bb7 100644 --- a/tests/test_resume.py +++ b/tests/test_resume.py @@ -1403,3 +1403,152 @@ def test_resume_historian_excerpt_filters_old_sessions( assert "old session work" not in excerpt # Should still have markdown structure. assert "# Feature: auth-flow" in excerpt + + +# ── T14: resume_summary unit tests ─────────────────────────────────────────── + + +_SUMMARY_KEYS = { + "last_visit", "first_visit", "new_commit_count", "new_thread_count", + "github_resolved_count", "ci_changed", "draft_replies_pending", + "memory_present", "degraded", +} + + +class TestResumeSummary: + """Unit tests for resume_summary — the counts-only view embedded in switch.""" + + def test_resume_summary_first_visit(self, canopy_toml_for_workspace): + """No prior anchor → first_visit=True, all counts zero, degraded=False.""" + from canopy.actions.resume import resume_summary + + ws = _load_workspace(canopy_toml_for_workspace) + summary = resume_summary(ws, "auth-flow") + + assert summary["first_visit"] is True + assert summary["last_visit"] is None + assert summary["new_commit_count"] == 0 + assert summary["new_thread_count"] == 0 + assert summary["github_resolved_count"] == 0 + assert summary["ci_changed"] is False + assert summary["draft_replies_pending"] == 0 + assert summary["degraded"] is False + # All required keys must be present. + assert _SUMMARY_KEYS <= set(summary.keys()), ( + f"Missing keys: {_SUMMARY_KEYS - set(summary.keys())}" + ) + + def test_resume_summary_with_prior_iso_explicit(self, canopy_toml_for_workspace): + """When prior_iso is passed explicitly, diff anchors to it even if current anchor differs.""" + from canopy.actions.resume import resume_summary + + ws = _load_workspace(canopy_toml_for_workspace) + # Bump the live anchor. + lv.mark_visited(ws, "auth-flow") + + # Pass an explicit prior that's much older — the summary should report it back. + summary = resume_summary(ws, "auth-flow", prior_iso="2020-01-01T00:00:00Z") + + assert summary["first_visit"] is False + assert summary["last_visit"] == "2020-01-01T00:00:00Z" + + def test_resume_summary_prior_iso_none_falls_back_to_live_anchor( + self, canopy_toml_for_workspace + ): + """When prior_iso is omitted, falls back to the current live anchor.""" + from canopy.actions.resume import resume_summary + + ws = _load_workspace(canopy_toml_for_workspace) + ts = lv.mark_visited(ws, "auth-flow") + + summary = resume_summary(ws, "auth-flow") # no prior_iso + + assert summary["first_visit"] is False + assert summary["last_visit"] == ts + + def test_resume_summary_degraded_when_threads_fail( + self, canopy_toml_for_workspace, monkeypatch + ): + """GH unreachable → degraded=True, thread counts zero.""" + from canopy.actions.resume import resume_summary + + ws = _load_workspace(canopy_toml_for_workspace) + lv.mark_visited(ws, "auth-flow") + + def boom(*a, **k): + raise RuntimeError("offline") + + monkeypatch.setattr("canopy.actions.resume._threads_delta", boom) + + summary = resume_summary(ws, "auth-flow", prior_iso="2020-01-01T00:00:00Z") + + assert summary["degraded"] is True + assert summary["new_thread_count"] == 0 + assert summary["github_resolved_count"] == 0 + # Commit count is unaffected by GH failure (local-only computation). + assert isinstance(summary["new_commit_count"], int) + + def test_resume_summary_counts_new_commits(self, canopy_toml_for_workspace): + """Commits on the feature branch after prior_iso appear in new_commit_count.""" + import os + import subprocess + from canopy.actions.resume import resume_summary + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + + # Capture the anchor BEFORE making commits. + prior_iso = lv.mark_visited(ws, "auth-flow") + time.sleep(1.1) + + # Make a commit on auth-flow in repo-a. + repo_a = ws.config.root / ws.config.repos[0].path + env = { + **os.environ, + "GIT_AUTHOR_NAME": "Test", + "GIT_AUTHOR_EMAIL": "test@test.com", + "GIT_COMMITTER_NAME": "Test", + "GIT_COMMITTER_EMAIL": "test@test.com", + } + subprocess.run(["git", "checkout", "auth-flow"], cwd=repo_a, env=env, check=True) + subprocess.run( + ["git", "commit", "--allow-empty", "-m", "feat: new work"], + cwd=repo_a, env=env, check=True, + ) + + summary = resume_summary(ws, "auth-flow", prior_iso=prior_iso) + + assert summary["first_visit"] is False + assert summary["new_commit_count"] >= 1, ( + "Expected at least one new commit in new_commit_count" + ) + + def test_resume_summary_memory_present_when_historian_has_content( + self, canopy_toml_for_workspace + ): + """memory_present=True when historian.format_for_agent returns non-empty.""" + from canopy.actions import historian + from canopy.actions.resume import resume_summary + + ws = _load_workspace(canopy_toml_for_workspace) + historian.record_event( + ws.config.root, "auth-flow", + summary="did some work", + at="2026-01-01T00:00:00Z", + ) + + summary = resume_summary(ws, "auth-flow") + + assert summary["memory_present"] is True + + def test_resume_summary_memory_not_present_when_no_content( + self, canopy_toml_for_workspace + ): + """memory_present=False when historian has no content for the feature.""" + from canopy.actions.resume import resume_summary + + ws = _load_workspace(canopy_toml_for_workspace) + # No historian entries written. + summary = resume_summary(ws, "auth-flow") + + assert summary["memory_present"] is False diff --git a/tests/test_switch.py b/tests/test_switch.py index fb7effa..6e843df 100644 --- a/tests/test_switch.py +++ b/tests/test_switch.py @@ -235,3 +235,99 @@ def test_switch_bumps_previous_visit_on_reswitch(workspace_with_canonical_only): after = lv.get_last_visit(ws, "Y") assert after["last_visit"] > ts1, "last_visit must have advanced" assert after["previous_visit"] == ts1, "previous_visit must hold the old anchor" + + +# ── T14: since_last_visit_summary embedded in switch return ─────────────── + + +_SUMMARY_KEYS = ( + "last_visit", "first_visit", "new_commit_count", "new_thread_count", + "github_resolved_count", "ci_changed", "draft_replies_pending", + "memory_present", "degraded", +) + + +def test_switch_result_includes_since_last_visit_summary(workspace_with_canonical_only): + """switch return carries since_last_visit_summary with all required keys.""" + ws = workspace_with_canonical_only + from canopy.actions.switch import switch + + result = switch(ws, "Y") + + assert "since_last_visit_summary" in result, ( + "switch must embed since_last_visit_summary in its return dict" + ) + s = result["since_last_visit_summary"] + for key in _SUMMARY_KEYS: + assert key in s, f"since_last_visit_summary missing key: {key}" + + +def test_switch_summary_first_visit_when_no_prior_anchor(workspace_with_canonical_only): + """No prior anchor for target feature → first_visit=True, counts zero.""" + ws = workspace_with_canonical_only + from canopy.actions.switch import switch + from canopy.actions import last_visit as lv + + # Confirm no anchor for Y before the switch. + assert lv.get_last_visit(ws, "Y") is None + + result = switch(ws, "Y") + + s = result["since_last_visit_summary"] + assert s["first_visit"] is True + assert s["last_visit"] is None + assert s["new_commit_count"] == 0 + assert s["new_thread_count"] == 0 + assert s["degraded"] is False + + +def test_switch_summary_uses_prior_anchor(workspace_with_canonical_only): + """summary's last_visit reflects the value BEFORE this switch's bump.""" + import time + ws = workspace_with_canonical_only + from canopy.actions.switch import switch + from canopy.actions import last_visit as lv + + # First switch to Y: establishes an anchor. + switch(ws, "Y") + ts1 = lv.get_last_visit(ws, "Y")["last_visit"] + + # Wait so the next bump has a strictly later timestamp. + time.sleep(1.1) + + # Re-switch to Y — the summary should anchor to ts1 (the PRIOR value). + result = switch(ws, "Y") + + s = result["since_last_visit_summary"] + assert s["first_visit"] is False, "second switch should not be first_visit" + assert s["last_visit"] == ts1, ( + "summary's last_visit must equal the anchor captured BEFORE this switch bumped it" + ) + + # And the live anchor must have advanced past ts1. + assert lv.get_last_visit(ws, "Y")["last_visit"] > ts1 + + +def test_switch_summary_degraded_when_threads_fail( + workspace_with_canonical_only, monkeypatch +): + """GH unreachable → degraded=True, thread counts zero, switch still succeeds.""" + ws = workspace_with_canonical_only + from canopy.actions import last_visit as lv + from canopy.actions.switch import switch + + # Pre-seed an anchor so we're not in first_visit territory. + lv.mark_visited(ws, "Y") + + def boom(*a, **k): + raise RuntimeError("offline") + + monkeypatch.setattr("canopy.actions.resume._threads_delta", boom) + + result = switch(ws, "Y") + + s = result["since_last_visit_summary"] + assert s["degraded"] is True + assert s["new_thread_count"] == 0 + # Switch itself must not have failed. + assert result["feature"] == "Y" From 77dd64a80d5ff988d3a71a4b2d5c4555ba5e89e7 Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 09:48:56 +0530 Subject: [PATCH 17/21] feat(cli): canopy resume --- src/canopy/cli/main.py | 192 +++++++++++++++++++++++++++++++++++++++++ tests/test_resume.py | 85 ++++++++++++++++++ 2 files changed, 277 insertions(+) diff --git a/src/canopy/cli/main.py b/src/canopy/cli/main.py index 66cbde6..a528221 100644 --- a/src/canopy/cli/main.py +++ b/src/canopy/cli/main.py @@ -3246,6 +3246,185 @@ def cmd_resolve_thread(args: argparse.Namespace) -> None: console.print() +def _render_resume_human(brief: dict, console) -> None: + """Paint the resume brief in a human-readable layout.""" + from .ui import SYM_ARROW, separator + + feature = brief.get("feature", "") + fstate = (brief.get("current_state") or {}).get("feature_state") or "unknown" + last_visit = brief.get("last_visit") + window_hours = brief.get("window_hours") + first_visit = brief.get("first_visit", False) + switch_performed = brief.get("switch_performed", False) + switch_summary = brief.get("switch_summary") or {} + since = brief.get("since_last_visit") or {} + current = brief.get("current_state") or {} + hints = brief.get("intent_hints") or [] + + console.print() + console.print( + f" [feature]{feature}[/] [muted]state: {fstate}[/]" + ) + + # Last visit line + if first_visit: + console.print(f" [muted]first visit[/]") + else: + ago = f" [muted]({window_hours:.0f}h ago)[/]" if window_hours is not None else "" + console.print(f" [muted]last visit: {last_visit}{ago}[/]") + + if switch_performed: + prev = switch_summary.get("previously_canonical") or "" + mode = switch_summary.get("mode", "active_rotation") + mode_label = {"active_rotation": "active_rotation", "wind_down": "wind_down"}.get(mode, mode) + arrow_str = f"{prev} {SYM_ARROW} {feature}" if prev else feature + console.print(f" [muted]switch: {arrow_str} [{mode_label}][/]") + + # ── Since last visit ────────────────────────────────────────────────── + separator() + console.print(f" [header]Since last visit[/]") + + commits = since.get("commits") or {} + if commits: + parts = " ".join( + f"[repo]{r}[/] {len(cs)}" for r, cs in commits.items() if cs + ) + if parts: + console.print(f" Commits {parts}") + + new_threads = since.get("threads_new") or [] + resolved_gh = since.get("threads_resolved_on_github") or [] + resolved_by_canopy = since.get("threads_resolved_by_canopy") or [] + if new_threads: + repos_hit = {t.get("repo") for t in new_threads if t.get("repo")} + suffix = f" [muted]({', '.join(sorted(repos_hit))})[/]" if repos_hit else "" + console.print(f" New threads {len(new_threads)}{suffix}") + if resolved_gh: + by_canopy = sum(1 for t in resolved_gh if t.get("by_canopy")) + canon_str = f" [muted](by canopy: {by_canopy})[/]" if by_canopy else "" + console.print(f" Resolved threads {len(resolved_gh)}{canon_str}") + elif resolved_by_canopy: + console.print(f" Resolved by canopy {len(resolved_by_canopy)}") + + drafts_pending = since.get("draft_replies_pending", 0) + if drafts_pending: + console.print(f" Draft replies {drafts_pending} pending") + + historian_excerpt = since.get("historian_excerpt") or "" + if historian_excerpt: + first_line = historian_excerpt.splitlines()[0] + console.print(f" Memory [muted]{first_line}[/]") + + if not any([commits, new_threads, resolved_gh, resolved_by_canopy, drafts_pending, historian_excerpt]): + console.print(f" [muted]nothing new[/]") + + # ── Current ─────────────────────────────────────────────────────────── + separator() + console.print(f" [header]Current[/]") + + linear_issue = current.get("linear_issue") + linear_url = current.get("linear_url") + if linear_issue: + url_str = f" [muted]{linear_url}[/]" if linear_url else "" + console.print(f" Linear [linear]{linear_issue}[/]{url_str}") + + ci = current.get("ci_summary_per_repo") or {} + if ci: + ci_parts = [] + for r, status in ci.items(): + if status in ("passing", "success"): + ci_parts.append(f"[repo]{r}[/] [success]{status}[/]") + elif status in ("failing", "failure"): + ci_parts.append(f"[repo]{r}[/] [error]{status}[/]") + else: + ci_parts.append(f"[repo]{r}[/] [muted]{status}[/]") + console.print(f" CI {' '.join(ci_parts)}") + + open_threads = current.get("open_thread_count", 0) + if open_threads: + console.print(f" Open threads {open_threads}") + + bot_unresolved = current.get("bot_unresolved_total", 0) + if bot_unresolved: + console.print(f" Bot unresolved {bot_unresolved}") + + branch_pos = current.get("branch_position_per_repo") or {} + if branch_pos: + for repo_name, pos in branch_pos.items(): + ahead = pos.get("ahead", 0) + behind = pos.get("behind", 0) + default = pos.get("default_branch", "main") + parts = [] + if ahead: + parts.append(f"[ahead]+{ahead}[/]") + if behind: + parts.append(f"[behind]-{behind}[/]") + div_str = " ".join(parts) if parts else "[muted]up to date[/]" + console.print( + f" Branch [repo]{repo_name}[/] {div_str} [muted]from {default}[/]" + ) + + # ── Next actions ────────────────────────────────────────────────────── + if hints: + separator() + console.print(f" [header]Next actions[/]") + for i, hint in enumerate(hints, 1): + kind = hint.get("kind", "") + summary = hint.get("summary", "") + tool = hint.get("suggested_tool", "") + tool_args = hint.get("suggested_args") or {} + args_str = ", ".join(f'{k}="{v}"' for k, v in tool_args.items() if v) + call_str = f"{tool}({args_str})" if tool else "" + console.print(f" {i}. [info]{kind}[/] [muted]{summary}[/]") + if call_str: + console.print(f" [muted]{SYM_ARROW} {call_str}[/]") + + console.print() + + +def cmd_resume(args: argparse.Namespace) -> None: + """Show a fresh resume brief: what changed since last visit.""" + from ..actions.resume import feature_resume + from ..actions.last_visit import reset_anchor + from ..actions.errors import ActionError + from .render import render_blocker + from .ui import console + + workspace = _load_workspace() + + if args.reset_anchor: + from ..actions.aliases import resolve_feature + try: + feature = resolve_feature(workspace, args.alias) + except ActionError as err: + if args.json: + _print_json(err.to_dict()) + else: + render_blocker(err, action="resume") + sys.exit(1) + cleared = reset_anchor(workspace, feature) + if args.json: + _print_json({"feature": feature, "cleared": cleared}) + return + console.print(f" [success]Cleared last_visit for {feature}[/]") + return + + try: + brief = feature_resume(workspace, args.alias) + except ActionError as err: + if args.json: + _print_json(err.to_dict()) + else: + render_blocker(err, action="resume") + sys.exit(1) + + if args.json: + _print_json(brief) + return + + _render_resume_human(brief, console) + + def cmd_context(args: argparse.Namespace) -> None: """Show detected canopy context for current directory (debug).""" from ..workspace.context import detect_context @@ -3860,6 +4039,18 @@ def main() -> None: help="Feature alias; defaults to canonical feature") resolve_p.add_argument("--json", action="store_true", help="Output as JSON") + # resume + resume_p = subparsers.add_parser( + "resume", + help="Fresh brief: what changed since last visit", + ) + resume_p.add_argument("alias", help="Feature alias (name, Linear ID, slot id, etc.)") + resume_p.add_argument("--json", action="store_true", help="Output as JSON") + resume_p.add_argument( + "--reset-anchor", action="store_true", + help="Clear last_visit so the next call is treated as a first visit", + ) + args = parser.parse_args() if not args.command: @@ -3907,6 +4098,7 @@ def main() -> None: "pr-checks": cmd_pr_checks, "resolve": cmd_resolve_thread, "reply": cmd_reply_thread, + "resume": cmd_resume, } if args.command == "feature": diff --git a/tests/test_resume.py b/tests/test_resume.py index 5c93bb7..e0fd23a 100644 --- a/tests/test_resume.py +++ b/tests/test_resume.py @@ -1552,3 +1552,88 @@ def test_resume_summary_memory_not_present_when_no_content( summary = resume_summary(ws, "auth-flow") assert summary["memory_present"] is False + + +# ── CLI smoke tests ─────────────────────────────────────────────────────────── + + +class TestResumeCLI: + """Smoke tests for the cmd_resume CLI wiring.""" + + def test_resume_command_json(self, canopy_toml_for_workspace, monkeypatch, capsys): + """cmd_resume --json prints the structured brief.""" + import argparse + import json as _json + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + monkeypatch.setattr("canopy.cli.main._load_workspace", lambda: ws) + + from canopy.cli.main import cmd_resume + args = argparse.Namespace(alias="auth-flow", json=True, reset_anchor=False) + cmd_resume(args) + + data = _json.loads(capsys.readouterr().out) + assert data["version"] == 1 + assert data["feature"] == "auth-flow" + assert "since_last_visit" in data + assert "current_state" in data + assert "intent_hints" in data + + def test_resume_command_human_renders_sections( + self, canopy_toml_for_workspace, monkeypatch, capsys + ): + """Human mode prints the feature name and at least one section header.""" + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + monkeypatch.setattr("canopy.cli.main._load_workspace", lambda: ws) + + import argparse + from canopy.cli.main import cmd_resume + args = argparse.Namespace(alias="auth-flow", json=False, reset_anchor=False) + cmd_resume(args) + + out = capsys.readouterr().out + assert "auth-flow" in out + + def test_resume_command_reset_anchor( + self, canopy_toml_for_workspace, monkeypatch, capsys + ): + """--reset-anchor drops the last_visit entry.""" + from canopy.actions.last_visit import mark_visited, get_last_visit + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + mark_visited(ws, "auth-flow") + assert get_last_visit(ws, "auth-flow") is not None + + monkeypatch.setattr("canopy.cli.main._load_workspace", lambda: ws) + + import argparse + from canopy.cli.main import cmd_resume + args = argparse.Namespace(alias="auth-flow", json=False, reset_anchor=True) + cmd_resume(args) + + assert get_last_visit(ws, "auth-flow") is None + + def test_resume_command_reset_anchor_json( + self, canopy_toml_for_workspace, monkeypatch, capsys + ): + """--reset-anchor --json returns {feature, cleared} shape.""" + import argparse + import json as _json + + from canopy.actions.last_visit import mark_visited + + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + mark_visited(ws, "auth-flow") + monkeypatch.setattr("canopy.cli.main._load_workspace", lambda: ws) + + from canopy.cli.main import cmd_resume + args = argparse.Namespace(alias="auth-flow", json=True, reset_anchor=True) + cmd_resume(args) + + data = _json.loads(capsys.readouterr().out) + assert data["feature"] == "auth-flow" + assert data["cleared"] is True From 09ad50e6ce54fc2e9bb8d6dc913f09567b80a641 Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 09:54:24 +0530 Subject: [PATCH 18/21] feat(mcp): feature_resume tool --- src/canopy/mcp/server.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/canopy/mcp/server.py b/src/canopy/mcp/server.py index 0411e1a..c1344b6 100644 --- a/src/canopy/mcp/server.py +++ b/src/canopy/mcp/server.py @@ -856,6 +856,36 @@ def resolve_thread(thread_id: str, feature: str | None = None) -> dict: return e.to_dict() +@mcp.tool() +def feature_resume(alias: str) -> dict: + """Fresh "what changed since last visit" brief. + + Refreshes GitHub + Linear on every call. Returns: + - since_last_visit: commits, new threads, resolved threads (GH + canopy), + ci status delta (v1: empty), draft_replies_pending, historian_excerpt + - current_state: feature_state, ci_summary_per_repo, bot_unresolved_total, + draft_replies_summary, branch_position_per_repo, linear link + - first_visit, last_visit, window_hours + - switch_performed, switch_summary + - intent_hints (prioritized next actions) + + The agent should call this on first activity in a feature per session + (or after returning from another feature). Switch already embeds a + counts-only summary; this is the full payload. + + Args: + alias: Feature name, Linear ID, PR URL, or slot ID. + """ + from ..actions.resume import feature_resume as _impl + from ..actions.errors import ActionError + + ws = _get_workspace() + try: + return _impl(ws, alias) + except ActionError as e: + return e.to_dict() + + @mcp.tool() def drift(feature: str | None = None) -> dict: """Compare recorded HEAD state vs feature lane expectations. From 51f8287114dae9d5441186cbfdcfde1470a11cd5 Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 10:00:26 +0530 Subject: [PATCH 19/21] docs(skill): teach feature_resume as session-start primitive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add "Session start — call feature_resume first" and "Closing out review threads" sections to the using-canopy skill, positioning feature_resume as the compound session-start primitive that supersedes switch alone. --- .../agent_setup/skills/using-canopy/SKILL.md | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/canopy/agent_setup/skills/using-canopy/SKILL.md b/src/canopy/agent_setup/skills/using-canopy/SKILL.md index 94ac198..06a0e48 100644 --- a/src/canopy/agent_setup/skills/using-canopy/SKILL.md +++ b/src/canopy/agent_setup/skills/using-canopy/SKILL.md @@ -13,6 +13,33 @@ The single biggest agent failure mode in multi-repo work is path mistakes — `c Canopy also returns **pre-classified state**: review comments are temporally filtered into `actionable_threads` vs `likely_resolved_threads`, features have computed states like `ready_to_commit` / `drifted` / `awaiting_review`, and every action returns structured `next_actions` you can follow without re-deriving the rules. +## Session start — call `feature_resume` first + +When the user opens a chat and references a feature alias (a Linear ID like `DOC-2226`, a feature name, a slot id like `worktree-1`, or a PR URL/`#`), call `mcp__canopy__feature_resume()` *before* acting on whatever intent they've stated. This is a compound action: it resolves the alias, switches the canonical slot if it's not already there, refreshes from GitHub + Linear, and returns a structured brief with `intent_hints` for the most likely next actions. + +Patterns that trigger this: +- A bare alias as the first non-trivial token: `"DOC-2226"`, `"DOC-2226, let's address comments"`, `"jump into auth-flow"`. +- Explicit return: `"I am back on DOC-2226"`, `"resuming auth-flow"`. +- Topic-shift to another feature mid-session. + +Once you have the brief, look at `intent_hints` (sorted by `priority`) and pair the top hint with what the user said. Examples: + +| User says | Top hints | Your move | +|---|---|---| +| "address PR comments" | `address_comments`, `post_drafts` | call `review_comments` with the alias from `intent_hints[0].suggested_args`, then walk through actionable threads | +| "align with dev" | `align_with_default` | inspect `current_state.branch_position_per_repo`, propose rebase or merge per repo | +| (nothing specific) | (read top 3 hints) | summarize the brief + hints in 3–5 lines, ask the user which to pursue | + +Do **not** call `feature_resume` more than once per session per feature unless you've done work that materially changes state (e.g. pushed, resolved a thread, posted replies). The brief is fresh-per-call (refreshes GH/Linear) so repeated calls are wasteful, not stale. + +`feature_resume` supersedes `mcp__canopy__switch` as the session-start primitive — it does the switch internally plus the full brief. Use `switch` directly only when you want to focus a slot without the overhead of a fresh brief (e.g. mid-session slot rotation). + +### Closing out review threads + +When you finish reviewing a thread: +- If your commit addresses it: `commit --address --resolve-thread` (or post-process with `mcp__canopy__resolve_thread`). +- If the comment is wrong: `mcp__canopy__reply_to_thread ` with concrete evidence. Pass `resolve_after=true` when the pushback closes the discussion. + ## Tool selection — what to use when | What you want to do | Canopy tool | Don't use | From 915c2976fada514137bd890425316a395131d277 Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 10:11:49 +0530 Subject: [PATCH 20/21] docs: feature_resume + thread mutations + state files + 3.1.0 --- CHANGELOG.md | 52 ++++++++++++++++++++++++++++++++++++++++++ CLAUDE.md | 15 ++++++++---- docs/agents.md | 24 ++++++++++++++++--- docs/commands.md | 14 +++++++++++- docs/concepts.md | 52 ++++++++++++++++++++++++++++++++++++++++++ docs/mcp.md | 17 +++++++++++++- src/canopy/__init__.py | 2 +- 7 files changed, 165 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d0a0e0..a1f11d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,58 @@ Tracks the Python side (CLI + MCP server). The VSCode extension has its own [vsc Versions follow semver. Pre-1.0 — minor bumps may add features or break behavior; the README is the source-of-truth contract. +## 3.1.0 — 2026-05-30 (Plan 2 — Feature Resume) + +### Added +- `canopy resume ` (+ `mcp__canopy__feature_resume`): switch-aware + compound action. One call: alias → switch-if-needed → refresh GitHub + Linear → + compute structured brief with `intent_hints` for the most likely next actions. + See `docs/concepts.md#returning-to-a-feature`. +- `canopy resolve ` (+ `mcp__canopy__resolve_thread`): close a + GitHub review thread + log to `.canopy/state/thread_resolutions.json` for + attribution in the resume brief. +- `canopy reply [--body | --body-file | stdin]` + (+ `mcp__canopy__reply_to_thread`): post a reply to a GH review thread. + `--resolve` (or `resolve_after=True`) closes the thread after posting. +- `canopy commit --address --resolve-thread`: optionally close the GH + review thread after the local commit. Augment + `auto_resolve_threads_on_address = true` in canopy.toml makes this the + default for the workspace. `--no-resolve-thread` overrides the augment + per-invocation. +- New state files: `.canopy/state/visits.json` (per-feature last-visit anchor + `{feature: {last_visit, previous_visit}}`); `.canopy/state/thread_resolutions.json` + (canopy-driven GH thread closures `{thread_id: {resolved_by_canopy_at, + feature, via_command, via_commit_sha}}`). +- `actions/last_visit.py` — get/mark/reset the per-feature visit anchor. +- `actions/resume.py` — `feature_resume` compound action + `resume_summary` + (counts-only view embedded in `switch` return). +- `actions/thread_actions.py` — `resolve_thread` + `reply_to_thread` wrappers + + local resolution log writer. +- `actions/thread_resolutions.py` — load/record/filter_since for the + thread-resolutions log. +- GraphQL thread API in `integrations/github.py`: `list_review_threads`, + `resolve_thread`, `unresolve_thread`, `reply_to_thread`. Every comment from + `get_review_comments` now carries a `thread_id` field (GraphQL-sourced when + available, `""` on REST fallback) and `author_type` from GraphQL `__typename`. +- Bundled `using-canopy` skill now teaches `feature_resume` as the + session-start primitive and documents the "Closing out review threads" + workflow. + +### Changed +- `switch(feature)` bumps `last_visit` on every successful switch and embeds + `since_last_visit_summary` in its return value — a counts-only view + (commits, threads, GH resolutions, draft replies) so the agent sees + "something changed" without a full `feature_resume` round-trip. Sets + `degraded: true` if GitHub is unreachable. +- `get_review_comments` prefers GraphQL when available (single round-trip for + thread IDs + `author_type`); falls back to REST with `thread_id=""`. + +### Notes +- `feature_resume` refreshes GitHub + Linear on every call — the brief is + never cached at the canopy layer. +- Plan 1's slot model is the prerequisite. If upgrading from pre-3.0, run + `canopy migrate-slots` first. + ## 3.0.0 — 2026-05-28 (Wave 3.0) **Breaking — slot model.** Worktree directories are now generic numbered slots (`worktree-1`, `worktree-2`, ...) instead of feature-named. `max_worktrees` renamed to `slots` (default 2). State unified in `.canopy/state/slots.json`; `active_feature.json` deleted. Run `canopy migrate-slots` once per workspace. diff --git a/CLAUDE.md b/CLAUDE.md index 7f04c70..1741b9c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -48,7 +48,11 @@ src/canopy/ │ ├── stash.py # feature-tagged stash save/list/pop │ ├── switch.py # WAVE 3.0: slot-model focus primitive (+ --to-slot / --evict-to) │ ├── switch_preflight.py # WAVE 3.0: predictable-failure detection for switch -│ └── triage.py # cross-repo PR enumeration + priority tiers (slot-enriched) +│ ├── triage.py # cross-repo PR enumeration + priority tiers (slot-enriched) +│ ├── last_visit.py # per-feature last-visit anchor (visits.json get/mark/reset) +│ ├── resume.py # feature_resume compound action + resume_summary (counts-only) +│ ├── thread_actions.py # GH thread resolve/reply wrappers + local resolution log +│ └── thread_resolutions.py # thread_resolutions.json load/record/filter_since ├── agent/ │ └── runner.py # canopy_run — directory-safe shell exec ├── agent_setup/ # ships bundled skills + setup_agent installer @@ -61,7 +65,7 @@ src/canopy/ │ ├── github.py # GitHub PR + comments (MCP or gh CLI fallback) │ └── precommit.py # detect + run pre-commit hooks └── mcp/ - ├── server.py # MCP server — 59 tools, stdio transport + ├── server.py # MCP server — 67 tools, stdio transport └── client.py # MCP client — stdio + HTTP+OAuth transports ``` @@ -75,7 +79,7 @@ src/canopy/ - **Per-repo branches map** — `FeatureLane.branches: dict[repo, branch]` overrides "branch == feature name" for legacy mismatched-naming features. Use `lane.branch_for(repo)` or `repos_for_feature(workspace, feature)` everywhere — never recompute as `[r for r in feature.repos]` with feature name as branch (regresses Gap 2). - **Feature lanes use real Git branches and worktrees.** No virtual branches. - **Feature metadata lives in `.canopy/features.json`. Worktrees in `.canopy/worktrees/worktree-N//` (generic numbered slots).** A slot holds one feature at a time; a feature's repos sit as siblings inside its slot. Canonical (main repo dirs) is the only place to *run* code; worktrees are passive branch storage. -- **State files** at `.canopy/state/heads.json` (post-checkout hook output), `.canopy/state/preflight.json` (preflight tracker), and `.canopy/state/slots.json` (canonical + warm slot occupancy + `last_touched` LRU map + `in_flight` transaction marker). OAuth tokens at `~/.canopy/mcp-tokens/`. +- **State files** at `.canopy/state/heads.json` (post-checkout hook output), `.canopy/state/preflight.json` (preflight tracker), `.canopy/state/slots.json` (canonical + warm slot occupancy + `last_touched` LRU map + `in_flight` transaction marker), `.canopy/state/visits.json` (per-feature last-visit anchor: `{feature: {last_visit, previous_visit}}`), and `.canopy/state/thread_resolutions.json` (log of GH review threads canopy itself resolved: `{thread_id: {resolved_by_canopy_at, feature, via_command, via_commit_sha}}`). OAuth tokens at `~/.canopy/mcp-tokens/`. - **MCP client supports two transports.** Stdio (existing) for npm/python servers. HTTP+OAuth (new) for hosted servers like Linear's `mcp.linear.app`. Tokens cache per server. - **GitHub fallback to gh CLI.** When no `github` MCP server is configured, `integrations/github.py` falls back to `gh api` / `gh pr` for the same return shapes. If neither is available, raises `BlockerError(code='github_not_configured')` with platform-aware install hints. - **Single source of truth for state.** `feature_state` uses live git (not heads.json) so it's correct even when the hook hasn't fired. `drift` uses heads.json for the fast cached path. @@ -107,7 +111,7 @@ For integration testing against real services, see `~/projects/canopy-test/` (me - **Skill bundling:** Bundled skills live at `src/canopy/agent_setup/skills//SKILL.md`. `canopy setup-agent` copies them to `~/.claude/skills//SKILL.md`. The default `using-canopy` skill always installs; opt-in extras (e.g. `augment-canopy`) install via `--skill ` (repeatable). Foreign skills with the same path are not overwritten without `--reinstall`. The `_SKILL_SOURCE` constant remains as a backward-compat alias pointing at `using-canopy`'s source. - **Version bumps:** When shipping a milestone, bump `__version__` in [`src/canopy/__init__.py`](src/canopy/__init__.py) and add a section to [`CHANGELOG.md`](CHANGELOG.md). The version handshake (`canopy --version`, `mcp__canopy__version`, doctor's `cli_stale` / `mcp_stale` checks) is only useful when this number actually moves — drift was the bug 0.5.0 caught. -## MCP Server (64 tools) +## MCP Server (67 tools) Grouped by topic. Run with `canopy-mcp` (entry point) or `python -m canopy.mcp.server`. @@ -121,7 +125,8 @@ Slots: slots, slot_load, slot_clear, slot_swap, migrate_slots # WAVE 3. Actions: switch, triage, drift, conflicts # switch is the slot-model focus primitive Reads: linear_get_issue, github_get_pr, github_get_branch, github_get_pr_comments, linear_my_issues, pr_checks # pr_checks = M10 CI rollup -Workflow: ship, draft_replies # M8 + M9 — capstone + addressed-comment drafts +Workflow: ship, draft_replies, feature_resume # M8 + M9 + Plan 2 — capstone + reply drafts + session resume +Threads: resolve_thread, reply_to_thread # Plan 2 — GH review thread mutations + local log Run/Pre: run, preflight, review_status, review_comments, review_prep Stash: stash_save_feature, stash_list_grouped, stash_pop_feature, stash_save, stash_pop, stash_list, stash_drop diff --git a/docs/agents.md b/docs/agents.md index b657afa..7dd1441 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -6,7 +6,7 @@ How AI coding agents (Claude Code primarily; others by analogy) integrate with c Three pieces, all installed in one step by `canopy init`: -1. **Canopy MCP server** (`canopy-mcp` binary) — 64 tools exposing every canopy operation. Registered in `/.mcp.json`. +1. **Canopy MCP server** (`canopy-mcp` binary) — 67 tools exposing every canopy operation. Registered in `/.mcp.json`. 2. **`using-canopy` skill** at `~/.claude/skills/using-canopy/SKILL.md` — tells the agent *when* to prefer canopy MCP over raw bash. 3. **Per-workspace MCP config** in `/.mcp.json` with `CANOPY_ROOT` set so the server scopes to the right workspace. @@ -54,6 +54,7 @@ The skill encodes this matrix; the agent reads it on session start. Mirror here | What you want | Canopy tool | Don't use | |---|---|---| +| Return to a feature in a new session | `mcp__canopy__feature_resume` | `switch` + `feature_state` + `github_get_pr_comments` separately | | What feature should I work on? | `mcp__canopy__triage` | per-repo `gh pr list` + manual grouping | | Show me everything about a feature | `mcp__canopy__feature_state` | composing many reads | | Switch a feature into main (the focus primitive) | `mcp__canopy__switch` | `cd repo && git checkout`, or guessing paths | @@ -72,6 +73,8 @@ The skill encodes this matrix; the agent reads it on session start. Mirror here | Free a slot without bringing a new feature in | `mcp__canopy__slot_clear(slot_id)` | manual stash + branch checkout | | Exchange two slots' occupants (e.g., shuffle warm order) | `mcp__canopy__slot_swap(slot_a, slot_b)` | two `slot_load` calls with `replace=True` | | Migrate a pre-3.0 workspace to the slot model | `mcp__canopy__migrate_slots` | hand-renaming `.canopy/worktrees/` dirs | +| Close a GitHub review thread + log it | `mcp__canopy__resolve_thread(thread_id)` | raw GraphQL or `gh api` | +| Reply to a GitHub review thread | `mcp__canopy__reply_to_thread(thread_id, body)` | raw GraphQL or `gh pr review` | ### Vocabulary note: hibernate ⇄ release_current @@ -86,11 +89,26 @@ Same operation. Different surface vocab. A future canopy release may add `hibern A feature in the resulting state is **hibernating** (synonyms in the wild: "branch only", "released to cold", "wound down" — all the same thing). +## Session start — returning to a feature + +When opening a new session on a feature you've worked on before, call `feature_resume` first: + +```python +mcp__canopy__feature_resume(alias="SIN-12-search") +# → switches if needed, refreshes GH + Linear, returns brief + intent_hints +# → bumps the last-visit anchor so the next resume gets an accurate delta +``` + +The brief tells you: what state the feature is in, what changed since you last visited (commits, new/resolved threads), and `intent_hints` for the most likely next action categories. It replaces the manual `switch → feature_state → github_get_pr_comments` chain at session start. + +The bundled `using-canopy` skill's "Session start" section encodes this as the expected agent protocol. See [concepts.md §5](concepts.md#5-returning-to-a-feature--the-resume-brief) for the full brief shape and freshness policy. + ## The daily loop ``` -1. triage() → pick a feature from the prioritized list -2. feature_state(feature) → get current state + next_actions +1. feature_resume(feature) → session-start brief + switch-if-needed (Plan 2) + OR triage() → if you don't know what to work on +2. feature_state(feature) → get current state + next_actions (also in resume brief) 3. follow next_actions[0] → primary CTA (canopy decided what to do next) 4. feature_state again → confirm state advanced 5. repeat diff --git a/docs/commands.md b/docs/commands.md index 90ed068..5ac0564 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -49,6 +49,9 @@ Write actions and execution. | Command | What it does | |---|---| +| `canopy resume [--reset-anchor]` | **Session-start primitive (Plan 2).** Switch-aware compound action: alias → switch-if-needed → refresh GitHub + Linear → compute structured brief → bump last-visit anchor. Returns `{feature, switch_performed, first_visit, window_hours, since_last_visit, current_state, next_actions, intent_hints}`. Use this instead of manually calling `switch` + `feature_state` + `github_get_pr_comments` at the start of a session. `--reset-anchor` sets the anchor to now (useful when you want a fresh delta window). See [concepts.md §5](concepts.md#5-returning-to-a-feature--the-resume-brief). | +| `canopy resolve [--feature ]` | **Plan 2.** Resolve a GitHub PR review thread via GraphQL + record the closure in `.canopy/state/thread_resolutions.json`. The log feeds `feature_resume`'s `since_last_visit.resolved_threads` count. `--feature` pins which feature the resolution is attributed to (defaults to the canonical feature). | +| `canopy reply [--body \| --body-file \| stdin] [--resolve] [--feature ]` | **Plan 2.** Post a reply to a GitHub review thread. Body comes from `--body`, `--body-file`, or stdin (pipe-friendly). `--resolve` closes the thread after posting (equivalent to `reply_to_thread(..., resolve_after=True)`) and logs the closure. | | `canopy switch [--release-current] [--no-evict] [--evict ] [--evict-to ] [--to-slot ]` | **The focus primitive (Wave 3.0 slot model).** Promote a feature to the canonical slot. Default (active rotation): previously-canonical evacuates into a warm slot (full stash → checkout → pop). When the destination is already warm, the swap is a fast 5-op-per-repo dance — no `mv`, no slot renaming. `--release-current` (wind-down): previous goes cold with a feature-tagged stash. `--evict-to ` pins which slot the outgoing canonical lands in. `--to-slot ` promotes whatever feature already occupies that slot (omit ``). Cap-reached blocker surfaces explicit fix actions (wind-down, evict a specific slot, raise cap). See [docs/concepts.md §4](concepts.md#4-the-slot-model). | | `canopy slot load [] [--replace] [--bootstrap]` | **Wave 3.0.** Warm a cold feature into a slot WITHOUT changing canonical. `` defaults to the lowest free slot. `--replace` evicts the slot's current occupant to cold first. `--bootstrap` runs the env-file copy + install_cmd + IDE workspace gen (same as `canopy worktree-bootstrap`). The feature must already be registered — create it with `canopy feature create` first. | | `canopy slot clear ` | **Wave 3.0.** Evict that slot's occupant to cold with a feature-tagged stash if dirty. The slot id stays — only the occupant moves. | @@ -57,7 +60,7 @@ Write actions and execution. | `canopy run [--feature]` | Run a shell command in a canopy-managed repo with cwd resolved internally. The "agent never `cd`s" tool — also useful from a CLI in a deeply nested directory. | | `canopy code\|cursor\|fork ` | Open the feature in VS Code / Cursor / Fork.app (alias-aware; generates `.code-workspace` for the IDE ones). | | `canopy sync` | Pull default branch + rebase feature branches across repos. | -| `canopy commit -m [--feature ] [--repo ] [--paths

] [--no-hooks] [--amend] [--address ]` | **Wave 2.3 + M3.** Commit across every repo in the canonical (or named) feature with a single message. Pre-flight refuses with `BlockerError(code='wrong_branch')` if any in-scope repo has drifted; per-repo hook failures don't cancel the others (status: `hooks_failed`). `--address ` (numeric id or GitHub URL) auto-suffixes the message with the bot comment's title + URL and records the resolution in `.canopy/state/bot_resolutions.json`. Non-bot comments raise `BlockerError(code='not_a_bot_comment')`. | +| `canopy commit -m [--feature ] [--repo ] [--paths

] [--no-hooks] [--amend] [--address ] [--resolve-thread \| --no-resolve-thread]` | **Wave 2.3 + M3 + Plan 2.** Commit across every repo in the canonical (or named) feature with a single message. Pre-flight refuses with `BlockerError(code='wrong_branch')` if any in-scope repo has drifted; per-repo hook failures don't cancel the others (status: `hooks_failed`). `--address ` (numeric id or GitHub URL) auto-suffixes the message with the bot comment's title + URL and records the resolution in `.canopy/state/bot_resolutions.json`. Non-bot comments raise `BlockerError(code='not_a_bot_comment')`. `--resolve-thread` additionally closes the corresponding GitHub review thread and logs it to `thread_resolutions.json`. `--no-resolve-thread` disables this even when `[augments] auto_resolve_threads_on_address = true` is set in canopy.toml. | | `canopy bot-status [--feature ] [--unresolved-only]` | **M3.** Per-feature rollup of bot review comments — total / resolved / unresolved per repo + an `all_resolved` flag. Bot vs human classification respects `[augments] review_bots` in canopy.toml. | | `canopy historian show []` | **M4.** Print the rendered memory file for a feature (3 sections: resolutions log, PR context, sessions). Returns empty when no memory has been recorded yet. | | `canopy historian compact [] [--keep-sessions ]` | **M4.** Trim the Sessions section to the most-recent N (default 5). Resolutions log + PR context are preserved regardless. v1 is mechanical (no LLM); future iterations will summarize. | @@ -150,6 +153,15 @@ Install-staleness (canopy's installation around the workspace): ## Common patterns +Session start — returning to a feature: + +```bash +canopy resume # switch-if-needed + fresh brief + bump last-visit anchor +# brief shows: window_hours, since_last_visit counts, current_state, intent_hints +canopy reply --body "Done — fixed in abc123." --resolve # close a thread +canopy resolve # close a thread without replying +``` + The daily loop: ```bash diff --git a/docs/concepts.md b/docs/concepts.md index 8f9aea2..e8cd277 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -202,3 +202,55 @@ Decoupling slot identity from feature identity matters because: - **Not branch-management.** `switch` doesn't create branches that don't exist (that's `feature_create`), doesn't open IDEs (that's `code`), doesn't commit/push (those are `commit`/`push`/`ship`). It only moves features between {canonical, warm, cold}. - **Not slot-allocation either.** Use `slot load` to warm a cold feature into a slot without changing canonical. Use `slot clear` to free a slot without bringing a new feature in. `switch` is specifically the "what's in focus" verb. - **Not unsafe.** Three layers of defense: a preflight catches predictable failures cheaply; a fast-path 5-op-per-repo swap when Y is already warm; a journaled rollback walker plus a `slots.json.in_flight` marker for the residual real-world failures (disk full, network blip, partial multi-repo failure). Either every repo finishes the switch or every repo rolls back to its pre-switch state. + +## 5. Returning to a feature — the resume brief + +When the agent (or human) returns to a feature in a new session, `canopy resume ` (or `mcp__canopy__feature_resume(alias)`) runs the full recovery chain: + +``` +alias → switch-if-needed → refresh GitHub + Linear → brief → bump last-visit anchor +``` + +One call gets you oriented. There's no separate "switch, then fetch PR state, then read comments" dance. + +### What the brief carries + +```json +{ + "feature": "SIN-12-search", + "switch_performed": true, + "first_visit": false, + "window_hours": 18.4, + "since_last_visit": { + "new_commits": 3, + "new_threads": 1, + "resolved_threads": 2, + "draft_replies_available": 1 + }, + "current_state": "needs_work", + "next_actions": ["address_review_comments"], + "intent_hints": ["review_comments", "check_ci"] +} +``` + +- `switch_performed` — whether `resume` had to call `switch` to move the feature to canonical. +- `first_visit` — true when no prior anchor exists; no delta computed. +- `window_hours` — wall-clock hours since the last visit anchor was set. +- `since_last_visit` — counts-only delta (commits, threads, GH thread closures, draft-reply availability) since the last visit. Does NOT re-read every comment body — just counts, so it fits in a summary line. +- `current_state` + `next_actions` — forwarded from `feature_state`, so the agent knows what to do next without an extra round-trip. +- `intent_hints` — canopy's best guess at the most likely next action categories (e.g., `review_comments`, `check_ci`, `push`). Use as a prompt, not a constraint. + +### Freshness policy + +- **Every `resume` call refreshes GitHub + Linear.** The brief is never cached at the canopy layer; upstream HTTP/MCP layers may cache. +- **Auxiliary state** (`bot_resolutions`, `thread_resolutions`, `visits.json`) is read live on every call. +- **`switch` also bumps `last_visit`.** When you call `switch` without `resume` — e.g., a quick focus change mid-session — the switch return includes a lightweight `since_last_visit_summary` (counts only, no intent hints) so you immediately see whether anything changed since you were last here. `degraded: true` appears in this field when GitHub is unreachable. + +### Last-visit anchor — the single-bump invariant + +`visits.json` stores `{feature: {last_visit: , previous_visit: }}`. The anchor advances exactly once per `resume` call, at the END of the call (after the brief is computed). It does NOT advance on `switch` alone, nor on repeated `resume` calls in the same logical session (within ~5 minutes of the same anchor time). + +This invariant means: +- The delta window always reflects the period since you last *consciously* resumed the feature, not since the last focus change. +- Repeated `resume` calls in quick succession return the same delta (not a 0-minute window the second time). +- `--reset-anchor` (CLI) / `reset_anchor=True` (MCP) explicitly resets the anchor to now — use when you want to start fresh without reopening a new session. diff --git a/docs/mcp.md b/docs/mcp.md index 743aeba..502add3 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -24,7 +24,7 @@ Register in any MCP-compatible client. `canopy init` writes this entry into the `CANOPY_ROOT` scopes the server to one workspace. To use canopy in multiple workspaces simultaneously, register separate entries with different `CANOPY_ROOT` values (or scope MCP per-project via `.mcp.json` at each workspace root). -### Tools (64) +### Tools (67) Grouped by topic. Every tool is alias-aware where it accepts a feature input. @@ -95,6 +95,21 @@ Grouped by topic. Every tool is alias-aware where it accepts a feature input. | `github_get_branch` | Branch HEAD/divergence/upstream per repo. Accepts feature or `:`. | | `github_get_pr_comments` | Temporally classified review comments. Same alias forms as `github_get_pr`. | +#### Workflow + +| Tool | Description | +|---|---| +| `ship` | **M8.** PR open/update orchestrator with cross-repo body links. Idempotent — `up_to_date` on re-run. | +| `draft_replies` | **M9.** File-history-based addressed-comment classifier. Returns draft reply text with high/medium/low confidence per comment. No LLM. | +| `feature_resume` | **Plan 2.** Switch-aware compound brief. One call: alias → switch-if-needed → refresh GitHub + Linear → compute `{feature, switch_performed, first_visit, window_hours, since_last_visit, current_state, next_actions, intent_hints}` → bump last-visit anchor. Refreshes GH + Linear on every call (never cached at the canopy layer). See [concepts.md §5](concepts.md#5-returning-to-a-feature--the-resume-brief). | + +#### Threads (Plan 2) + +| Tool | Description | +|---|---| +| `resolve_thread` | Close a GitHub PR review thread via GraphQL + record the closure in `.canopy/state/thread_resolutions.json`. Params: `thread_id` (str), `feature` (optional — defaults to canonical). The log feeds `feature_resume`'s `since_last_visit.resolved_threads` count. | +| `reply_to_thread` | Post a reply to a GitHub review thread. Params: `thread_id` (str), `body` (str), `feature` (optional), `resolve_after` (bool, default `False`). When `resolve_after=True`, closes the thread after posting and logs via `thread_resolutions`. | + #### Run / preflight | Tool | Description | diff --git a/src/canopy/__init__.py b/src/canopy/__init__.py index c23428d..e746bfd 100644 --- a/src/canopy/__init__.py +++ b/src/canopy/__init__.py @@ -1,2 +1,2 @@ """Canopy — workspace-first development orchestrator.""" -__version__ = "3.0.0" +__version__ = "3.1.0" From 1dbfe94d113ccc9a296759e6801330621ff0ff00 Mon Sep 17 00:00:00 2001 From: Ashmit Biswas Date: Sat, 30 May 2026 10:26:01 +0530 Subject: [PATCH 21/21] fix(milestone-3): doc shape fidelity + augment-canopy + cross-repo + unresolved tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - docs/concepts.md: replace stale brief example with actual feature_resume shape; fix next_actions→intent_hints; fix switch/resume bump contradiction; drop false debounce claim; document single-bump invariant correctly - docs/mcp.md: drop next_actions from feature_resume description; fix resolved_threads→threads_resolved_by_canopy in resolve_thread row - docs/agents.md: remove "(also in resume brief)" parenthetical for next_actions - augment-canopy SKILL.md (bundled + user): add auto_resolve_threads_on_address row - tests/test_resume.py: add multi-repo threads_new aggregation test and unresolved-with-stale-resolved_at exclusion test --- docs/agents.md | 2 +- docs/concepts.md | 49 ++++++++++++----- docs/mcp.md | 4 +- .../skills/augment-canopy/SKILL.md | 1 + tests/test_resume.py | 55 +++++++++++++++++++ 5 files changed, 94 insertions(+), 17 deletions(-) diff --git a/docs/agents.md b/docs/agents.md index 7dd1441..e5dd43e 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -108,7 +108,7 @@ The bundled `using-canopy` skill's "Session start" section encodes this as the e ``` 1. feature_resume(feature) → session-start brief + switch-if-needed (Plan 2) OR triage() → if you don't know what to work on -2. feature_state(feature) → get current state + next_actions (also in resume brief) +2. feature_state(feature) → get current state (brief.current_state has feature_state + intent_hints) 3. follow next_actions[0] → primary CTA (canopy decided what to do next) 4. feature_state again → confirm state advanced 5. repeat diff --git a/docs/concepts.md b/docs/concepts.md index e8cd277..3d1ddfd 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -217,38 +217,59 @@ One call gets you oriented. There's no separate "switch, then fetch PR state, th ```json { + "version": 1, "feature": "SIN-12-search", - "switch_performed": true, + "now": "2026-05-30T10:00:00Z", + "last_visit": "2026-05-29T15:30:00Z", "first_visit": false, - "window_hours": 18.4, + "window_hours": 18.5, + "switch_performed": true, + "switch_summary": {"status": "ok"}, + "intent_hints": [ + {"kind": "review_comments", "summary": "2 open threads", "suggested_tool": "github_get_pr_comments", "suggested_args": {}, "priority": 1} + ], "since_last_visit": { - "new_commits": 3, - "new_threads": 1, - "resolved_threads": 2, - "draft_replies_available": 1 + "commits": { + "backend": [{"sha": "abc1234", "short_sha": "abc1234", "at": "2026-05-30T09:00:00Z", "author": "alice", "subject": "fix: auth token refresh"}] + }, + "threads_new": [ + {"thread_id": "PRRT_1", "comment_id": 42, "author": "bob", "path": "src/auth.py", "line": 10, "body_excerpt": "This needs a guard.", "created_at": "2026-05-30T08:00:00Z", "url": "https://github.com/...", "repo": "backend", "pr_number": 7} + ], + "threads_resolved_on_github": [], + "threads_resolved_by_canopy": [], + "ci_status_delta": {}, + "draft_replies_pending": 1, + "historian_excerpt": "Last session: implemented token refresh. Left off before adding tests." }, - "current_state": "needs_work", - "next_actions": ["address_review_comments"], - "intent_hints": ["review_comments", "check_ci"] + "current_state": { + "feature_state": "needs_work", + "open_thread_count": 2, + "ci_summary_per_repo": {"backend": "passing"}, + "bot_unresolved_total": 0, + "draft_replies_summary": {"addressed_total": 1, "unaddressed_total": 1}, + "branch_position_per_repo": {"backend": {"branch": "SIN-12-search", "default_branch": "main", "ahead": 3, "behind": 0, "last_sync_at": "2026-05-30T09:00:00Z"}}, + "linear_issue": "SIN-12", + "linear_url": "https://linear.app/..." + } } ``` - `switch_performed` — whether `resume` had to call `switch` to move the feature to canonical. - `first_visit` — true when no prior anchor exists; no delta computed. - `window_hours` — wall-clock hours since the last visit anchor was set. -- `since_last_visit` — counts-only delta (commits, threads, GH thread closures, draft-reply availability) since the last visit. Does NOT re-read every comment body — just counts, so it fits in a summary line. -- `current_state` + `next_actions` — forwarded from `feature_state`, so the agent knows what to do next without an extra round-trip. -- `intent_hints` — canopy's best guess at the most likely next action categories (e.g., `review_comments`, `check_ci`, `push`). Use as a prompt, not a constraint. +- `since_last_visit` — full delta since the last visit: `commits` (per-repo list), `threads_new` (unresolved threads whose first comment is newer than last_visit), `threads_resolved_on_github` and `threads_resolved_by_canopy` (two separate resolution logs), `draft_replies_pending` count, `historian_excerpt`. +- `current_state` — live snapshot from `feature_state` + branch positions + Linear link. NOT forwarded verbatim; the brief extracts specific fields into this sub-object. +- `intent_hints` — canopy's best guess at the most likely next action categories (e.g., `review_comments`, `check_ci`, `push`). Derived from the brief data, not from `feature_state`. Use as a prompt, not a constraint. ### Freshness policy - **Every `resume` call refreshes GitHub + Linear.** The brief is never cached at the canopy layer; upstream HTTP/MCP layers may cache. - **Auxiliary state** (`bot_resolutions`, `thread_resolutions`, `visits.json`) is read live on every call. -- **`switch` also bumps `last_visit`.** When you call `switch` without `resume` — e.g., a quick focus change mid-session — the switch return includes a lightweight `since_last_visit_summary` (counts only, no intent hints) so you immediately see whether anything changed since you were last here. `degraded: true` appears in this field when GitHub is unreachable. +- **`switch` always bumps `last_visit`.** Every `switch` call (whether invoked directly or triggered internally by `resume`) bumps `last_visit` for the incoming feature after the slot state is written. When you call `switch` without `resume` — e.g., a quick focus change mid-session — the switch return includes a lightweight `since_last_visit_summary` (counts only, no intent hints) so you immediately see whether anything changed since you were last here. `degraded: true` appears in this field when GitHub is unreachable. ### Last-visit anchor — the single-bump invariant -`visits.json` stores `{feature: {last_visit: , previous_visit: }}`. The anchor advances exactly once per `resume` call, at the END of the call (after the brief is computed). It does NOT advance on `switch` alone, nor on repeated `resume` calls in the same logical session (within ~5 minutes of the same anchor time). +`visits.json` stores `{feature: {last_visit: , previous_visit: }}`. The anchor advances exactly once per `feature_resume` call. If `resume` triggered a `switch`, the bump happened inside `switch` — `resume` does NOT bump again. If no switch ran, `resume` bumps at the END of the call (after the brief is computed). Either way, exactly one bump per `feature_resume` invocation. This invariant means: - The delta window always reflects the period since you last *consciously* resumed the feature, not since the last focus change. diff --git a/docs/mcp.md b/docs/mcp.md index 502add3..30482eb 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -101,13 +101,13 @@ Grouped by topic. Every tool is alias-aware where it accepts a feature input. |---|---| | `ship` | **M8.** PR open/update orchestrator with cross-repo body links. Idempotent — `up_to_date` on re-run. | | `draft_replies` | **M9.** File-history-based addressed-comment classifier. Returns draft reply text with high/medium/low confidence per comment. No LLM. | -| `feature_resume` | **Plan 2.** Switch-aware compound brief. One call: alias → switch-if-needed → refresh GitHub + Linear → compute `{feature, switch_performed, first_visit, window_hours, since_last_visit, current_state, next_actions, intent_hints}` → bump last-visit anchor. Refreshes GH + Linear on every call (never cached at the canopy layer). See [concepts.md §5](concepts.md#5-returning-to-a-feature--the-resume-brief). | +| `feature_resume` | **Plan 2.** Switch-aware compound brief. One call: alias → switch-if-needed → refresh GitHub + Linear → compute `{feature, switch_performed, first_visit, window_hours, since_last_visit, current_state, intent_hints}` → bump last-visit anchor. Refreshes GH + Linear on every call (never cached at the canopy layer). See [concepts.md §5](concepts.md#5-returning-to-a-feature--the-resume-brief). | #### Threads (Plan 2) | Tool | Description | |---|---| -| `resolve_thread` | Close a GitHub PR review thread via GraphQL + record the closure in `.canopy/state/thread_resolutions.json`. Params: `thread_id` (str), `feature` (optional — defaults to canonical). The log feeds `feature_resume`'s `since_last_visit.resolved_threads` count. | +| `resolve_thread` | Close a GitHub PR review thread via GraphQL + record the closure in `.canopy/state/thread_resolutions.json`. Params: `thread_id` (str), `feature` (optional — defaults to canonical). The log feeds `feature_resume`'s `since_last_visit.threads_resolved_by_canopy` list. | | `reply_to_thread` | Post a reply to a GitHub review thread. Params: `thread_id` (str), `body` (str), `feature` (optional), `resolve_after` (bool, default `False`). When `resolve_after=True`, closes the thread after posting and logs via `thread_resolutions`. | #### Run / preflight diff --git a/src/canopy/agent_setup/skills/augment-canopy/SKILL.md b/src/canopy/agent_setup/skills/augment-canopy/SKILL.md index 89251fd..649297c 100644 --- a/src/canopy/agent_setup/skills/augment-canopy/SKILL.md +++ b/src/canopy/agent_setup/skills/augment-canopy/SKILL.md @@ -48,6 +48,7 @@ augments = { preflight_cmd = "uv run pytest tests/fast" } # api-only override | `preflight_cmd` | string | `canopy preflight` (and `review_prep` path inside `coordinator.py`) | Runs via `sh -c` so pipes / `&&` chains work | | `test_cmd` | string | future `canopy test` (not v1) | Schema-reserved; safe to set | | `review_bots` | list[string] | M3 bot-comment tracking (when shipped) | Workspace-level only; per-repo overrides ignored for this key | +| `auto_resolve_threads_on_address` | bool | `canopy commit --address ` | When true, `canopy commit --address ` auto-resolves the corresponding GH review thread after push. `--no-resolve-thread` overrides. Default: false. | Unknown keys are silently preserved by the parser — future augments don't require schema migration. diff --git a/tests/test_resume.py b/tests/test_resume.py index e0fd23a..d2c21ad 100644 --- a/tests/test_resume.py +++ b/tests/test_resume.py @@ -1637,3 +1637,58 @@ def test_resume_command_reset_anchor_json( data = _json.loads(capsys.readouterr().out) assert data["feature"] == "auth-flow" assert data["cleared"] is True + + +class TestThreadsAggregation: + """Tests for multi-repo threads_new aggregation and edge cases.""" + + def test_resume_threads_new_aggregates_across_repos(self, canopy_toml_for_workspace, monkeypatch): + """When both repos have new threads, all surface in threads_new with repo+pr_number.""" + from canopy.actions.last_visit import mark_visited + from canopy.actions.resume import feature_resume + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + mark_visited(ws, "auth-flow") + import time; time.sleep(1.1) + + monkeypatch.setattr("canopy.actions.resume._pr_coords_per_repo", + lambda ws, f: { + "repo-a": {"owner": "o", "repo_slug": "a", "pr_number": 1}, + "repo-b": {"owner": "o", "repo_slug": "b", "pr_number": 2}, + }) + + def fake_threads(root, owner, slug, pr): + return [{"thread_id": f"PRRT_{slug}_1", "is_resolved": False, "resolved_at": None, + "comments": [{"comment_id": pr * 10, "created_at": "2999-01-01T00:00:00Z", + "author": "u", "path": "x", "line": 1, "body": "new", "url": "u"}]}] + monkeypatch.setattr("canopy.integrations.github.list_review_threads", fake_threads) + + brief = feature_resume(ws, "auth-flow") + new = brief["since_last_visit"]["threads_new"] + repos_seen = {t["repo"] for t in new} + assert repos_seen == {"repo-a", "repo-b"} + assert all(t["pr_number"] in (1, 2) for t in new) + + def test_resume_unresolved_thread_with_stale_resolved_at_excluded(self, canopy_toml_for_workspace, monkeypatch): + """A thread with is_resolved=False but stale resolved_at (from prior resolve+unresolve) is excluded + from both threads_new (it's old) and threads_resolved_on_github (it's not resolved).""" + from canopy.actions.last_visit import mark_visited + from canopy.actions.resume import feature_resume + ws = _load_workspace(canopy_toml_for_workspace) + _make_canonical(ws, "auth-flow") + mark_visited(ws, "auth-flow") + import time; time.sleep(1.1) + + monkeypatch.setattr("canopy.actions.resume._pr_coords_per_repo", + lambda ws, f: {"repo-a": {"owner": "o", "repo_slug": "a", "pr_number": 1}}) + + monkeypatch.setattr("canopy.integrations.github.list_review_threads", + lambda *a, **k: [{ + "thread_id": "PRRT_x", "is_resolved": False, "resolved_at": "2999-01-01T00:00:00Z", + "comments": [{"comment_id": 1, "created_at": "1900-01-01T00:00:00Z", + "author": "u", "path": "x", "line": 1, "body": "old", "url": "u"}], + }]) + + brief = feature_resume(ws, "auth-flow") + assert brief["since_last_visit"]["threads_new"] == [] + assert brief["since_last_visit"]["threads_resolved_on_github"] == []