Repository navigation
feat(skills): one Skills tab — own and shared skills, Run, Requires approval (abilityai/trinity-enterprise#754) - #3412
Merged
Merged
Conversation
…val (ent#754 C1) Requirements first (Rule #1): requirements/skills.md §22 becomes the merged Skills tab (own + shared, Run, Requires approval), superseding PLAYBOOK-001; the AC5 canon-role deviation is named and linked to trinity-enterprise#848. GET /api/skills now reports per skill: - source: "platform" when the platform's .trinity-skill.json marker is in the directory (the same isfile test the backend uses), else "agent"; - dir: the directory name, which the gate fingerprint resolves before the frontmatter name; - approval: the closed set {recommended}, read with the backend contract's precedence (trinity: block first, then the flat key). All three are informational only. A behavioural parity table runs the same documents through both parsers. Refs Abilityai/trinity-enterprise#754 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…(ent#754 C2) The /playbooks proxy existed three times (agent page, public link, MCP connector). It now lives once, in services/agent_skills_listing.py (Invariant #1), and every live success keeps the listing in Redis under agent:skills_list:{name} with no TTL (the user's ruling on Q1). - GET /api/agents/{name}/playbooks?last_known=true is opt-in. When the agent is stopped or unreachable it serves the copy, labelled last_known: {captured_at, reason}. With no copy, or without the param, the original errors stand, so the other consumers are unaffected. - Only a real listing is kept, and a live empty list replaces the copy. A failed read never overwrites it. A listing over 256 KB is not kept and drops the older copy. - Redis is fail-open on every path. - The keyspace is registered in CLEARED_KEYSPACES and cleared on teardown and on the create path, never on a stop or start. - The public link keeps to its pre-existing per-skill fields, so the new source/dir/approval never reach an anonymous visitor. Refs Abilityai/trinity-enterprise#754 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…forces it (ent#754 C3)
GET /api/agents/{name}/skill-gates gains two additive reads (no new route):
- approvers: [{kind, reachable, viewer_fills}] for every kind the install
resolves. viewer_fills is decided by the same two functions enforce uses
for self-approval (requester_from_principal plus the casefolded approver
list, through a new non-raising approver_people wrapper). So the card's
"you approve this" agrees with what a run does, and a parity test runs
enforce for owner, viewer, admin and agent key. Booleans only, no person
data.
- ?probe=true adds hook, the agent's /health -> skill_gate_hook (ent#752).
It is one direct read with a 3 s timeout and no breaker bookkeeping, and is
honoured only for a person who may manage the agent's skills (the #3052
precedent). A 200 without the field is "predates", meaning an image older
than the hook. No answer is "unknown", never "not ok".
Refs Abilityai/trinity-enterprise#754
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…54 C4) The executions list and detail gain gate_self_approved and gate_self_approved_by_viewer. They are derived from ent#752's existing self_approved records in skill_gate_requests (UNIQUE dispatched_execution_id), so there is no new column and no migration (UC1, approved at the plan gate). - List: one batch read per page (get_self_approved_runs, scoped to the agent and the ids). The PERF-001 column list the portal shares is untouched. - Viewer compare: the record's requester key vs the caller's casefolded email. It is done server-side and only for a person principal, so the email never leaves the server and a machine key is never "you". - The read never fails the executions read. A record whose write failed (fail-open by design) reads false, which matches what the in-agent hook did. Driven end to end: the record is written through the real record_self_approval and read back through the real routes. Refs Abilityai/trinity-enterprise#754 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…uires approval (ent#754 C5-C6) The Playbooks tab (the agent's own skills, with Run) and the library-only Skills tab (assignment) become one Skills tab of fixed-size cards, built to the approved design. - Own skills: the agent's .claude/skills/, live. When the agent is stopped, its last-known list is shown with Run disabled and a stopped banner, never an empty tab. - Shared skills: library assignments. The picker moves into an "Assign skills" dialog, sets show as status chips with a "Manage sets" dialog (AgentSkillSets unchanged), and the #2914 conflict, delivery warnings and superseded text open from the card's note line. - Every card has one top-left badge area. The author's automation value is relabelled by meaning (runs unattended / asks mid-run / start by hand, Q4) on a new BaseBadge size="sm". - Gate line, shown to everyone: names the approver kind, never a person, and says "you approve this" to the person who fills it. - Owner or admin (not on a ghost or the system agent): Requires approval toggle plus approver picker; a kind nobody fills is shown but can't be selected. A gate with no matching skill is kept on its own card, with Clear. - Run sends POST /task {"/<name>", async_mode} (the shared client times out at 30 s). A 202 shows the approval notice and nothing opens, a refusal goes to the error toast, and any other failure stays on the card. - An in-agent enforcement warning appears for the owner when the hook reports anything but ok (predates, missing, altered, unsupported runtime). - A self-approved run is marked on its Tasks row and its execution page. - ?tab=playbooks resolves to skills (TAB_ALIASES, now in utils/agentTabs.js). Every rule is in utils/skillCards.js; data is in stores/skills.js and the new agent-scoped stores/skillGates.js. PlaybooksPanel.vue and SkillsPanel.vue are deleted. Their specs are ported, not dropped, and the mutation checks still go red. The two ratchet entries are removed by hand; the raw-colour baseline is not regenerated. Refs Abilityai/trinity-enterprise#754 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The chat and Workspace / menus head their lists "Skills" (Q5: both). The composer placeholders, the Workspace overflow and empty lines, the exposed-skills panel, the connector and sharing copy, and the dashboard's update button follow. API and field names (/playbooks, exposed_playbooks, run_playbook, use-playbook) are unchanged, as the AC says. Refs Abilityai/trinity-enterprise#754 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…#754 C9) This is the 10-03 eyeball case itself. The person who fills a gated skill's approver role asked for it in the Workspace chat, the gate self-approved, and the reply landed with no sign a gate applied. Since #3166 every Workspace message names the turn that wrote it, so the history read now marks each agent reply whose run has an ent#752 self_approved record. This is the same derivation as the Tasks marker: no new column, one read per thread, a casefolded viewer compare, and no email added. The reply poll's narrow read carries it too, so a reply that has just landed shows "Ran without approval: you are the approver" under the bubble without a reload (assistantRow and replyFromHistory carry the two fields). The hotspot gets a two-line call in get_history; the logic lives in skill_gate_map_service.annotate_self_approved_turns. This is its own commit, so it can be cut. portalReplyRateable.spec.js pins the mapper shapes and the reattach call, and is extended rather than loosened. Refs Abilityai/trinity-enterprise#754 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ead store field (ent#754) From the feature-flow sync's code read: - When the sets read failed, the set-chips line vanished, so a viewer (who can't open the sets dialog) never learned it. The line now says "Couldn't read this agent's sets" with a Retry, and adds "Some sets need credentials" when any set lacks them (the plan's error registry). - skillGates.warnings was written and asserted but never rendered. The map is re-read after every write and its reachable:false is already what the card's "nobody fills it yet" says, so the state is removed. Refs Abilityai/trinity-enterprise#754 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er docs (ent#754 C8)
- Feature flows:
- new skills-tab.md (the whole slice);
- playbooks-tab.md becomes a pointer;
- skill-assignment, skill-injection, library-page, skill-gate,
playbook-autocomplete, workspace-composer-typeahead, mcp-connector,
public-agent-links, tasks-tab, execution-detail-page and run-agent-loop
are brought up to date;
- the index gets one row plus one Recent Updates row.
- Architecture:
- backend (agent_skills_listing, the keyspace, the map service's reads);
- api-endpoints (/playbooks?last_known, the executions flags,
skill-gates approvers / probe);
- frontend (the Skills tab paragraph);
- agent-runtime (the SkillInfo facts);
- workspace (the self-approved reply).
- Requirements: §22.3 now names SkillsTab.
- Screenshot manifest: sources point at SkillsTab. The image itself still
shows the old tab and needs a localhost capture.
- User docs: 14 pages move from the Playbooks tab to the Skills tab, plus a
new "Requiring approval for a skill" section. Anchors and FAQ headings
are kept.
The disclosure-guard pattern finds 0 hits. All 3,084 unit tests that read
the docs pass.
Refs Abilityai/trinity-enterprise#754
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ting (ent#754) Two of the listing's lifecycle properties were pinned by reading source (AST call lists), not by running the code. Both now execute: - the create path, through the #1484 characterization harness that runs the real create_agent_internal: the name's listing is dropped beside the breakers and before the container exists (order clear -> forget -> run); - the breaker sweep a start runs: the real clear_agent_breakers leaves the copy in place. Mutation: removing forget(config.name) from crud.py turns test_ent754_create_drops_a_recycled_names_skills_listing red; adding a forget to clear_agent_breakers turns test_a_start_does_not_clear_the_copy red. Both restored byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…the next (ent#754)
AgentDetail is KeepAlive'd and the merged Skills tab is open to everyone,
so the same tab instance carries from one agent to the next. Two things
leaked across that switch:
- The tab's own verb state: the last outcome line ("Unassigned x."), card
errors, a pending-sync emphasis, open dialogs, the filter, and a run still
starting. A run that answered after the switch opened the NEW agent's
Tasks tab on the previous agent's execution id, or toasted there. The
state now resets on a switch, and Run / Unassign / Sync drop an answer for
an agent the page has left.
- The skills store's reads and writes (load, sync, save, set assign and
unassign) applied whatever answered last. A slow load for the previous
agent overwrote the next agent's assignments, so an Unassign there could
PUT the previous agent's list onto it. Each answer is now checked against
the agent it was asked for (a load also against a newer load), a write's
follow-up read targets that agent, and the per-agent busy flags reset
with the agent. saveAssignments answers null when superseded, so the
assign dialog doesn't report a save on the wrong agent.
Plan §3 promised these guards ("stale-response guards keyed on agent");
only the gate store had them.
Mutation: dropping the reset turns "the previous agent's outcome line and
card errors do not follow" red; dropping the Run guard turns "a run that
answers after the switch…" red; dropping the load guard turns the store's
"a load" red. Restored byte-identical.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…swered (ent#754)
The Shared section counted itself loaded on the platform-wide library
status, which survives an agent switch, and every card was drawn before the
gate map answered. So, until (or instead of) a real answer:
- a switched-to agent read "No shared skills yet" with an Assign button
whose draft started empty, and saving it replaced the real list;
- a library skill the agent holds was shown under Own as "from library: the
next sync removes it";
- a failed assignments or library read was invisible ("The library is
configured but has no skills yet");
- every approval toggle read "off", and turning one on wrote the default
approver over an existing gate. A failed gate-map read hid every gate line
from everyone, so the AC "a gated row shows its gate to everyone" failed
silently.
The store now records sharedLoaded (status, assignments, the library list,
then the sets read) per agent. The sections draw once the reads they are
built from have answered or failed: Own needs the list, the assignments and
the gate map; Shared needs the assignments and the map. A skill therefore
never moves between sections, and the sets and enforcement lines land in the
same paint as the cards. A failed read says so: Shared gets LoadFailed with
Retry; a failed gate map gets a line for everyone, with Retry, and the
toggles are held. Assign, Manage sets and Sync wait for the assignments.
Also:
- a running agent that isn't answering yet offers "Check again", since
nothing else re-asks;
- a failed refresh of the own list keeps the list and names it (the
stale-banner copy, with Retry);
- a 200 that is not a listing is a failure, never an empty list (plan §3,
row 5).
Mutation:
- the old sharedView gate turns the failed-library case red;
- the unguarded leftover rule turns the pure case and the failed-assignments
mount red;
- dropping the malformed-list check turns its case red;
- an Own view that doesn't wait turns the two waiting cases red;
- unheld shared toggles turn the pure case red.
All restored byte-identical.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rrent default (ent#754)
- A skill whose frontmatter name differs from its directory can be gated
under the directory; the card found that gate, but every write used the
name. Turning approval off sent DELETE /skill-gates/<name>, so the toggle
sprang back on, and changing the approver created a second gate. An
existing gate is now changed and cleared under its own key. Only a new
gate takes the name: that is what a request types, so the request-time
check sees it, and the in-agent check matches either key.
- The kind that turning approval on sends was fixed when the card was
drawn. After the map was re-read (every write re-reads it), a card could
still send a kind nobody fills: the bodiless-reset case S7 was meant to
prevent. The default is now computed from the current map; a pick holds
the select only until its write settles.
- Every approval switch was announced as "Requires approval". Each is now
named for its skill. BaseToggle lets an explicit aria-label name the
switch beside a visible label; its only aria-label caller has no label,
so it is unchanged.
- The enforcement line names every hook state in words. The raw code
("(not_root_owned)") is on hover only.
- The "via <sets>" badge, the one that can grow long, comes last with its
full text on hover, so it can't push "name conflict" or "failed" out of
the fixed badge area.
- Five gate-store fields that nothing outside its own spec read are gone.
Mutation: the name-keyed own gate turns the pure key case and both
dir-keyed mount cases red; a default frozen at draw time turns "the default
follows the map when it is re-read" red. Restored byte-identical.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the safe action (ent#754)
Design principle 19: destructive actions restate the consequence and focus
the safe action first.
- A card's Unassign removed the library skill in one click, and with it the
skill's explicit approval gate (the server drops it on unassign). It now
asks first ("Unassign /x?") and says the approval requirement goes with
it when the skill is gated. The dialog opens on Cancel. The old tab's
untick-then-Save flow had no confirm either, but it took a second
deliberate step.
- The details dialog put initial focus on "Unassign library skill". The
Manage sets dialog, which now holds AgentSkillSets, put it on the first
"Unassign set". Both buttons are now marked data-destructive, so focus
lands on a safe control.
Mutation: wiring Unassign straight to the write turns the confirm case
(and the switch case that goes through it) red; dropping either
data-destructive turns its dialog's focus case red. Restored byte-identical.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lter (ent#754) Two lines carried their behaviour with no test that could fail without them. Each surviving mutation was found in review. - `?probe=true` is honoured only for a person. can_manage_agent_skills alone admits a system-scoped key, and an agent key that holds skills.manage on an agent its owner owns. Forcing is_person_principal to True left all 26 gate-map tests green. Both keys are now driven through the real route: hook stays null and no /health call is made. - A run someone else approved is dispatched under the same UNIQUE dispatched_execution_id as a self-approved one, and only the self_approved state means "ran without approval". Dropping that filter left all 9 execution and Workspace tests green. A dispatched record, written through the real create, claim and transition calls, now reads (False, False) on the list and the detail. Mutation: each turns its new test red (3 cases), restored byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…gs and the library contract (ent#754) Listing proxy (services/agent_skills_listing.py and its three readers): - A container removed between the lookup and the reload raised Docker's NotFound inside each route's catch-all. Its text (the Docker API URL and the container id) reached the caller, including the anonymous public link. It is now a plain not_found: 404 "Agent not found" on the agent page, 503 "Agent is not running" on the public link. - Every httpx transport failure is now "unreachable". A connection dropped mid-answer (ReadError, RemoteProtocolError) was a 500 that last_known could not cover. - The connector keeps its own "Agent error: …" wording. A body that is not a listing is a 500 again, never "no skills". - Two `except HTTPException: raise` branches nothing could reach are gone. Self-approved flags: - The batch read is chunked (500 ids per statement). The page is the caller's unbounded `limit`, and SQLite and PostgreSQL both cap bound parameters; one oversized IN would raise and fail open to "no run is marked". - Workspace: a machine principal (a system key on the platform session, is_person false) sees the fact but is never "you", as on the executions path. The history route passes principal.is_person. - Workspace: the synchronous fallback (POST …/chat) now answers both flags. `_persist_reply` computes them, PortalChatResponse declares them, and the client reads either spelling through portalUtils.turnGateFlags. That closes the gap C9 had named, as plan C9 said: "the completion payload carries the flag". Hook probe: an unexpected failure (not httpx, not JSON) is still `unknown`, but it is now logged at debug with exc_info, so a fault that recurs on every call leaves a trace. Library: GET /skills/library builds SkillInfo field by field and never named `approval`, so it was always null over REST. A Shared card's "author recommends approval" (AC8) had no source for a stopped agent or an older image. Mutation: each fix reverted turns its own new test red (11 cases across the cache, gate-map, executions, Workspace and #2580 suites), restored byte-identical. 55 neighbouring backend files: 1,238 passed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…orkspace architecture (ent#754) - requirements §22.1: the gate-write key and the live default approver; nothing drawn from an unanswered read, with each failure named; Unassign asks first; an agent switch carries nothing over. - skills-tab.md: a "Loading, failures and agent switches" subsection, the new and extended test rows, the Workspace fallback gap removed from the known limits, a revision row. - architecture/workspace.md: the synchronous fallback answers the self-approved pair, and "you" is for a person only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A tab that outlives an agent switch carries its component state, every async answer and its "loaded" flag across unless each is keyed to the agent. This extends the 10-07 write-then-reload fragment to reads, view state and the loaded flag. - A field declared on a response model is still dropped by a route that builds the model field by field. That is the third shape of the #2580 / ent#2320 allowlist class. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Operator ruling (review round): beside a run's source_user_email, "ran without approval" tells the reader that this person fills the gate's approver kind. On enterprise that names a role holder. The gate map already withholds `set_by` from machine keys for this reason (#715). An agent, MCP, connector or system key now reads both flags false: - self_approved_flags returns no flags for a non-person principal (executions list and detail, so MCP list_executions / get_execution); - annotate_self_approved_turns takes viewer_is_person, and the Workspace history read and the synchronous reply pass the caller's. People who can see the agent (owner, admin, shared users) still see the marker, and "you" only on their own runs. Mutation: dropping either branch turns its machine-key test red (3 cases: executions, Workspace history route, #2580 sync reply). Restored byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…w round (ent#754) - requirements §22.1, skills-tab.md and architecture/workspace.md state the people-only rule for the self-approved marker. - 75 `path:line` citations in docs/memory are remapped from the C8-era files (f51472b) to the current ones: - 44 explicit citations, each resolved to its full path first, so a shared basename (models.py, service.py) is never guessed; - 26 relative `:N` citations, resolved to the file named before them in the same section; - five citations into the listing service, where that heuristic picked the wrong file, corrected by hand. Citations into other files, and those already out of range, are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…g internal (ent#754) cso-diff 2026-10-08, finding 1 (LOW, independently verified). The listing service reloads the container, and the public link has done so only since this branch. Only Docker's NotFound was mapped, so any other Docker failure (a daemon 500, a dropped socket) reached the routes' catch-all. Its text went out in the 500: the Docker transport and API version, the full container id, and the daemon's own message. That reached the unauthenticated public link too. A caller cannot cause the fault, but the leak was new. - fetch_live maps any other reload failure to `unreachable` (503 "Could not read the agent's state", logged with the cause). A kept copy is served for it, as for an agent that is not answering. - The public link's catch-all answers a fixed sentence and logs the cause. An anonymous caller never gets an exception's own text. Mutation: narrowing the new except turns the two daemon-fault cases red; echoing the exception again turns the public fixed-sentence case red. Restored byte-identical. Public-link suites: 135 passed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Daily gate, diff scope 51b01c7..7ab9439: - one LOW finding: Docker daemon text echoed by the listing's reload, including on the anonymous public link. Independently verified, fixed in-branch (852fcc4), mutation-proven; - 11 security guard suites green (277 tests); - no secrets, invisible Unicode or v-html in the added lines; - coverage gaps named: no Docker pass, no PostgreSQL run, and the live end-to-end run pending /verify-local. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…(ent#754) Run on the merged Skills tab sends `async_mode: true`, so the Tasks tab opens on the run while it is still going. TasksPanel read its rows once on mount and its 5 s poll refreshed only the queue chip, so the row read "running" until Refresh (found on the localhost eyeball). - The poll re-reads the list while any loaded row is queued, running or pending_retry, and stops when none is. It never stacks a read on one still out, and it holds off while a task typed into the panel is awaited (its local row stands for it; a read meanwhile listed it twice). - Only the latest read is applied, so a slow read for the agent the KeepAlive'd page left is dropped. - The highlighted row is opened and scrolled to once, on the first read that answers. Before, every load re-opened it. - A row whose status changes drops the details read while it ran; the open one re-reads them in place, so its result shows where it is. Mutation-proven (tasksPanelRunRefresh.spec.js, 14 tests): dropping the re-read reddens 8, and each guard reverted reddens its own test (highlight once, typed-task hold-off, latest-read guard, no stacking, open-row re-read, cache drop, the in-flight status set). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… card row (ent#754) The Skills card's approver picker sits in a 40px row beside a 20px toggle and sm buttons. The `field` recipe is a form field, 38.3px in Chromium, edge to edge of that row; `ghost` is the composer's 44px box (it overflowed the row) and drops its border, so a disabled picker read as faint text. The approved card design draws a compact bordered select. `size="sm"` sizes the `field` recipe to BaseButton sm's box: 12.5 ink, padding 4x10, ghost's 12px chevron at right 8px (28.8px measured). Both sizes come from one builder in fieldClasses.js, so they differ in the box only, and FIELD_CLASS (BaseInput, BaseTextarea) is byte-identical. `ghost` takes no size. Cataloged in design-system.md §5 and the contract. Mutation-proven (baseSelectSize.spec.js): ignoring the size, or keeping the md chevron, reddens the sm test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From the localhost eyeball:
- The Own meta line read "From .claude/skills, .claude/skills". The
agent server scans /home/developer/.claude/skills and ~/.claude/skills,
one folder in the container (HOME=/home/developer, USER developer).
The store now names each folder once (every image), and the agent
server scans and names it once (new images).
- The Unassign confirm read as already done ("is removed"). It now says
the library skill will be removed, along with its approval requirement
when the card is gated.
- The approver picker uses BaseSelect size="sm" (previous commit), so it
sits inside the 40px approval row; disabled, it is still a bordered box.
- "Own skills 0" sat above a kept-gate card: the count now counts every
card the section lists.
- The not-synced line again says statuses appear after a sync (the
Shared cards' delivery badges come from the last sync).
Flow docs follow, and their stale citations are corrected (onEditRun,
onSetGate/onClearGate, showOwnerRow/showManage, and skill-assignment's
Unassign, Manage sets and Sync now rows).
Mutation-proven: each change reverted reddens its test
(skillsTab.mount.spec.js: folder once, both Unassign wordings, the
picker's size and its ghost alternative, Own count, sync hint;
test_ent754_agent_server_skillinfo.py: the folder scanned once).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Tasks tab's own flow states the eyeball-round poll: re-read while a row is queued / running / pending_retry, only the latest read applied, the highlighted row opened once, a settled row's details re-read. The loadExecutions excerpt and its line citation follow the code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ent#754) `test_the_attribution_reaches_both_rows` pins `portal_chat`'s call to `_persist_reply(..., voice_call_id, execution_id)` with its closing paren. The review round passes `viewer_is_person=` after the execution id, so the literal no longer matched and the test failed in /verify-local's stage 1 (it was outside the neighbour runs). The pin now ends at the execution id, as the same test's `_persist_user_turn` pin already does for #3265's `attachments=`. Still bites: dropping the call id or the execution id from the reply write reddens it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ent#841 and ent#838 moved lines in PortalConversation.vue, portalUtils.js, client_portal/router.py and service.py, database.py, db_models.py and db/schema.py. The 15 citations this branch wrote into those files point at the same code again (each checked: the cited line's text is the same before and after the merge). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
webmixgamer
added
the
ui
vybe
added a commit
that referenced
this pull request
Oct 9, 2026
…orable secret value is a named 422 (#3325) (#3417) Fixes #3325 — independent fix, one of ten small bug fixes in the trinity-pm `chain-easy-1008` run. Base is `dev`. **Draft until the operator merges.** ## What - `src/backend/services/credential_encryption.py::decrypt`: every malformed envelope shape raises `ValueError`, as the function promises — JSON that is not an object (`[]`, `null`), and a non-string nonce or ciphertext. Importing such a `.credentials.enc` now returns **400** instead of a 500 with a stack trace in the log. The envelope format and every successful decrypt are unchanged. - `src/backend/services/secret_settings.py::encrypt_secret_setting`: the key is checked first, in its own `try`, and only that check can raise `MissingEncryptionKeyError`. A value that cannot be encoded (for example a lone surrogate) raises the new `SecretSettingValueError(ValueError)`. Its message names the setting, never the value. - `src/backend/error_handlers.py` + `src/backend/main.py`: one app-level handler maps `SecretSettingValueError` to **422**. `src/backend/routers/settings/credentials.py`: three pass-through lines in the routes whose catch-all would have turned it into a 500 (Anthropic, GitHub PAT, `PUT /slack`). - Tests: the three strict-xfail markers in `test_ec_credential_crypto_edges.py` are removed; new `tests/unit/test_3325_credential_error_types.py` (registered in `tests/registry.json`) asserts no value in the message or the exception chain, a missing key reported before a bad value, `PUT /api/settings/api-keys/anthropic` → 422, and that `main.py` registers the handler. ## Rulings carried (orchestrator, on the operator's behalf — plan file) - TD-1: one app-level handler plus the three pass-through lines — as recommended. - TD-2: a `.credentials.enc` that decrypts correctly but whose contents are not an object is deferred — as recommended; forging one needs the platform key. - TD-3: 422, matching the existing 422 for refused cleartext writes — as recommended. ## Review + security `/review` reading pass (claude-fable-5-1, report-only): **MERGEABLE**, no critical findings. The secret cannot leave through the new error: it is raised after the `except` block, so `__context__` is cleared, not merely hidden; the handler returns only the message; nothing logs it; audit rows are written only after a successful write. The only residual carrier is traceback frame locals, which nothing in the backend renders. The handler is keyed on the subclass, so no other `ValueError` is swallowed or relabelled. `/cso --diff`: no findings. Evidence sweep (claude-opus-5-5): **GREEN**. 23 unit files import the touched modules; the 16 most direct ran shuffled (seed 12345), one file per process, all passed. All 7 tests in the new file ran, 0 skipped. With both service files taken from `dev`, exactly the three formerly-xfail tests fail (8 cases). Seven less direct files are left to CI and named in the review file. `git merge-tree` against `dev` f6bedcd is clean. ## Tests Targeted counts above. Not run here: the full unit island (CI). ## Before merge - Startup auto-import (`lifecycle.py`) on a malformed envelope now lands in its `ValueError` retry arm: it retries without a warning per attempt and ends in the same "failed" result, with the existing error log after the last attempt. - Enterprise callers of `decrypt` get `ValueError` where they got `AttributeError` / `TypeError` on a malformed envelope. Two catch broadly, three propagate as before; no change needed there. - The new test file has a module-level `importorskip` for `sqlalchemy` and `cryptography`: it skips silently where those are missing. They are present on the engineer and in CI. - Ten open PRs share `tests/registry.json` or `src/backend/main.py` with this branch (#3412, #3411, #3407, #3304, #3271, #3256, #2984, #2956, #2713, #2709). A test-merge of this head against each adds no conflict beyond what that PR already has with `dev`. ## Handoffs (not in this diff) - TD-2 above: the decrypted-but-not-an-object case can still 500 on import. - `PUT /api/settings/slack` saves the client ID before a later secret write fails (partial update, pre-existing). - The GitHub PAT, Resend and Gemini routes are covered by the app-level handler but not exercised by the new route test. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
vybe
added a commit
that referenced
this pull request
Oct 9, 2026
…L is refused, not stored (#3323) (#3420) Fixes #3323 — independent fix, one of ten small bug fixes in the trinity-pm `chain-easy-1008` run. Base is `dev`. **Draft until the operator merges.** ## What - `src/backend/utils/url_validation.py::reject_embedded_credentials`: strips its input and treats a leading `//` as already having a host, the same rule `strip_url_credentials` uses. `//<token>@github.com/o/r` is now refused like the `https://` form, including with leading whitespace or mixed case. All three write paths that store a skills-library URL go through this one function (`routers/skills.py` create and update, legacy adoption in `services/skill_service.py`), so the token is no longer saved in plaintext to `skill_sources.url` or the audit details. - If `urlparse` refuses an input that starts with `//`, the function falls back to the module's existing userinfo regex instead of raising, so this fix adds no new 500. - Tests: the D3 strict-xfail marker is removed; four more refused shapes and a four-case "must not raise / must not over-reach" test are added. ## Rulings carried (orchestrator, on the operator's behalf — plan file) - TD-1: the fallback applies only to inputs starting with `//`; the bare `ValueError` for every other malformed input is #3322's and is unchanged (D4 stays a strict xfail) — as recommended. - TD-2: keep the username/password check; `//@host` carries no secret and is not refused — as recommended. - TD-3, TD-4: deferred, see Handoffs — as recommended. ## Review + security `/review` reading pass (claude-fable-5-1, report-only): **MERGEABLE**, zero critical, five informational. The pre-fix and post-fix modules were executed side by side over forty inputs: nothing that was refused before now passes, credential-free inputs are unchanged, the fallback regex is anchored and linear. `/cso --diff`: no findings. Fixed after review: - `546f329a` — the `tests/registry.json` descriptions no longer call #3323 a strict xfail. - `a75a5e04` — the reject stub in `test_ent183_skill_packages.py` mirrors the fixed `//` rule. It is deliberately stricter than production on a malformed bracket (`//[oops/x` raises). Evidence sweep after the fix (claude-opus-5-5): **GREEN**. 28 unit files import the module; the 14 most direct ran shuffled (seed 12345), one file per process. Three did not come back clean, none because of this branch: - `test_ent236_skills_lifecycle.py::TestNonRepoDirectoryRecovers::test_real_repo_still_pulls` and `test_skill_service_user_agent.py` (collection error) fail the same way on `dev` f6bedcd in the engineer's environment. - `test_ent237_skill_sources.py` hit the 240 s per-file limit at about 72 % with no failure printed — left to CI. `git merge-tree` against `dev` f6bedcd is clean. ## Tests 895 passed across the 11 files that completed cleanly. Not run here: the 14 less direct importer files, `test_ent237_skill_sources.py` to completion, and the full unit island (CI). ## Before merge - Ten open PRs share `tests/registry.json` with this branch (#3417, #3412, #3411, #3407, #3304, #3271, #3256, #2984, #2956, #2709). A test-merge of this head against each adds no conflict beyond what that PR already has with `dev`. - One stale docstring remains at `tests/unit/test_ec_url_validation_properties.py:36` (still calls D3 a strict xfail). ## Handoffs (not in this diff) - TD-3 (P3): shapes that still pass both reject and strip — `/\t/tok@`, `/\n/tok@`, `///tok@`, `https:/tok@` (`urlparse` removes tab and newline after the check). Fixing it needs the same change in `strip_url_credentials` and cases in both test families. - TD-4: a property test that reject and strip always agree over the shared `test_2052` corpus; it needs exceptions for empty userinfo. - #3322 (bare `ValueError` / `UnicodeError` → 500) is the sibling issue in the same file and is untouched. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
vybe
added a commit
that referenced
this pull request
Oct 9, 2026
… that way (#3134) (#3422) Fixes #3134 — independent fix, one of ten small bug fixes in the trinity-pm `chain-easy-1008` run. Base is `dev`. **Draft until the operator merges.** ## What - `src/backend/services/skill_packaging.py::extract_contract`: `user_invocable` is read with the existing `_first_present(sources, ("user-invocable", "user_invocable"))`. A library skill marked `user-invocable: false` (the Claude Code frontmatter spelling, the only one the agent server reads) is now reported `user_invocable: false` on `GET /api/skills/library` and MCP `list_skills`. The wire name stays `user_invocable`; the underscore spelling keeps working. - Precedence: a `trinity:` block wins over both flat spellings, whichever spelling it uses; within one level the hyphen wins; a present-but-null key does not hide the other spelling; a non-bool still reads as `True`. - Docs: `docs/memory/feature-flows/skill-injection.md`, `docs/memory/requirements/skills.md`, and `docs/user-docs/automation/skills-and-playbooks.md` (the user doc taught the underscore spelling, which the agent server never reads). - Tests: eight cases in `tests/unit/test_ent183_skill_packages.py`. ## Rulings carried (orchestrator, on the operator's behalf — plan file) - TD-1: `_first_present` over the sources, **not the issue's suggested one-liner** — as recommended. The one-liner reads the merged dict, so a flat hyphen key would beat a `trinity:` block that uses the underscore, breaking the acceptance criterion "a `trinity:` block still wins". The sweep confirmed it: with the one-liner in place, two of the eight cases fail. - TD-2: real booleans only — as recommended. - TD-3: the same cross-spelling flaw in `allowed-tools` stays out — as recommended; see Handoffs. ## Review + security `/review` reading pass (claude-fable-5-1, report-only): **MERGEABLE**, zero critical. Every reader of the platform-side flag is display or catalog only (Library → Skills chip, the agent Skills tab chip, MCP `list_skills`). Everything that gates execution, connector exposure or portal hints reads the agent server's own parser, which this branch does not touch; assignment never consults the flag. `/cso --diff`: no findings. **Correction to the issue (review I1):** on the library's current tip only `project-intake` carries `user-invocable: false`. `adjust-playbook` is explicitly `true` and `skill-builder` has no key. One chip flips, not three. Fixed after review: `d44b207a` — I2, the user-doc line. Evidence sweep after the fix (claude-opus-5-5): all 10 unit files that import `skill_packaging` ran shuffled (seed 12345), one file per process — 387 passed, 1 failed. The failure is `test_ent236_skills_lifecycle.py::TestNonRepoDirectoryRecovers::test_real_repo_still_pulls` (`'cloned' == 'pulled'`). It fails the same way on `dev` f6bedcd on the engineer, without shuffling, and it tests `skill_source_clone`, which this branch does not change; it looks environment-dependent (real git against the test's empty fake `.git`). The builder marked the sweep RED on principle; nothing this branch touches is red. With `dev`'s parser, four of the eight spelling cases fail. `git merge-tree` against `dev` is clean. ## Tests Counts above. Not run here: the full unit island (CI). ## Before merge - **Merge-order call:** this branch conflicts with open PR #3412 (`feature/754-skills-tab-merge`) in `docs/user-docs/automation/skills-and-playbooks.md` — a one-line edit here against that PR's rewrite of the same section. Whichever lands second keeps the hyphen spelling `user-invocable:` on that line. #3412's edits to the two `docs/memory` files merge cleanly, as does #3420's edit to the shared test file. - Read CI's backend run for `test_real_repo_still_pulls`: if it is green there, the failure above is the engineer's environment only. ## Handoffs (not in this diff) - `allowed-tools` (`skill_packaging.py`, around line 208) has the same cross-spelling flaw. - `abilityai/trinity-skills` `tools/validate.py` runs a copy of this parser pinned by SHA. It asserts only on `frontmatter_invalid*` warnings, so nothing it expects changes; the library picks the new behaviour up when that pin moves. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Contributor
|
Heads-up: #3422 ( |
vybe
added a commit
that referenced
this pull request
Oct 9, 2026
…t the database (#3385) (#3421) Fixes #3385 — one of ten small bug fixes in the trinity-pm `chain-easy-1008` run. **Stacked on #3414** (the fix for #3314): base is `feature/3314-queue-fingerprint-falsy`, because both issues edit `_clamp_ingested_item` and the fingerprint helpers. Merge #3414 first, then retarget this to `dev`. **Draft until the operator merges.** ## What - `src/backend/services/operator_queue_service.py`: new `OPERATOR_QUEUE_TYPE_MAX = 64` and `_bounded_type`. An agent-written `type` longer than 64 characters is shortened with the existing `…[truncated]` marker at ingest (`_clamp_ingested_item`); the ask is never refused or lost. `_comparable_type` applies the same bound, so a shortened row does not read as a rewrite (#2915) and a row stored before this fix at full length still compares equal. - `src/backend/db/operator_queue.py::_insert_values`: `_DB_BELT_TYPE_MAX_BYTES = 1024`; above it the insert raises `ValueError`, like the id / title / question / context belts. All three create paths go through it. - Docs: five lines in `architecture/api-endpoints.md`, `feature-flows/operating-room.md` and `requirements/security.md` that list the clamp and belt fields now name `type`. - Tests in `tests/unit/test_1632_operator_queue_caps.py`: an oversized type is shortened with the marker; every platform-emitted type and every type `ask_operator` accepts is unchanged; the belt rejects an oversized type and passes 64 four-byte characters; `test_3385_clamped_type_is_not_a_rewrite` (clamped row vs raw entry, legacy full-length row, a real rewrite inside the first 52 characters). ## Rulings carried (orchestrator, on the operator's behalf — plan file) - T1: a fixed constant, no environment variable — as recommended. - T2: 64 characters — as recommended. The longest type the platform emits is `workspace_problem_report`, 24. - T3: the DB belt raises, like its siblings — as recommended. - Orchestrator: stacked on #3314 instead of branching from `dev`, so the two fixes do not hand the operator a conflict in the same two functions. ## Review + security `/review` reading pass (claude-fable-5-1, report-only, against the stacked base): **MERGEABLE**, zero critical. Census of every literal `type` the platform and the enterprise submodule pass to the insert paths: longest is 24 characters, none is altered. Every truncated value ends in the marker, which no set member can match, so truncation cannot move a value into or out of a set that decides control flow (`_BUDGETED_ALERT_TYPES` and neighbours). The new `ValueError` is reachable only from the file sync loop, which quarantines it under its existing handler. `/cso --diff`: no findings; the error text carries the constant, never the value. Fixed after review: `eab91c10` — I1, the five doc lines. Noted, not changed: I2, a non-string `type` over 1 KiB now raises the named `ValueError` where the base failed at the bind — same quarantine outcome. Evidence sweep after the fix (claude-opus-5-5): **GREEN**. 13 operator-queue files ran shuffled (seed 12345), one file per process: 836 passed, 7 xfailed. With both production files taken from the #3314 branch, the two `TestClampType` cases, the belt test and the clamped-row fingerprint case go red. `git merge-tree` against `dev` f6bedcd is clean; the base branch has not moved (`923b1edc`). ## Tests Counts above. Not run here: `test_ent815_queue_walk`, `test_ent815_broad_list_agent_scope`, `test_ent751_gate_entries` (2–4 minutes each; they ran green on the #3314 base in that PR's sweep) and the full unit island. ## Before merge - **This PR's checks are weaker than a dev-based PR's:** a PR whose base is a feature branch runs a reduced CI set and `backend-unit-test` does not run. The first full run happens when it retargets to `dev` after #3414 merges — read that run before merging. - Accepted trade-off: an agent that rewrites an oversized type only after character 52 is not detected as a change. Titles and questions already behave this way. - Thirteen open PRs share a file with this branch (mostly `tests/registry.json` and `docs/memory/architecture/api-endpoints.md`; #3256 also `db/operator_queue.py` and the three docs). A test-merge of this head against each of #3420, #3417, #3412, #3411, #3407, #3304, #3271, #3256, #2984, #2956, #2713 and #2709 adds no conflict beyond what that PR already has with `dev`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Brings the branch up to ed59049 (12 commits, incl. #3406 public-link sessions, #3422 `user-invocable:` in the library, #3418 safe_yaml). One conflict, in docs/user-docs/automation/skills-and-playbooks.md: this PR's prose kept, with the `user-invocable:` key spelling (the hyphen is the only spelling the agent server reads). The Skills flow's Run table now names the listing field and the frontmatter key apart. PublicChat.vue merged cleanly with #3406 (this PR's "/ for skills" placeholder kept). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…(ent#754) PR review (#3412). SkillAssignModal seeded its draft once and reset it only when the agent's individual assignments changed. The Skills tab outlives an agent switch, and two agents with the same assignments (most often none) gave that watcher nothing to see: a tick left unsaved on one agent was offered on the next, and Save wrote it there. The draft now resets whenever the dialog opens; closing without saving discards it. A draft still survives a sync while the dialog is open. Mutation-proven: without the reset, the new skillsTab.mount.spec.js case (the reviewer's repro) fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PR review (#3412). AgentDetail is KeepAlive'd, so leaving the page deactivates TasksPanel without unmounting it, and its 5 s interval kept running. This PR added a list re-read to that interval while a run is in flight, with no bound: a stuck pending_retry row kept it going, each read carrying the self-approved lookup (measured: 12 list reads in a minute away). The poll now stops on deactivate and starts again on activate; the queue chip's poll, which already leaked on dev, stops with it. Mutation-proven (tasksPanelRunRefresh.spec.js, mounted under a real KeepAlive): dropping either hook fails the new test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ault serves the kept copy (ent#754) PR review (#3412): - The public link handed an agent's non-200 body (a traceback, a path) to the anonymous visitor word for word, while its comment promised a fixed sentence. It now keeps the status and answers "The agent could not list its skills", logging the body. `public_view` is an allow-list for the top level too (`skills`, `count`, `skill_paths`): a key the agent server adds, or an entry that is not a skill, never reaches a visitor, and a 200 that is not a skills list is a 502 with the same sentence. - The container lookup collapsed "no container" and "Docker unreadable" (`get_agent_container` swallows every error), so a daemon fault read as "agent gone": a 404, never the kept copy the comment promised. The lookup is now `docker_service.agent_container_state` (tri-state, a fresh read, #2196's class): a fault is `unreachable` and serves the kept copy; Docker's own text still reaches no caller. The reload step it replaced is gone. - Two fixes had no test that failed without them: the stopped-agent tests for the public link and the connector now keep a copy first. - The listing's docstring and backend.md no longer call it the one place that reads the container's skills (the Workspace's read is the known gap). Mutation-proven (test_ent754_skills_list_cache.py): echoing the agent's body, the public link or the connector reading `last_known`, a collapsed lookup, a top-level passthrough and a non-listing passthrough each fail their own test. The daemon-fault and missing-container cases drive the real `agent_container_state` through a fake daemon. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ved (ent#754) PR review (#3412): deleting `loadAgentList`'s guard kept the suite green. Two store cases now pin it: a slow listing for the previous agent never replaces the next agent's, and it does not land before the next agent has asked for its own (the case only the agent clause catches; the sequence number alone does not move on a switch). Mutation-proven: removing the guard fails both; keeping only the sequence check fails the second. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…y included (ent#754) PR review (#3412): the PR said agent, MCP, connector and system keys always read the self-approved flags false, but `self_approved_flags` gates on `requester.is_person`, and `PERSON_SCOPES` is {None, "user"}: a person's own user-scoped MCP key reads what they read in the browser, the same line `enforce` draws to let that key self-approve. The code is kept (operator decision); the docstring, requirements §22.1, the skills-tab and skill-gate flows now say so, and the `viewer_fills` notes with them. The requirement also states that people the agent is shared with read the marker (the review's design note, accepted). A new test pins the decided line: the approver's user-scoped key reads (True, True), another person's (True, False). Narrowing the gate to browser sessions fails it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PR review (#3412): the probe passed 3 s to httpx, whose timeout applies per phase, so an agent trickling its `/health` answer held one probe for 10 s. The request now runs under `asyncio.wait_for(..., 3.0)`; httpx keeps its own timeout as well. A timeout reads `unknown`, as before. Mutation-proven: without the cap, the new slow-agent test fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PR review (#3412): switching to an agent in another state fired both the agent and the status watchers, so the list was read and the in-agent check probed twice (the router promises one `/health` read per tab load). A third watcher, on the skills-changed tick, fired too when the next agent had a tick of its own, and it ran before the store switched, so it re-read the agent being left. One watcher now takes the agent and its state together (a switch is one load; a start or stop on the same agent re-reads and re-probes), and the tick watcher ignores a switch. Mutation-proven (skillsTab.mount.spec.js): the separate status branch on a switch, the tick watcher without its switch check, and a start/stop without its probe each fail a test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…m is cataloged (ent#754) PR review (#3412), design system: - SkillsTab's inline text links (Retry ×3, Check again, "+N more", the admin's Settings link) are hand-rolled, not BaseButtons, and carried no focus-visible ring. They now share one constant with BaseButton's ring recipe. - BaseBadge `size="sm"` (11/550, padding 1.5×7, the Skills card's author- mode chip) was added by this PR without a catalog entry, and the contract still read 11.5/550 only. design-system.md §5 and the contract name it now, and a mounted spec pins both sizes. Mutation-proven: dropping the ring from the gates Retry or the stale-list Retry fails its test; rendering `sm` as `md` fails baseBadgeSize.spec.js. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ecorded (ent#754) PR review (#3412): skills-tab.md's citations for `list_skills`, `remember`, `recall`, `forget`, `public_view`, the field constants, `approver_status`, `self_approved_flags`, `hook_status` and the public route had drifted, and public-agent-links.md cited public.py lines for code far below them. Each is re-pointed and checked against the symbol it names, as are the SkillsTab citations the watcher and focus-ring commits moved. Known limits now name the two review items decided against a change: rename clears the last-known copy (deferred), and Shared cards show no file count or size (kept; both are in the Assign dialog). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tchers From the #3412 review round (trinity-enterprise#754): an interval opened on mount kept running while the KeepAlive'd page was away, and three watchers on one agent switch each re-read, one of them for the agent being left. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
Thanks for the review. Everything is addressed at Fix before merge
Claims
Nits
Design note: confirmed with the ruling's owner that shared viewers reading the marker is within the ruling. Requirements §22.1 now says so. Merge: merged Tests at
|
AndriiPasternak31
approved these changes
Oct 9, 2026
AndriiPasternak31
left a comment
AndriiPasternak31
left a comment
Contributor
There was a problem hiding this comment.
Approving. Both blockers are fixed, and I checked them in a real browser this time, not only in jsdom. Every other point is fixed or reasonably declined.
Re-reviewed at 42aaf1b08.
- Frontend: full vitest 5278/5278, and
npm run buildpasses. - Backend: the five ent754 test files pass (110).
- CI: green.
- Merge: the branch merges cleanly with today's dev (
c90c711ee). Theaaf5bcc5cmerge differs from a plain auto-merge only in theuser-invocable:line, which follows vybe's note, and one matching row in skills-tab.md.
Live click-through. I ran this round against a throwaway stack:
- the PR's backend and agent image;
- two
test-echoagents; - the community library, synced;
- this branch's frontend, served by Vite;
- headless Chromium, logged in through the admin form, with every agent switch done as an in-app click so the kept-alive AgentDetail is reused.
at fe5ec98ee |
at 42aaf1b08 |
|
|---|---|---|
Tick add-memory on alpha, Close, open bravo → Assign skills |
already ticked, Save enabled, Save sent PUT /api/agents/bravo/skills |
not ticked, Save disabled, no PUT |
60 s on Library with one in-flight row on alpha (list reads / queue reads) |
12 / 12 | 0 / 0 |
Back on alpha's Tasks tab, 15 s |
n/a | 3 / 3 (the poll resumes) |
Switch from running alpha to stopped bravo |
n/a | playbooks?last_known=true once, then skill-gates plain and with probe (the known gap), and no reads of alpha |
I also ran the old dev panel as a control for the first row. The tick carries over there too, but Save writes to alpha, the agent you just left. So my "the old SkillsPanel did the same" was wrong: dev was worse, and this PR already fixed the target before this round.
Mutation spot-checks. I removed each of these in turn, and each time exactly its new test went red:
- the reset-on-open watcher;
onDeactivated(stopPolling);asyncio.wait_foraround the probe;public_viewreturningNonefor a non-listing answer.
Declined, and fine with me:
- the connector and
agent_fileskeep the agent's text (authenticated, and they behaved this way before this PR); - rename doesn't carry the kept copy;
- the shared card has no file size.
All three are listed in Known limits.
This was referenced Oct 9, 2026
Merged
vybe
added a commit
that referenced
this pull request
Oct 9, 2026
…in: import-line union with #3412)
vybe
added a commit
that referenced
this pull request
Oct 9, 2026
…eature-flows index keep-both with #3412)
3 tasks
webmixgamer
added a commit
that referenced
this pull request
Oct 10, 2026
…erges) into the review round The remote PR branch carried two merge-train merges of dev on top of 37829f0 (#3412, #3428, #3479 and others). One conflict, Settings.vue: - the whitelist add now goes through #3479's settingsStore.addWhitelistEmail, which returns { email, existingAccountRole } so the form can still name an existing account's role (emailWhitelist.spec updated, pass-through case added); - the add's refusals use #3455's InlineError under the form, beside the account notice, and the page-wide error is no longer set (the eyeball of d9473d3: a 409 landed at the bottom of Settings); pinned in workspaceOnlyWiring.spec; - #3479's guarded workspaceOnlyUsers computed, plus this round's filter reset. test_3455's whitelist fake gains the two reads the route now makes. Re-run on the merge: targeted backend 3983 passed (134 files incl. every unit test the merges changed), frontend 5501 passed (3 known host-only files), MCP 829/831 (2 = the host's pnpm layout), CLI suites green. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.claude/skills/; Shared skills are its skill-library assignments. Each skill is one fixed-size card with Run, Edit & Run, the author's mode chip ("runs unattended", "asks mid-run", "start by hand") and the approval line. The legacy?tab=playbookslink lands on Skills.false./menus. API names are unchanged (/playbooks,exposed_playbooks,run_playbook,list_playbooks).Changes
Agent server (
docker/base-image/agent_server/routers/skills.py)GET /api/skillsreports, per skill:source: whether the platform's.trinity-skill.jsonmarker is present;dir: the directory name;approval: a closed set, read with the backend's precedence.Backend
services/agent_skills_listing.py(new) is the one proxy behindGET /api/agents/{name}/playbooks, the public link and the connector read. It adds?last_known=true.docker_service.agent_container_state): a missing container is a 404, while a Docker daemon fault isunreachableand serves the kept copy, never "agent gone". Docker's own text reaches no caller.GET /api/agents/{name}/skill-gatesgains:approvers[{kind, reachable, viewer_fills}];?probe=true→hook, honoured only for a person who may manage the agent's skills.gate_self_approved/gate_self_approved_by_viewer(booleans only) are derived from the existingskill_gate_requestsself-approval records with a chunked read. They are added to:GET /api/skills/librarynow passesapprovalthrough. It was declared but never sent.Frontend
components/skills/(SkillsTab,SkillCard,SkillAssignModal,SkillDetailsModal,ExecutionGateMarker),stores/skills.js,stores/skillGates.jsandutils/skillCards.js.PlaybooksPanelandSkillsPanelare removed.size="sm": a new primitive size, at BaseButton sm's box, so the approver picker fits the 40px card row.fieldvariant only;ghosttakes no size.FIELD_CLASSis byte-identical, so BaseInput and BaseTextarea are unchanged.design-system.md§5 and the contract.Docs
feature-flows/skills-tab.md, which absorbsplaybooks-tab.md, plus pointers from the related flows.requirements/skills.md§22, the architecture area files and the user docs./cso --diffreport:docs/security-reports/cso-diff-2026-10-08-ent754-skills-tab.md. It has one LOW finding (daemon error text), fixed in this branch.Notes for review
docker/base-image/. After merge, rebuild the base image and recreate agents forsource/dir/approval. An older image still works: the directory comes frompath, and there's no "author recommends approval" line.enforcedraws for self-approval. People the agent is shared with read the marker too, which is within the ruling.path, as before.?probe).docs/screenshots/agent-skills.pngstill shows the old tab.Test plan
tests/unit/test_ent754_agent_server_skillinfo.py,test_ent754_skills_list_cache.py,test_ent754_gate_map_viewer_and_hook.py,test_ent754_execution_self_approved.py,test_ent754_workspace_self_approved.py.test_1484_create_agent_characterization.py,test_2580_reply_message_id.py,test_ent551_voice_background_tasks.py.dev(ed5904906): 24,821 passed. The only failures are existing order-dependent families that pass alone (heretest_retention_floor.py;dev's own run failed 19 intest_ent666_objective_join.py) and two environment-specific tests that fail ondevtoo.skillsTab.mount,skillsStoreAgentSwitch,skillGatesStore,skillCards,executionGateMarker.mount,portalSelfApprovedTurn,tasksPanelRunRefresh(including a realKeepAlive),baseSelectSize,baseBadgeSize./verify-local, full (agent stage, global mode, fleet seeder disabled):/health.?tab=playbooksalias, the own card, owner approval, the viewer role, self-approval (Tasks, the execution page and the Workspace), a stopped agent with Check again, the Shared dialogs and the Unassign confirm, agent switching, both themes, and both/menus. The eyeball-round fixes were re-checked and pass.Fixes abilityai/trinity-enterprise#754
🤖 Generated with Claude Code