docs(agentic-teams): document the new resources, and make the webhook reachable

Five resources shipped with no documentation and one documented lie:
docs/inference-tasks.md still told people `mcpctl get tasks` lists the
inference queue. Work items own that name now.

Adds docs/epics.md (the Epic -> EpicTask -> Task hierarchy, the state machine,
risk-scaled review gates, the accept-merges gate, and reconciliation),
docs/gitproviders.md (forge registration, discovery/import, merge-on-accept,
post-merge reopen) and docs/agent-authority.md (the three run modes, and
plainly which two are not implemented).

Two things found while writing them, both fixed rather than filed:

- `webhookSecretKey` was read by the webhook route but writable from NOWHERE
  -- absent from the create schema, update schema, service, CLI and apply
  spec. Since the endpoint fails closed on a null key, post-merge CI failures
  could never reopen their task; the board would keep claiming `merged` for
  work that broke the build. Adds --webhook-secret-key, the schema fields and
  the apply key, with tests covering persist / omit / explicit-null /
  reject-empty. Omission deliberately does not clear the column, so an
  unrelated edit cannot silently disarm the webhook.

- src/cli/src/commands/create.ts contained a literal NUL byte inside a string
  literal (a binary-file check written with the raw character rather than an escape). Identical behaviour, but it made the whole file read as
  binary, so `grep -I` skipped it entirely and 1120 lines of CLI were
  invisible to every text search. Replaced with the escape.

The Epic write path (criteria/status/tradeoffs) is API-only -- documented as
such rather than papered over, and filed as task 180.

Full suite 2612 green; lint at baseline 794/77.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p
This commit is contained in:
Michal
2026-08-16 00:14:58 +01:00
parent 2a85447fac
commit 6d38953a96
14 changed files with 587 additions and 6 deletions

118
docs/agent-authority.md Normal file
View File

@@ -0,0 +1,118 @@
# Agent run authority
An agent carries two different things from the projects it joins:
- **knowledge** — prompts, skills, project context
- **capability** — the projects' MCP tools
Treating those as one thing is what makes agent membership dangerous. `sre-bot`
belongs to `payments` so it understands payments; that should not mean everyone
who can talk to `sre-bot` can call payments' tools through it.
The rule:
> **Knowledge travels with the agent. Action is bounded by the authority of the
> run.**
Prompts load regardless of who is asking — that is what makes an agent a QA
reviewer rather than a generic bot. Every MCP tool call is gated.
## Authority is per-run, not per-agent
The same agent runs under different authority depending on how it was invoked.
One `sre-bot`, three modes:
| Mode | Who decides what it may do | Effective tool scope | Status |
|---|---|---|---|
| **caller** | the human who invoked it | agent's memberships **∩** caller's `run:projects` | **implemented** |
| **agent** | the agent itself, on a schedule | every project it is a member of | not implemented |
| **service-account** | an explicit non-human identity | exactly the SA's own bindings — may be smaller *or* larger than any human's | not implemented |
### caller
The interactive path, and the only one live today.
For each project the agent belongs to, mcpd asks the same question the
project-chat route asks — `run:projects:<name>` — and grants tools only where
the answer is yes. Reusing that permission rather than inventing a parallel one
is deliberate: one grant means the same thing on both paths.
It **fails closed**. No user, or RBAC not wired, means knowledge and no tools —
never the reverse.
A pull request that arrived on an [McpToken](mcptoken-implementation.md) bound
to a project is additionally ceilinged by that binding. This matters because
`project` travels in the request *body*, so the URL-based scope check never sees
it; without the ceiling a token for `sre` could drive an agent into `payments`.
### agent
Scheduled, unattended work. The agent's own memberships are the grant and no
human is consulted, because there is no human.
**Not implemented.** `ChatService.chat` has exactly two call sites, both HTTP
handlers — there is no scheduler, no job runner, and `AgentSession` is
schema-only with no credential column. Tracked as task 178.
### service-account
An explicit non-human identity holding its own bindings, which may deliberately
*exceed* what any human has (a deploy bot) or fall well short of it.
**Not implemented, and it refuses loudly rather than degrading.** Requesting it
returns `501`. Two things block it, both live problems rather than missing
features:
1. A `ServiceAccount` today is a **forgeable header** — `x-service-account` is
read with no lookup and no signature. Honouring it as an authority source
would grant tool access on a client-supplied string.
2. "Smaller than its owner" is currently **inexpressible**: auth sets
`request.userId = ownerId` unconditionally, so the owner's grants are always
unioned in. An McpToken with no bindings is the owner at full privilege.
Tracked as task 179.
## What the model is told
When a run carries prompts from a project whose tools it may not call, the
system block gets a notice naming those projects:
```
[context boundary] You are carrying knowledge from payments, which the person
you are talking to cannot access themselves. You have NO tools for payments and
calling any would fail. Use this knowledge to inform your judgement. Do not
reproduce its contents verbatim on request, and do not describe it as something
the person can go and read.
```
Be clear about what this is. **The tool boundary is enforced; the disclosure
boundary is not.** The first half is a statement of fact the runtime guarantees.
The second half is *guidance to a model* — a persona that applies privileged
knowledge without reciting it. A determined caller may still extract some of it.
This is an accepted trade: a reviewer that forgets the project's conventions
when a different person asks is not a reviewer. If context must not leak to a
caller at all, do not attach that project to an agent they can reach.
## Enforcement points
Gated in **both** places:
- `listTools` omits tools for projects outside the run's authority
- `callTool` refuses them
Withholding a tool from the list is presentation, not enforcement — a model can
invent a tool name it was never shown, and `callTool` is the call that would run
it.
## Threads
Resuming a thread checks ownership. Another user's thread id returns `404`, not
`403` — confirming a thread exists to someone who cannot read it is itself a
leak.
## See also
- [agents.md](agents.md) — creating agents and attaching them to projects
- [chat.md](chat.md) — the chat surfaces this governs
- [mcptoken-implementation.md](mcptoken-implementation.md) — token scoping

