Skip to content

feat(skills): a Settings panel for the skill runner's operator half (abilityai/trinity-enterprise#242) - #2772

Merged
vybe merged 3 commits into
devfrom
feature/242-skill-runner-admin-panel
Sep 20, 2026
Merged

vybe merged 3 commits into
devfrom
feature/242-skill-runner-admin-panel

Conversation

@dolho

@dolho dolho commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Fixes abilityai/trinity-enterprise#242

Draft — the panel's entitled render has not been exercised on a live instance yet (see Verification). Everything else is done.

What this closes

The six admin endpoints shipped with no writer outside curl. run_skill / list_runnable_skills are advertised over MCP but gate on an ACL table nothing could populate, so an entitled operator could enable the feature and grant access only by hand-rolling HTTP.

One entitlement-gated Settings tab now covers enable → provision → sync → per-(caller, skill) grants.

No backend change

All six endpoints already exist, are typed, and are require_human_admin-gated. I verified that before planning rather than assuming it, because it decides the PR shape: had anything needed the private submodule, this would have been three PRs in the ent#545 order (public first, private draft, then a pointer bump) instead of one.

Scope ruling

The issue offered "a Settings/agent-detail panel (or MCP admin tools)" and did not resolve it. Operator ruling: the Settings panel. The MCP half is deferred and recorded on the issue rather than dropped — Invariant #13's third surface stays open, so an ops agent still cannot enable or grant without curl.

Design

