clawmates/docs/CAPABILITY-REVIEW.md

# ClawMates capability review

*2026-08-19. What exists, what is wired, what was repaired in this pass, and
what is deferred with reasons.*

## Inventory

| | |
|---|---|
| Crates | 19 libraries + 4 binaries (`clawmates-server`, `-broker`, `-node`, `fcagent`) |
| HTTP | 181 routes / 207 method+handler pairs across 39 route modules |
| Workers | 21 background loops, 2s (approval resume) to 24h (tool versions) |
| Database | 79 migrations, ~90 tables |
| Templates | 6 workflow recipes, 11 team templates, **53 authored skills** (was 30) |
| Frontend | 6 tiers (VIZ · MISSIONS · AGENT · PODCAST · REPOS · INFRA), ~57 dashboard components |
| Execution | 5 paths: container/ZeroClaw, microVM solo, composed microVM graph, local herdr, direct headless session |

Dead-code hygiene is genuinely excellent: no `todo!`, no `unimplemented!`, no
stray `TODO` in production paths. **Every defect found in this review was a
wiring defect** — code that was correct, reachable by nothing, and reported as
working.

## Where skills come from

Asked directly, and worth recording because it was not answerable from any
document before now.

- **All 53 catalogue skills are hand-authored markdown** committed to
  `skills/**/*.md`, ingested at boot by `skills_loader` with
  `source_kind='builtin'`, `workspace_id = NULL`. None are generated at runtime
  and none are pulled from anywhere.
- **An LLM-authoring path exists and is fully wired**: `level_up.rs:435` calls a
  model (default `glm:glm-4.7`) whose prompt asks for `skill_candidate` items
  carrying a complete `draft.body`. Nothing is written to `skills` at propose
  time — the payload sits in `level_up_proposals` as `pending`.
- **Application WAS gated on a human click**, and as of 2026-08-19 is not.
  `apply()` still honours `approved_item_ids` for the manual path, but
  `skill_self_authoring` now sweeps pending proposals every two minutes and
  applies their `skill_candidate` items autonomously, by operator decision.
  What replaces the gate is not another gate but four properties, each held by
  a test in `tests/skill_self_authoring.rs`:
  - the write is **workspace-scoped** (`workspace_id` set, never NULL), so it
    can never modify a hand-authored skill;
  - a draft **cannot take a hand-authored skill's name** — ids are scoped and
    bindings resolve by `skill_id`, so it could not overwrite or shadow one
    anyway, but two procedures under one name means nobody reading a transcript
    can tell which the agent followed;
  - every revision **appends a `skill_versions` row**, so a self-authored skill
    can be reverted and a past run can be read back against the text it was
    actually judged under;
  - the proposal lands with **`approved_by = NULL`** — an agent's decision is
    never attributed to a person who did not make it.

  `identity_refinement` and `brain_consolidation` still wait for a human: they
  change what an agent *is* rather than adding a procedure it can consult.
  Disable with `CLAWMATES_SKILL_SELF_AUTHORING=0`; the state is announced at
  boot either way.
- **No external registry feeds the catalogue.** ClawBrainHub trades `.brain`
  files and never touches the `skills` table. `cm_db::repo::skills::create` —
  the only path that would produce `source_kind='hand_authored'` — has no
  production caller.

So the only way a skill enters the catalogue today is a committed markdown file
or an operator approving an agent's draft.

## The theme

Everything below is one shape, the project's own "silent-success" class: a
capability is built, a second layer does not connect to it, and every layer
reports success. It is not carelessness. In four of the six cases the *belief*
that the wire existed was written down in a doc comment, and no test asked.

That is the actionable lesson: **a comment asserting a connection is not
evidence of one.** Two of the audit's own findings that fed this plan were
themselves wrong in the same way, and the registry whose job is to record which
config keys are read was inaccurate in both directions.

## What was repaired

### 1. Skill bindings — 55 of 85 resolved to nothing

Only 30 skills were authored; 10 of 11 templates referenced names that did not
exist. Three roles bound **zero** skills while their prompts described
procedures to follow.

Invisible because both existing tests assert `authored ⊆ referenced` (30/30,
green) and one explicitly declines to check the other direction.

Fixed: 23 skills authored, renames onto real skills, 22 aspirational references
deleted. Two tests now hold it — one against the files, one against the database
(different questions: resolution goes through catalogue rows, so a file that
exists but fails to ingest still leaves the role empty).

