CLAUDE.md@src/grove/core/issueops git:20260712.bbc007cA

Immutable. This exact content is served forever at /api/v1/blob/7a473cc3150d31e7.

# grove.core.issueops — turn issue-comment events into workspace actions

> ↑ [grove.core](../CLAUDE.md) · [root](../../../../CLAUDE.md)

One nameable concern with two faces (epic #192, Wave 2): a forwarded, normalized issue-comment event in, a workspace action out (#196), and the workspace's progress mirrored back onto the ticket as a live sticky comment (#197). The engine is pure policy over the *existing* lifecycle seams — it adds **zero new lifecycle logic**: steering is `manager.send_message` verbatim, creation is the ticket-aware `manager.create` path, verbs are the matching manager ops. The wire shapes live in [contracts/issueops.py](../contracts/CLAUDE.md); the `IssueOpsConfig` submodel cascades with the rest of config in [config.py](../CLAUDE.md#config-cascade). The design rationale (industry-converged inbound defaults, the stateless-forwarder verdict, sticky-comment discipline, Gitea capability floors) is the "Issue-ops research verdicts" section in [tickets/CLAUDE.md](../tickets/CLAUDE.md) — read it first; it is binding.

## The atoms (no free helpers)

- **`CommandParser` (parser.py) is pure and takes the trigger as an argument, never a config object** — so it stays a table-testable unit and the engine owns config resolution. Grammar: the trigger must be the FIRST token, word-boundary matched (`(?!\w)` after it, matched case-insensitively — `@grovebot` and a mid-comment mention never fire); a lone verb from the fixed set (`status|pause|resume|stop`) is a `verb`; that verb with trailing junk is `usage`; a non-verb first word is `prompt` free-text (never a mistyped verb to second-guess); a bare trigger is `usage`; not-triggered is `None`. Leading `:`/`,` after the trigger is stripped so `@grove: fix` / `@grove, status` read cleanly.
- **`IssueOpsEngine` (engine.py) is the routing + gate policy; all I/O is at the edges.** `handle(event)` runs a terminal-drop pipeline in one fixed order: **dedupe → bot → marker → resolve-repo → parse → permission → route**. The early-return count IS the pipeline (PLR0911 silenced, like `manager.create`'s branch-count lints) — collapsing the gates behind a flag reads worse.
- **`TicketStatusPublisher` (publisher.py) is the outbound face** — see its own section below.
- **`marker.py` holds TWO constants shared by both faces, split so recovery and the anti-loop guard can't collide.** `SIGNATURE_MARKER` rides *every* Grove comment — the engine's replies AND the publisher's sticky comment — and the engine drops any inbound comment carrying it (the bot never answers itself). `STICKY_MARKER` (`<!-- grove:issue-ops:status -->`, more specific) rides *only* the sticky status comment, and the publisher's cold-start recovery scans for **that** one — never the general signature, because every engine reply also carries the signature, so a signature scan would adopt an old refusal/usage reply and edit the status render over it (the #197-review collision this split fixes). The sticky footer stamps both; `_reply` stamps only the signature. Both live in this tiny module precisely because both faces import them and *neither may depend on the other* — the engine must not import render logic, the publisher must not import the router. HTML comments (invisible in rendered markdown), substring-matched.

## Load-bearing decisions (engine, #196)

- **`IssueOpsConfig` lives in `config.py`, NOT `issueops/config.py`.** Every other config submodel (Tickets, Notifications, Telemetry, …) lives in `config.py`, and putting it under `issueops/` would force `config.py` to `import grove.core.issueops.config`, which runs `issueops/__init__.py` → imports the engine → imports `manager` + `config` mid-import → **cycle**. So the submodel sits with its siblings and the package holds engine/parser/marker/publisher only. The brief's "config module" is satisfied by the submodel + its default prompt template constant living in `config.py`.
- **Repo resolution is always repo+ticket, never a fleet-wide ticket scan (carried from #195's review).** `_resolve_manager` scans `registry.known_roots()` and matches the event's `owner`/`repo` against the target repo's `tickets.<provider>` config, then `find_by_ticket` runs on THAT manager only — a bare ticket id is ambiguous across repos. The match is **generic over the provider**: `event.provider` is exactly a `TicketsConfig` field name, so one `getattr` reaches the right submodel with no per-provider branching (Linear has no owner/repo, so it resolves nothing — issue-ops targets the numeric forges). The resolved repo's own cascade supplies the trigger, permission policy, agent, and prompt template, so every knob cascades per-repo.
- **The engine is injected the `RepoRegistry` and reads `mgr.config.issueops` per event — no separate config injection.** One daemon-wide engine serves every repo; per-repo policy rides the registry's per-repo cascade. Tests duck-type a fake registry → fake manager (holding a REAL `GroveConfig`) + capturing fake provider; `make lint` only mypy-checks `src`, so a structural test fake needs no ABC.
- **Permission default is strict; the allowlist only ever WIDENS.** `_permitted` = forwarder-asserted write-or-above (`{write,admin,maintain,owner}`, case-insensitive) OR an `issueops.allowed_actors` entry. The allowlist never narrows what write access already grants — it's the "separately-named opt-in widening knob" the research verdict prescribes.
- **Routing "running" means a LIVE session (`status in LIVE_STATUSES`), and the paused case is refused, not auto-resumed.** Free-text prompt: hit + live → `send_message` (steered); hit + not-live → refuse `workspace_not_running` with a reply telling the user to `resume` first (resume is an explicit verb — no compound lifecycle op here); miss → `create`. This keeps routing a clean state machine and adds no lifecycle logic.
- **Every ack/reply write is best-effort and swallowed (carried from #193's review, and TESTED).** `_reply` catches `GroveError` (covers `TicketProviderNotConfigured`/`TicketProviderError`/`TicketCommentsUnsupported`) and logs; a failed reply never re-raises into routing. Same for the `status` seam (`_render_status` catches broadly — a publisher may raise anything). A bad command always gets a reply (never silence); the **reactions** (👀→🚀/👎) are the CI action's job (it holds the forge event token), not the engine's — the engine only names the outcome.
- **The outcome vocab is a closed literal the CI action maps to a reaction:** `created|steered|paused|resumed|stopped|status` are terminal verbs; `refused` (code: `usage`/`insufficient_permission`/`no_workspace`/`workspace_not_running`/`{verb}_failed`/`create_failed`/`steer_failed`) and `ignored` (code: `duplicate`/`bot`/`signature_marker`/`unknown_repo`/`not_triggered`) carry a `code`. `workspace_id` is set whenever a concrete workspace was touched.
- **Dedupe is a bounded, thread-safe LRU on `(provider, owner, repo, comment_id)`.** CI retries deliver at-least-once; `_RecentKeys.check_and_add` is atomic (a `threading.Lock`) because the daemon dispatches `handle` into the executor — concurrent threads must not both treat one comment as fresh. Mark-first (record at the top of the pipeline) so a retried comment can't double-act, even one dropped as bot/marker.
- **Daemon route: `POST /issue-ops/events` (bearer dep, 202).** `handle` does blocking git/tmux/network I/O, so it runs in the executor (the `/activity` discipline). 202 = accepted-and-acted (the create/steer/reply already happened synchronously in the engine); it returns an `IssueOpsOutcome`, not a raised error, for an ordinary refusal. Injectable via `build_app(issue_ops_engine=…)`, mirroring `notification_broker`.
- **`StatusPublisher` seam:** a `Protocol` with `publish(event, manager)` and a `_NullStatusPublisher` no-op default, invoked (best-effort) only on the `status` verb. The real publisher is injected at daemon wiring; it must not import the engine.

## The status publisher (`publisher.py`, #197)

`TicketStatusPublisher` is the activity bus's **third subscriber** (alongside `_SseHub` and `NotificationBroker`) — a live sticky comment that mirrors a workspace's blended state + todo checklist + branch/commit onto its ticket.

- **It subscribes to the activity bus but deliberately does NOT ride the notification broker — wrong granularity, right discipline.** The broker is an edge-triggered, *debounced* push for human attention (WAITING/BLOCKED/ERROR rising edges, one buzz per episode); the publisher needs the *opposite* — the continuous WORKING-state todo churn that never crosses an attention edge. So it `bind(ActivityService.subscribe)`s the same bus and reads the whole story, then copies the broker's *mechanics* wholesale: `bind(subscribe)`, a pure decision/I-O dispatch split, a single-worker `ThreadPoolExecutor`, and best-effort per-delivery isolation. Copy the pattern from [notifications/CLAUDE.md](../notifications/CLAUDE.md), never the broker's edge/debounce *policy*.
- **Coalescing, not debouncing — the load-bearing difference (the Sweep lesson).** The broker drops flapping edges; the publisher *folds* every render-relevant change into per-workspace state and flushes the merged result **at most once per window** (`update_window_seconds`, default 5s). Forges apply secondary rate limits to same-comment PATCH storms. `dirty_since` is stamped on the clean→dirty edge and **not** refreshed while dirty, so the window measures from the *first* unflushed change (staleness bounded to one window, not indefinitely deferred by a busy workspace).
- **`observe` (fold, poll thread) / `flush_pending` (clock-driven) / `dispatch` (I/O) is the whole design, and it's what makes it testable with no threads.** `observe` only mutates in-memory coalescing state and, for a terminal, hands one job to the worker — it never blocks the poll. `flush_pending(now)` is the clock-driven seam the coalescing test drives directly with a fake clock; in production a single self-arming `threading.Timer` calls it. **The timer only arms while bound** (`_scheduling`): unbound — every test — runs each step inline on the caller's thread and drives `flush_pending` by hand, so no real timer fires mid-test.
- **`render(snapshot) -> str` is a pure classmethod over `PublishSnapshot` — full rebuild every time, never a diff-patch of the prior body.** The forge holds the last render; we replace it wholesale (the sticky-comment rule). A `terminal` snapshot swaps the live status + checklist for a final summary (outcome + branch + link) and the workspace latches `done` — no further publishes.
- **"Terminal" is one axis of mechanism, no policy baked in.** The always-on terminal is the **lifecycle `killed` `workspace_changed` delta** (the workspace is gone — render the finale from the last cached row, since the store record is already deleted). A second, **caller-supplied** `terminal_states` frozenset (default *empty*) lets a deployment mark an agent state (e.g. ERROR) terminal without the code deciding "ERROR means done" — the "no policy in code" rule.
- **Render-relevance is a custom fingerprint, NOT the poll's.** Gating dirty on `_render_fingerprint` (state/current_task/tool_calls/replies/branch/latest-commit-sha, **excluding** diff/dirty counts) stops a pure `git add` from scheduling a redundant PATCH. `tool_calls`/replies stand in for todo/checklist progress, because the `TodoList` isn't on the row and reading it is I/O we keep off the poll thread.
- **The todo comes from `WorkspaceManager.latest_todo` (#194), resolved at dispatch time, not on the poll thread**, via an injected `TodoResolver` — read off-thread in `dispatch`, best-effort (`None` on a miss, e.g. a killed workspace whose dir is gone).
- **All forge writes go through the Wave-1 `TicketProvider` comment I/O; every call is swallowed at THIS call site.** `list_comments`/`post_comment`/`edit_comment` only — no direct `httpx`. The #193 swallow obligation lives with the caller (the publisher): a `TicketProviderError`/`TicketCommentsUnsupported` is logged and dropped, never re-raised into the activity poll path (tested). On a publish failure the sticky id is *forgotten* so the next flush re-scans — which also recreates a comment a human deleted (marker scan finds nothing → post).
- **Sticky identity: in-memory id + `STICKY_MARKER` cold-start recovery (the sanctioned v1 store).** The comment id is a non-persisted per-workspace map. It doesn't need to persist because the *forge thread itself* holds the recovery key: on a cold start (no id) the publisher scans `list_comments` for the **sticky** marker (never the general signature — that also tags every engine reply, and adopting one would clobber it) and adopts that comment before posting a fresh one, so a daemon restart re-adopts its own status comment instead of duplicating it. Persisting the id would duplicate a source of truth the thread already owns.
- **Routing: first ticket ref that resolves to an issue-ops-enabled provider — one sticky comment per workspace.** `_route` walks `state.ticket_refs` and takes the first whose `ProviderResolver` returns non-`None` (an unconfigured/uncapable provider resolves to `None` → skipped silently, as does a workspace with no refs). Multi-ticket mirroring is out of scope (YAGNI).

### Final wiring (config placement · daemon lifespan · the status verb)

- **One canonical `IssueOpsConfig` in `grove.core.config`.** The outbound publisher knobs (`enabled` / `update_window_seconds` / `deep_link_base_url`) fold in beside the inbound ones (`trigger` / `allowed_actors` / `agent` / `prompt_template`). `enabled` gates ONLY the outbound status mirror — the daemon builds the publisher only when it's set; inbound routing has no on/off flag (its opt-in is the CI workflow + an enabled ticket provider). It is deliberately **not** re-exported from this package — it lives with its config siblings, and `TicketStatusPublisher.from_config` / the daemon import it from `grove.core.config`.
- **Daemon lifespan mirrors the notification broker (`daemon/app.py`).** `TicketStatusPublisher.from_config(cfg.issueops, registry=registry)` (`None` when disabled) is bound to `activity_service.subscribe` right after `sse_hub.start` and `close()`d on shutdown, injectable via `build_app(status_publisher=…)`. The **same instance** is injected into the engine's `StatusPublisher` seam, so one object both mirrors continuous progress and answers `@grove status`.
- **`@grove status` re-renders immediately through the structural `StatusPublisher` seam.** `TicketStatusPublisher.publish(event, manager)` maps ticket → workspace via `find_by_ticket`, then `flush_now` dispatches that workspace's latest coalesced row at once, bypassing the window. The two classes stay decoupled by structural typing: the publisher takes only the wire `IssueOpsEvent` + the already-resolved `WorkspaceManager` (both under `TYPE_CHECKING`) and never imports the engine module.
- **Both faces stamp `SIGNATURE_MARKER`; only the sticky comment also stamps `STICKY_MARKER`.** The engine stamps the signature on every `_reply` body, and the publisher's sticky footer stamps *both* — so the ingest anti-loop guard (`SIGNATURE_MARKER in body`) drops both bot artifacts (the bot never re-triggers itself, independent of the bot-actor check), while cold-start recovery scans for `STICKY_MARKER` alone and so re-adopts the status comment without ever clobbering a reply.