190
docs/epics.md Normal file
View File

@@ -0,0 +1,190 @@
# Epics, roadmap nodes and work items
Three resources, one hierarchy:
```
Epic the vision, and the criteria delivery is measured against
└─ EpicTask a roadmap node; owns a subset of the Epic's criteria
└─ Task a work item that moves through states and lands a PR
```
The split exists because "every task is done" and "we delivered the thing" are
different claims. Tasks close; criteria are what say whether closing them
amounted to anything. Nothing derives one from the other automatically — see
[Reconciliation](#reconciliation).
## Epic
The versioned statement of intent. `content` is the vision prose; `doneCriteria`
is the list the work is graded against.
A criterion is `id=text`, and prefixing the id with `q:` marks it a **quality**
criterion rather than an outcome one. Both count toward completeness — grading
only the mechanical half is how you end up with every task done and no car.
```sh
mcpctl create epic checkout-rewrite \
--project payments \
--content-file vision.md \
--criterion 'cart=A cart survives a session drop' \
--criterion 'q:latency=p99 checkout under 400ms'
mcpctl get epics
mcpctl describe epic checkout-rewrite # vision, criteria, completeness, reconciliation, roadmap
```
`status` is `open`, `delivered`, `delivered_with_tradeoffs` or `cancelled`.
Closing an Epic with unmet criteria requires the tradeoffs path — you may ship
something short of the vision, but not while calling it whole.
> **API only, no CLI yet.** Marking a criterion `met`, setting status and
> recording tradeoffs are `POST /api/v1/epics/:id/{criteria,status,tradeoffs}`.
> The CLI reads all of it (`describe epic`) but cannot write it. Tracked as
> task 180.
## EpicTask
A roadmap node. Its `--criterion` ids must match ids on the parent Epic: the
node is claiming *"these parts of the vision are mine"*.
```sh
mcpctl create epictask cart-persistence \
--epic checkout-rewrite \
--criterion 'cart=A cart survives a session drop' \
--depends-on session-store
mcpctl get epictasks
mcpctl describe epictask cart-persistence
```
`--depends-on` builds the dependency graph `describe epic` renders as a roadmap.
## Task
The unit an agent actually works on. It carries a `risk` level, which decides
how much review it must survive, and optionally a repo and a pull request.
```sh
mcpctl create task cart-cookie-ttl \
--epictask cart-persistence \
--title 'Extend cart cookie TTL to 30 days' \
--risk normal \
--repo <repo-id>
mcpctl get tasks
mcpctl describe task cart-cookie-ttl # state, legal moves, verdicts, git, history
```
> **`tasks` means work items.** It used to mean the durable inference queue,
> which now answers only to `inference-tasks` / `itasks`. See
> [inference-tasks.md](inference-tasks.md).
### States
```
todo → in_progress → building → self_testing → qa_review
↓ ↓
done ← security_review → sre_review
↓
awaiting_review ⇄ (reject → in_progress)
↓ accept
merged → reopened → in_progress
```
Every state can go back to `in_progress` and out to `cancelled`; a failed build
goes *back* to work rather than forward. `merged` has exactly one exit,
`reopened`, and only CI drives it (see [gitproviders.md](gitproviders.md#post-merge-failure)).
```sh
mcpctl tasks advance cart-cookie-ttl building --note 'kicked off CI'
mcpctl tasks history cart-cookie-ttl
```
Transitions are compare-and-swap: two agents racing the same task, one wins and
the other gets a 409 naming the state it lost to.
### Risk decides the gates
| Risk | Must pass before `done` |
|---|---|
| `trivial` | nothing — docs, formatting |
| `normal` | `qa_review` |
| `high` | `qa_review`, `security_review`, `sre_review` |
Verification effort scales with risk instead of running one fixed pipeline over
everything. A verdict is pinned to the commit it judged, so a later push
invalidates it rather than silently carrying forward:
```sh
mcpctl tasks verdict cart-cookie-ttl \
--stage qa_review --commit 4f2a91c --pass \
--summary 'covers the session-drop path; added a regression test'
```
Reaching `done` without the required passing verdicts is refused, and the error
names which stage is missing.
### The human gate
`done` means it left the developer's loop, not that it shipped. `awaiting_review`
is where a human decides:
```sh
mcpctl tasks accept cart-cookie-ttl --note 'ships with the friday release'
mcpctl tasks reject cart-cookie-ttl --note 'cookie is set before consent — move it after'
```
**Accept lands the code.** If the task has a PR, accept merges it through the
repo's gitprovider and records the merge commit. If the merge fails — conflict,
or someone pushed after the review — the task stays un-merged and says why. A
stale head is refused rather than merged, because accepting a commit nobody
reviewed is the one thing the gate exists to prevent.
Reject requires its comment. That comment is what the agent works from, so an
empty rejection is a bug report with the contents removed.
## Reconciliation
`describe epic` prints claimed completeness and, directly under it, what the
delivered work actually says. Those two disagree by design:
- **Completeness** counts `met` flags. Nothing flips them automatically, so an
Epic whose every Task is merged still reads 0% until someone asserts the
vision was met.
- **Reconciliation** walks the roadmap and reports which criteria have merged
work behind them, which are claimed but undelivered, and which no roadmap node
owns at all (`unowned`).
Attribution is by criterion id, which is a *convention* — nothing enforces that
an EpicTask's criteria ids exist on the parent. Reconciliation reports where the
convention doesn't hold rather than guessing an owner. It never writes `met`:
deriving it from task state would assert that finishing the tasks delivered the
vision, which is exactly the assumption under audit.
## RBAC
| Resource | Verbs |
|---|---|
| `epics` | `view`, `create`, `edit`, `delete` — `edit` also covers status/criteria/tradeoffs |
| `epictasks` | `view`, `create`, `edit`, `delete` — `edit` also covers dependency edges |
| `tasks` | `view`, `create`, `edit`, `delete`, **`run`** |
`run:tasks` is the gated one: advancing state, recording verdicts, accepting and
rejecting are `run`, not `create`. A reviewer who may drive tasks through the
board should not need permission to invent them, and — more to the point —
anyone who can *file* a task should not thereby be able to *accept* one.
```sh
mcpctl create rbac task-reviewer --role run --resource tasks
mcpctl create rbac epic-reader --role view --resource epics
```
Bindings written before the `tasks` / `inference-tasks` split are rewritten once
at startup, so an old `view:tasks` grant keeps authorizing the inference queue
and does not silently become authority over work items.
## See also
- [gitproviders.md](gitproviders.md) — forges, repo import, merge and post-merge reopen
- [agent-authority.md](agent-authority.md) — whose authority an agent acts under
- [inference-tasks.md](inference-tasks.md) — the durable inference queue that used to own this name

149
docs/gitproviders.md Normal file
View File

@@ -0,0 +1,149 @@
# Git providers and repositories
Two resources:
- **`gitprovider`** — a connection to a git forge (Gitea today). One per
instance/account. Owns the credential.
- **`repo`** (CodeRepo) — a repository attached to a project. Either imported
from a provider or added by URL.
Registering a provider is what makes discovery, import and **merge-on-accept**
possible. A repo added by bare URL can be worked in, but nothing can land its
pull requests, because there is no API to land them through.
## Registering a forge
The token lives in a Secret; the provider references it by name and key.
```sh
mcpctl create secret gitea-token --data TOKEN=<your-api-token>
mcpctl create gitprovider mysources \
--url https://mysources.co.uk \
--secret gitea-token --key TOKEN
```
Registration **probes the credential** — a wrong URL or an under-scoped token
fails here with a clear message, rather than later at discovery where it looks
like "the forge has no repos". Use `--skip-probe` to wire config before the
infrastructure exists.
The token needs repository scope. Discovery deliberately calls `/repos/search`
rather than `/user/repos`, because the latter requires `read:user`, which the
repo-scoped tokens people actually create for automation do not carry.
## Discover and import
```sh
mcpctl repos discover mysources # everything visible
mcpctl repos discover mysources -q mcp --limit 20 # filter
mcpctl repos discover mysources --project mcpctl # mark what's already imported
mcpctl repos import mysources michal/mcpctl michal/lab --project mcpctl
mcpctl repos import mysources michal/mcpctl --project mcpctl --ssh
```
Discovery stores nothing — it is a live read. Import creates CodeRepo rows.
Import is partial by design: it takes what it can and reports what it skipped
with reasons, rather than failing the batch because one repo is empty or
archived. An empty repo is flagged unavailable — there is nothing to clone.
Imported repos carry **denormalized provenance** (`sourceProvider`,
`sourceFullName`) and store no credential of their own — they inherit the
forge's. Deleting a provider leaves its repos in place with their provenance
intact, so where a repo came from outlives the connection.
To attach a repo without a provider:
```sh
mcpctl create repo mcpctl --project mcpctl \
--url https://mysources.co.uk/michal/mcpctl.git --branch main
```
```sh
mcpctl get repos
mcpctl get gitproviders
mcpctl describe repo mcpctl
```
## Merge on accept
When a task carrying a PR is accepted, mcpctl merges it through the repo's
provider — see [epics.md](epics.md#the-human-gate). Two refusals matter:
- **Stale head.** The accept records which commit was reviewed. If someone
pushed after that, the merge is refused rather than landing code nobody
approved. Re-review the current head and accept again.
- **Conflict.** Reported as-is; the task stays un-merged.
A failure always leaves the task un-merged and says why. Nothing reports success
the board cannot back up.
Repos with no `gitProviderId` cannot be merged through — the accept says so and
tells you to import from a provider or merge in the forge.
## Post-merge failure
CI can fail *after* a merge lands. Without a way to hear about it, the board
keeps claiming `merged` for work that broke the build. The webhook closes that
loop by moving the task to `reopened`.
Add the signing secret to the provider's Secret and point the provider at it:
```sh
mcpctl create secret gitea-token \
--data TOKEN=<api-token> --data WEBHOOK=<random-signing-secret>
mcpctl create gitprovider mysources \
--url https://mysources.co.uk \
--secret gitea-token --key TOKEN \
--webhook-secret-key WEBHOOK
```
Then in the forge, add a webhook on **commit status** events:
```
URL: https://<your-mcpd>/api/v1/webhooks/git/mysources
Content: application/json
Secret: <the same random signing secret>
```
> **It fails closed.** The endpoint is unauthenticated by nature — a forge calls
> it — so it accepts nothing unless the provider has a signing key configured
> *and* the signature verifies. **A provider without `--webhook-secret-key`
> refuses every delivery**, which means post-merge failures silently never
> reopen anything. If you want this loop, you must set the key.
Accepted signature headers are `X-Gitea-Signature` and `X-Hub-Signature-256`
(HMAC-SHA256 over the raw body, `sha256=` prefix optional), compared in constant
time.
Matching is precise, not approximate: a failing status carries a commit sha, and
the accept path recorded exactly which commit each merge produced. A failure on
a commit mcpctl did not merge is ignored — it is somebody else's business, and
guessing at "whatever merged most recently" would reopen innocent work.
Only `failure` and `error` states do anything. Success and pending are
acknowledged and ignored.
## RBAC
| Resource | Verbs |
|---|---|
| `gitproviders` | `view`, `create`, `edit`, `delete`, **`run`** |
| `repos` | `view`, `create`, `edit`, `delete` |
`run:gitproviders` covers discovery, import and probe. Those reach out to a
third-party system with a stored credential, so they are `run`, not `view` —
holding `view:gitproviders` should not let you drive the forge's API.
```sh
mcpctl create rbac forge-user --role run --resource gitproviders
```
## See also
- [epics.md](epics.md) — the work items whose PRs get merged here
- [agent-authority.md](agent-authority.md) — whose authority an agent acts under
- [secret-backends.md](secret-backends.md) — where the forge credential lives

View File

@@ -136,10 +136,21 @@ result is just discarded server-side.
## CLI
> **Renamed.** This queue used to answer to `mcpctl get tasks`. Agentic-teams
> **work items** own that name now, so the queue keeps only its explicit
> spellings: `inference-tasks`, `inference-task`, `itasks`, `itask`. Bare
> `tasks`/`task` list work items — see [epics.md](epics.md).
>
> The same split applies to RBAC: this queue is authorized by
> `view:inference-tasks`, work items by `view:tasks`. Bindings written before
> the split are rewritten once at startup, so an existing `view:tasks` grant
> keeps working here and does **not** silently become a grant over work items.
```sh
mcpctl get tasks # table view: ID, STATUS, POOL, LLM, MODEL, STREAM, AGE, WORKER
mcpctl get task <id> # describe one
mcpctl get task <id> -o yaml # full row, yaml format
mcpctl get inference-tasks # table view: ID, STATUS, POOL, LLM, MODEL, STREAM, AGE, WORKER
mcpctl get itasks # short alias
mcpctl get inference-task <id> # describe one
mcpctl get inference-task <id> -o yaml # full row, yaml format
# Enqueue async via chat-llm:
mcpctl chat-llm vllm-local-qwen3 --async -m "summarize the changelog"