### 2. Skills could not reach a mission agent at all

The larger finding, and the reason #1 was never noticed. The catalogue's only
delivery channel was the `clawmates_skills` MCP server, and a mission claw could
not reach it for **three independent reasons**:

- `provision_claw` wrote the constant `["clawmates_door"]`, ignoring the
  template's `mcp_bundles` — which `mission_orchestrator` had already stored.
- The runtime config defines no `clawmates_skills` bundle. (The live local
  config defines **no bundles at all**, not even the door.)
- Mission claws run on `claude_cli`, which the runtime's own config comments
  document as text-only: it cannot surface a tool call, so no MCP server is
  reachable from a mission turn regardless.

And a mission turn's entire system context is two sentences synthesised from the
role slot (`topology_exec::build_prompt`) — the template's role prose is not
used either, which `mission_orchestrator` documents.

So every skill authored for a mission role was unreachable prose, and the
Skill-Use measurement this review planned could only ever have returned zero.

Fixed **on the container/ZeroClaw tier only**: `provision_claw` now provisions
the template's bundles (door always added), and the pinned skills are injected
as **bodies** into the turn prompt — not an index, because there is no
`skills.read` tool on this path and an index would advertise a capability that
does not exist.

The correction matters, because the first version of this document said
"mission agents" without qualifying the tier. `compose_turn_prompt` /
`pinned_skills_text` have exactly one production caller
(`topology_exec.rs:745`), which `topology_worker` drives. `phase_runner`'s three
other paths do not call it:

| Path | Entry point | Skills reach the agent? |
|---|---|---|
| Container / ZeroClaw (the default) | `topology_worker` → `ZeroClawDriveExecutor` | **yes** |
| Composed microVM | `phase_runner::launch_composed_microvm_phase` | not yet |
| Solo microVM | `microvm_executor::run_phase_in_vm` | not yet |
| Direct session | `phase_runner::launch_direct_session` | not yet |

### 3. `upsert_task` raised 42P10 on every call

`mission_tasks_external_uniq` is a **partial** unique index. Postgres will not
match a partial index to an `ON CONFLICT` target unless the statement repeats
the predicate. Both callers — the task-card parser that turns INT markers into
tasks, and the security scanner — map the error to a string their caller logs.

Two features were broken for as long as they have existed, and nothing was red.

### 4. The security scan phase never scanned

`security_scan::run` was reachable only from an operator button. So
`security_hardening.toml` — a workflow whose entire first phase is a scan — ran
an agent that was never told to scan, and never fired the scanner either. Now
swept by `phase_runner`, mirroring the benchmark baseline sweep added earlier
for the identical defect, guarded on a completion marker (a clean scan writes no
findings, so a findings-guard would rescan forever).

### 5. Two recipes could not fail

`security_hardening.toml` and `benchmark.toml` carried no `task` and no
`done_when` on any phase. Without `done_when` a phase never enters `evaluating`,
is never judged, and reports `completed` whatever it did. Both now state the
work and the condition.

### 6. Smaller wiring

- `CLAWMATES_JUDGE_MODEL` had two different defaults and a doc comment naming a
  third; one source now.
- `GITEA_TOKEN` absence is stated rather than degrading into the same message a
  private repo produces.
- `phase_config`'s registry corrected: `harness` and `tools` were listed NOT
  IMPLEMENTED while fully read; `bench_name`/`cmd` added; `test_command`
  deleted (no reader **and** no writer).

## Verification

- Full workspace suite green.
- Every fix has a test, and **every test was negative-controlled**: the skills
  test failed naming all 55; reverting the `ON CONFLICT` predicate reproduces
  42P10 exactly; reverting one skill name fails naming that exact role.
- Local stack boots; test-server path (`scripts/test-server.sh up`) is required
  for the integration suite, otherwise `cm-testkit` falls back to a
  testcontainer that times out.

## Deferred, with reasons

- ~~**`mission_plan` / `mission_roster` have no frontend.**~~ **Shipped
  2026-08-19.** `MissionProposalDrawer` reaches both flows from a mission's
  SETUP tab. Verified end to end against the live backend: a model proposed a
  roster, approval flipped the mission to the `composed` engine, and approval on
  a non-draft mission is refused.

  Building it surfaced a defect in the backend it consumes: every refusal path
  computed a precise reason, logged it to stderr, and returned a bare
  `{"error":"bad request"}`. The person who needed the sentence was the one
  clicking Approve. `ApiError::Refused(String)` now carries it — the same
  argument `ApiError::Unavailable` was added for, one status code down.
