Skip to content

feat(agents): the self-change permissions as four admin-only toggles in Agent Settings (abilityai/trinity-enterprise#756) - #3330

Merged
vybe merged 2 commits into
devfrom
feature/ent756-self-change-toggles
Oct 8, 2026
Merged

vybe merged 2 commits into
devfrom
feature/ent756-self-change-toggles

Conversation

@dolho

@dolho dolho commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Stacked on #3236 (base: feature/ent164-self-change-grants). Review the top commit only; this PR retargets to dev when #3236 merges.

Summary

  • Agent Settings → Permissions to change itself: four toggles (skills, schedules, instructions, other agents) over feat(auth): an agent changes its own shape only with a grant (ent#164) #3236's GET/PUT /api/agents/{agent}/capability-grants/{cap}. Each toggle has a plain line saying what the agent can do with it and what is refused without it. Each also shows granted by · relative time, with the absolute time on hover.
  • Who sees and edits it:
    • Only an instance admin can flip a toggle. The owner sees the same state with toggles disabled and the line Only an instance admin can change these. The PUT (admin and interactive) remains the real gate.
    • For a non-owner the grants read returns 404, so the card does not render at all.
  • States:
    • A fresh agent shows all four off, with a one-line explanation.
    • A failed first load shows LoadFailed with retry.
    • A refused toggle (calibrating, system or ephemeral agent) shows an InlineError beside that toggle, in the backend's own words.
  • Autonomy is shown beside the four as a person's switch, never a grant.
  • Open permission requests: the card lists pending queue items from this agent whose title contains a permission id, each linking to Operations. The ask surface has no field that marks an ask as a permission request. So the 403 from feat(auth): an agent changes its own shape only with a grant (ent#164) #3236 now tells the agent to put the id in its ask title verbatim (title includes 'schedules.manage'), and the card matches on that id.

Changes

  • src/frontend/src/components/settings/SelfChangePermissionsPanel.vue (new), mounted in SettingsPanel.vue
  • src/frontend/src/stores/capabilityGrants.js (new, agent-scoped, generation-guarded)
  • src/frontend/src/utils/capabilityGrants.js (new, pure: copy, display order, the title match)
  • src/backend/dependencies.py: _ask_for_it(capability) replaces the shared _ASK_FOR_IT constant, so the refusal names the exact id to put in the title
  • Docs: requirements/auth.md §2.9, feature-flows/self-change-grants.md, architecture/frontend.md

Open-core

Ungated (OSS-core). This is the UI for #3236's grants and follows whatever #3236's open-core ruling is. That ruling is still pending on the issue.

Test Plan

  • src/frontend/tests/unit/selfChangePermissionsPanel.spec.js is a mounted spec with 9 tests:
    • fresh agent shows all four off
    • an admin's grant reaches the PUT
    • the owner's view is disabled
    • a refusal lands on its own toggle
    • a failed load shows LoadFailed
    • a non-owner sees nothing
    • autonomy is shown beside the four
    • the request list, and the title match
  • Mutation check: with canEdit forced true, the owner test goes red.
  • tests/unit/test_ent164_self_change_grants.py: 46 passed. The refusal names title includes '<cap>'.
  • Ratchets pass: raw-colour, loading-gate, source-text, contrast (48 tests). check:tokens is OK.
  • Human check in the browser, light and dark (design contract principle 29): open an agent's Settings tab as admin and as owner, flip a toggle, and confirm nothing below it moves.

Fixes abilityai/trinity-enterprise#756

🤖 Generated with Claude Code

@dolho dolho added the ui PR touches the frontend UI — triggers Playwright e2e tests label Oct 7, 2026
@dolho
dolho requested a review from vybe October 7, 2026 09:39
dolho added a commit that referenced this pull request Oct 7, 2026
…witch (#3330 review)

setGranted read the store's current agent before the PUT and reloaded it
after; switching agents mid-PUT reset the store to the OLD agent, so the
next toggle on screen granted on the agent no longer shown. The agent now
comes from the caller and every post-await step runs only while it is still
current. Also: busy is per capability (one write finishing no longer
re-enables another still in flight), and a failed refresh keeps the toggles
on screen with a stale banner. Three mounted regression tests, each red
with the fix reverted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dolho

dolho commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

/review Report

Branch: feature/ent756-self-change-toggles → feature/ent164-self-change-grants (stacked on #3236; merge-base e64aaa56e)
Files Changed: 10 (+555/−22) at review time, plus the fix commit 73ea1c002
Scope: CLEAN. The diff is the Settings card, its store, a pure helper, the refusal copy it depends on, and docs.
Plan Completion (ent#756 acceptance criteria): 5 done / 0 partial / 0 not done / 1 changed / 1 unverifiable

AC Status Evidence
A Permissions to change itself group with four toggles DONE SelfChangePermissionsPanel.vue, utils/capabilityGrants.js::SELF_CHANGE_CAPABILITIES
Visible to the owner, editable only by an admin, granted-by and granted-at on each DONE canEdit (cosmetic; the PUT require_admin + reject_non_interactive_principal is the gate); self-change-granted-*
Plain-language can / without lines DONE SELF_CHANGE_CAPABILITIES[].can/.without
Autonomy beside them, not one of them DONE self-change-autonomy, read-only, from GET /{agent}/autonomy
Open permission-request asks linked CHANGED The ask surface has no ask_class. The 403 now tells the agent to put the id in the title (dependencies._ask_for_it), and the card matches on that id. The link goes to /operations, not to the item, because Operations has no per-item deep link.
Defaults off; a fresh agent shows four off plus a line DONE self-change-all-off
Looks right in both themes, nothing shifts UNVERIFIABLE Needs a human in the browser (contract principle 29)

Execution coverage (Step 2.5)

changed symbol / test file executed by live consumer verdict
SelfChangePermissionsPanel.vue selfChangePermissionsPanel.spec.js (mounted, 12) settings/SettingsPanel.vue ✅ executed
stores/capabilityGrants.js (load, setGranted) same spec, through the panel the panel ✅ executed
utils/capabilityGrants.js::permissionRequests direct unit test + the mounted list test stores/capabilityGrants.js ✅ executed
dependencies._ask_for_it test_ent164…::test_a_non_holder_gets_a_named_refusal… (asserts title includes '<cap>' per capability) _CAPABILITY_REFUSALS → every capability 403 ✅ executed

No source-text assertions were added. The new spec mounts the component.

Critical Findings

[C1] Race: a grant write could land on the wrong agent after an agent switch (Confidence: 9/10). Fixed in 73ea1c002.
File: src/frontend/src/stores/capabilityGrants.js::setGranted
Evidence (before):

async function setGranted (capability, granted) {
  const name = agentName.value
  ...
  await api.put(`/api/agents/${encodeURIComponent(name)}/capability-grants/…`, { granted })
  await load(name)          // load() resets the store when name !== agentName

Issue: if the panel switches from A to B while the PUT is in flight, load(A) resets the shared store back to A. The panel for B then shows A's grants, and its next toggle PUTs to A. The read-side generation guard doesn't cover this, because it is the write's follow-up reload that re-targets the store.
Fix (applied): the caller passes the agent (store.setGranted(agentName, cap, v)). The reload, the error and the busy flag each run only while that agent is still current. Reproduced first with a probe (expected 'A' to be 'B'). Now pinned by a PUT that lands after the panel switched agents never retargets the panel, which goes red with the fix reverted.

Informational Findings

[I1] A finished write re-enabled another toggle still in flight (Confidence: 8/10). Fixed in 73ea1c002. busy was a single value, so the first write's finally cleared it while a second toggle's PUT was still running. It is now a per-capability map. The test goes red on revert.

[I2] A failed refresh after a successful toggle was silent (Confidence: 8/10). Fixed in 73ea1c002. view.stale was computed but never rendered. A failed reload left the old toggle state on screen with no message, while the grant had in fact been written. The card now keeps the toggles and shows a retryable stale banner (ent#253). The test goes red on revert.

[I3] The permission-request link is generic (Confidence: 7/10, open). Each request links to /operations because Operations has no per-item route. On a busy queue, a reviewer has to find the item by title. A follow-up could add ?item= to Operations.

[I4] The permission-request match depends on what the agent writes (Confidence: 6/10, by design). An agent that ignores the 403's instruction won't be listed. Any of the agent's own pending asks whose title contains a permission id will be listed. That is bounded to this agent's own asks, rendered as interpolated text (no v-html), and shown in a scrolling list capped at max-h-40. A structured capability field on asks is the durable fix and would be its own change across backend, MCP and UI.

Clean Categories

  • SQL / data safety: no new queries or schema. The only backend change is refusal copy.
  • Auth boundary: no new endpoints. The card uses feat(auth): an agent changes its own shape only with a grant (ent#164) #3236's GET (OwnedAgentByName) and PUT (require_admin + reject_non_interactive_principal). A non-owner's 404 sets hidden, never a failure. The Settings tab itself is already canShare && !isSystem (utils/agentTabs.js), so shared users and the system agent never mount the card.
  • Credential exposure: none. No secrets are rendered or logged.
  • Enterprise disclosure: the enterprise-docs-guard pattern matches nothing in the added docs/ lines.
  • XSS: agent-authored ask titles are rendered by interpolation only, with no v-html in the file.
  • Frontend states: skeleton → ready, or LoadFailed, all keyed on viewState({hasLoaded}), plus the new stale banner. Raw-colour, loading-gate, source-text and contrast ratchets pass. check:tokens is OK. Only Base primitives are used (BaseToggle, InlineError, LoadFailed, SkeletonLoader).
  • Enum completeness: the four ids match db/capability_grants.CAPABILITIES, and the refusal map covers all four.

Summary

  • Critical: 1, fixed on the branch (C1). No open criticals.
  • Informational: 4. Two are fixed (I1, I2). I3 and I4 are open, with a follow-up suggested.
  • Scope: clean.
  • Learning: fragment docs/memory/learnings/2026-10-07-write-then-reload-must-carry-its-own-key.md.
  • Still needed: a human visual pass in light and dark. A test instance from this branch is running locally for that.

🤖 Generated with Claude Code

@dolho

dolho commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

/review Report: re-review at 73ea1c002

Branch: feature/ent756-self-change-toggles → feature/ent164-self-change-grants (stacked on #3236)
Scope: CLEAN. The diff since the first review is the fix commit only: the store, the panel, the spec, and one learnings fragment.
Plan Completion: unchanged from the first review (5 done / 1 changed / 1 unverifiable). The visual pass is still open.

Prior findings: status

Finding Status Verified by
C1 a grant write re-targets the panel after an agent switch ✅ resolved setGranted(name, …) takes the agent from the caller. Reload, error and busy run only while name === agentName. a PUT that lands after the panel switched agents never retargets the panel passes now, and fails when the store and panel from acd8376ba are restored.
I1 one write's finally re-enabled another in-flight toggle ✅ resolved busy is a per-capability map. The test fails on revert.
I2 a failed refresh was silent ✅ resolved view.stale → self-change-stale banner, retryable, with the toggles kept on screen. The test fails on revert.
I3 permission-request link goes to /operations, not the item open Operations has no per-item route. This is a follow-up.
I4 request matching depends on the agent following the 403 copy open, by design A structured field on asks is the durable fix.

Execution coverage (Step 2.5)

changed symbol executed by live consumer verdict
capabilityGrants.setGranted(name, capability, granted) mounted spec: grant, refusal, switch-mid-PUT, two in flight, failed refresh SelfChangePermissionsPanel.vue (toggle and InlineError retry, both pass agentName) ✅ executed
busy map one in-flight toggle does not re-enable another… :disabled="!canEdit || !!store.busy[cap.id]" ✅ executed
stale banner a failed refresh keeps the toggles on screen and says so panel ✅ executed

Mutation check: with the store and panel reverted to acd8376ba, all 3 new tests fail. With the fix, 12/12 pass. The raw-colour and loading-gate ratchets pass.

Critical Findings

None.

Informational Findings

[I5] Switching A→B→A while a save is in flight re-enables that toggle early (Confidence: 7/10, low impact)
File: src/frontend/src/stores/capabilityGrants.js::reset
Evidence: reset() does busy.value = {}, and load(A) after B calls reset(A), so A's in-flight flag is lost. A probe confirms it: after A→B→A the toggle is not disabled while A's PUT is still pending.
Impact: low. The reload shows the server state from before the PUT landed, so a second click re-sends the same granted value. The PUT is idempotent (changed: false) and both writes are audited only on change. No wrong-target write is possible; C1's guard still holds.
Suggestion: key busy by `${name}:${capability}` and leave it out of reset().

[I6] The stale banner can repeat itself (Confidence: 6/10, nit)
The banner text is "Couldn't refresh the permissions — showing the last ones that loaded." followed by store.loadError. When the server sends no detail, that error is the fallback "Could not load the permissions.", so the banner says nearly the same thing twice. Suggestion: append the detail only when the server sent one.

Clean Categories

  • Auth boundary: unchanged. No new endpoints. The PUT gate is on the server, so canEdit is cosmetic only.
  • Race and concurrency: the write path is now keyed by its own agent. Reads keep the generation guard. Two concurrent loads (the switch watch plus a post-write reload) are serialised by generation.
  • Credentials, SQL, enterprise disclosure: no new surface. The learnings fragment carries no private-module detail.
  • Frontend states: loading, failed, ready and stale all exist and share one footprint. No v-html.
  • CI (base is a feature branch, so this is a subset): guard ×3, schema-parity, gitleaks, verify-* pass. build, e2e and prod-image-smoke were pending at the time of this review.

Summary

🤖 Generated with Claude Code

dolho and others added 2 commits October 8, 2026 07:57
…in Agent Settings (trinity-enterprise#756)

Agent Settings gets a "Permissions to change itself" card: skills,
schedules, instructions and other agents, each with what it allows, what is
refused without it, and who granted it when. Owners see the state; only an
instance admin can flip it (the PUT stays the gate). A fresh agent shows all
four off with a line saying so; a non-owner's read 404s and the card does
not render. Autonomy is shown beside them as a person's switch, never a
grant. The agent's open permission requests are listed with a link to
Operations.

The ask surface has no field for "this is a permission request", so the 403
now tells the agent to put the permission id in its ask title verbatim, and
the card matches on that id (pure utils/capabilityGrants.js).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…witch (#3330 review)

setGranted read the store's current agent before the PUT and reloaded it
after; switching agents mid-PUT reset the store to the OLD agent, so the
next toggle on screen granted on the agent no longer shown. The agent now
comes from the caller and every post-await step runs only while it is still
current. Also: busy is per capability (one write finishing no longer
re-enables another still in flight), and a failed refresh keeps the toggles
on screen with a stale banner. Three mounted regression tests, each red
with the fix reverted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vybe
vybe changed the base branch from feature/ent164-self-change-grants to dev October 8, 2026 06:58
@vybe
vybe force-pushed the feature/ent756-self-change-toggles branch from 73ea1c0 to 4785b99 Compare October 8, 2026 06:58

@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.

Validated (/validate-pr, lane C). Rebased onto dev after #3236 squash-merged; Tier 1 + journey-smoke + e2e green; local: 4984 frontend + 87 auth unit tests pass; mutation of dependencies.py turns 4 ent164 cases red.

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

Labels

ui PR touches the frontend UI — triggers Playwright e2e tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants