feat(skills): gated skills — the approval comes before the agent sees the request (abilityai/trinity-enterprise#751)

[webmixgamer]

Summary

A skill marked "requires approval by role X" is no longer dispatched when a request invokes it as a /<name> token. The platform freezes the request, raises an approval for that role, and answers the requester at once. Approve runs it once, exactly as written. Reject, expiry or cancel runs nothing. A proven person who fills the approver role runs their own request directly, and that is audited.

Ships inert: skill_gate_service.list_skill_gates returns {} until the gate map lands (trinity-enterprise#753).

How it works

• Where it's checked: skill_gate_service.enforce runs at the /chat and /task admission seams, plus a backstop at the top of execute_task for every other producer (scheduler, loops, fan-out, channels, the Workspace, rooms, sessions, A2A, public links, paid calls, operator resumes, validation). The voice tool refuses instead. The gate reads everything a requester put in front of the executor: message, user_message, system_prompt, the quoted reply, the chosen answer, and the rendered loop iteration. It never reads the platform's own framing.
• The record: skill_gate_requests on both tracks: the SQLite runner, and Alembic 0088_skill_gate_requests, re-parented on 0087_pull_sync (one head). Both agent columns cascade on delete.
  • Each approval dispatches exactly once, via a compare-and-set claim and a UNIQUE run id.
  • Skill fingerprints are re-checked at approval; a changed skill is stale and runs nothing.
  • A sweep reconciles what a dead process leaves behind, and never re-runs anything.
• Who decides: may_end in the ask sink. Only an addressee can answer; an admin may cancel but not approve. Gate rows are hidden from machine-key reads (test: gate dev → open-PR merges on the unit-test suite #715), and agents learn the outcome, never who decided.
• Requesters:
  • A Workspace caller is a person, proven by its route.
  • Each schedule is its own requester, read from the pre-created row, and Run now is the person who pressed it.
  • An approved result returns to the thread or channel it came from.
  • A person is not told about a decision they made themselves.
• Surfaces:
  • HTTP: 202 pending_approval, or a named refusal with X-Trinity-Error-Code.
  • MCP: structured pending_approval / refused results.
  • The Workspace answers a held turn with the notice as a normal reply.
  • The Operations card and /m show a not_addressee refusal next to the controls.

Requirement: docs/memory/requirements/security.md §26.13 (OPS-001-GATE). Flow: docs/memory/feature-flows/skill-gate.md.

Review trail

• /review: four ways around the gate, each a field the requester controls that the gate did not read. All fixed.
• /cso --diff: one HIGH (a system_prompt invocation was not scanned) and one MEDIUM (person emails reached agents), both fixed. Report: docs/security-reports/cso-diff-2026-10-02-ent751-gated-skills.{md,json}.
• Localhost eyeball with two accounts: seven issues, all fixed with tests that failed first:
  • the Workspace was treated as an anonymous channel;
  • a waiting request showed as Failed;
  • approved results did not come back to the thread;
  • a role id leaked into the notice;
  • schedules shared one requester budget;
  • people were notified of their own decisions;
  • a refusal was hidden in a refresh banner.
• Independent review of the post-eyeball changes: two LOW findings fixed (schedule names could forge lines on the card; /m invited a retry that can never succeed). The rest are stated limits or follow-ups.

Tests

• New tests: tests/unit/test_ent751_*.py (10 files), plus additions to the ent#329, ent#611, ent#551 and refactor(db): reduce database.py facade signature width — param objects for execution/chat writers #1482 suites; src/frontend/tests/unit/queueCardNotAddressee.spec.js and mobileAdminNotAddressee.spec.js; src/mcp-server/src/chat-gate.test.ts.
• Backend unit suite on the merged tree: 20,800 passed. The remaining failures are local only:
  • one order-dependent test in an untouched module, which passes alone;
  • the runtime route census, which can't import main.py without the OpenTelemetry exporter package.
• Frontend unit: 4,546 passed. MCP: chat-gate and chat-depth, 26 passed.

Not in this PR

• trinity-enterprise#753 (gate storage, UI and API). UI handling of the 202 answer has to land with it.
• trinity-enterprise#754 (the Skills tab "Requires approval" row, and a marker when an approver's own request skips approval).
• trinity-enterprise#755 (the approval card, including hiding the decision buttons from non-addressees).
• trinity-enterprise#752 (the in-container hook for skills named only in prose).

Fixes abilityai/trinity-enterprise#751

🤖 Generated with Claude Code

[github-actions]

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

[github-actions]

⚠️ Live-instance suite skipped — merge conflict against dev.

Resolve by merging dev locally and pushing the result; the next nightly re-tests.

[vybe]

merge-train (2026-10-04): on today's train, merging as-is. Validated at 14a4a50a with /validate-pr, /review and /cso --diff: no critical or high finding. Both findings from the 10-02 /cso report hold as fixed.

What made it READY:

• Coverage is behavioural. Neutering either admission seam, the execute_task backstop, the claim's state == pending predicate, or unregistering the SQLite migration each turns tests red.
• The inert path is a no-op. With list_skill_gates returning {}, enforce() and the backstop touch no DB and cannot raise.
• Both migration tracks agree on all 22 columns and three indexes; one Alembic head (0088 on 0087_pull_sync).
• Auth is only narrowed. require_admin, assert_admin and ADMIN_GATE_SCOPES are untouched; agent keys cannot approve, cancel or self-approve.

To close before trinity-enterprise#753 supplies a gate map. None of these is reachable while the map is empty.

1. Schedule name is not scanned. The backstop reads only message (task_execution_service.py:1926), but the schedule name goes into the executor's system prompt (platform_prompt_service.py:740). A schedule named /<gated-skill> … with a neutral message dispatches every tick unapproved. source_mcp_key_name is rendered the same way (:726); we did not verify whether a key name is free text.
2. Telegram quoted reply is not scanned. message_router.py:870 passes only message.text, while the prompt carries the quote (:159). Replying "@bot do this" to an untagged /<gated-skill> message dispatches with the command in the quote. client_portal/service.py:3211 fixes exactly this for the Workspace. Group history (:587-589) is the same door and needs your call.
3. Three wiring lines are unpinned. Each can be deleted with the suite green:
   • dependencies.py:704 (is_event_loopback=bool(loopback)); without it the event loopback is a person and can self-approve.
   • main.py:595 (register_ending_observer()) and operator_queue_service.py:1231 (await skill_gate_service.sweep()); without them an approval is recorded and nothing runs.
4. An approved /task run replays the concatenation. The seam freezes message + user_message + system_prompt (chat_execution_service.py:2130) and _dispatch_approved sends that as message (skill_gate_service.py:925). The command can appear twice and a system prompt arrives as user text.
5. The inline connector tier has no gate result. inlineConnectorChat (client.ts:2946-2965) does not call readGateResult, so a refusal loses its named code.
6. Matcher gaps (utils/skill_invocation.py:25,31). Not matched: -/name, _/name_, 1/name, and a soft hyphen (U+00AD), U+180E, U+034F, a variation selector or a tag character after the slash.
7. Approver email reaches the requester. _notify (skill_gate_service.py:1064-1075) names the decider to a person requester, which includes an external Workspace client.

Smaller, no urgency:

• Connector test. run_playbook now sends /<name> <input>, and that is live at merge. connector.test.ts:55-62 asserts only /cso/ and /scan repo/, which the old prose form also satisfied; assert.equal(chats[0].message, "/cso scan repo") would pin it.
• Hot-path read. The seams build requester_from_principal(...) before enforce can short-circuit, which costs one execution-row read on every agent→agent /chat and /task. Building it after read_gates removes that.
• Write-only columns. requester_mcp_key_id, state_detail and dispatched_at are written and never read.
• Catalogs. architecture/database.md, backend.md, mcp-server.md and security.md have no skill_gate rows.

One item is the owner's decision rather than a fix, and is already a stated limit in the docs: a user-scoped MCP key counts as its owner and can self-approve.

[vybe]

merge-train: validated on 2026-10-04 at 14a4a50 (/validate-pr, /review, /cso --diff). No critical or high finding; ships inert. Follow-ups to close before trinity-enterprise#753 are in the merge-train comment.