- **Skill-Use measurement (Trigger / Compliance / Boundary).** Deferred not for
  cost but because it was **unmeasurable until this pass**: with no delivery
  channel, trigger rate was structurally zero. It is now worth running, and it
  is the natural next step.
- **Provenance layer.** Assessed only, by decision — see
  [PROVENANCE-ASSESSMENT.md](PROVENANCE-ASSESSMENT.md).
- **Graph memory / `clawhdf5-agent`.** Declared in the workspace manifest, used
  by no crate. The matched study (`MemoryLake on MemoryArena`) shows structured
  memory winning modestly on low absolute numbers, and `Harness the Memory`
  finds excessive retrieval actively harms agent decisions. Measure against a
  baseline before migrating.
- **Test coverage, re-examined.** The original entry ranked crates by raw test
  count. That was a shallow metric and it was misleading: `cm-safety`'s seven
  tests already cover the CAS on decide, grant double-consume, expiry, and the
  approved/rejected split, and the `audit_log` immutability trigger is tested in
  `cm-db`. Counting tests found the wrong crates.

  Reading the API surface against the tests found the right ones:
  - **`verify_slack_signature` had no replay protection** — fixed, see below.
  - **`credits_for_tokens` was untested** — pure pricing arithmetic, now pinned
    including the round-up contract and an overflow case.

  Genuinely still thin: `cm-brain`, where 6 of 9 tests need live
  `clawbrainhub.com` so the whole hub client is unexercised offline. Stubbing it
  means reproducing an external registry protocol we have no spec for, which is
  its own piece of work rather than a coverage chore.

- **A Slack request could be replayed forever.** `slack_signature_valid`
  verified the HMAC correctly and nothing anywhere checked the timestamp's age —
  it was only an input to the basestring, so an old request's signature verified
  exactly as well as a fresh one. Anyone holding one captured signed request
  could replay it indefinitely. Slack's documented 5-minute window is now
  enforced **in the broker**, not the caller, because the broker does not trust
  its caller and a check the caller can forget will eventually be forgotten.
  Symmetric, so a far-future timestamp cannot mint a long-lived request.

- ~~**`ZEROCLAW_GATEWAY_URL` / `_TOKEN` fail at *first use*, not boot.**~~
  **Fixed 2026-08-19.** `gateway_preflight` reports at startup, the third
  sibling of `runtime_preflight` and `validator_preflight`. A report, not a
  gate — every read path works without a gateway, and refusing to boot would
  turn a degraded deployment into a dead one. The message names the
  consequence ("container-tier missions cannot run") rather than just the
  missing variable.
- ~~**`gitea_forge` resolves to nothing.**~~ **Resolved 2026-08-19 by removing
  the name.** It was harmless while `provision_claw` ignored the bundle list;
  once the list was honoured, an undefined name became a capability an agent is
  told it has and does not. Removed from seven team templates, one workflow
  recipe, the auto-provision path and a user-selectable dropdown. `web_fetch`
  went the same way — and a new test
  (`team_template_loader::bundle_tests`) now asserts every bundle a template
  names is defined in the runtime config, which is what found `web_fetch` in
  two templates I had missed by hand. Agents reach the forge through `git` over
  HTTPS with the ambient `GITEA_TOKEN`, which is why nothing ever broke.
- **The deployed runtime config is not the example.** `[mcp_bundles.clawmates_skills]`
  is now in `agent.config.example.toml`, but the live local config carries no
  bundle definitions at all — a fresh deploy needs the example's blocks. The
  prompt-injection path does not depend on this, which is why it was the fix
  chosen for the mission tier.

## The one process change worth making

Every defect above was found by **checking a claim instead of reading it**. The
existing `runtime_preflight` module is the pattern that works: it asks the
running container what it actually has and says so at boot, because "the code is
right and the machine is not" produced no error anywhere.

The equivalent check for this pass — does a provisioned agent actually receive
the bundles and skills its template names — does not exist yet, and is the
cheapest guard against all of this recurring.