diff --git a/CHANGELOG.md b/CHANGELOG.md index 6642cb0..ec1e0bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,24 @@ Upgrade notes: - A server without `GET /api/v1/groups/unread` (older than read-state-sync phase 5b) shows no unread markers at all, where the local rule used to light them. +### Added + +- Agent skill: an agent token can also run `notes list|search|show|edit --room ` on the + text notes of a room it belongs to (pepper#5537); the guide now lists seventeen commands, keeps + personal notes, other artifact kinds and note creation/deletion out of scope, and tells an agent + to read the whole note with `notes show` before answering a comment. +- Room messages show note comments for agents: `messages list` and `agents claude-channel` events + print an anchored comment's quoted passage (about 160 chars) above its body and a one-line + header on a note's comments topic pointing at `comments list --note`; `--json` passes `anchor`, + `resolved_at`, `resolved_by_name` and the topic's `subject_artifact_id` through. The agent skill + says to reply under the thread root. +- `pepper comments list|create --note N`: comments on a room note, optionally on a quoted passage; + `--reply-to`, `--mention`, `--idempotency-key`, `--json`. Prototype against the new + `/api/v1/artifacts/{id}/comments` routes. +- `pepper comments resolve|reopen --note N THREAD_ID`; `comments list` hides resolved threads + (`--all` shows them, marked "resolved by X"; `--json` always has every thread plus + `resolved_count`). `resolve` joins the verb set (IA §3.1). + ### Changed - Unread dots and `@` mention markers come from the server's `unread` rows (`unread_count`, diff --git a/README.md b/README.md index 9a0f547..35d8fca 100644 --- a/README.md +++ b/README.md @@ -272,6 +272,10 @@ Every command except `login` and the interactive REPL takes `--json`, and also s | `pepper notes show [--room ] [--json]` | Print one note's raw markdown body to stdout; `--json` for the full detail (labels, `updated_at`, `etag`, `shared`). | | `pepper notes create "" [--room ] [--file PATH \| -]` | Create a note. Body from `$EDITOR`, or `--file PATH`/`--file -` for stdin. | | `pepper notes edit [--room ] [--file PATH \| -] [--name NAME] [--if-match ETAG \| --force]` | Edit a note; sends nothing when the text is unchanged. | +| `pepper comments list --note [--room ] [--all] [--json]` | A room note's comment threads, oldest first: the quoted passage (or whole note), a `detached` marker when the quote is gone from the note, then author, time and body, with replies indented. Resolved threads are hidden (footer counts them); `--all` shows them marked "resolved by X"; `--json` always has every thread and `resolved_count`. | +| `pepper comments create --note [--room ] [--quote TEXT] [--reply-to ID] [--mention PERSON_ID]... [MESSAGE \| -] [--idempotency-key K] [--json]` | Comment on a room note: on an exact passage (`--quote`, which must appear once; exit 2 otherwise), or the whole note, or reply to a thread. Personal notes cannot be commented on (exit 2). | +| `pepper comments resolve\|reopen --note [--room ] THREAD_ID [--json]` | Mark a thread resolved, or reopen it (idempotent; `THREAD_ID` is the root message id from `comments list --json`). A reply to a resolved thread reopens it. | +| `pepper messages list` on a note's comments topic | Prints a one-line header naming the note (and `pepper comments list --note `), and a `> "quoted passage"` line above each anchored comment. | | `pepper notes delete [--room ] [--yes]` | Delete a note; asks for confirmation unless `--yes` (off a terminal, `--yes` is required). | | `pepper artifacts list [--kind note\|link\|file] [--label L] [--search TEXT] [--limit N] [--room R] [--json]` | Everything you have asked Pepper to keep, of any kind (pictures, PDFs, recordings and recipe photos are files; reports and recipes are notes), newest first, numbered. `--kind`, `--label` and `--search` go to the server as query parameters (`--search` is its substring match over names and note bodies); the CLI filters nothing. The numbers are saved per profile (`artifacts-view.json`, next to the credentials, removed at `logout` and `login`) for `show`/`download`/`delete N`; if they cannot be saved the list says so and the numbers do not resolve; in `--json` each item carries `n`. `--room` lists a room's. | | `pepper artifacts show ARTIFACT [--json]` | One artifact: kind, type, size, labels, and its text if it has any (a note's body, a text file's preview). `ARTIFACT` is a number from the last `list` or a full id. `--json` is the server's detail (minus the rendered `body_html`). | @@ -293,7 +297,7 @@ layer on top. Note this is unrelated to `pepper skills`, which manages your account's own skills. A test holds the document to the real parser: every command in it must be a full `pepper …` invocation whose subcommand and flags exist, every `--flag` it mentions anywhere must exist, and outside its "cannot run" section -it may only show the `rooms`, `topics`, `messages`, `attachments` and `members list` commands the server accepts an agent token for. +it may only show the `rooms`, `topics`, `messages`, `attachments`, `members list`, `comments` and room-scoped `notes list|search|show|edit` commands the server accepts an agent token for. A renamed flag, a shortened example or a personal-token-only command fails CI until the document is updated. diff --git a/planning/cli-information-architecture.md b/planning/cli-information-architecture.md index 5f74505..5896310 100644 --- a/planning/cli-information-architecture.md +++ b/planning/cli-information-architecture.md @@ -58,6 +58,7 @@ picks one and names the evidence on both sides. Paths are in the pepper server r | `trajectory` | "Trajectories", "Activity history" | `trajectories_list.html` | **`trajectories`** | | `scheduled-tasks`, `runs`, `thread`, `destinations` | "Scheduled tasks", "Pause task", "Resume task", "Keep it going", "Run history", "Thread", "Delivery" | `scheduled_tasks_list.html`, `scheduled_task_detail.html`, `settings/_scheduled_tasks.html` | `scheduled-tasks`; `runs`, `thread` kept; `destinations` folds into `edit --to` (the product has one "Delivery" setting, not a separate object); `renew` for "Keep it going" | | `skills` | "Skills"; "Sharing a skill with a group" | `settings/_skills.html`, `reference/skills.md` | `skills` with `share`/`revoke` | +| `comments` (remarks on a room note, optionally on a quoted passage) | "Comments" on the notes applet | notes applet | **`comments`** with `list` (`--all`; resolved threads are hidden by default, a display filter only), `create` (`--quote`, `--reply-to`, `--mention`), `resolve`/`reopen THREAD_ID`; `--note` container flag. The server finds the quote and builds the anchor; the CLI only sends text. Prototype | | `notes` (kind `inline_text`) | "Notes" (sidebar, room "Notes board"); "Artifacts" on the settings page | `_sidebar.html`, `reference/artifacts.md`, `settings/_artifacts.html` | `notes`, the text-editing surface: `notes edit` takes the body (`$EDITOR`, `--file`, `If-Match`), which no other kind has. It is not the whole collection: see the `artifacts` row | | `artifacts` (every kind: `inline_text`, `external_url`, `blob`) | "Artifacts": the app is titled "Artifacts" (`agent/apps/registry.py` `ARTIFACTS_APP`), as are the room page surface (`features/groups/surfaces.py`, `key="artifacts"`), the settings section ("The notes, links and files you've asked Pepper to keep", `settings/_artifacts.html`) and the help topic (`features/help/content.py`, `reference/artifacts.md`). The kinds are "Note", "Link", "File" (`KIND_LABELS`, `agent/artifacts/applet.py`); images, PDFs, recordings, recipe photos are Files, a research report or a recipe is a Note. The iOS app has `ArtifactDetail.swift` | as cited | **`artifacts`** with `list` (`--kind note\|link\|file`, `--label`, `--search`, `--limit`, `--room`; the server filters, the CLI never does), `show`, `download`, `create` (uploads a file), `delete`. Replaces the earlier "never `artifacts`" rule (maintainer, 2026-10-16: all kinds are supported). `notes` stays for writing note text; `artifacts` is for finding, reading, fetching, uploading and deleting anything. Text search is `artifacts list --search` (the maintainer's spelling), not a separate `search` leaf. Routes: `GET /api/v1/artifacts`, `/{id}/detail`, `/{id}/raw`, `POST /upload`, and the `artifacts` app's `delete` action | | timezone, calendar preferences | "Preferences" (Calendar sub-page, gear titled "Preferences", `

Preferences