Gate { id: 'skill-runner', adminOnly: true, requires: 'skill_runner' } — the same declarative seam sso and credential-vault use
Rules skillRunnerPanel.js, pure — vitest runs environment: 'node' with no mount harness, so a rule inside an SFC is one no test can reach (ent#392 precedent)
Pickers GET /api/agents + OSS GET /api/skills/library

The frontend gate is UX, not containment. The server refuses regardless, which is what keeps a stale entitlement list from becoming an escalation.

Every state stays distinguishable. SkillRunnerStatus reports each fact independently and the panel preserves that: "no runner yet" / "provisioned but not running" / "running" are three states with three badges, and unconfigured / never-pulled / unreadable library are three more. None collapse into a spinner, and a blocked operator is told which screen fixes it.

The revoke confirm names the CALLER, not just the skill — "Revoke summarise" reads harmlessly, while the decision is which agent loses the ability to execute it.

Two things found while building

There is no endpoint listing the library for a grant picker. /available is agent-facing — ACL ∩ library for one caller, and it 422s for a user-scoped key with no caller_agent. The picker uses the OSS GET /api/skills/library. Whoever builds the MCP half needs the same.

The suite caught a real design violation. I reached for ScanlineReveal; portalLoadingTreatment.spec.js failed because that primitive has an importer allowlist — #2540 ruled the beam is chart-loading only and panels load with a skeleton keyed on "no data yet". Fixed the panel rather than widening the allowlist, and pinned the choice with a test so it isn't repeated.

Docs

No feature-flow doc, deliberately. A public doc describing this module would name enterprise internals under docs/** — the enterprise-docs guard's scope and the CLAUDE.md standing rule. No API or architecture changed, so tiered docs don't require one. Verified my staged files carry no token the guard flags (the one permissions_matrix hit in Settings.vue is pre-existing on dev, outside this diff, and src/frontend/** isn't in the guard's scope anyway).

Verification

check result
New rule tests 32 passed
Full frontend suite 2845 / 2845
npm run build clean
Raw-color ratchet green — zero raw non-gray classes in new code
Loading-gate ratchet (#1927) green — gates on view.state, never a bare loading
Step 10 hygiene staged by path; both gitlinks clean in the index; no conflict markers

Not verified: the local instance reports enterprise_features: [] and the endpoints 404, so the entitled render is unexercised. The unentitled path — tab absent, no dead panel — is what the gate actually decides and is covered by source guards on requires: + adminOnly. Draft until the entitled path is seen on a real instance.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ

@dolho

dolho commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Verified on a live instance — the entitled path now exercised

The draft caveat is discharged. I mounted the enterprise submodule into the running dev stack, confirmed skill_runner registered, and drove the panel in a browser. The instance was restored to its exact prior state afterwards.

Trinity Enterprise modules registered
enterprise_features: 17 modules · skill_runner entitled: True
GET /api/enterprise/skill-runner/status  -> 200
GET /api/enterprise/skill-runner/access  -> 200

The live /status happened to return the most interesting state — a usable library, no runner yet:

{ "enabled": false,
  "library": { "configured": true, "synced": true, "skill_count": 41 },
  "runner_agent": "trinity-skill-runner",
  "runner_exists": false, "runner_running": false,
  "exposed_skill_count": 0, "grant_count": 0 }

Every rule confirmed against that payload

rule expected observed
Tab renders only when entitled present present (absent before the module registered)
Runner badge "No runner agent yet" "No runner agent yet"
Counts shown while OFF 0 / 0 / 41 "0 skills exposed · 0 access grants · 41 in library"
Provision enabled — library is usable isDisabled() === false
Sync disabled — nothing to push onto isDisabled() === true
Grants empty state names the next action "No agent can run any skill yet. Grant access below to let one start."
Grant button, nothing picked disabled disabled
Grant button, pair picked enabled enabled
Pickers populate 24 agents, 41 skills 25 options / 42 options (incl. placeholder), both endpoints 200

Toggling the feature on and off round-tripped through PUT /enabled and the panel re-read status. Left off, as found.

Two measurement corrections worth recording

I nearly reported two bugs that were not bugs, and both are the same mistake — measuring before asserting I was on the right screen:

  1. A run reported "skill options = 0". The tab switch had silently failed and I was counting selects on the General tab. The fix is asserting a panel-specific element (Access grants) is visible before measuring anything.
  2. A second run reported both pickers holding only their placeholder. They load asynchronously after the panel renders; the assertion fired too early. With a wait they read 25 / 42.

Neither reached the PR as a finding, but they would have if the first number had been trusted.

One real bug found — pre-existing, not from this PR

The screenshot shows a blue "How it works" box below the panel, describing the Trinity Prompt. It is ungated in Settings.vue and renders under every tab — confirmed on SSO and Retention on unmodified dev. Filed as #2774. It is not in this diff (8 lines, all tab registration), but it is visible in the screenshot, so: named rather than left for a reviewer to wonder about.

Instance restored

Enterprise checkout removed, the three copied frontend files reverted, feature toggled back off, backend restarted — enterprise_features: [], /health green, trinity-demo tree clean.

@dolho
dolho marked this pull request as ready for review September 14, 2026 10:46
@dolho
dolho marked this pull request as draft September 14, 2026 10:47
@dolho

dolho commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Held as draft until v0.9.5 lands

Decision (2026-09-14): this PR stays a draft until the 0.9.5 release is cut. Not a quality hold — the work is complete and verified; see the verification comment above.

Why. ent#242 is a new operator surface, and 0.9.5 is already at v0.9.5-rc2. Adding a Settings tab during a release candidate widens the surface the release is being stabilised against, for a feature that has waited since v0.8.5 and can wait one more cut. It is also not in the 0.9.5 floor — unlike the canvas stack (ent#553/554/555), which was ruled in on 2026-09-08 and is landing.

State when parked:

  • 32 new tests; full frontend suite 2845/2845; build clean
  • Both design-system ratchets green (zero raw non-gray in new code)
  • Verified live against an entitled instance — tab gating, all runner/library states, action enablement, grant gating, pickers populated (25 / 42)
  • Backend unchanged, so nothing here affects the release build

What unblocks it: the 0.9.5 release cut (dev → main). After that, mark ready — no rework is expected. If dev has moved by then, the only likely conflict is views/Settings.vue's tab list, which is an append.

One caveat for whoever picks it up: the entitled render was verified by temporarily mounting the enterprise submodule into a local stack. That is a real check, not a simulated one, but it is not the same as an instance that ships entitled — worth a look on a genuinely entitled deployment before release.

Pre-existing bug found while testing and filed separately: #2774 (the ungated "How it works" box, visible in the screenshot, not from this diff).

…ent#242)

The six admin endpoints (`enabled`, `provision`, `sync`, and the three
`access` routes) shipped with no writer outside curl: the consumer tools
`run_skill` / `list_runnable_skills` are advertised over MCP but gate on an ACL
table that nothing could populate. An entitled operator could activate the
feature and grant access only by hand-rolling HTTP.

This is that writer — one entitlement-gated Settings tab covering
enable -> provision -> sync -> per-(caller, skill) grants.

**No backend change.** All six endpoints already exist, are typed, and are
`require_human_admin`-gated (admin AND a human principal — widening an
allow-list must not be reachable by a prompt-injected agent's own key). That
premise was checked before planning rather than assumed, because it decides the
PR shape: had anything needed the private submodule, this would have been three
PRs in the ent#545 order instead of one.

Gated with `requires: 'skill_runner'`, the same declarative seam `sso` and
`credential-vault` use, so an unentitled install renders no tab rather than a
panel whose every control 404s. That gate is UX, not containment — the server
refuses regardless — which is what keeps a stale entitlement list from becoming
an escalation.

Decidable rules live in `skillRunnerPanel.js`, not the SFC: vitest runs
`environment: 'node'` with no mount harness, so a rule inside a component is one
no test can reach (the ent#392 precedent). 32 tests cover them.

Two details worth naming:

* `SkillRunnerStatus` reports each fact independently, and the panel keeps it
  that way: "no runner yet", "provisioned but not running" and "running" stay
  three states with three badges, and the library's own unconfigured /
  never-pulled / unreadable states are three more. None collapse into a
  spinner, and a blocked operator is told which screen fixes it.
* There is no endpoint listing the library for a grant picker — `/available` is
  agent-facing (ACL ∩ library for ONE caller, and 422s for a user-scoped key
  with no `caller_agent`). The picker uses the OSS `GET /api/skills/library`.
  Worth knowing for whoever builds the MCP half.

The revoke confirmation names the CALLER, not just the skill: "Revoke
summarise" reads harmlessly, while the decision being made is which agent loses
the ability to execute it.

Loading uses a skeleton keyed on "no data yet", not `ScanlineReveal` — #2540
ruled the beam is chart-loading only, and that primitive's importer allowlist
caught this panel reaching for it. Fixed the panel rather than widening the
allowlist, and pinned the choice with a test.

No feature-flow doc: a public doc describing this module would name enterprise
internals under `docs/**`, which is the enterprise-docs guard's scope and the
CLAUDE.md standing rule. No API or architecture changed, so tiered docs do not
require one.

Deferred, recorded on the issue: the MCP admin tools half (Invariant #13's third
surface), so an ops agent still cannot enable or grant without curl.

Fixes Abilityai/trinity-enterprise#242

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ
@dolho
dolho force-pushed the feature/242-skill-runner-admin-panel branch from 0441c6d to be198aa Compare September 18, 2026 08:48
@dolho
dolho marked this pull request as ready for review September 18, 2026 08:48
@dolho

dolho commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

/review — post-rebase (onto dev @ 026cc68)

Scope: CLEAN — Settings-panel half only; MCP half deferred on the issue (operator comment 2026-09-14). Open-core ruling present: entitlement-gated (requires: 'skill_runner' on the tab, requires_entitlement("skill_runner") router-wide + require_human_admin on all six routes in the private submodule). Plan completion: 4 done / 1 partial (grants) / 1 deferred-recorded.

Critical (block merge)

[C1] Contract mismatch: the grants list is a dict, the panel expects an array (9/10)
SkillRunnerPanel.vue load():

axios.get(`${BASE}/access`, hdr()) … grants.value = Array.isArray(a.data) ? a.data : []

Server (enterprise/backend/skill_runner/service.py:169-170, returned unwrapped by router.py:118):

def list_access(caller_agent: Optional[str] = None) -> dict:
    return {"grants": db.list_access(caller_agent)}

a.data is {"grants": [...]}, never an array, so grants is always []: every grant renders as "No agent can run any skill yet", isDuplicateGrant never fires, the filter never appears, and there is no revoke path from the UI — the only human writer for the ACL is write-only. The PR body's "Not verified: the entitled render is unexercised" is exactly where this hides. Fix: unwrap a.data?.grants, and extract a grantsFrom(payload) rule into skillRunnerPanel.js so the node-env suite pins the {grants} shape.

Informational

Clean

Every endpoint require_human_admin + entitlement; pickers use OSS GET /api/agents / GET /api/skills/library (shapes verified on dev); no v-html; busy serialises actions; failureText matches _raise's shape; ratchets green (3159/3159).

…#242)

`GET /api/enterprise/skill-runner/access` answers `{ grants: [...] }`
(`service.list_access` wraps `db.list_access`), and the panel read the
payload AS the array — so `grants` was always `[]`, every grant rendered as
"No agent can run any skill yet", `isDuplicateGrant` never fired, and the
only human writer for the ACL had no revoke path. The PR's own "the entitled
render is unexercised" note is exactly where it hid.

`grantsFrom(payload)` is the one unwrap rule, pure so the node-env suite can
pin the envelope shape; a bare list is still accepted, anything else is an
empty list rather than a throw (the status half of the same load must still
render). A source guard pins that the panel reads through it — reverting the
call site turns the spec red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dolho

dolho commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

/review — re-review after e142f3f (C1 fix)

Scope: CLEAN — the fix touches only the three files C1 named. Plan completion: 5 done / 1 deferred-recorded (MCP half); the grants AC is now DONE.

C1 closure — verified end to end

  • load() → grants.value = grantsFrom(a.data); grantsFrom unwraps payload.grants, matching service.list_access → {"grants": db.list_access(...)} returned unwrapped by the router. Bare-list and garbage shapes handled; a source guard pins that Array.isArray(a.data) ? a.data never returns (reverting the call site turns the spec red).
  • Row shape {id, caller_agent, skill_name, granted_by, created_at} (db.py:61): every consumer — isDuplicateGrant, filterGrants, revokePrompt, the list :key and rendered spans — reads only caller_agent/skill_name. ✅
  • DELETE /access: panel sends params: { caller_agent, skill_name } ↔ router Query(...) params. ✅ POST body ↔ SkillAccessGrant. ✅ PUT /enabled ↔ SkillRunnerToggle, response is the SkillRunnerStatus shape. ✅
  • /status (same-class risk): service.get_status() → enabled, library{configured, synced, skill_count, commit_sha | error}, runner_agent, runner_exists, runner_running, exposed_skill_count, grant_count, pinned by SkillRunnerStatus; the panel reads nothing else. No second mismatch. ✅
  • Header grant_count (db.count_grants) and the list (db.list_access) read the same table, so they now agree.

Critical

None — C1 closed.

Informational

Clean

Auth unchanged (require_human_admin + requires_entitlement("skill_runner") on every route); {{ }} only; grantsFrom never throws, so a malformed access payload cannot blank the status half. Spec 36/36, suite 3163/3163 on the tip.

Ready to merge — base dev, no stack dependency.

…#242) — mechanical, per the merge-train note on the PR

blockingAction() emitted navigate-tab with 'skills', an id absent from
Settings.vue's ALL_TABS (the Skills Library renders under 'agents'), and
the handler assigned it straight to activeTab, bypassing selectTab()'s
validTabIds guard — so a fresh entitled install's only blocked-state
action blanked the Settings content area. Emit 'agents' and route the
event through selectTab so an unknown id is a no-op rather than a blank
page, and pin the corrected value in the spec.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@vybe

vybe commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

merge-train — mechanical fix pushed to this branch (fd6fd3b2a)

Validated on the 2026-09-20 train. One finding was mechanical, so it was fixed here rather than sending the PR back — one commit, three lines, nothing else touched:

  • skillRunnerPanel.js:43 — blockingAction() returned tab: 'skills'. No such id exists in Settings.vue's ALL_TABS (never has — git log -S"id: 'skills'" is empty); the Skills Library (SkillSourcesPanel) renders under 'agents'. Changed to 'agents'.
  • Settings.vue:77 — @navigate-tab="activeTab = $event" assigned the emitted id directly, bypassing selectTab()'s validTabIds guard and doing no router.push. With the dead id, a fresh entitled install (library unconfigured → blocked → the one offered button) landed on a blank Settings content area with no tab highlighted. Now @navigate-tab="selectTab", so an unknown id is a no-op and a known one updates the URL like a tab click.
  • skillRunnerPanel.spec.js:48 — pinned the corrected value (it pinned 'skills' as correct).

Confirmed at runtime by mounting the panel under jsdom with library.configured: false and clicking the button: the emit was 'skills' ∉ ALL_TABS. skillRunnerPanel.spec.js + both design-system ratchets: 56/56 on the fixed tree.

Follow-ups noted, not conditions on this merge (worth their own issues):

Cross-tracker keyword: ent#242 closes on merge but does not relabel — status-in-dev will be set by hand.

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge-train: batch validated on train/20260920-1811 (#2913)

@vybe
vybe merged commit 58a1bb4 into dev Sep 20, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants