git:20260821.3dfe12f to git:20260908.bbb0fcb
8 added, 6 removed. Audit A to A.
# grove.core.tickets — the branch-aware ticket-provider layer
> ↑ [grove.core](../CLAUDE.md) · [root](../../../../CLAUDE.md)
One `TicketProvider` interface, implemented once per tracker (Gitea, GitHub, Linear) and reused by every client (TUI / API / MCP) through the `TicketProviderRegistry`. Wire shapes (`TicketRef` / `TicketSelector` / `TicketProviderView`) live in [contracts/tickets.py](../contracts/CLAUDE.md) — they cross the client boundary; this package is the engine-side mechanism over them.
## The provider boundary (an adapter normalizes shape, not semantics)
- **A provider has two faces, split by the side-effects-at-the-edge rule.** The *pure* face (`parse_branch_refs` / `format_branch_name`) translates branch ⇄ canonical key with no I/O — this is the **engine's only entry point**, so `create()`/`attach_ticket` stay deterministic and offline-safe. The *I/O* face (`list_assigned` / `get_ticket` / comment I/O) hits the tracker's REST/GraphQL API and is called **only at the daemon `/tickets` edge**, never from the engine. Keep that split: a new provider method that does network must not become reachable from a lifecycle path.
- **`HttpTicketProvider` carries the shared wire plumbing (the `core/mewbo.py` pattern): lazy httpx client + one typed-error `_request`.** The client is built on first I/O call, so a provider used only for pure branch parsing (the engine's path) never opens a socket. Every failure narrows to `TicketProviderError`; httpx exceptions never leak. Auth is the env-ref pattern — config holds the env-var NAME (`token_env`), never a value.
- **`NumberTicketProvider` factors the numeric branch grammar shared by Gitea + GitHub; Linear's keyed grammar is its own class.** This is shared *kind of grammar*, not cross-provider leakage: each concrete class still owns its API endpoints, JSON shape, and built-in keyword set (`gh` / `gitea`+`gtea`). The numeric grammar recognizes three forms, all word-boundary-anchored so a digit inside a slug (`fix-v2-bug`) is never an id: leading numeric segment (`123-…` / `repo/123-…`), built-in keyword (`gh-123`), and the configured `branch_prefix`.
- **Config-bound providers, no `RepoContext` argument (YAGNI).** Each provider is constructed bound to its config submodel (owner/repo/team_key/base_url), exactly like `MewboClient` is bound to `MewboConfig`, so the methods take only the data they need.
## The registry is the one shared surface
- **`TicketProviderRegistry` owns the only non-provider-specific logic: branch-ref aggregation + ambiguity.** `parse_workspace_refs(branch)` runs every enabled provider's pure parse and, when the total match count across providers exceeds one, marks **every** resulting ref `ambiguous=True`. A bare number (`123-fix`) deliberately matches BOTH numeric providers when both are enabled → ambiguous; a keyed form (`ENG-123`, `gh-42`, `gtea-5`) is unambiguous. The manager and the daemon both reach providers via `WorkspaceManager.ticket_providers` (cached, built from `cfg.tickets`) — there is no second parse path.
- **Branch name is the single source of truth for association.** `create()` re-parses the *final* branch (ticket-aware auto, user-named, or an adopted checkout) — so create-from-ticket and adopt-existing-branch share one code path. `attach_ticket`/`detach_ticket` are the manual override; they store a **bare** ref (provider+id), since display enrichment (title/status) is the daemon's on-demand fetch, never persisted — attach stays pure. Both gated by `ensure_can_update` (refused only on ORPHANED).
- **`WorkspaceManager.find_by_ticket(provider, ticket_id)` is the "does a workspace already exist for this ticket" lookup — a scan over `list()`, no new index.** It reuses the ref list `attach_ticket`/branch-parse already populate; no new persisted state. Tie-break when more than one live workspace tracks the same ticket: newest by `created_at` wins. "Non-killed" needs no explicit filter — `kill()` deletes the persisted record outright (there is no KILLED status), so a killed workspace can never appear in `list()`'s output. `GET /workspaces?ticket=<provider>:<id>` (daemon) is the one wire consumer, dispatched like `/branches`'s `repo` query param.
## Comment I/O is a base-default capability, not a per-provider stub
- **`list_comments`/`post_comment`/`edit_comment`/`react` live on `HttpTicketProvider` as concrete (non-abstract) methods whose base body raises `TicketCommentsUnsupported`.** A bare `NotImplementedError` would leak an untyped exception past the provider boundary, so the base raises the typed sibling of `TicketProviderError`/`TicketProviderNotConfigured` instead (same section in `errors.py`, same "callers handle one exception family" contract). Only Gitea and GitHub override them; **Linear inherits the raise untouched** — no per-method stub. Adding a fourth non-implementing provider costs zero lines.
- **Ruff's ARG002 only exempts a *literal* `raise NotImplementedError(...)` body, not a custom exception.** Raising a typed subclass (even via a one-line helper) trips "unused method argument" on every base-default parameter, so each base-default method carries `# noqa: ARG002` — deliberate, matching the `close()` `# noqa: B027` precedent in `notifications/channel.py` for "this override intentionally does nothing with its args."
- **Gitea and GitHub's comment JSON shape is byte-for-byte identical** (`id`/`body`/`user.login`/`created_at`) — tempting to share `_to_comment` on `NumberTicketProvider`. Deliberately duplicated per-provider instead, matching this package's `_to_ref`/`_assignee` precedent: each concrete class owns its own JSON-shape normalization even when two trackers happen to overlap, so `NumberTicketProvider` stays scoped to the *branch grammar* it was introduced for.
- **`_scoped(op)` replaces the repeated `if not (self._owner and self._repo): raise ...` guard**, one private method per provider class — not shared across Gitea/GitHub, because the message text names the concrete provider.
- **`TicketReactionKind` is a closed `Literal` in `contracts/tickets.py`, not a bare `str`** — Gitea and GitHub accept the identical eight-value reaction vocabulary verbatim, so one shared literal catches a typo'd reaction name at the type-checker instead of a silent 4xx at the daemon edge.
## Building a browser URL is a base default too, and `None` is an ANSWER
- **`web_root` sits BESIDE `host` rather than replacing it, and the difference is the PORT.** `host` answers "which tracker does a pasted URL belong to", where a port is noise a human's pasted link may or may not carry; a *built* URL needs the port back, or a self-hosted forge on `:3000` is simply unreachable. `web_root` composes scheme + `host` + port and reuses the identical `api.`-stripping rule, so the matcher and the builder can never disagree about which host serves the browser.
- **`commit_url`/`branch_url` are base-default `None` methods on `HttpTicketProvider`, and `None` is a real answer** — "this tracker fronts no repo" (Linear), never a failure to report. The consumer's rule is what makes that shape correct: never manufacture a link with no destination, because a dead link spends the reader's click and is worse than plain text. Same base-default discipline as comment I/O, minus the raise: an absent capability that a caller can *render around* answers `None`, where one it cannot answer around raises the typed error.
- **The commit path is identical on both numeric forges, so it lives once on `NumberTicketProvider`; only the branch VIEW segment differs** (`src/branch` vs `tree`), carried as a one-line `ClassVar` — empty meaning "no branch view", which falls back to the `None` contract. **This is the opposite call from `_to_comment` above, and the discriminator is what the shared code IS:** that one is JSON-shape normalization, which each adapter owns for its own tracker even when two shapes coincide today; this is one URL *grammar* with a single differing segment, so sharing it means the two forges cannot drift apart on the parts that are genuinely the same.
- **A branch name reaching a URL keeps `/` and quotes everything else** — a slash is a real path separator in the forge's own route, every other character is a value that must not become syntax. Same class as the branch-name flag guard and the tmux-target validation: the fix belongs where the value enters the syntax.
- ## Credentials resolve at USE, from the section's env source
+ ## Credentials have a repository-owned snapshot
- - **A frozen credential and a long-lived cache are each survivable; together they are unfixable from outside.** Reading `os.environ[token_env]` in `__init__`, baking it into an `Authorization` header and freezing `configured = bool(token)` captures the value once, in a process whose environment nothing can mutate afterwards — and `WorkspaceManager.ticket_providers` caches the registry while a daemon caches one manager per repo for the whole process. A credential produced by a workspace init script, fetched from a secret manager, or rotated in place would never be seen, and the provider would stay `configured: false` forever with no way to notice. `_token()` is the ONE read site (`configured` and `_request` both go through it) and `auth_headers(token)` composes the header per request.
- - **When a seam is already a mapping, a smarter mapping is a smaller diff than a new parameter.** Providers take `env: Mapping[str, str]` (defaulted to `os.environ` by the registry, injected as a dict by tests). `TicketEnv` is a LAZY `Mapping` that consults `tickets.env_file`/`env_command` on every `__getitem__` and falls back to the base mapping — late binding with zero call-site churn, and `os.environ` is held live rather than copied so even the bare `token_env` path is late-bound.
- - **Order is source-then-process, and the source wins for the same reason the feature exists.** An operator naming a file is saying "read it from here"; the value baked into the daemon's environment at exec is precisely the one that cannot be updated, so letting it shadow the file would reinstate the frozen-credential failure. Fallback, not replacement: a file holding only the Gitea token leaves a `GROVE_GITHUB_TOKEN` exported in the shell perfectly usable.
- - **One source per SECTION, not per provider, because the lookup is keyed by env-var NAME.** `tickets.env_file`/`env_command` serve Gitea, GitHub and Linear from one resolution.
+ - **A frozen credential and a long-lived cache are each survivable; together they are unfixable from outside.** Reading `os.environ[token_env]` in `__init__`, baking it into an `Authorization` header and freezing `configured = bool(token)` captures the value once, in a process whose environment nothing can mutate afterwards — and `WorkspaceManager.ticket_providers` caches the registry while a daemon caches one manager per repo for the whole process. `_token()` is the ONE read site (`configured` and `_request` both go through it) and `auth_headers(token)` composes the header per request.
+ - **When a seam is already a mapping, a smarter mapping is a smaller diff than a new parameter.** Providers take `env: Mapping[str, str]` (defaulted to `os.environ` by the registry, injected as a dict by tests). Production's `TicketEnv` owns one immutable, generation-numbered source snapshot for its repository; ordinary mapping, `configured`, and capability reads reuse it with zero source subprocesses. The live base mapping remains a fallback for keys absent from the source.
+ - **Order is source-then-process, and the source wins for the same reason the feature exists.** An operator naming a source is saying "read it from here"; the value baked into the daemon's environment at exec is precisely the one that cannot be updated, so letting it shadow the source would reinstate the frozen-credential failure. Fallback, not replacement: a file holding only the Gitea token leaves a `GROVE_GITHUB_TOKEN` exported in the shell perfectly usable.
+ - **One source per SECTION, not per provider, because the lookup is keyed by env-var NAME.** `tickets.env_file`/`env_command` serve Gitea, GitHub and Linear from one `TicketEnv` instance, so concurrent providers share its single-flight first resolution. The registry remains repository-scoped; snapshots never cross repository roots.
+ - **Refresh is an edge, never a timer.** `TicketEnv.invalidate()` drops the prior source result; `refresh()` resolves and atomically publishes the next generation. A refresh failure replaces a formerly valid result with a typed denial — fail closed, never continue authorizing with a stale token. Concurrent initial and refresh consumers wait for one resolver and receive its same success or refusal; an unexpected resolver failure is a generic denial so untrusted exception text cannot disclose a secret. The shared admission foundation is not involved: resolution is synchronous ownership, not cross-thread delivery.
+ - **Shutdown is a credential edge too.** `TicketEnv.close()` clears its snapshot, wakes waiters, and permanently refuses later reads. A registry integration must call it alongside providers' HTTP close, so a cached manager cannot retain credentials after shutdown.
+ - **File sources supply a temporary consumption-time edge; commands require a caller edge.** `TicketEnv` compares a file signature before serving its snapshot, so create/replace/delete causes one refresh even before the shared filesystem owner supplies explicit invalidation. An `env_command` has no observable change feed and remains unchanged until `invalidate()` or `refresh()` is called. The integration hook still needed is for the future configuration/filesystem owner to invoke `TicketEnv.invalidate()` for command rotations (and to replace the file-signature bridge where it has stronger change evidence); do not add polling or another event bus.
- **`configured` swallows a resolution failure; `_request` does not — that asymmetry is the contract.** `configured` feeds render paths (the picker's gray-out, the daemon's "skip unconfigured providers" aggregation over `/tickets/assigned`), where one broken tracker must not fail the whole request, so a missing `env_file` logs and reads false. An actual call raises the typed reason instead of sending an unauthenticated request. Same value, two questions: *can I offer this* vs *what happened*.
- **`EnvSourceError` is narrowed to `TicketProviderError` at this package's boundary** (`credentials.py`), exactly as the container arm narrows it to `ContainerError` in `manager._container_env`. The shared resolver raises its own type precisely so neither consumer has to pretend the other's family covers it.
- - **No cache, deliberately — the argument is lifetime, not correctness.** The registry lives for the daemon's whole life, so any memo would hold secrets in memory for days and serve a rotated token. Cost is one resolution per request (the token is resolved once in `_request` and used for both the gate and the header), and the contract on `env_command` is "idempotent and cheap".
## A pull request is another ref in the SAME list (the link contract)
- **A PR needs a `kind` discriminator, not a parallel `pr_refs` list or a second publisher — because the forges already agree with that shape.** Both Gitea and GitHub number issues and PRs in ONE space and thread PR comments through `/issues/{id}/comments` (verified live: comment I/O against a PR number works unchanged on both), so a PR reuses the provider registry, the comment I/O, the sticky publisher and `find_by_ticket` with zero new machinery. `TicketRef.kind` / `TicketSelector.kind` are `Literal["issue","pull_request"]` defaulting to `"issue"` — additive-with-default, so every record already on disk and every client that never sends the field keeps decoding, and no store migration exists to get wrong.
- **Only the `pulls` namespace carries the merge state, and MERGED is not CLOSED.** The issues endpoint reports a merged PR as `state: "closed"` — true and useless to an orchestrator deciding whether work landed. `get_pull_request` therefore goes to `GET /repos/{o}/{r}/pulls/{n}` on both forges and normalizes `merged`/`merged_at` to `status="merged"` ahead of the bare state. `_status` also checks the NESTED `pull_request` member, because the same normalizer is reached from the issues endpoint, where the merge fact lives one level down.
- **`TicketRef.draft` is provider metadata, defaulting false for legacy stored refs.** Both Gitea and GitHub map the `draft` member from their pull payloads in their own `_to_ref`; Gitea had supplied it all along, so the correction was to inspect the existing response rather than add a special request. Like `status`, it is display enrichment; its Pydantic default makes the stored shape additive.
- **PR-ness is read where each forge states it, and `list_assigned` stays issues-only on purpose.** Gitea flags `is_pull` on search results and a non-null `pull_request` member on the single-issue read; GitHub adds a `pull_request` key — so `get_ticket` on a PR number reports `kind="pull_request"`, while `get_pull_request` passes `kind` EXPLICITLY rather than sniffing (its endpoint already answered). The assigned listing is deliberately not widened to PRs: a PR arrives by explicit attach or branch parse, so "what should I work on" keeps its meaning.
- **Parsing a human-typed link is SEGMENTATION; validation belongs on the Pydantic fields.** `TicketLink.parse` only decides which of the four shapes the text is (URL / `#42` / `42` / `owner/repo#42`) and pulls pieces out; the id grammar, the `owner/repo` shape, the host charset and the lowercasing are field rules stated once, so four regex branches cannot disagree about what a valid id is. A piece failing them is narrowed to `TicketLinkError` — a `ValidationError` must never escape this layer. The URL arm SCANS the path for an `issues`/`pull`/`pulls` segment followed by a number rather than anchoring one regex at the root, which is what makes a subpath-hosted Gitea and a deep link (`.../pull/301/files`) both parse.
- **The link knows its hints; only the REGISTRY knows the enabled set, so resolution lives there** (`resolve_link`, beside `parse_workspace_refs`). Two reuses keep it policy-free: a bare id's candidacy is answered by the provider's own **`parse_branch_refs`**, so there is no second copy of the numeric-vs-keyed rule (`42` is claimed by both numeric trackers, `ENG-7` only by Linear), and a URL's host is matched against `provider.host`, derived from the configured `base_url` with a leading `api.` stripped — the one place the API host and the browser host differ (`api.github.com` serves `github.com`). No provider NAME is hard-coded in the comparison.
- **Ambiguity is a typed refusal here, where a branch parse only FLAGS it.** `parse_workspace_refs` marks a two-provider branch match `ambiguous=True` because an inferred association may honestly be uncertain; an explicit attach must name one ticket, so `resolve_link` raises `TicketLinkAmbiguous` listing the candidates instead of taking the first. Attach stays idempotent on `(provider, id)` — the key the forge itself links on — and re-attaching a ref whose `kind` was wrong (a branch parse assumed `issue`) CORRECTS it in place rather than keeping the stale kind or growing a duplicate row.
- **`can_comment` (capability AND credential) is the seam a "first mirrorable ref" picker must gate on.** Checking *enabled* alone lets an enabled-but-tokenless Gitea — or Linear, which cannot comment at all — permanently shadow every later ref on the workspace, so the sticky comment silently never appears. The predicate is answered once on `HttpTicketProvider` (`comments_supported` ClassVar × live `configured`), never re-derived per caller; a test pins `comments_supported` against which classes actually override `post_comment` so the declaration cannot drift from the implementation.
## Assignee writes: the token IS the bot, and the forges disagree about shape
- **The configured token's own account is the only identity there is, and that is a FEATURE with a stated risk.** `assign_self`/`unassign_self`/`viewer_login` never take a login: "assign Grove" means "assign whoever this credential is", so no deployment names a bot account in config and no config can drift from the credential in use. `list_assigned` is the same identity pointed the other way, which is what makes it the whole inbound read. The residual is that a deployment configuring a *human's* token turns the pickup poll into "start a workspace for everything this person is assigned". Not refusable in code — there is nothing else to check against — so the poller resolves `viewer_login()` once per provider and **logs which account it is draining**. Say the assumption out loud where it cannot be enforced.
- **`assigned=true` means "I am AN assignee", not "the sole assignee" — verified live** against this Gitea on an issue carrying a human assignee alongside Grove's, which came back in `/api/v1/repos/issues/search?type=issues&assigned=true`. That is what lets the pickup rule be "fire whenever the bot is among the assignees" with **no widening, no new search parameter and no per-issue GET**. GitHub's `filter=assigned` is the documented analogue and is **inferred**, not exercised against a live GitHub.
- **The additive-assign shape is where the two forges genuinely differ, and the difference is a whole extra round-trip.** GitHub ships `POST`/`DELETE /repos/{o}/{r}/issues/{n}/assignees`, which adds and removes without touching the others. Gitea has only `PATCH /repos/{o}/{r}/issues/{n}` with an `assignees` array that **REPLACES**, so its arm is a read-modify-write and a blind `PATCH {"assignees": ["grove-ai"]}` **evicts every human already on the ticket** — a bug that passes any test asserting only that the bot ended up assigned. `with_login` returns `None` for "unchanged" so a repeated assign sends no PATCH at all: idempotent at the wire, not merely in effect.
- **`assign_self` returns whether THIS call assigned the ticket, and that is a different question from what the ticket now says.** Because the write is idempotent, an account a human already assigned leaves the finished state indistinguishable from one Grove created — so a caller deciding whether it may later UNASSIGN cannot read the state, it has to be told. Gitea gets the answer free from the unchanged-list branch it already computes; **GitHub's arm pays one GET it would not otherwise make**, deliberately, because the alternative is Grove undoing an assignment somebody else made. The inverse deliberately stays `None`-returning: nothing needs to know whether an unassign was a no-op. Consumed by the issue-ops release rule in [issueops](../issueops/CLAUDE.md).
- **`assignees_supported` is a THIRD capability flag, not a fold into `comments_supported`, even though the same two forges back both.** Assigning needs repo **write**, a strictly stronger grant than commenting, so a deployment can honestly have one and not the other; `can_assign` = capability × credential, and a caller treats `False` as "log it and carry on" — the assignment is never worth failing the work it was going to describe. A test pins the ClassVar against which classes actually override `assign_self`, the same drift guard `comments_supported` has.
- **`read_body`/`update_body` are the description seam, and `body_supported`/`can_edit_body` are a THIRD capability rather than a fold into the comment one.** Rewriting a description edits what a *human* wrote, a strictly larger claim than adding a comment beside it, so a deployment can honestly hold one grant and not the other — same argument that split `assignees_supported` off, same declaration-not-derivation rule, same drift-guard test, and its own `TicketBodyUnsupported` so a caller can tell the two refusals apart. `update_body` is a blind WHOLE-body write on both forges (`PATCH …/issues/{n}` with `{"body": …}`), so **the caller owns the merge** — which is what forces the issue-ops footer to be marker-delimited rather than appended. `read_body` is separate from `read_thread` on cost: the thread read also pulls every comment, the expensive half on a long ticket and pure waste when only the description is wanted.
- **`read_thread` returns a plain dataclass, and the reason it exists at all is that `TicketRef` deliberately has no `body`.** An agent handed a ticket needs the description and the discussion; a ref carries *display* enrichment and is persisted on `WorkspaceState` and streamed on every dashboard tick, so widening it would put unbounded text on the wire (and force a `types.gen.ts` regeneration) to serve one create path. `TicketThread` crosses no client boundary → dataclass, filed under the COMMENT capability because comments are exactly what it is short of.
- **`viewer_path` lives on `NumberTicketProvider` while `_to_comment` stays per-provider — the same discriminator as `branch_view_segment`.** One grammar with one differing path segment and an identical `login` key is shared; JSON-shape normalization is owned per adapter even when two shapes coincide.
## Gotchas
- **Linear GraphQL must POST to the *absolute* configured endpoint.** httpx turns an empty relative path into a trailing slash (`/graphql/`), which Linear rejects; `_graphql` passes `self._base_url` as the path so the URL is exact. GraphQL also answers 200 on error — the error envelope rides `body.errors`, checked in `_graphql`, never leaked.
- **`TicketRef` is Pydantic but persisted on the `WorkspaceState` dataclass.** `asdict` won't serialize a nested Pydantic model, so the store handles `ticket_refs` explicitly (`model_dump(mode="json")` out, `model_validate` back) — the same explicit-field treatment enums/datetimes get. `WorkspaceState` imports `TicketRef` under `TYPE_CHECKING` only (default `field(default_factory=list)` needs no runtime symbol) so the engine state stays on the depended-upon side of the import arrow — see [contracts](../contracts/CLAUDE.md).