`) holds Home address, Work address, **Timezone** ("The same timezone as in Settings."), work days, hours, meals, workouts, weather (`web/apps/defaults/calendar.html`); Settings > Profile also has "Timezone" (`settings/_profile.html`) | as cited | **`preferences`** with `show` and `edit` (`--timezone`, `--home-address`, `--work-days`, ...). No `timezone` noun: the Preferences route takes the account timezone and the page says it is the same one, so one noun covers both. `whoami` keeps printing the timezone as a read. Which calendars count as busy is not on this route (it is per booking page, `booking.set_conflicts`) and is not covered | @@ -112,7 +113,7 @@ pepper [--profile P] [--room ROOM [--topic TOPIC]] [-- | Read | `list`, `show`, `search`, `watch`, `activity`, `runs` | `list` = many, filterable; `show OBJECT` = one; `search QUERY` = list by text (help names the fields matched); `watch` = stream; `runs OBJECT` = the run history of an object that runs (`scheduled-tasks`, `research-projects`), a J3 list paged with `--before` | | Write | `create`, `edit`, `delete` | `create` makes, `edit` changes fields, `delete` destroys the object | | Association | `invite`/`remove`, `share`/`revoke`, `install`/`uninstall`, `leave` | break a link, never destroy the object | -| Domain | `post`, `react`/`unreact`, `archive`/`unarchive`, `mute`/`unmute`, `pause`/`resume`/`renew`, `done`/`dismiss`/`reopen`, `download`, `run`, `guide`, `steer`, `take`, `reconsider` | one each, product-named. `run` starts work and returns its receipt (J5); `--wait` waits for it (§3.5). `steer OBJECT [TEXT…\|-]` queues guidance the object's next run reads (research projects). `take ITEM` marks a delivered item as taken; `reconsider ITEM [--reason]` puts a rejected item back as a candidate (research items) | +| Domain | `post`, `react`/`unreact`, `archive`/`unarchive`, `mute`/`unmute`, `pause`/`resume`/`renew`, `done`/`dismiss`/`resolve`/`reopen`, `download`, `run`, `guide`, `steer`, `take`, `reconsider` | one each, product-named. `run` starts work and returns its receipt (J5); `--wait` waits for it (§3.5). `steer OBJECT [TEXT…\|-]` queues guidance the object's next run reads (research projects). `take ITEM` marks a delivered item as taken; `reconsider ITEM [--reason]` puts a rejected item back as a candidate (research items) | `rename` is gone as a verb: it is `edit --name`. Destructive = `delete`, `remove`, `revoke`, `uninstall`, `leave`, and `members edit --role`. They prompt on a TTY; off a TTY or under `--json` @@ -316,6 +317,7 @@ today); **S** needs a server change (or verification first); **N** a room is not | `skills list/show/share/revoke` | optional / optional / required / required | R | `/groups/{gid}/skills…` | | `skills create/edit/delete` | — | N | a room holds a grant of someone's skill, not a skill; `share` is the room verb | | `notes *` | optional | R | `/artifacts…?principal=`, `/notes/{id}…?principal=` | +| `comments list/create/resolve/reopen` | optional (scopes `--note` name resolution) | R | `/artifacts/{id}/comments[/{thread}/resolve\|reopen]`; `--note` is the required container flag (§3 rule 2); room notes only, a personal note answers 409 | | `trajectories list/show` | optional | S-verify | `/trajectories` has no `principal`; help claims room-shared ones list — verify, else add `?principal=` | | `calendar --planning` | optional | C | `/calendars/planning?principal=` | | `calendar` (range) | optional | S | `/calendars/range` has no `principal`; or defer to `apps show calendar --room` | diff --git a/src/pepper_cli/api_client.py b/src/pepper_cli/api_client.py index b8aa5d9..f158762 100644 --- a/src/pepper_cli/api_client.py +++ b/src/pepper_cli/api_client.py @@ -43,6 +43,10 @@ #: Ordinary API request timeout (seconds). Chat streams may have long gaps #: between frames while the agent works, so they get a separate read limit. DEFAULT_TIMEOUT = 60.0 + +#: An agent member's token. It authenticates as the agent, not a Pepper account, so it reaches +#: only the room routes (and a room's notes, named with ``?principal=``). +AGENT_TOKEN_PREFIX = "pep_agt_" DEFAULT_CHAT_STREAM_READ_TIMEOUT = 600.0 #: Longest the agent event stream may go without a byte before the read is #: abandoned and the caller reconnects. The server sends a ``: ping`` comment @@ -1241,6 +1245,8 @@ class GroupTopicSummary: #: lands in. Only the topic rail #: (``GET .../topics``) says so; the rooms list leaves it ``False``. is_default: bool = False + #: The note's id when this is a note's comment topic; ``None`` otherwise. + subject_artifact_id: str | None = None # MIRROR: src/pepper/api/groups.py::list_groups @@ -1436,6 +1442,12 @@ class GroupMessage: #: from an older server. is_agent: bool = False for_display_name: str | None = None + #: Note comments: the quoted passage a thread root is anchored to + #: (``{exact, prefix, suffix, etag}``), and when/by whom the thread was + #: resolved. ``None`` on ordinary messages and on rows from an older server. + anchor: dict[str, Any] | None = None + resolved_at: str | None = None + resolved_by_name: str | None = None # MIRROR: src/pepper/api/groups.py::topic_messages @@ -1561,6 +1573,9 @@ def group_message_from_dict(row: dict[str, Any]) -> GroupMessage: is_agent=bool(row.get("is_agent", False)), for_display_name=row.get("for_display_name"), parent_message_id=row.get("parent_message_id"), + anchor=row.get("anchor") if isinstance(row.get("anchor"), dict) else None, + resolved_at=row.get("resolved_at"), + resolved_by_name=row.get("resolved_by_name"), ) @@ -1596,6 +1611,7 @@ def _group_topic_from_dict(row: dict[str, Any]) -> GroupTopicSummary: archived_at=row.get("archived_at"), imported_history=row.get("imported_history", False), is_default=row.get("is_default", False), + subject_artifact_id=row.get("subject_artifact_id"), ) @@ -1647,6 +1663,90 @@ class ArtifactListResponse: items: list[ArtifactListItem] = field(default_factory=list) +# The note-comment routes answer untyped JSON the server has not published in the vendored +# snapshot yet, so these carry no contract marker; add one once the snapshot refresh lands. +# They follow the prototype contract (GET/POST /api/v1/artifacts/{id}/comments, and +# POST .../{thread_id}/resolve|reopen). +@dataclass(frozen=True) +class NoteComment: + """One message of a note-comment thread (a root or a reply).""" + + id: str + body: str + author: str + created_at: str + mentions: list[str] = field(default_factory=list) + anchor: dict[str, Any] | None = None + resolved_at: str | None = None + resolved_by: str | None = None + deleted: bool = False + + +@dataclass(frozen=True) +class NoteCommentThread: + """A thread: its root message, where the quote sits now, and its replies.""" + + message: NoteComment + position: dict[str, int] | None + detached: bool + replies: list[NoteComment] = field(default_factory=list) + + +@dataclass(frozen=True) +class NoteComments: + """``GET /api/v1/artifacts/{id}/comments``.""" + + artifact_id: str + group_id: str | None + topic_id: str | None + etag: str | None + threads: list[NoteCommentThread] = field(default_factory=list) + resolved_count: int = 0 + + +def _note_comment_from_dict(row: dict[str, Any]) -> NoteComment: + anchor = row.get("anchor") + resolver = row.get("resolved_by") + return NoteComment( + id=str(row["id"]), + body=str(row.get("body") or ""), + author=str((row.get("author") or {}).get("display_name") or ""), + created_at=str(row.get("created_at") or ""), + mentions=_mention_ids( + [ + {"person_id": m["id"]} if isinstance(m, dict) and "id" in m else m + for m in row.get("mentions") or [] + ] + ), + anchor=anchor if isinstance(anchor, dict) else None, + resolved_at=row.get("resolved_at"), + resolved_by=str(resolver.get("display_name") or "") if isinstance(resolver, dict) else None, + deleted=row.get("deleted") is True, + ) + + +def _note_thread_from_dict(t: dict[str, Any]) -> NoteCommentThread: + position = t.get("position") + return NoteCommentThread( + message=_note_comment_from_dict(t["message"]), + position=position if isinstance(position, dict) else None, + detached=bool(t.get("detached", False)), + replies=[_note_comment_from_dict(r) for r in t.get("replies") or []], + ) + + +def _note_comments_from_dict(body: dict[str, Any]) -> NoteComments: + threads = [_note_thread_from_dict(t) for t in body.get("threads") or []] + return NoteComments( + artifact_id=str(body.get("artifact_id") or ""), + group_id=body.get("group_id"), + topic_id=body.get("topic_id"), + etag=body.get("etag"), + threads=threads, + resolved_count=int(body.get("resolved_count") or 0), + ) + + # MIRROR: src/pepper/agent/artifacts/applet.py::read_detail # (the detail route answers an untyped JSONResponse, no snapshot schema) @dataclass(frozen=True) @@ -1776,6 +1876,12 @@ def server_url(self) -> str: server-relative links a message carries against it).""" return self._server_url + @property + def is_agent_token(self) -> bool: + """Whether this client authenticates as a room's agent member (a ``pep_agt_`` token), + which the server lets list notes only of a room it names.""" + return self._token.get_secret_value().startswith(AGENT_TOKEN_PREFIX) + def __enter__(self) -> Self: return self @@ -3565,6 +3671,61 @@ def get_artifact_detail(self, artifact_id: str) -> ArtifactDetail: body = self._request("GET", f"/api/v1/artifacts/{artifact_id}/detail").json() return _artifact_detail_from_dict(body) + def list_note_comments(self, artifact_id: str) -> NoteComments: + """``GET /api/v1/artifacts/{id}/comments`` — a note's comment threads, oldest first.""" + body = self._request("GET", f"/api/v1/artifacts/{artifact_id}/comments").json() + if not isinstance(body, dict): + raise ApiError(status=200, body=str(body), detail="malformed response") + return _note_comments_from_dict(body) + + def create_note_comment( + self, + artifact_id: str, + *, + body: str, + quote: str | None = None, + reply_to: str | None = None, + mentions: list[str] | None = None, + idempotency_key: str | None = None, + ) -> dict[str, Any]: + """``POST /api/v1/artifacts/{id}/comments`` — comment on a quote, the whole note, or + reply to a thread. The server finds the quote and builds the anchor; the answer + (``thread_root_id``, ``message``, ``topic_id``) is returned as the server's own dict.""" + payload: dict[str, Any] = {"body": body, "quote": quote, "start": None} + payload["reply_to"] = reply_to + payload["mentions"] = mentions or [] + if idempotency_key: + payload["idempotency_key"] = idempotency_key + headers = {"Idempotency-Key": idempotency_key} if idempotency_key else None + resp = self._request( + "POST", + f"/api/v1/artifacts/{artifact_id}/comments", + json=payload, + extra_headers=headers, + ).json() + if not isinstance(resp, dict): + raise ApiError(status=200, body=str(resp), detail="malformed response") + return resp + + def _set_note_thread_resolved( + self, artifact_id: str, thread_id: str, action: str + ) -> NoteCommentThread: + resp = self._request( + "POST", f"/api/v1/artifacts/{artifact_id}/comments/{thread_id}/{action}" + ).json() + thread = resp.get("thread") if isinstance(resp, dict) else None + if not isinstance(thread, dict): + raise ApiError(status=200, body=str(resp), detail="malformed response") + return _note_thread_from_dict(thread) + + def resolve_note_thread(self, artifact_id: str, thread_id: str) -> NoteCommentThread: + """``POST /api/v1/artifacts/{id}/comments/{thread_id}/resolve`` (idempotent).""" + return self._set_note_thread_resolved(artifact_id, thread_id, "resolve") + + def reopen_note_thread(self, artifact_id: str, thread_id: str) -> NoteCommentThread: + """``POST /api/v1/artifacts/{id}/comments/{thread_id}/reopen`` (idempotent).""" + return self._set_note_thread_resolved(artifact_id, thread_id, "reopen") + def create_artifact( self, *, diff --git a/src/pepper_cli/claude_channel.py b/src/pepper_cli/claude_channel.py index 78ec463..70207d9 100644 --- a/src/pepper_cli/claude_channel.py +++ b/src/pepper_cli/claude_channel.py @@ -46,7 +46,7 @@ from mcp.server.stdio import stdio_server from mcp.shared.message import SessionMessage -from pepper_cli import api_client, render +from pepper_cli import api_client, credentials, render from pepper_cli._version import get_version from pepper_cli.cli import ( _WATCH_BACKOFF_CAP, @@ -195,6 +195,8 @@ class TopicInfo: room_id: str room_name: str topic_name: str + #: The note's id when this is a note's comment topic. + subject_artifact_id: str | None = None TopicMap = dict[str, TopicInfo] @@ -228,7 +230,9 @@ def build_topic_map(api: api_client.ApiClient, groups: api_client.GroupList) -> mapping: TopicMap = {} for room in groups.groups: for topic in api.list_group_topics(room.group_id): - mapping[topic.topic_id] = TopicInfo(room.group_id, room.display_name, topic.name) + mapping[topic.topic_id] = TopicInfo( + room.group_id, room.display_name, topic.name, topic.subject_artifact_id + ) return mapping @@ -285,9 +289,24 @@ def message_event( if not meta["created_at"]: del meta["created_at"] return content, meta + preface = "" + if info.subject_artifact_id: + note = sanitize_line(info.subject_artifact_id) + preface += ( + f"[comments topic of note {note}; read it with " + f"`{credentials.command_prefix()} comments list --note {note} --room {info.room_id}`; " + "reply under the thread root, never top-level]\n" + ) + if row.anchor and isinstance(row.anchor.get("exact"), str): + quoted = " ".join(sanitize_line(row.anchor["exact"]).split()) + if len(quoted) > 160: + quoted = quoted[:159].rstrip() + "…" + preface += f'> on the passage: "{quoted}"\n' content, truncated = clip_content( - f"{sanitize_line(row.display_name)} in {where}:\n{row.raw_body}" + f"{sanitize_line(row.display_name)} in {where}:\n{preface}{row.raw_body}" ) + if info.subject_artifact_id: + meta["note_id"] = info.subject_artifact_id if row.parent_message_id: meta["parent_id"] = row.parent_message_id if row.author_person_id: diff --git a/src/pepper_cli/cli.py b/src/pepper_cli/cli.py index 0c3150c..01c317b 100644 --- a/src/pepper_cli/cli.py +++ b/src/pepper_cli/cli.py @@ -1902,6 +1902,81 @@ def _build_parser() -> tuple[argparse.ArgumentParser, frozenset[str]]: p_notes_rm.add_argument("--room", help=_GROUP_ARG_HELP + " Omit for your own notes.") p_notes_rm.set_defaults(handler=cmd_notes_delete) + p_comments = sub.add_parser( + "comments", + help=( + "Comments on a room note, optionally on a quoted passage of it. ``list`` the " + "threads, ``create`` a comment or a reply, ``resolve``/``reopen`` a thread. Every " + "command takes --note (with " + "--room, a note of that room). Personal notes cannot be commented on." + ), + ) + sub_comments: Any = p_comments.add_subparsers( + dest="comments_action", required=True, parser_class=_NoteFlagParser + ) + p = sub_comments.add_parser( + "list", + help="List a note's comment threads, oldest first; resolved ones are hidden.", + parents=[_comments_room(), _note_parent(), _json_parent], + ) + p.add_argument( + "--all", + action="store_true", + help="Also show resolved threads (human output only; --json always has every thread).", + ) + p.set_defaults(handler=cmd_comments_list) + for verb, handler, text in ( + ("resolve", cmd_comments_resolve, "Mark a comment thread resolved."), + ("reopen", cmd_comments_reopen, "Reopen a resolved comment thread."), + ): + p = sub_comments.add_parser( + verb, help=text, parents=[_comments_room(), _note_parent(), _json_parent] + ) + p.add_argument( + "thread_id", + metavar="THREAD_ID", + help="The thread's root message id (from 'comments list --json').", + ) + p.set_defaults(handler=handler) + p = sub_comments.add_parser( + "create", + help="Comment on a note: on a quoted passage (--quote), the whole note, or a reply.", + parents=[_comments_room(), _note_parent(), _json_parent], + ) + p.add_argument( + "message", + nargs="*", + help="Comment text. Multi-word OK without quotes. Pass '-' alone to read stdin.", + ) + p.add_argument( + "--quote", + metavar="TEXT", + help=( + "Comment on this exact passage of the note (the server finds it; it must appear " + "once). Omit to comment on the whole note." + ), + ) + p.add_argument( + "--reply-to", + metavar="ID", + help="Reply in the thread whose root message has this id (from 'comments list --json').", + ) + p.add_argument( + "--mention", + action="append", + metavar="PERSON_ID", + help=( + "Structurally mention this person id (from 'members list'); repeatable. " + "Keep their '@Display Name' in the text." + ), + ) + p.add_argument( + "--idempotency-key", + metavar="KEY", + help="Send this Idempotency-Key instead of a fresh one, so a re-run cannot post twice.", + ) + p.set_defaults(handler=cmd_comments_create) + p_takeout = sub.add_parser( "takeout", help="Download a zip archive of everything Pepper holds about your account.", @@ -2117,7 +2192,7 @@ def _login_manual(server_url: str, raw_token: str | None) -> int: #: An agent member's token. It authenticates as the agent, not a Pepper account, so it reaches #: only the room routes — ``whoami`` refuses it by design. -AGENT_TOKEN_PREFIX = "pep_agt_" +AGENT_TOKEN_PREFIX = api_client.AGENT_TOKEN_PREFIX def _login_agent(server_url: str, token: SecretStr) -> int: @@ -5450,6 +5525,21 @@ def run(api: api_client.ApiClient) -> int: page = api.list_group_messages( group_id, topic_id, since=args.since, before=args.before, limit=args.limit ) + # A note's comments topic gets a header line. Best effort and human output only: the + # lookup failing (an HTTP error or a transport one) must not stop the messages printing. + note_id = None + if not _json_mode(args): + try: + note_id = next( + ( + t.subject_artifact_id + for t in api.list_group_topics(group_id) + if t.topic_id == topic_id + ), + None, + ) + except (api_client.ApiError, httpx.RequestError): + note_id = None # Mark before printing: a failed mark exits non-zero with nothing on stdout. if args.mark and page.messages: newest = max(m.created_at for m in page.messages) @@ -5457,7 +5547,7 @@ def run(api: api_client.ApiClient) -> int: _emit_list( page.messages, args, - lambda: render.render_group_messages(page), + lambda: render.render_group_messages(page, note_id), next_before=page.next_before, ) return 0 @@ -6888,6 +6978,209 @@ def _resolve_note( return None +class _NoteFlagParser(argparse.ArgumentParser): + """A ``comments`` leaf: a missing ``--note`` says it is a flag (IA §3 rule 2).""" + + def error(self, message: str) -> NoReturn: + if "--note" in message: + message += f" (the note is a flag here, not a positional: {self.prog} --note NAME)" + super().error(message) + + +def _note_parent() -> argparse.ArgumentParser: + """``--note NOTE``, required: the container flag of ``comments``.""" + parent = argparse.ArgumentParser(add_help=False) + parent.add_argument( + "--note", + metavar="NOTE", + required=True, + help=_NOTE_ARG_HELP + " Always this flag, never a positional.", + ) + return parent + + +def _comments_room() -> argparse.ArgumentParser: + """``--room ROOM`` for ``comments``: scopes note-name resolution to that room's notes.""" + return _room_parent(omitted="Scopes the note name to that room's shared notes.") + + +def _report_comments_api_error(exc: api_client.ApiError, args: argparse.Namespace | None) -> int: + """:func:`_report_notes_api_error` plus the comment routes' own refusals (all exit 2): + 409 for a personal note, 422 ``quote_not_found`` / ``quote_ambiguous`` / ``quote_mismatch``.""" + error: object = None + count: object = None + try: + payload = json.loads(exc.body) + except ValueError: + payload = None + if isinstance(payload, dict): + detail = payload.get("detail") + found = detail if isinstance(detail, dict) else payload + error, count = found.get("error"), found.get("count") + if exc.status == 422 and error == "quote_not_found": + _emit_error( + args, + "validation", + "that quote does not appear in the note; copy it exactly as it reads", + status=422, + ) + return 2 + if exc.status == 422 and error == "quote_ambiguous": + _emit_error( + args, + "ambiguous", + f"that quote appears {count if isinstance(count, int) else 'more than once'} " + "times in the note; quote a longer, unique passage", + status=422, + ) + return 2 + if exc.status == 422 and error == "quote_mismatch": + _emit_error(args, "validation", "the quote does not match the note text", status=422) + return 2 + if exc.status == 409: + _emit_error( + args, + "conflict", + "personal notes cannot be commented on; copy the note into a room" + if error == "personal_note" or not exc.detail + else exc.detail, + status=409, + ) + return 2 + return _report_notes_api_error(exc, args) + + +def _comments_note(api: api_client.ApiClient, args: argparse.Namespace) -> str | None: + """The note ``--note`` names. A note name resolves through ``GET /api/v1/artifacts``, + which an agent token may call only with the room named (``?principal=``); without + ``--room`` the server would refuse it, so say what to pass instead of reporting that.""" + if args.room is None and not _valid_uuid(args.note) and api.is_agent_token: + _emit_error( + args, + "usage", + "an agent finds a note by name only within a room: add --room ROOM, " + "or give the note's id", + ) + return None + room_principal = _opt_group(api, args.room, args=args) + return _resolve_note(api, room_principal=room_principal, value=args.note, args=args) + + +def cmd_comments_list(args: argparse.Namespace) -> int: + """``pepper comments list --note N [--room R] [--json]``. + + ``--json`` is the J3 envelope: ``items`` are the threads (``message``, ``position``, + ``detached``, ``replies``), with ``artifact_id``, ``group_id``, ``topic_id``, ``etag`` and + ``resolved_count`` beside them. ``--all`` is a display filter of the human output only: the + server returns every thread, and ``--json`` passes them all through. + """ + + def run(api: api_client.ApiClient) -> int: + note_id = _comments_note(api, args) + if note_id is None: + return 2 + result = api.list_note_comments(note_id) + _emit_list( + result.threads, + args, + lambda: render.render_note_comments(result, show_resolved=args.all), + artifact_id=result.artifact_id, + group_id=result.group_id, + topic_id=result.topic_id, + etag=result.etag, + resolved_count=result.resolved_count, + ) + return 0 + + return _with_api(run, _report_comments_api_error, args) + + +def _cmd_comments_set_resolved(args: argparse.Namespace, *, resolve: bool) -> int: + thread_id = args.thread_id.strip() + if not thread_id: + _emit_error(args, "usage", "THREAD_ID cannot be empty") + return 2 + + def run(api: api_client.ApiClient) -> int: + note_id = _comments_note(api, args) + if note_id is None: + return 2 + call = api.resolve_note_thread if resolve else api.reopen_note_thread + thread = call(note_id, thread_id) + + def human() -> None: + render.info("Resolved." if resolve else "Reopened.") + print(sanitize_line(thread.message.id)) + + _emit(thread, args, human) + return 0 + + return _with_api(run, _report_comments_api_error, args) + + +def cmd_comments_resolve(args: argparse.Namespace) -> int: + """``pepper comments resolve --note N [--room R] THREAD_ID [--json]``: idempotent. A thread + id that is not a top-level comment of the note is a 404 (exit 1, as elsewhere).""" + return _cmd_comments_set_resolved(args, resolve=True) + + +def cmd_comments_reopen(args: argparse.Namespace) -> int: + """``pepper comments reopen --note N [--room R] THREAD_ID [--json]``: idempotent.""" + return _cmd_comments_set_resolved(args, resolve=False) + + +def cmd_comments_create(args: argparse.Namespace) -> int: + """``pepper comments create --note N [--room R] [--quote Q] [--reply-to ID] + [--mention PERSON_ID ...] [MESSAGE | -] [--idempotency-key K] [--json]``. + + The server finds ``--quote`` in the note, builds the anchor and creates the note's topic on + the first comment; the CLI sends the text through. ``--json`` prints the server's answer + (``thread_root_id``, ``message``, ``topic_id``). Exit 2 for bad input: an empty body, a quote + that is missing or ambiguous, a personal note, an unknown note. + """ + for flag, value in ( + ("--idempotency-key", args.idempotency_key), + ("--reply-to", args.reply_to), + ("--quote", args.quote), + *(("--mention", m) for m in args.mention or []), + ): + if value is not None and not value.strip(): + _emit_error(args, "usage", f"{flag} cannot be empty") + return 2 + if args.quote is not None and args.reply_to is not None: + _emit_error(args, "usage", "--quote and --reply-to are exclusive: a reply has no quote") + return 2 + parts = args.message or [] + text = sys.stdin.read().strip() if parts == ["-"] else " ".join(parts).strip() + if not text: + _emit_error( + args, "usage", "comment text cannot be empty (pass text, or '-' to read it from stdin)" + ) + return 2 + + def run(api: api_client.ApiClient) -> int: + note_id = _comments_note(api, args) + if note_id is None: + return 2 + created = api.create_note_comment( + note_id, + body=text, + quote=args.quote, + reply_to=args.reply_to, + mentions=args.mention or None, + idempotency_key=args.idempotency_key or str(uuid.uuid4()), + ) + + def human() -> None: + render.info("Commented.") + print(sanitize_line(str(created.get("thread_root_id") or ""))) + + _emit(created, args, human) + return 0 + + return _with_api(run, _report_comments_api_error, args) + + def _note_not_found(args: argparse.Namespace | None) -> None: _emit_error(args, "not_found", "no such room or note, or you are not a member", status=404) diff --git a/src/pepper_cli/credentials.py b/src/pepper_cli/credentials.py index b3b663f..2281370 100644 --- a/src/pepper_cli/credentials.py +++ b/src/pepper_cli/credentials.py @@ -97,6 +97,13 @@ def active_profile() -> str | None: return _profile +def command_prefix() -> str: + """``pepper``, plus ``--profile NAME`` when one is selected: the start of a command the CLI + suggests, so following it runs as the same login (an agent's is always a named profile). + Profile names are validated by :func:`set_profile`, so the name needs no quoting.""" + return f"pepper --profile {_profile}" if _profile else "pepper" + + def _config_dir() -> Path: """Return the directory the credentials file lives in. diff --git a/src/pepper_cli/data/agent-skill/SKILL.md b/src/pepper_cli/data/agent-skill/SKILL.md index 7661b32..49605ee 100644 --- a/src/pepper_cli/data/agent-skill/SKILL.md +++ b/src/pepper_cli/data/agent-skill/SKILL.md @@ -17,7 +17,7 @@ rest describes how the commands behave. - An agent member authenticates with its own token, which starts `pep_agt_`. It is not a Pepper user account. It can reach the rooms it has been added to and nothing else, and only through - the nine commands listed in section 8. Apart from `login`, every other command that talks to + the seventeen commands listed in section 8. Apart from `login`, every other command that talks to the server answers an agent token with a 401 (section 9). - Because of that, `pepper --profile whoami` fails with a 401 for an agent token. That is by design, not a broken login. Check a login with `pepper --profile rooms list` instead. @@ -323,6 +323,45 @@ EOF the text before you send it, and correct a mistake with a new reply only if you are addressed again or your operator allows it. +### Comments on a room note + +`pepper --profile comments list --room --note ` prints a room note's comment +threads: per thread the quoted passage (or "whole note"), a `detached` marker when the quote is no +longer in the note, then author, time and body, replies indented (`--json`: `items` are threads +with `message`, `position`, `detached`, `replies`). A withdrawn comment shows as `(deleted)` +(`deleted` in `--json`). `pepper --profile comments create --room --note +[--quote TEXT] [--reply-to MESSAGE_ID] [--mention PERSON_ID] -` with the text on stdin adds a +comment: `--quote` names an exact passage of the note, which must appear once (otherwise exit 2 +with a message saying the quote was not found or is ambiguous: quote a longer passage), no +`--quote` comments on the whole note, and `--reply-to` answers a thread by its root message id. A +personal note cannot be commented on (exit 2). The note is always the `--note` flag: its name +(or a unique prefix) when you also give `--room`, otherwise its id — an agent token finds a note +by name only within a room, and without `--room` a name exits 2 saying so. + +The list hides resolved threads and says how many (`--all` shows them, marked "resolved by +X"; `--json` always has every thread, each message with `resolved_at` and `resolved_by`, plus +`resolved_count`). `pepper --profile comments resolve --room --note +THREAD_ID` closes a thread and `pepper --profile comments reopen` reopens it, both +idempotent; `THREAD_ID` is the thread's root message id. A new reply to a resolved thread reopens +it on its own. + +#### When you're addressed in a note's comments + +You are in a note's comments when the topic names a note (`pepper --profile messages list` +and the channel's event text say "comments topic of note "; `--json` topics carry +`subject_artifact_id`) or the message has an `anchor` (`exact` is the passage the comment is on; +shown as a `> "..."` line above the body). Read the context before answering: `pepper --profile + notes show --room ` prints the whole note, which you need for a comment on the +whole note (it has no quoted passage) and for any question about what the note says around the +passage; then `pepper --profile comments list --room --note ` gives the +threads and the quoted passages. Reply +as a child of the comment's thread root, never top-level: `pepper --profile comments create +--room --note --reply-to -` with the text on stdin (or `pepper +--profile messages post --room --topic --reply-to -`). The +thread root is the thread's first message; if you were mentioned in a reply, use that reply's +`parent_message_id`. Resolving (`pepper --profile comments resolve`) is fine once the +asker's question is settled, when the room's norms allow it; otherwise leave it for them. + ## 7. Etiquette (a default; a deployment may override it) These are neutral defaults for sharing a room with people and other agents. Whoever deploys you @@ -349,7 +388,7 @@ set. ## 8. What an agent token can run -An agent token is accepted by exactly these nine commands (the channel server in section 4 +An agent token is accepted by exactly these seventeen commands (the channel server in section 4 uses the same routes as the watch, the reads, the posts and the reactions, and adds none). They are also the prefixes a harness that asks before running commands needs to allow. Each is scoped to one profile name and one subcommand, so allowing them does not allow `login`, `logout` or anything in section 9. @@ -364,8 +403,31 @@ pepper --profile messages react pepper --profile messages unreact pepper --profile members list pepper --profile attachments download +pepper --profile comments list +pepper --profile comments create +pepper --profile comments resolve +pepper --profile comments reopen +pepper --profile notes list +pepper --profile notes search +pepper --profile notes show +pepper --profile notes edit ``` +The four `notes` commands reach only the notes of a room whose roster names you, so always give +them `--room `. They list that room's notes (text notes only), print one note, or replace +its body; `NOTE` is the note's name or id: + +- `pepper --profile notes list --room ` and + `pepper --profile notes search QUERY --room ` find notes. +- `pepper --profile notes show NOTE --room ` prints a note's markdown body + (`--json`: the full detail, with its `etag`). +- `pepper --profile notes edit NOTE --room --file -` replaces the body from stdin + (an agent has no editor; `--name` renames). Edit only when you were asked to. An edit sends the + etag it just read, so a note someone changed in between answers a conflict (exit 1) rather than + being overwritten: read it again and decide, and never pass `--force` to get past it. + +Without `--room` these commands aim at your personal notes, which an agent token cannot reach. + In Claude Code, for example, each becomes an allow rule such as `Bash(pepper --profile messages list:*)`. Log in once by hand (section 1) rather than allowing `login`. @@ -381,6 +443,9 @@ is changed. That covers: agent member cannot change or remove a message, including its own. - `pepper --profile rooms show`: room details. - Two flags on commands that otherwise work: `--mark` on a read and `--attach` on a post. +- `pepper --profile notes create` and `pepper --profile notes delete`: an agent + member cannot create or delete a note. Personal notes (any `notes` command without `--room`) and + every other kind of artifact (images, links, files) are out of reach too. - Every other `rooms`, `topics`, `messages`, `attachments`, `members`, `sharing` or `apps` command, which are the ones that change a room, its topics, its members or its apps, or that read or search across rooms. - Every command outside those nouns that talks to the server, `whoami`, chat and `tui` included @@ -397,9 +462,9 @@ binary has, not commands your token may use. - 401: the CLI prints `Authentication failed (401). Run 'pepper login' to refresh your token.` Do not follow that advice; it is written for a person. First check the command against - sections 8 and 9. A 401 from a command or flag that is not on the list of nine is the expected + sections 8 and 9. A 401 from a command or flag that is not on the list of seventeen is the expected refusal of an agent token and says nothing about your login: stop using that command. A 401 - from one of the nine means the token is unknown, revoked or expired, or its owner's access was + from one of the seventeen means the token is unknown, revoked or expired, or its owner's access was removed. Stop and tell your operator. - A paused or retired agent keeps a valid token, so it is not a 401. The groups service stops acting for it: the stream ends with `agent_unavailable` (section 4), and other commands fail diff --git a/src/pepper_cli/render.py b/src/pepper_cli/render.py index 63666a4..24fcc54 100644 --- a/src/pepper_cli/render.py +++ b/src/pepper_cli/render.py @@ -42,6 +42,7 @@ from rich.segment import Segment from rich.text import Text +from pepper_cli import credentials from pepper_cli.terminal_text import sanitize_line, sanitize_text if TYPE_CHECKING: @@ -52,6 +53,7 @@ GroupMessagePage, GroupRosterMember, GroupTopicSummary, + NoteComments, ScheduledTaskDetail, ScheduledTaskMessage, ScheduledTaskRunEntry, @@ -897,6 +899,44 @@ def render_group_detail(detail: dict[str, Any]) -> None: _stdout.print("apps: (none installed)") +def render_note_comments(result: NoteComments, *, show_resolved: bool = False) -> None: + """Pretty-print ``pepper comments list``: per thread the quote (or "whole note"), a + ``detached`` marker when the quote no longer appears in the note, then author, time and + body, with replies indented beneath. Resolved threads are hidden unless ``show_resolved`` + (a display filter only), with a footer counting what was hidden; shown, they carry a + "resolved by X" marker.""" + threads = [t for t in result.threads if show_resolved or not t.message.resolved_at] + hidden = len(result.threads) - len(threads) + if not threads: + _stdout.print("(no comments)" if not hidden else "(no open comments)") + for i, thread in enumerate(threads): + if i: + _stdout.print("") + anchor = thread.message.anchor + quote = str(anchor.get("exact") or "") if anchor else "" + head = f"[bold]\u201c{_s(quote)}\u201d[/bold]" if quote else "[bold]whole note[/bold]" + if thread.detached: + head += " [yellow]detached[/yellow]" + if thread.message.resolved_at: + by = f" by {_s(thread.message.resolved_by)}" if thread.message.resolved_by else "" + head += f" [green]resolved{by}[/green]" + _stdout.print(head, soft_wrap=True) + for depth, msg in enumerate([thread.message, *thread.replies]): + pad = " " if depth else "" + _stdout.print( + f"{pad} [cyan]{_s(msg.author)}[/cyan] " + f"[dim]{_s(_format_stamp(msg.created_at))} {_s(msg.id)}[/dim]", + soft_wrap=True, + ) + if msg.deleted: + _stdout.print(f"{pad} [dim](deleted)[/dim]", soft_wrap=True) + continue + for line in sanitize_text(msg.body).splitlines() or [""]: + _stdout.print(f"{pad} {escape(line)}", soft_wrap=True) + if hidden and not show_resolved: + _stdout.print(f"\n{hidden} resolved hidden \u2014 use --all", soft_wrap=True) + + def render_note_list(result: ArtifactListResponse) -> None: """Pretty-print a ``pepper notes list``/``search`` page to stdout. @@ -949,7 +989,30 @@ def group_message_tags(m: GroupMessage) -> tuple[str, str]: return via, agent -def render_group_messages(page: GroupMessagePage) -> None: +ANCHOR_QUOTE_CAP = 160 + + +def note_topic_header(note_id: str) -> str: + """One line naming a note's comment topic and how to read it as threads.""" + return ( + f"[note comments] This is the comments topic of note {sanitize_line(note_id)}; " + f"read it as threads with: {credentials.command_prefix()} comments list " + f"--note {sanitize_line(note_id)}" + ) + + +def anchor_quote_line(anchor: dict[str, Any] | None) -> str: + """``> "quoted passage"`` for an anchored comment (truncated), else ``""``.""" + exact = anchor.get("exact") if isinstance(anchor, dict) else None + if not isinstance(exact, str) or not exact.strip(): + return "" + text = " ".join(sanitize_text(exact).split()) + if len(text) > ANCHOR_QUOTE_CAP: + text = text[: ANCHOR_QUOTE_CAP - 1].rstrip() + "…" + return f'> "{text}"' + + +def render_group_messages(page: GroupMessagePage, note_id: str | None = None) -> None: """Pretty-print one page of a topic's messages to stdout, oldest first. Timestamps use ``created_at_display`` verbatim — server-preformatted in @@ -977,6 +1040,8 @@ def render_group_messages(page: GroupMessagePage) -> None: than opening a blank one (the same guard as the web room's ``group_room.js``). """ + if note_id: + _stdout.print(note_topic_header(note_id), markup=False, highlight=False, soft_wrap=True) if not page.messages: _stdout.print("(no messages)") last_day_key: str | None = None @@ -1003,6 +1068,12 @@ def render_group_messages(page: GroupMessagePage) -> None: _stdout.print( f"[dim]{_s(m.created_at_display)}[/dim] [bold]{display_name}[/bold]{via}{edited}:" ) + quoted = anchor_quote_line(m.anchor) + if quoted: + _stdout.print(quoted, markup=False, highlight=False, soft_wrap=True) + if m.resolved_at: + by = f" by {m.resolved_by_name}" if m.resolved_by_name else "" + _stdout.print(f"(resolved{by})", markup=False, highlight=False) if m.is_broken: _stdout.print("[dim](message could not be rendered)[/dim]") else: diff --git a/tests/test_agent_skill.py b/tests/test_agent_skill.py index de21b31..3f12e06 100644 --- a/tests/test_agent_skill.py +++ b/tests/test_agent_skill.py @@ -45,7 +45,19 @@ _PLACEHOLDER = re.compile(r"^<[^<>\s]*>$") # Commands the document always shows under a profile; ``agent-skill`` and ``--version`` # need no login, so the document may show them bare. -_API_NOUNS = frozenset({"rooms", "topics", "messages", "attachments", "members", "sharing", "apps"}) +_API_NOUNS = frozenset( + { + "rooms", + "topics", + "messages", + "attachments", + "members", + "sharing", + "apps", + "comments", + "notes", + } +) _NEEDS_PROFILE = _API_NOUNS | {"login", "logout", "whoami"} _HEREDOC = re.compile(r"<<-?\s*(['\"]?)(\w+)\1") @@ -74,6 +86,14 @@ "messages unreact", "members list", "attachments download", + "comments list", + "comments create", + "comments resolve", + "comments reopen", + "notes list", + "notes search", + "notes show", + "notes edit", } ) _AGENT_REFUSED_FLAGS = frozenset({"--mark", "--attach"}) @@ -347,6 +367,8 @@ def test_document_names_the_tempting_commands_as_unavailable() -> None: "messages edit", "messages delete", "rooms show", + "notes create", + "notes delete", ): assert f"`pepper --profile {needle}`" in cannot, needle for flag in _AGENT_REFUSED_FLAGS: diff --git a/tests/test_api_client_note_comments.py b/tests/test_api_client_note_comments.py new file mode 100644 index 0000000..2c0bd48 --- /dev/null +++ b/tests/test_api_client_note_comments.py @@ -0,0 +1,249 @@ +"""Tests for the ``ApiClient`` note-comment methods: wire shape and response parsing.""" + +from __future__ import annotations + +import json + +import httpx +import pytest + +from pepper_cli import api_client +from pepper_cli.secret import SecretStr + +_NID = "00000000-0000-0000-0000-0000000000d1" +_GID = "00000000-0000-0000-0000-0000000000a1" +_PATH = f"/api/v1/artifacts/{_NID}/comments" + + +def _client(handler) -> api_client.ApiClient: + return api_client.ApiClient( + server_url="http://example.com", + token=SecretStr("pep_pat_abc"), + client=httpx.Client(transport=httpx.MockTransport(handler)), + ) + + +def _msg(mid: str, **over) -> dict: + base = { + "id": mid, + "author": {"id": "p1", "display_name": "Ada", "is_agent": False}, + "body": "hello", + "body_html": "

hello

", + "created_at": "2026-10-26T10:00:00Z", + "edited_at": None, + "deleted": False, + "parent_message_id": None, + "mentions": [], + "anchor": None, + "resolved_at": None, + "resolved_by": None, + } + base.update(over) + return base + + +def test_list_parses_threads_replies_and_resolution() -> None: + anchor = {"exact": "sunscreen", "prefix": "- ", "suffix": "\n", "etag": "e1"} + body = { + "artifact_id": _NID, + "group_id": _GID, + "topic_id": "t1", + "etag": "e1", + "viewer_person_id": "p9", + "resolved_count": 1, + "threads": [ + { + "id": "m1", + "message": _msg( + "m1", + anchor=anchor, + resolved_at="2026-10-26T11:00:00Z", + resolved_by={"id": "p2", "display_name": "Bo", "is_agent": False}, + ), + "position": {"start": 2, "end": 11}, + "detached": False, + "replies": [ + _msg( + "m2", parent_message_id="m1", mentions=[{"id": "p3", "display_name": "Cy"}] + ), + _msg("m3", parent_message_id="m1", body="", deleted=True), + ], + }, + {"id": "m4", "message": _msg("m4"), "position": None, "detached": True, "replies": []}, + ], + "future_field": 1, + } + seen: dict[str, object] = {} + + def handler(request: httpx.Request) -> httpx.Response: + seen["method"], seen["path"] = request.method, request.url.path + return httpx.Response(200, json=body) + + with _client(handler) as api: + result = api.list_note_comments(_NID) + + assert (seen["method"], seen["path"]) == ("GET", _PATH) + assert (result.artifact_id, result.group_id, result.topic_id) == (_NID, _GID, "t1") + assert result.etag == "e1" and result.resolved_count == 1 + first, second = result.threads + assert first.message.anchor == anchor + assert first.message.resolved_at == "2026-10-26T11:00:00Z" + assert first.message.resolved_by == "Bo" + assert first.position == {"start": 2, "end": 11} and not first.detached + assert [r.id for r in first.replies] == ["m2", "m3"] + assert first.replies[0].mentions == ["p3"] + assert first.replies[0].author == "Ada" + assert [r.deleted for r in first.replies] == [False, True] + assert second.detached and second.position is None + assert second.message.resolved_at is None and second.message.resolved_by is None + + +def test_list_tolerates_a_sparse_answer() -> None: + with _client(lambda r: httpx.Response(200, json={"artifact_id": _NID})) as api: + result = api.list_note_comments(_NID) + assert result.threads == [] and result.resolved_count == 0 and result.topic_id is None + + +def test_list_rejects_a_non_object_answer() -> None: + with ( + _client(lambda r: httpx.Response(200, json=[1])) as api, + pytest.raises(api_client.ApiError), + ): + api.list_note_comments(_NID) + + +def test_create_sends_the_full_payload_and_idempotency_key() -> None: + seen: dict[str, object] = {} + + def handler(request: httpx.Request) -> httpx.Response: + seen["method"], seen["path"] = request.method, request.url.path + seen["json"] = json.loads(request.content) + seen["key"] = request.headers.get("Idempotency-Key") + return httpx.Response(200, json={"thread_root_id": "m1", "topic_id": "t1"}) + + with _client(handler) as api: + out = api.create_note_comment( + _NID, body="Nice", quote="hello", mentions=["p3"], idempotency_key="k-1" + ) + + assert out == {"thread_root_id": "m1", "topic_id": "t1"} + assert (seen["method"], seen["path"]) == ("POST", _PATH) + assert seen["json"] == { + "body": "Nice", + "quote": "hello", + "start": None, + "reply_to": None, + "mentions": ["p3"], + "idempotency_key": "k-1", + } + assert seen["key"] == "k-1" + + +def test_create_without_optionals_sends_no_key() -> None: + seen: dict[str, object] = {} + + def handler(request: httpx.Request) -> httpx.Response: + seen["json"] = json.loads(request.content) + seen["key"] = request.headers.get("Idempotency-Key") + return httpx.Response(200, json={}) + + with _client(handler) as api: + api.create_note_comment(_NID, body="x", reply_to="m1") + + assert seen["json"] == { + "body": "x", + "quote": None, + "start": None, + "reply_to": "m1", + "mentions": [], + } + assert seen["key"] is None + + +@pytest.mark.parametrize( + ("status", "payload"), + [ + (422, {"detail": {"error": "quote_ambiguous", "count": 2}}), + (409, {"detail": {"error": "personal_note"}}), + ], +) +def test_create_errors_surface_as_api_error_with_the_body(status, payload) -> None: + with ( + _client(lambda r: httpx.Response(status, json=payload)) as api, + pytest.raises(api_client.ApiError) as exc, + ): + api.create_note_comment(_NID, body="x", quote="q") + assert exc.value.status == status + assert json.loads(exc.value.body) == payload + + +@pytest.mark.parametrize("action", ["resolve", "reopen"]) +def test_resolve_and_reopen_post_to_the_thread_and_parse_it(action) -> None: + seen: dict[str, object] = {} + thread = { + "id": "m1", + "message": _msg("m1", resolved_at="2026-10-26T11:00:00Z"), + "position": None, + "detached": False, + "replies": [], + } + + def handler(request: httpx.Request) -> httpx.Response: + seen["method"], seen["path"] = request.method, request.url.path + return httpx.Response(200, json={"thread": thread}) + + with _client(handler) as api: + call = api.resolve_note_thread if action == "resolve" else api.reopen_note_thread + result = call(_NID, "m1") + + assert (seen["method"], seen["path"]) == ("POST", f"{_PATH}/m1/{action}") + assert result.message.id == "m1" + assert result.message.resolved_at == "2026-10-26T11:00:00Z" + + +def test_resolve_without_a_thread_is_malformed() -> None: + with ( + _client(lambda r: httpx.Response(200, json={"ok": True})) as api, + pytest.raises(api_client.ApiError), + ): + api.resolve_note_thread(_NID, "m1") + + +def test_group_message_and_topic_parse_the_note_fields() -> None: + row = { + "id": "m1", + "is_pepper": False, + "is_you": False, + "is_deleted": False, + "is_broken": False, + "run_start": True, + "display_name": "Ada", + "initial": "A", + "avatar_tint": "t1", + "raw_body": "x", + "rendered_html": "

x

", + "created_at": "2026-10-26T10:00:00Z", + "created_at_display": "10:00", + "day_key": "k", + "day_kicker": "k", + "day_heading": "k", + "day_label": "k", + "quote_excerpt": "", + "anchor": {"exact": "hello"}, + "resolved_at": "2026-10-26T11:00:00Z", + "resolved_by_name": "Bo", + } + parsed = api_client.group_message_from_dict(row) + assert parsed.anchor == {"exact": "hello"} + assert (parsed.resolved_at, parsed.resolved_by_name) == ("2026-10-26T11:00:00Z", "Bo") + plain = api_client.group_message_from_dict( + {k: v for k, v in row.items() if k not in ("anchor", "resolved_at", "resolved_by_name")} + ) + assert plain.anchor is None and plain.resolved_at is None + + +@pytest.mark.parametrize(("token", "agent"), [("pep_agt_abc", True), ("pep_pat_abc", False)]) +def test_is_agent_token_reads_the_token_prefix(token, agent) -> None: + api = api_client.ApiClient(server_url="http://example.com", token=SecretStr(token)) + with api: + assert api.is_agent_token is agent diff --git a/tests/test_claude_channel.py b/tests/test_claude_channel.py index 3c19b0b..b1b9d7f 100644 --- a/tests/test_claude_channel.py +++ b/tests/test_claude_channel.py @@ -189,6 +189,8 @@ def __init__(self, *streams: str | int, features: tuple[str, ...] = ("watch_scop self.requests: list[httpx.Request] = [] self.messages: dict[str, dict] = {_MESSAGE: _message_json()} self.topics = {_TOPIC: "Dinner", _OTHER_TOPIC: "Chores"} + #: topic id -> the note id when that topic holds a note's comments. + self.subjects: dict[str, str] = {} self.message_status = 200 self.fail_messages = 0 self.fail_status = 503 @@ -226,7 +228,12 @@ def __call__(self, request: httpx.Request) -> httpx.Response: ) if path == f"/api/v1/groups/{_ROOM}/topics": rows = [ - {"topic_id": tid, "name": name, "last_activity_at": None} + { + "topic_id": tid, + "name": name, + "last_activity_at": None, + "subject_artifact_id": self.subjects.get(tid), + } for tid, name in self.topics.items() ] return httpx.Response(200, json={"topics": rows}) @@ -594,6 +601,66 @@ def test_a_message_becomes_one_channel_notification_with_the_body_and_ids(monkey assert result.code == 0 +_NOTE = "00000000-0000-0000-0000-0000000000e9" + + +def test_a_comment_in_a_note_topic_carries_the_note_header_quote_and_note_id(monkeypatch): + fake = _Fake(_CONNECTED + _scope() + _event(7)) + fake.subjects[_TOPIC] = _NOTE + fake.messages[_MESSAGE] = _message_json(anchor={"exact": "the oven\ndoor"}) + + result = run_channel(monkeypatch, fake, ["--room", _ROOM]) + + (note,) = result.notifications + assert note["content"] == ( + "Alice Example in Kitchen \u203a Dinner:\n" + f"[comments topic of note {_NOTE}; read it with " + f"`pepper comments list --note {_NOTE} --room {_ROOM}`; " + "reply under the thread root, never top-level]\n" + '> on the passage: "the oven door"\n' + "Can you check the oven?" + ) + assert note["meta"]["note_id"] == _NOTE + assert all(_META_KEY.match(k) and isinstance(v, str) for k, v in note["meta"].items()) + + +def test_the_note_header_command_keeps_the_channel_profile(monkeypatch): + credentials.set_profile("helper") + credentials.save(server_url=_SERVER, token=SecretStr("pep_agt_abc")) + fake = _Fake(_CONNECTED + _scope() + _event(7)) + fake.subjects[_TOPIC] = _NOTE + fake.messages[_MESSAGE] = _message_json() + + result = run_channel(monkeypatch, fake, ["--room", _ROOM]) + + (note,) = result.notifications + assert ( + f"`pepper --profile helper comments list --note {_NOTE} --room {_ROOM}`" + in (note["content"]) + ) + + +def test_a_long_anchor_quote_is_cut_and_a_plain_topic_has_no_note_fields(monkeypatch): + fake = _Fake(_CONNECTED + _scope() + _event(7) + _event(8, message=_MESSAGE_2)) + fake.subjects[_TOPIC] = _NOTE + fake.messages[_MESSAGE] = _message_json(anchor={"exact": "w" * 400}) + fake.messages[_MESSAGE_2] = _message_json(_MESSAGE_2) + fake.topics[_OTHER_TOPIC] = "Chores" + + result = run_channel(monkeypatch, fake, ["--room", _ROOM]) + + quoted = next( + ln for ln in result.notifications[0]["content"].splitlines() if ln.startswith("> on") + ) + assert quoted.endswith('\u2026"') and len(quoted) < 200 + + plain = _Fake(_CONNECTED + _scope() + _event(7)) + plain.messages[_MESSAGE] = _message_json(anchor={"exact": "ignored"}) + (note,) = run_channel(monkeypatch, plain, ["--room", _ROOM]).notifications + assert "note_id" not in note["meta"] + assert "comments topic" not in note["content"] + + def test_sender_kind_is_agent_or_pepper_and_a_top_level_post_has_no_parent(monkeypatch): fake = _Fake(_CONNECTED + _scope() + _event(7) + _event(8, message=_MESSAGE_2)) fake.messages[_MESSAGE] = _message_json(is_agent=True) diff --git a/tests/test_cli_comments.py b/tests/test_cli_comments.py new file mode 100644 index 0000000..0eb685c --- /dev/null +++ b/tests/test_cli_comments.py @@ -0,0 +1,541 @@ +"""End-to-end tests for ``pepper comments`` and the note lines of ``messages list``. + +Same approach as ``test_cli_notes.py``: ``cli.main([...])`` against a monkeypatched +``api_client.ApiClient``, so nothing touches the network. +""" + +from __future__ import annotations + +import io +import json +import sys + +import httpx +import pytest + +from pepper_cli import api_client, cli, credentials +from pepper_cli.secret import SecretStr + +_GID = "00000000-0000-0000-0000-0000000000a1" +_NID = "00000000-0000-0000-0000-0000000000d1" +_TID = "00000000-0000-0000-0000-0000000000b1" + + +def _login(tmp_home) -> None: + credentials.save(server_url="http://example.com", token=SecretStr("pep_pat_abc")) + + +def _c(cid: str, body: str = "hello", **over) -> api_client.NoteComment: + base = {"id": cid, "body": body, "author": "Ada", "created_at": "2026-10-26T10:00:00+00:00"} + base.update(over) + return api_client.NoteComment(**base) + + +def _thread(root: api_client.NoteComment, *replies, **over) -> api_client.NoteCommentThread: + base = {"message": root, "position": None, "detached": False, "replies": list(replies)} + base.update(over) + return api_client.NoteCommentThread(**base) + + +def _comments(*threads, resolved_count=0) -> api_client.NoteComments: + return api_client.NoteComments( + artifact_id=_NID, + group_id=_GID, + topic_id=_TID, + etag="e1", + threads=list(threads), + resolved_count=resolved_count, + ) + + +class _FakeApi: + instances: list[_FakeApi] = [] + + def __init__(self, *, server_url, token, **canned) -> None: + self._canned = canned + self.calls: list[tuple] = [] + self.is_agent_token = token.get_secret_value().startswith(api_client.AGENT_TOKEN_PREFIX) + _FakeApi.instances.append(self) + + def __enter__(self): + return self + + def __exit__(self, *exc_info): + return None + + def _result(self, key): + value = self._canned.get(key) + if isinstance(value, Exception): + raise value + if value is None: + raise AssertionError(f"test forgot to set {key}") + return value + + def list_artifacts(self, *, principal=None, **kwargs): + # The server's rule for an agent token: notes of a room it names, nothing else. + self.calls.append(("artifacts", principal, kwargs)) + if self.is_agent_token and principal is None: + raise _err(422, {"detail": "an agent names the room with ?principal="}) + return self._result("notes") + + def list_note_comments(self, artifact_id): + self.calls.append(("list", artifact_id)) + return self._result("comments") + + def create_note_comment(self, artifact_id, **kwargs): + self.calls.append(("create", artifact_id, kwargs)) + return self._result("created") + + def resolve_note_thread(self, artifact_id, thread_id): + self.calls.append(("resolve", artifact_id, thread_id)) + return self._result("thread") + + def reopen_note_thread(self, artifact_id, thread_id): + self.calls.append(("reopen", artifact_id, thread_id)) + return self._result("thread") + + +@pytest.fixture +def patch_api(monkeypatch): + _FakeApi.instances.clear() + + def install(**canned): + monkeypatch.setattr(api_client, "ApiClient", lambda **kw: _FakeApi(**canned, **kw)) + + return install + + +def _calls() -> list[tuple]: + return _FakeApi.instances[0].calls + + +def _err(status: int, payload: object = None) -> api_client.ApiError: + body = json.dumps(payload) if payload is not None else "" + return api_client.ApiError(status=status, body=body, detail="x") + + +# --- note names under an agent token ----------------------------------------------------- + + +def _agent_login(tmp_home) -> None: + credentials.save(server_url="http://example.com", token=SecretStr("pep_agt_abc")) + + +def _notes(*names: str) -> api_client.ArtifactListResponse: + items = [ + api_client.ArtifactListItem( + id=_NID, name=n, kind="inline_text", updated_at="2026-10-26T10:00:00+00:00", etag="e" + ) + for n in names + ] + return api_client.ArtifactListResponse(truncated=False, items=items) + + +def test_agent_resolves_a_note_name_within_the_named_room(tmp_home, patch_api, capsys) -> None: + _agent_login(tmp_home) + patch_api(notes=_notes("Runbook"), comments=_comments(_thread(_c("m1", "hi")))) + assert cli.main(["comments", "list", "--room", _GID, "--note", "Runbook"]) == 0 + assert _calls()[0][:2] == ("artifacts", _GID) + assert _calls()[-1] == ("list", _NID) + assert "hi" in capsys.readouterr().out + + +def test_agent_note_name_without_room_says_to_add_one(tmp_home, patch_api, capsys) -> None: + _agent_login(tmp_home) + patch_api() + assert cli.main(["comments", "list", "--note", "Runbook"]) == 2 + assert "--room" in capsys.readouterr().err + assert _calls() == [] + + +def test_agent_note_id_without_room_needs_no_lookup(tmp_home, patch_api) -> None: + _agent_login(tmp_home) + patch_api(comments=_comments()) + assert cli.main(["comments", "list", "--note", _NID]) == 0 + assert _calls() == [("list", _NID)] + + +# --- list --------------------------------------------------------------------------------- + + +def test_list_renders_quote_replies_and_deleted(tmp_home, patch_api, capsys) -> None: + _login(tmp_home) + quoted = _thread( + _c("m1", "Is this right?", anchor={"exact": "sunscreen"}), + _c("m2", "Yes", author="Bo"), + _c("m3", "", author="Cy", deleted=True), + ) + whole = _thread(_c("m4", "General remark"), detached=True) + patch_api(comments=_comments(quoted, whole)) + assert cli.main(["comments", "list", "--note", _NID, "--room", _GID]) == 0 + out = capsys.readouterr().out + assert "“sunscreen”" in out + assert "Is this right?" in out and "Yes" in out + assert "(deleted)" in out + assert "whole note" in out and "detached" in out + assert _calls() == [("list", _NID)] + + +def test_list_empty(tmp_home, patch_api, capsys) -> None: + _login(tmp_home) + patch_api(comments=_comments()) + assert cli.main(["comments", "list", "--note", _NID]) == 0 + assert "(no comments)" in capsys.readouterr().out + + +def _mixed(): + done = _thread( + _c("m1", "Done thing", resolved_at="2026-10-26T11:00:00+00:00", resolved_by="Bo") + ) + return _comments(done, _thread(_c("m2", "Open thing")), resolved_count=1) + + +def test_list_hides_resolved_threads_and_points_at_all(tmp_home, patch_api, capsys) -> None: + _login(tmp_home) + patch_api(comments=_mixed()) + assert cli.main(["comments", "list", "--note", _NID]) == 0 + out = capsys.readouterr().out + assert "Open thing" in out and "Done thing" not in out + assert "1 resolved hidden" in out and "--all" in out + + +def test_list_all_shows_resolved_with_who_resolved(tmp_home, patch_api, capsys) -> None: + _login(tmp_home) + patch_api(comments=_mixed()) + assert cli.main(["comments", "list", "--note", _NID, "--all"]) == 0 + out = capsys.readouterr().out + assert "Open thing" in out and "Done thing" in out + assert "resolved by Bo" in out and "hidden" not in out + + +def test_list_with_only_resolved_threads_says_no_open_comments(tmp_home, patch_api, capsys) -> None: + _login(tmp_home) + done = _thread(_c("m1", resolved_at="2026-10-26T11:00:00+00:00")) + patch_api(comments=_comments(done, resolved_count=1)) + assert cli.main(["comments", "list", "--note", _NID]) == 0 + assert "(no open comments)" in capsys.readouterr().out + + +def test_list_json_always_has_every_thread_whatever_all_says(tmp_home, patch_api, capsys) -> None: + _login(tmp_home) + patch_api(comments=_mixed()) + assert cli.main(["comments", "list", "--note", _NID, "--json"]) == 0 + doc = json.loads(capsys.readouterr().out) + assert [t["message"]["id"] for t in doc["items"]] == ["m1", "m2"] + assert doc["items"][0]["message"]["resolved_by"] == "Bo" + assert doc["items"][0]["message"]["deleted"] is False + assert doc["resolved_count"] == 1 + assert (doc["artifact_id"], doc["group_id"], doc["topic_id"], doc["etag"]) == ( + _NID, + _GID, + _TID, + "e1", + ) + + +def test_list_requires_note_and_names_the_flag(tmp_home, patch_api, capsys) -> None: + _login(tmp_home) + with pytest.raises(SystemExit) as exc: + cli.main(["comments", "list"]) + assert exc.value.code == 2 + assert "the note is a flag here" in capsys.readouterr().err + + +def test_list_404_prints_the_uniform_message(tmp_home, patch_api, capsys) -> None: + _login(tmp_home) + patch_api(comments=_err(404)) + assert cli.main(["comments", "list", "--note", _NID]) == 2 + assert "no such room or note" in capsys.readouterr().err + + +def test_list_503_exits_1(tmp_home, patch_api, capsys) -> None: + _login(tmp_home) + patch_api(comments=_err(503)) + assert cli.main(["comments", "list", "--note", _NID]) == 1 + assert "try again" in capsys.readouterr().err + + +def test_list_409_personal_note_json_error(tmp_home, patch_api, capsys) -> None: + _login(tmp_home) + patch_api(comments=_err(409, {"detail": {"error": "personal_note"}})) + assert cli.main(["comments", "list", "--note", _NID, "--json"]) == 2 + captured = capsys.readouterr() + assert captured.out == "" + error = json.loads(captured.err)["error"] + assert error["code"] == "conflict" and error["status"] == 409 + + +# --- create ------------------------------------------------------------------------------- + + +_CREATED = {"thread_root_id": "m9", "topic_id": _TID, "message": {"id": "m9"}} + + +def test_create_sends_quote_mentions_and_a_fresh_idempotency_key( + tmp_home, patch_api, capsys +) -> None: + _login(tmp_home) + patch_api(created=_CREATED) + rc = cli.main( + [ + "comments", "create", "--note", _NID, "--quote", "sunscreen", + "--mention", "p1", "--mention", "p2", "Pack", "more", "of", "it", + ] + ) # fmt: skip + assert rc == 0 + cap = capsys.readouterr() + assert "m9" in cap.out and "Commented." in cap.err + cap.out + (_, note, kwargs) = _calls()[0] + assert note == _NID + assert kwargs["body"] == "Pack more of it" + assert kwargs["quote"] == "sunscreen" and kwargs["reply_to"] is None + assert kwargs["mentions"] == ["p1", "p2"] + assert kwargs["idempotency_key"] + + +def test_create_reply_with_explicit_key(tmp_home, patch_api) -> None: + _login(tmp_home) + patch_api(created=_CREATED) + rc = cli.main( + ["comments", "create", "--note", _NID, "--reply-to", "m1", "--idempotency-key", "k1", "ok"] + ) + assert rc == 0 + kwargs = _calls()[0][2] + assert kwargs["reply_to"] == "m1" and kwargs["idempotency_key"] == "k1" + assert kwargs["quote"] is None and kwargs["mentions"] is None + + +def test_create_reads_the_body_from_stdin(tmp_home, patch_api, monkeypatch) -> None: + _login(tmp_home) + patch_api(created=_CREATED) + monkeypatch.setattr(sys, "stdin", io.StringIO(" line one\nline two\n")) + assert cli.main(["comments", "create", "--note", _NID, "-"]) == 0 + assert _calls()[0][2]["body"] == "line one\nline two" + + +def test_create_json_prints_the_server_answer(tmp_home, patch_api, capsys) -> None: + _login(tmp_home) + patch_api(created=_CREATED) + assert cli.main(["comments", "create", "--note", _NID, "hi", "--json"]) == 0 + assert json.loads(capsys.readouterr().out) == _CREATED + + +@pytest.mark.parametrize( + "argv", + [ + ["comments", "create", "--note", _NID], + ["comments", "create", "--note", _NID, " "], + ["comments", "create", "--note", _NID, "--quote", " ", "x"], + ["comments", "create", "--note", _NID, "--reply-to", "", "x"], + ["comments", "create", "--note", _NID, "--mention", "", "x"], + ["comments", "create", "--note", _NID, "--idempotency-key", "", "x"], + ["comments", "create", "--note", _NID, "--quote", "q", "--reply-to", "m1", "x"], + ], +) +def test_create_bad_input_exits_2_without_a_call(tmp_home, patch_api, argv) -> None: + _login(tmp_home) + patch_api(created=_CREATED) + assert cli.main(argv) == 2 + assert not _FakeApi.instances or _calls() == [] + + +def test_create_empty_stdin_exits_2(tmp_home, patch_api, monkeypatch) -> None: + _login(tmp_home) + patch_api(created=_CREATED) + monkeypatch.setattr(sys, "stdin", io.StringIO("\n")) + assert cli.main(["comments", "create", "--note", _NID, "-"]) == 2 + + +@pytest.mark.parametrize( + ("status", "payload", "expect"), + [ + (422, {"detail": {"error": "quote_not_found"}}, "does not appear in the note"), + (422, {"detail": {"error": "quote_ambiguous", "count": 3}}, "appears 3 times"), + (422, {"error": "quote_ambiguous"}, "more than once"), + (422, {"detail": {"error": "quote_mismatch"}}, "does not match"), + (409, {"detail": {"error": "personal_note"}}, "personal notes"), + ], +) +def test_create_maps_server_refusals_to_messages( + tmp_home, patch_api, capsys, status, payload, expect +) -> None: + _login(tmp_home) + patch_api(created=_err(status, payload)) + assert cli.main(["comments", "create", "--note", _NID, "--quote", "q", "x"]) == 2 + assert expect in capsys.readouterr().err + + +def test_create_quote_invalid_and_unparseable_bodies_fall_back_to_generic( + tmp_home, patch_api, capsys +) -> None: + _login(tmp_home) + patch_api(created=_err(422, {"detail": {"error": "quote_invalid"}})) + assert cli.main(["comments", "create", "--note", _NID, "--quote", "q", "x"]) != 0 + assert capsys.readouterr().err.strip() + patch_api(created=api_client.ApiError(status=422, body="not json", detail="x")) + assert cli.main(["comments", "create", "--note", _NID, "--quote", "q", "x"]) != 0 + + +# --- resolve / reopen --------------------------------------------------------------------- + + +@pytest.mark.parametrize(("verb", "word"), [("resolve", "Resolved."), ("reopen", "Reopened.")]) +def test_resolve_and_reopen(tmp_home, patch_api, capsys, verb, word) -> None: + _login(tmp_home) + patch_api(thread=_thread(_c("m1"))) + assert cli.main(["comments", verb, "--note", _NID, "m1"]) == 0 + cap = capsys.readouterr() + assert word in cap.out + cap.err and "m1" in cap.out + assert _calls() == [(verb, _NID, "m1")] + + +def test_resolve_json_prints_the_thread(tmp_home, patch_api, capsys) -> None: + _login(tmp_home) + patch_api(thread=_thread(_c("m1", resolved_at="2026-10-26T11:00:00+00:00", resolved_by="Bo"))) + assert cli.main(["comments", "resolve", "--note", _NID, "m1", "--json"]) == 0 + doc = json.loads(capsys.readouterr().out) + assert doc["message"]["resolved_by"] == "Bo" + + +def test_resolve_unknown_thread_is_the_uniform_404(tmp_home, patch_api, capsys) -> None: + _login(tmp_home) + patch_api(thread=_err(404)) + assert cli.main(["comments", "resolve", "--note", _NID, "nope"]) == 2 + assert "no such room or note" in capsys.readouterr().err + + +def test_resolve_blank_thread_id_exits_2(tmp_home, patch_api, capsys) -> None: + _login(tmp_home) + patch_api(thread=_thread(_c("m1"))) + assert cli.main(["comments", "resolve", "--note", _NID, " "]) == 2 + assert "THREAD_ID cannot be empty" in capsys.readouterr().err + + +# --- messages list: note header, quote, resolved -------------------------------------------- + + +def _group_message(**over) -> api_client.GroupMessage: + base = { + "id": "m1", + "is_pepper": False, + "is_you": False, + "is_deleted": False, + "is_broken": False, + "run_start": True, + "display_name": "Dana", + "initial": "D", + "avatar_tint": "t1", + "raw_body": "see this", + "rendered_html": "

see this

", + "created_at": "2026-09-16T17:30:00+00:00", + "created_at_display": "5:30 PM", + "day_key": "2026-09-16", + "day_kicker": "WEDNESDAY", + "day_heading": "September 16, 2026", + "day_label": "Wednesday, September 16", + "quote_excerpt": "", + } + base.update(over) + return api_client.GroupMessage(**base) + + +class _FakeRoomApi(_FakeApi): + def list_group_topics(self, group_id): + return self._result("topics") + + def list_group_messages(self, group_id, topic_id, **kwargs): + return self._result("page") + + +def _topics(subject: str | None): + return [ + api_client.GroupTopicSummary( + topic_id=_TID, + name="Note: Packing", + last_activity_at="2026-09-16T17:30:00+00:00", + dedicated_conversation=False, + archived_at=None, + imported_history=False, + subject_artifact_id=subject, + ) + ] + + +def _patch_room(monkeypatch, subject, *messages) -> None: + page = api_client.GroupMessagePage(messages=list(messages), next_before=None) + monkeypatch.setattr( + api_client, + "ApiClient", + lambda **kw: _FakeRoomApi(topics=_topics(subject), page=page, **kw), + ) + + +def test_messages_list_in_a_note_topic_prints_header_quote_and_resolved( + tmp_home, monkeypatch, capsys +) -> None: + _login(tmp_home) + _patch_room( + monkeypatch, + _NID, + _group_message( + anchor={"exact": "sunscreen SPF\n50"}, + resolved_at="2026-10-26T11:00:00+00:00", + resolved_by_name="Bo", + ), + _group_message(id="m2", anchor={"exact": "x" * 300}, resolved_at="t"), + ) + assert cli.main(["messages", "list", "--room", _GID, "--topic", _TID]) == 0 + out = capsys.readouterr().out + assert f"pepper comments list --note {_NID}" in out.splitlines()[0] + assert '> "sunscreen SPF 50"' in out + assert "(resolved by Bo)" in out and "(resolved)" in out + long_line = next(ln for ln in out.splitlines() if ln.startswith('> "x')) + assert long_line.endswith('…"') and len(long_line) < 175 + + +def test_messages_list_note_header_keeps_the_profile(tmp_home, monkeypatch, capsys) -> None: + credentials.set_profile("helper") + _login(tmp_home) + _patch_room(monkeypatch, _NID, _group_message()) + argv = ["--profile", "helper", "messages", "list", "--room", _GID, "--topic", _TID] + assert cli.main(argv) == 0 + first = capsys.readouterr().out.splitlines()[0] + assert f"pepper --profile helper comments list --note {_NID}" in first + + +def test_messages_list_in_an_ordinary_topic_has_no_note_lines( + tmp_home, monkeypatch, capsys +) -> None: + _login(tmp_home) + _patch_room(monkeypatch, None, _group_message()) + assert cli.main(["messages", "list", "--room", _GID, "--topic", _TID]) == 0 + out = capsys.readouterr().out + assert "note comments" not in out and '> "' not in out and "resolved" not in out + + +def test_messages_list_json_skips_the_topic_lookup(tmp_home, monkeypatch, capsys) -> None: + _login(tmp_home) + _patch_room(monkeypatch, _NID, _group_message(anchor={"exact": "a"}, resolved_at="t")) + assert cli.main(["messages", "list", "--room", _GID, "--topic", _TID, "--json"]) == 0 + doc = json.loads(capsys.readouterr().out) + row = doc["items"][0] + assert row["anchor"] == {"exact": "a"} and row["resolved_at"] == "t" + + +@pytest.mark.parametrize( + "failure", + [_err(500), httpx.ReadTimeout("timed out")], + ids=["http-500", "transport-timeout"], +) +def test_messages_list_still_prints_when_the_topic_lookup_fails( + tmp_home, monkeypatch, capsys, failure +) -> None: + _login(tmp_home) + page = api_client.GroupMessagePage(messages=[_group_message()], next_before=None) + monkeypatch.setattr( + api_client, + "ApiClient", + lambda **kw: _FakeRoomApi(topics=failure, page=page, **kw), + ) + assert cli.main(["messages", "list", "--room", _GID, "--topic", _TID]) == 0 + assert "see this" in capsys.readouterr().out diff --git a/tests/test_cli_conformance.py b/tests/test_cli_conformance.py index bf13218..e130840 100644 --- a/tests/test_cli_conformance.py +++ b/tests/test_cli_conformance.py @@ -52,6 +52,8 @@ "scheduled-tasks", "skills", "notes", + # `comments` is the thread of remarks on a room note, always inside one (--note). + "comments", # `research-projects` and `research-items` are the product's words "Research projects" # and their ledger (planning/research-projects.md §2). "research-projects", @@ -113,6 +115,8 @@ "done", "dismiss", "reopen", + # `resolve`/`reopen THREAD` close and reopen a comment thread (IA §3.1). + "resolve", "download", "run", # `runs OBJECT` is the run history of an object that runs (IA §3.1). @@ -395,7 +399,7 @@ def test_room_is_a_flag_where_it_means_something() -> None: #: Nouns whose leaves each take a required container flag (IA §3 rule 2). -CONTAINER = {"research-items": "--project"} +CONTAINER = {"research-items": "--project", "comments": "--note"} def test_room_comes_before_the_container_flag() -> None: @@ -635,6 +639,20 @@ def test_pending_lists_name_real_leaves() -> None: } #: path -> body for every GET the leaves below make (query strings are ignored). +_COMMENT_THREAD = { + "message": { + "id": "m1", + "body": "hi", + "author": {"id": "p1", "display_name": "Ada", "is_agent": False}, + "created_at": "2026-10-26T10:00:00Z", + "resolved_at": "2026-10-26T11:00:00Z", + "resolved_by": {"id": "p1", "display_name": "Ada", "is_agent": False}, + }, + "position": None, + "detached": False, + "replies": [], +} + ROUTES: dict[str, object] = { "/api/v1/whoami": {"user_id": "u", "display_name": "U", "timezone": "UTC"}, f"/api/v1/trajectories/{_TRAJ}": { @@ -844,6 +862,14 @@ def test_pending_lists_name_real_leaves() -> None: "workout": {"at": "07:00", "minutes": 45}, }, f"/api/v1/artifacts/{_NOTE}/detail": {**_NOTE_ROW, "shared": False, "body": "hello note"}, + f"/api/v1/artifacts/{_NOTE}/comments": { + "artifact_id": _NOTE, + "group_id": _G, + "topic_id": None, + "etag": "e1", + "threads": [], + "resolved_count": 0, + }, } #: Non-GET routes (POST, DELETE): the room post answers ``{"message": {...}}``. @@ -890,6 +916,13 @@ def test_pending_lists_name_real_leaves() -> None: f"/api/v1/artifacts/{_NOTE}/edit": { "artifact": {**_NOTE_ROW, "shared": False, "body": "changed"} }, + f"/api/v1/artifacts/{_NOTE}/comments/m1/resolve": {"thread": _COMMENT_THREAD}, + f"/api/v1/artifacts/{_NOTE}/comments/m1/reopen": {"thread": _COMMENT_THREAD}, + f"/api/v1/artifacts/{_NOTE}/comments": { + "thread_root_id": "m1", + "topic_id": "t1", + "message": {"id": "m1", "body": "hi"}, + }, "/api/v1/apps/notes/actions/delete": {"result": {"name": "Runbook"}}, "/api/v1/apps/artifacts/actions/delete": {"result": {"name": "Receipt"}}, "/api/v1/artifacts/upload": { @@ -1090,6 +1123,10 @@ def fake_watch(self, *, since=0, last_event_id=None, **_kw): "notes create": ["notes", "create", "Runbook", "--file", "body.md"], "notes edit": ["notes", "edit", _NOTE, "--file", "body.md"], "notes delete": ["notes", "delete", _NOTE, "--yes"], + "comments list": ["comments", "list", "--note", _NOTE], + "comments resolve": ["comments", "resolve", "--note", _NOTE, "m1"], + "comments reopen": ["comments", "reopen", "--note", _NOTE, "m1"], + "comments create": ["comments", "create", "--note", _NOTE, "--quote", "hello", "Nice"], "artifacts list": ["artifacts", "list", "--kind", "file"], "artifacts show": ["artifacts", "show", _BLOB], "artifacts download": ["artifacts", "download", _BLOB, "--out", "got.png", "--force"], diff --git a/tests/test_cli_groups.py b/tests/test_cli_groups.py index 50d7365..5d72e35 100644 --- a/tests/test_cli_groups.py +++ b/tests/test_cli_groups.py @@ -137,6 +137,9 @@ def get_group(self, group_id): def list_group_topics(self, group_id): self.calls.append(("list_group_topics", group_id)) + # ``messages list`` looks the topic up for a note header; most tests need no topics. + if "topics" not in self._canned: + return [] return self._result("topics") def list_group_messages(