feat(skill-gates): the per-agent skill gate map (abilityai/trinity-enterprise#753)

[webmixgamer]

Description

This adds the per-agent skill gate map: for each agent, which skills need approval before they run, and which kind of person approves. The dispatch check (#3208) and the in-container hook (#3252) already read it through skill_gate_service.list_skill_gates, which returned {} until now. Once this merges, gating is live.

• Storage. A new table, agent_skill_gates, holds one row per agent and skill: the approver (primary or approver), an optional deadline (1–168 hours; empty means 24), and where the row came from (set, library_default, or cleared, a tombstone that gates nothing).

  • Both migration tracks: the SQLite entry agent_skill_gates, and Alembic 0094_agent_skill_gates on top of 0093_platform_alert_responded_heal (re-chained after fix(operator-queue): acknowledging a platform alert ends it (#2372) #3338 landed its own 0093). One head.
  • A CASCADE AgentRef, so the rows follow a rename and are purged with the agent.
  • A failed read raises, so the check refuses with 503 gate_unavailable instead of treating the agent as ungated.

• REST: GET/PUT/DELETE /api/agents/{agent_name}/skill-gates[/{skill_name}]. MCP: list_skill_gates, set_skill_gate, clear_skill_gate. list_skills now also carries the library's approval key.

• Who can write a gate:

  • a person (a signed-in session, or the person's own user-scoped key) who owns the agent or is an admin;
  • an agent holding skills.manage, only on agents its owner owns, and never on itself: an agent cannot set, loosen or lift its own gate.

  Everyone else is refused with a 403 before the target is looked up, including the system key, connector keys and a principal with no scope.

• Who can read: anyone with access to the agent. An agent key can read its own gates, or, holding skills.manage, the gates of an agent its owner owns. set_by is withheld from machine keys (test: gate dev → open-PR merges on the unit-test suite #715).

• Refusals by name:

  • 422: invalid_skill_name, invalid_approver, approver_unavailable, invalid_deadline. approver_unavailable comes back when no assignments provider is registered; in that case only primary is offered, and approver_kinds says so.
  • 409: ephemeral_agent.

  Each gate reports approver_reachable. A write whose approver kind reaches nobody returns the warning approver_unassigned. That is the default case on an install whose admin has no email.

• Library default. A library skill whose SKILL.md says approval: recommended is gated before its package is delivered. Agents that already held the skill get the gate at their next start or library sync. The library can only tighten: a default is never removed because the metadata changed. When the owner clears a default, the clear sticks.

• Removal. No gate goes while the skill's files may still be on the agent.

  • A default goes once its package removal has completed, or once a start's prune has run.
  • An explicit gate goes only when a person unassigns the skill and the package removal completed.
  • It stays in place for an agent's unassign (on itself or a sibling), the system key, a deferred removal, a Sync, a start or the sweep. The response reports those as gates_kept, so unassigning and re-assigning a skill cannot launder a gate away.

• Marker ordering (feat(skills): a gated skill is refused inside the agent unless its run was approved for it (abilityai/trinity-enterprise#752) #3252). The fail-closed marker is written before an agent's first gate (decided under the per-agent lock) and removed after its last. A stopped agent is never exec'd.

• Audit. Every change is audited, but only when a row actually changed. Each audit row records who made the change, via (ui / api / orchestrator / system) and the trigger.

Deviations from the issue's acceptance criteria (recorded scope decisions)

• Own skills: an own skill's gate stays until someone clears it. The platform gets no event when an agent deletes its own skill folder, so dropping the gate when the skill disappears would let the agent ungate itself.
• Library skills: "removing a skill removes its gate" holds when a person removes it and the package is gone. In every other case listed above the gate stays.
• Ephemeral agents cannot hold gates in v1.

Merge order

• fix(skill-gate): close the #3208 validation gaps before the gate map lands (#3274) #3345 has merged (547deb2c6), and dev is merged into this branch (9fea4bd38). The merge-order requirement is met. The two doc conflicts kept both sides.
• feat(auth): an agent changes its own shape only with a grant (ent#164) #3236 has merged. This branch merged dev (7d7b350e4) and now uses feat(auth): an agent changes its own shape only with a grant (ent#164) #3236's require_person_or_capability. The duplicate copy is gone; dev's version is kept, including _refuse_unless_owners_agent.

Related Issue

Fixes abilityai/trinity-enterprise#753

It also covers item 2 of #3344, the architecture catalog rows for skill_gate_requests, the gate services, the MCP gate results and the dispatch gate. Items 1 and 3 there stay open.

Journey Impact

Journey Impact: none: no promise in tests/journeys/catalog.yaml covers gated skills yet; this adds the configuration surface for the existing gate

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Testing

• I have tested this locally: eyeball of 737d4db71 on localhost, all 12 steps passed, each checked in the DB (test plan below)
• New tests added
• All existing tests pass. The only failures are the pre-existing ones listed below.
• Every new test executes the changed path

New tests:

• tests/unit/test_ent753_*.py: 136 tests over the real per-test DB, the real routes and real dependencies.
• src/mcp-server/src/tools/skill-gates.test.ts: 6 tests.
• The AST guard (test_ent753_assignment_hooks.py) makes every agent_skills writer in src/backend reach the gate reconcile, whether it calls the writer or passes it by reference.

Results:

• The related suites pass in fixed order (1,098 tests).
• The MCP suite passes (810), and tsc is clean.
• Both migration guards pass: check_alembic_heads and check_alembic_parity.
• Seven pre-existing test files fail on this host, none of them caused by this branch:
  • test_ent666_objective_join, test_ent477_* (two files) and test_retention_floor fail only in some random orders and pass alone;
  • test_ent751_gate_callers::TestPortal fails under one random seed and fails the same way on the base commit;
  • test_ent679_payments_pin_parity fails on the host's package pins;
  • test_2996_route_census_runtime errors because the host venv lacks opentelemetry.exporter.
• /verify-local --skip-unit --skip-agent:
  • build, import smoke, boot with the migration applied, and test preflight all pass;
  • integration: 69 passed, 1 failed. The failure is test_circuit_breaker dormant-alert, already failing on dev and tracked as bug: circuit-breaker integration test still counts the old operator-queue call for the dormant alert (stale since #3246) #3343.
• A smoke test of REST and MCP against the sibling stack's production images: set_skill_gate then list_skill_gates through a user-scoped key, audited via=api.

Mutation: n/a — not a fix. For the record: 28 hand mutations, covering the write paths, the auth bounds, the marker ordering, the drop-only-after-the-package-is-gone rule, the partial-update merge and the set_by redaction. Each turned a test red, and each was restored byte-for-byte.

Test plan (localhost)

1. Back up the DB first. The backend's /data is a Docker volume, so do it through the container: run Python sqlite3 .backup to /data/backups/pre-eyeball-753-<ts>.db, then docker cp it to ~/trinity-backups. Then switch the checkout to the PR head; the backend reloads and applies the migration.
2. Optional: rebuild mcp-server if you want the MCP tools. REST works without it.
3. Pick an agent. Use one whose skill-gate hook reports ok (agent /health → skill_gate_hook). If it has no skills, add a temporary own skill for the test and remove it afterwards.
4. Read the empty map: GET /api/agents/<agent>/skill-gates returns no gates. approver_kinds is ["primary"] on OSS, plus "approver" where an assignments provider is registered.
5. Set a gate: PUT …/skill-gates/<skill> with {}.
   • Expect changed: true.
   • If warnings says approver_unassigned, give the owner an email first.
   • The marker /opt/trinity/skill-gates-active exists in the container, owned by root, mode 0444.
6. Audit: a JWT write via curl is audited via=ui (by design, a JWT is the UI). A user-scoped key is audited via=api.
7. Raise an approval:
   • As a non-approver, ask for /<skill> … in the Workspace chat (or, now that fix(skill-gate): close the #3208 validation gaps before the gate map lands (#3274) #3345 is on dev, the agent page's Chat tab).
   • Expect the "Not run: … needs approval … Request gate-… is waiting" notice, and an approval card for the approver.
   • The requester's execution row is skipped.
8. Approve it: exactly one run happens, and it succeeds.
9. Plain-text request: as the non-approver, ask in plain text. The in-container hook refuses it: audit skill_gate_refused with via=skill_tool and reason=not_cleared, and no card is raised.
10. Self-approval: as the approver, ask for /<skill> …. It runs directly, audited skill_gate_self_approved.
11. Agent keys:
    • The agent's own key can read its own gates, and set_by comes back null.
    • A PUT or DELETE on itself gets 403 person_required, whether or not the agent holds skills.manage, because the self bound is checked first.
    • On a sibling without the grant it gets 403 skill_management_not_permitted, and nothing is written.
12. Clear the gate: DELETE …/skill-gates/<skill> returns changed: true, and the marker is gone.

Security: docs/security-reports/cso-diff-2026-10-07-ent753-skill-gate-map.md. One LOW finding (set_by reaching machine keys), fixed in this branch with a test.

Checklist

• My code follows the project's style guidelines
• I have updated the documentation (if applicable): requirements §26.13, feature-flows/skill-gate.md §6, the skills flows, the architecture catalogs, and a learnings fragment
• I have not committed any sensitive data (API keys, credentials, etc.)
• I have added appropriate logging for new functionality

🤖 Generated with Claude Code

[github-actions]

⚠️ Alembic head fork if this PR is merged into dev.

dev has advanced since this PR's checks last ran. GitHub recomputes the merge ref when the base moves but does not re-trigger workflows, so a green schema-parity here can describe a base that no longer exists (#2533).

scripts/ci/check_alembic_heads.py against dev + this PR, merged in memory

alembic-heads: FAIL — src/backend/migrations/versions resolves to 2 heads across 95 revision(s); exactly 1 is required.
  • 0093_agent_skill_gates  (0093_agent_skill_gates.py)
  • 0093_platform_alert_responded_heal  (0093_platform_alert_responded_heal.py)

They fork at: 0092_portal_messages_attachments  (0092_portal_messages_attachments.py)

`alembic upgrade head` is singular and resolves its target BEFORE applying anything,
so this graph applies ZERO revisions — every revision since the fork stops arriving,
not only the one that forked. Fix by chaining the newer revision off the real head,
or — if the forked revision may already be applied somewhere — by adding a merge
revision (`alembic merge -m "…" <head-a> <head-b>`), whose tuple `down_revision`
converges the line from any starting state. See Architectural Invariant #3.
alembic-heads: src/backend/enterprise/backend/migrations/versions — version directory absent (submodule not initialised) — skipped.

Evaluated on the version line only — this PR also conflicts with dev in 2 unrelated file(s), which do not change this verdict but must be resolved before merge:

src/backend/db/migrations.py
tests/registry.json

Fix: rechain this PR's revision off 0093_platform_alert_responded_heal (the current dev head), or — if the forked revision may already be applied somewhere — add a merge revision (alembic merge -m "…" 0093_agent_skill_gates 0093_platform_alert_responded_heal), whose tuple down_revision converges the line from any starting state. See Architectural Invariant #3.

Push the fix and schema-parity re-checks it against the merge ref immediately; this comment clears on the next push to dev touching src/backend/migrations/versions/**.

Advisory — this check does not block merge. · head_sha: 7d7b350e4ccfa646b630a1f483c5479b36ed4e1b · run

[vybe]

merge-train (mechanical): merged origin/dev (now carrying #3345 and #3338) into this branch (9fea4bd). Two doc conflicts with #3345 resolved keep-both: feature-flows/skill-gate.md status line keeps this PR's "the gate map makes it live" plus #3345's "#3208 gaps closed by #3274 (plus #3233)"; requirements/security.md keeps this PR's gate-map block with the Tests line unioned (test_3274_* + the four frontend specs). Your own e34a145a had already rechained the revision to 0094_agent_skill_gates ← 0093_platform_alert_responded_heal — same as the train used; heads check: 1 head. Validated on train #3349 (all Tier 1 + Tier 2 green).

[vybe]

merge-train: batch validated on train/20261007-1609 (#3349)