Conversation
…ity-enterprise#454) Rule #1: requirements before implementation. Records only what lives in this repo — the edition-agnostic membership primitive, the license-blind MCP proxy and the Vue gate — and states plainly that the teardown control plane is an entitlement-gated module in the private submodule (404 on an OSS build). The paid module's surface and internals stay in the private tree (ent#45). Also cross-references §33.1: a teardown is N of that soft-delete composed through the same endpoint, so the 180-day window, name reservation and schedule freeze are preserved by construction rather than re-implemented. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…bilityai/trinity-enterprise#454) `system_member_names` keeps its signature and becomes a thin wrapper; the body moves verbatim into `system_membership`, which additionally returns the two facts the flat name list discarded: * `tags_readable` — the body degrades to the raw prefix on a tag-read failure, argued there for `restart` (over-capture restarts one container too many and logs it; under-capture restarts a subset and reports success). A verb that DELETES inverts the cheap half, so it can refuse on unverified membership instead of degrading — at its own call site, without forking THE ONE predicate into a fourth rule. * `evidence` (tag|prefix|both) — derived from the two sets that already decided membership, so it cannot disagree with them. Makes the prefix fallback's documented residual visible to a caller instead of silent. `SYSTEM_VIEW_AUTO_DESCRIPTION` replaces the inline f-string: nothing persists a system<->view link, so that description IS the identifier for the view a deploy auto-created, and a reader that has to find it again must not restate the literal. Behaviour-preserving, not asserted to be: the #2373 suite (33) and the whole ent#124/125/126 + systems set (325) pass unchanged against the wrapper. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rties (Abilityai/trinity-enterprise#454) #2373's fallback rule was wrong three times and each wrong rule passed the case test written for its predecessor, so the fallback is pinned as a PROPERTY through the new entry point: members ⊇ raw prefix match whenever the tags are unreadable, plus tagged ⊆ members whenever they are readable. Any future narrowing fails by construction, whatever shape it narrows on. Also pinned: the wrapper is a projection of `system_membership` (generated, AND source-anchored — a property cannot see a copied body that happens to agree today), evidence is keyed by exactly the members and true of each, and the auto-view description is asserted THROUGH `create_system_view` rather than by restating the literal. The named incidents stay as discrete cases: the eleven `vc-due-diligence-dd-*`, an agent named exactly `acme`, the sibling-prefixed `acme-api-worker`, and the `acme` vs `acme-extra` floor from the AC. One counterexample found by Hypothesis and fixed as a TEST bug, not a code one: `roster=[]` short-circuits before the tag read and so reports `tags_readable: True` by design — a destructive caller refuses on False, and refusing a teardown of a memberless system is a 503 where 404 is the truth. 17 passed under the derandomized `ci` profile and under `explore` (5000 examples); both lint guards clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…Abilityai/trinity-enterprise#454) Invariant #13's third surface. The tool ships in the OSS bundle and knows nothing about entitlement: it proxies DELETE /api/enterprise/system-teardown/{name} and reports what the route says. `dry_run` defaults to TRUE — the opposite of `deploy_system`, deliberately: an unwanted preview costs a round trip, an unwanted execute costs a deleted fleet. The default lives in tool code, so it is pinned by a test that an `|| false` typo would fail. Refusals degrade rather than throw, and stay DISTINCT: 404 (no such build) / 403 (not licensed, or an agent key on a human-only verb) / 503 (membership unverified — flagged retryable, and says nothing was removed). A thrown error reaches the agent as an opaque transport failure it cannot reason about, and flattening the three is how a human on a user key gets told a licensed feature "doesn't exist". A `failed` report arriving as a 500 is kept as a RESULT — its per-member reasons are the only actionable output. Reads `ApiError.body` rather than regexing `ApiError.message`: the field is retained verbatim for exactly this purpose (ent#443), and its own docstring says the regex is what breaks when the message format changes. Found while writing the test, which constructs a real ApiError rather than a `{status}` stand-in that would have passed over broken body parsing. No server.ts change: createSystemTools is already in toolGroups, so the tool inherits the operatorOnly allow-list. tsc --noEmit clean; npm test 383 pass / 0 fail (364 before). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…inity-enterprise#454) Library -> Systems gains the inverse of install: preview -> per-agent checklist -> acknowledgement -> remove. Gated on `system_teardown` in `enterprise_features`, so an OSS build's Systems section looks exactly as it did rather than offering a control whose route 404s. The preview is a CHECKLIST, not a list, and that is the design rather than decoration: a member whose `evidence` is `prefix` matched by name with no deploy tag to confirm it, so it may belong to a sibling system sharing this one's prefix. The server cannot tell — only the operator can — so the badge plus the opt-out IS the over-capture escape hatch, and the confirmed list goes back on execute to be intersected with freshly-resolved membership. Remove is gated on four conditions, each blocking something the others do not: the preview is current for the typed name, membership was verified, the consequence is acknowledged (ent#126's ack contract reused, not a second pattern), and at least one agent is checked. A disabled button says why. Honest reporting: tone switches on `status`, never the HTTP code (`partial` is 200, `failed` is 500 with the report as the body). skipped / failed / aborted render as three different things with per-reason operator copy, because a refusal is not a breakage and an un-attempted member is neither; `discarded` never claims recoverability. A timeout is "outcome unknown" and offers no retry — the server keeps deleting. `ManifestPreview`'s "there is no un-deploy" now tracks the CAPABILITY: still exactly true where the module is absent. Retiring it everywhere because one edition gained the verb would have been the dishonest fix. `DeployResult` gains a de-emphasised "Remove this system", also entitlement-gated (AC 7). Design contract: token-only (scanner reports 0 non-gray / 0 hex across all six systems files), both themes, primitives only, bounded member viewport with a stated total, no bare loading gate — the in-flight flags reach only BaseButton. Tests: 21 vitest store specs + 25 source-anchored contract assertions. Mutation-checked, and it found a real gap: "the preview is bound to its name" passed with the name comparison replaced by `true`, because `setTeardownName` nulls the preview and the null-check carried the assertion alone. The comparison guards a direct-assignment bypass, so the test now uses one. Frontend suite 2795 passed (2774 before); check:tokens clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…es (Abilityai/trinity-enterprise#454) `system-manifest.md` gains entry-point rows (the gated route, the license-blind MCP tool, the UI's new status) and a "Teardown — the public half" section covering what lives in THIS repo: the membership split and why it is a wrapper rather than a fork, the one-constant auto-view description, the MCP degradation contract, the checklist UI, and the four tests. The gated verb's own design stays in the private tree (#151 doc-follows-the-code). Feature-flows index row added, per the standing rule that a row goes in even when the flow doc already exists. (The "newest ~20" cap on that table is already at 140 rows — pre-existing drift, deliberately not pruned here, since dropping 120 rows would bury this diff.) Area files: mcp-server.md `systems.ts (4)` -> `(5)`, backend.md's `system_service.py` line records the membership split and the constant, frontend.md records the gated half of the Systems section. Public docs carry the seam and the gate's name only — no paid-module internals. Verified by running the enterprise-docs-guard's own grep locally: clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…bilityai/trinity-enterprise#454) `/sync-feature-flows` mapped every changed file to a flow; two changed files belong to a second flow each, and those were the gap: * `mcp-orchestration.md` — the System Tools table and the enumerated tool-count comment both said four. `teardown_system` makes five, with its license-blind degradation contract and its inverted `dry_run` default recorded where the other tools are catalogued. * `library-page.md` — the `#systems` section now hosts two panels, and the second renders nothing without the entitlement. Worth stating there because that page's doc is where someone checks why a tab looks different between editions. The index row's Flow column now names all three flows rather than only `system-manifest.md`. `system_service.py` also appears in `scheduling.md` and `agent-tags.md`, but only as the module that writes schedules and tags — nothing this change touched, so no update. Index is 552 lines against the skill's 400-line guidance. That is the pre-existing Recent Updates table at 140 rows against its own documented "~20" cap; my row is one line of it, and condensing 120 rows would bury this diff. Flagged, not fixed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…y-enterprise#454) Reusing ent#126's acknowledgement contract was right for the prop/emit shape and wrong for the `data-testid`. Install and remove can both be previewed on the same Library page, so a shared id puts two elements at one address — and `e2e/system-install.spec.js` addresses the install ack as `getByTestId('ack-checkbox')` in three places, which would be strict-mode ambiguous on an entitled build. It would NOT have failed CI: the teardown panel is unentitled there and renders nothing, so the collision is invisible to every check that runs today. That is exactly why it gets a test rather than a CI run. Found by reading the spec that shares the surface, while waiting on the e2e job. The pattern is worth sharing; the address is not. `ManifestPreview` keeps `ack-checkbox`; the teardown ack is `teardown-ack-checkbox`; the contract test now asserts BOTH — the shared prop/emit shape and the distinct ids. Frontend suite 2795 passed, check:tokens clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…bilityai/trinity-enterprise#454) The per-id fix in the previous commit was the wrong SHAPE, and looking for a second instance found one immediately: `goto-fleet`, owned by DeployResult.vue and copied into TeardownResult.vue. Two collisions is the signal to write the rule instead of fixing instances. Install and remove are two panels on one Library page, and on an entitled build both can show a preview AND a result at once — so any id they share is strict-mode ambiguous for a Playwright `getByTestId(...)`, which is exactly how `e2e/system-install.spec.js` addresses both `ack-checkbox` and `goto-fleet`. None of it fails CI today: the teardown panel is unentitled there and renders nothing, so the collision is invisible to every check that runs. Hence a test rather than a CI run — `test_no_test_id_is_shared_with_the_install_panel_it_sits_beside` intersects the two panels' id sets and fails on any overlap, so a third collision cannot be introduced quietly. Measured: reintroducing the `goto-fleet` collision turns it red; control clean. Also drops a now-stale docstring line in TeardownPreview that still claimed the shared testid. Frontend suite 2795 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ai/trinity-enterprise#454) `normalizeError` is shared with deploy, and its unknown-outcome copy read "Deployment may still be running — re-deploying creates duplicate agents." On a teardown timeout that is the opposite advice at the worst possible moment: the in-flight work is removal, and the members removed so far are already gone. The two sentences that are about WHAT was in flight move into `UNKNOWN_OUTCOME_COPY`, keyed by verb — everything else in the normalizer is verb-independent, which is why this stays one function. `teardown-preview` gets its own arm because a dry run writes nothing, so neither sentence may imply work in flight. The MCP tool copy is corrected alongside it: the 403 said "an agent-scoped key", which understates a gate that is now an allowlist refusing every machine credential; the 503 said "membership could not be verified", which no longer covers the case where the roster itself was unreadable; and `dry_run` now states that a member with no container is removed the same way, since that is what frees its name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…not (Abilityai/trinity-enterprise#454) Three docs claimed the teardown acknowledgement reuses ent#126's `data-testid="ack-checkbox"`. The code deliberately does the opposite and a guard test requires it to: both panels render on one Library page, so a shared address makes every `getByTestId('ack-checkbox')` strict-mode ambiguous. The PATTERN is what is reused (`:acknowledged` / `update:acknowledged`); the address is not. The flow and roadmap gain the roster-completeness ruling behind the private fix: every OSS sibling on this surface resolves its roster from containers, which is cosmetic for `get_system` / `restart_system` / `export_manifest` and is under-capture-reporting-success for a verb that claims completion — plus why the #2196 tri-state reader is what makes the ownership supplement safe, and why an unreadable Docker refuses rather than 404s. `learnings.md` records C1's class for the next planner: a guard that tests membership against a list the disqualified entries were just appended to is dead for the case its own comment describes, and the narrow test beside it passes either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…base string's (Abilityai/trinity-enterprise#454) BaseButton's shared class string carried `border-transparent` next to the reserved `border` width, while `secondary` — the one variant drawn BY its border — set `border-gray-300`. Both are single-class selectors, so specificity ties and emission order decides: Tailwind emits `.border-transparent` after `.border-gray-300` (rule 729 vs 698 on the live dev stylesheet), and the resting keyword won. Every secondary button rendered borderless in light mode — a white button on a white card with no outline — across 24 call sites. It survived review because dark mode won by accident rather than by design: `dark:border-gray-700` compiles to `.dark .border-gray-700`, one class heavier, which outranks the tie. The theme most of us work in was correct. The fix is the one `fieldClasses.js` already documents for FIELD_GHOST_CLASS one primitive over: the base string keeps the bare `border` (so the focus ring still costs no layout shift) and every variant names its own colour. Measured, not reasoned — resting border on the teardown panel's "Preview removal" control, before → after: light: rgba(0,0,0,0) → rgb(209,213,219) (gray-300) dark: rgb(55,65,81) → rgb(55,65,81) (gray-700, unchanged) Guard: tests/unit/baseButtonVariantBorder.spec.js pins the base string colour-free, asserts secondary is outlined in BOTH themes (the bug was theme-asymmetric, so a light-only pin would let the next edit break dark while staying green), and fails closed on a fifth variant that names no border colour. Mutation-checked: restoring `border-transparent` to the base string turns it red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ilityai/trinity-enterprise#454) BaseInput is a three-part stack — label, control, help — so a flex row that holds one whole aligns against that whole box. `items-end` therefore put the button's bottom edge on the HELP TEXT's bottom rather than the control's, leaving "Preview removal" 22px below the field's centre where it read as a stranded caption instead of the field's action. Fixed by composing the row the way BaseInput documents and the way TemplateRegistryPanel already does it for the same control-plus-action shape: the panel owns the label and the help, and the row holds two control boxes, so alignment means what it says. `items-stretch` rather than `items-start` because the two primitives are deliberately different heights (field py-2 on a 20px line box = 38px; button py-[7px] on 13.5px/1.35 = 34px) and stretching the shorter one costs no magic number and overrides neither recipe. Measured on the live dev stack, before → after: input top/bottom 1295/1333 → 1295/1333 button top/bottom 1319/1353 → 1295/1333 centre delta +22px → 0px BaseInput carries the second half. Its `:aria-describedby` binding is applied AFTER `v-bind="controlAttrs"`, so with no `help` prop of its own it did not leave a caller's `aria-describedby` alone — it ERASED it, and the help paragraph stopped being announced the moment the panel took ownership of it. It now hands the attr back instead of winning a merge the caller never knew it had entered. Strictly additive: error and help still win, in that order, so no existing caller changes. There is no mount harness in this project (no @vue/test-utils, vitest runs environment: 'node'), so the a11y half is browser-verified rather than unit-pinned — on the live render the control resolves aria-describedby="v-1-help" to the help paragraph, and label[for] to "System name". Full unit suite green: 131 files, 2937 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sits in (Abilityai/trinity-enterprise#454) Two h4s in this PR's own files were written without a size class and fell through to the UA default of 1em. On a 16px root that made the DEEPEST headings on the page the LARGEST — bigger than their four `text-sm` siblings and bigger than the h3 that contains all of them: h3 "Remove a deployed system" 14px h4 "Agents to remove" / "Also removed" / "Notes" 14px h4 "This removes running agents" 16px ← TeardownPreview.vue:149 h4 "Outcome unknown" 16px ← SystemTeardownPanel.vue:38 Both are warning callouts whose anatomy is identical to the danger callout at TeardownPreview.vue:8 — bordered box, icon span, heading, list — and that one already carries `text-sm`. So this was an inconsistency inside a single file, not a scale that needed inventing. All five h4s now measure 14px on the live render, matching the h3 they sit under. Scope is pinned to the three components this PR ADDS. Six more unsized headings exist on the install side (ManifestPreview ×2, DeployResult ×3, SystemInstallPanel ×2) and are left alone: they predate this branch, and TeardownResult's banner h3 is deliberately byte-identical to DeployResult's, so changing one half would break a parallel that was built on purpose. They are reported for their own issue rather than folded in here. Full unit suite green: 131 files, 2937 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
# Conflicts: # docs/memory/feature-flows.md # docs/memory/learnings.md
…lityai/trinity-enterprise#454) dev's ent#628 policy table landed while this branch was out. Every registered tool must carry a TOOL_ACCESS_POLICY row, and policyFor checks it AT REGISTRATION — so merging dev left the MCP server crash-looping on boot (ToolAccessPolicyError: tool 'teardown_system' has no entry; nine restarts on a live stack). PR CI never said so: the PR was CONFLICTING, which runs zero checks. The row is `baselined`, not `none`. The siblings can honestly say "a system name, not an agent" (deploy_system, restart_system); this one cannot — it takes the confirmed removal set as `agents` and deletes them. `agents` is absent from AGENT_TARGET_PARAMS, so a `none` row would have passed the shape check while saying something false. The owner names the fence that actually holds the line: the gated route requires role 'creator' AND a human caller. Proof: src/mcp-server npm test 440/440, including access.test.ts's "createServer boots: every registered tool has a row"; rebuilt image boots and stays up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ilityai/trinity-enterprise#454) A fresh teardown preview ticked EVERY member regardless of `evidence`. A member the server matched by NAME with no deploy tag is one it cannot vouch for — and arriving ticked makes the escape hatch an opt-OUT on a verb that deletes. The 503 refusal does not already cover this. It fires only when the tag read FAILED (`membership_verified: false`, Remove disabled). A `prefix` member also appears in the HEALTHY state — tags read fine, that one agent simply has no tag row — where membership is verified, Remove is ENABLED, and the unconfirmable agent was pre-ticked beside it. #2373 ranks under-capture as cheaper than over-capture, which is right for `restart` and inverts for a verb that deletes; that inversion is why teardown refuses rather than degrades everywhere else, and the default handed it back in the one state the refusal misses. The rule moves to the STORE (`teardownDefaultSelection`) rather than staying in the panel's watcher, because `vitest.config.js` pins `environment: 'node'` with no component mounting: a default written into a `.vue` watcher is a decision no runnable test can execute, which is how this shipped. The panel now applies it and nothing else. Found by clicking through a live stack, not by any test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…or an opt-in (Abilityai/trinity-enterprise#454) The rendered half of the pre-tick fix. A `prefix` member now arrives UNTICKED, so its own copy had to stop saying "uncheck it if so" — that sentence describes a box that arrives ticked, and left in place it would have told the operator to undo a safeguard that had not been applied. The short selection is also stated above the list. Without it the unticked rows read as a miscount against the "N of M selected" counter beside them, and an operator who re-ticks them to "fix" the count has re-created the defect by hand. Guards are source-level because nothing renders this component: vitest is `environment: 'node'` with zero mounts, and the green `e2e` runs `system-install.spec.js`, which never touches teardown. They go through a new `_prose` helper that strips comments AND collapses wrapping — the first version of one of them matched its own JSDoc line, which is the trap `_code_only` already existed for, landing a third time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d no retry that cannot help (Abilityai/trinity-enterprise#454) Two defects in one banner. "Try the preview again in a moment" is advice for a transient fault. The condition this renders under is a failed database read — a schema or permission fault that does not clear on its own — so re-previewing produces the identical screen forever, which trains the operator to read a real stop as flakiness. It now says the fault is standing and offers the one thing that does work: the per-agent path the server's own refusal detail already names. The banner also named the TAG read specifically, and it cannot know that. `membership_verified` is `tags_readable AND roster_complete` — one boolean over two independent faults — so on the roster arm the copy was simply false: the tags read perfectly and it is the agent LIST that is incomplete. The banner now states the consequence, which IS identical either way, and quotes the server for the cause. That required routing the warnings. The roster line used to fall through to "Notes", which put the real fault in a footnote underneath a banner naming the other one. Both membership lines are now claimed by the banner while it renders, and `otherWarnings` is defined by SUBTRACTING what the banner took rather than by a second independent regex — two filters that must stay complementary are two chances to drop a line off the screen entirely. Also repairs an existing assertion that broke on the re-wrap: the phrase it pinned was split across two lines without a word changing, so it now goes through `_prose` like the rest. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…Abilityai/trinity-enterprise#454) "The acme tag goes with the agents (2 tagged)" was rendering `SystemTeardownTag.member_count`, which the service sets to `len(members)` — every candidate, tagged OR matched by name. So the word "tagged" overstated the tag the moment any member was `prefix`. In the refusal state it was worse than imprecise: the banner at the top of the same screen says the membership could not be confirmed, and this line then quoted a figure derived from that very read. Two statements about one read, opposite in confidence, on one screen. Dropped rather than reworded. The total the operator actually needs is already stated on the list itself ("N of M selected"), and the sentence's job is the other clause — that the tag is not a separate record to go and delete. The field is not wrong; `member_count` counts members and says so. Only the noun beside it in the UI was. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lityai/trinity-enterprise#454) The flow doc described the old checklist ("the badge plus the opt-out is the mechanism") and said nothing about which boxes arrive ticked, which is the half that was wrong. It now records the opt-in/opt-out split and where the decision lives, why the refusal banner names no fault, how warning routing partitions, and that the tag line carries no count. Tests table and change history updated with it. The ledger entry is the transferable part: a UI default written in a `.vue` watcher is a decision no node-environment suite can execute, so it is reviewed by eye forever — move it to the store and the spec that already exists starts covering it. Plus the three smaller shapes this round produced: a boolean over two faults must name neither, a count rendered with a different noun than the field counts is a lie with a number on it, and warning routing partitions by subtraction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nity-enterprise#454) The preview watcher was not `immediate`, and the store outlives the component. `Library.vue` mounts the systems section lazily and nothing resets teardown state on unmount, so leaving the page and coming back remounts this panel onto a preview that is STILL current — and the watcher never fires for it. The removal set rendered with every box empty and "Select at least one agent", which makes this component's own stated default untrue on the second visit. Pre-existing (the previous all-members default had the same hole) and fail-safe in both directions, which is exactly why it survived: nothing breaks, the panel is just wrong and asks the operator to re-tick a set it had already reasoned about. Found by driving the real panel in a browser — the store spec cannot see it (the rule is correct; the wiring never ran) and neither can a source regex, until one is told to look for `immediate`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…pt-in one (Abilityai/trinity-enterprise#454) The server's prefix warning ends "Uncheck anything that does not belong before confirming" — the opt-OUT instruction this branch reversed. It rendered under "Notes", six lines below the panel's own "tick it only if it belongs", so one screen gave opposite instructions about the same row and the stale half read as the authoritative one, because it came from the server. Suppressed, which is the rule this component's docstring already states: a warning that has a dedicated block above is not repeated as prose. The prefix warning simply had no such block until the short-selection note gave it one, so it was the only member of that family still falling through. The excluded, ephemeral and unverified lines were already suppressed for exactly this reason. SUPPRESSED rather than MEMBERSHIP_FAULT: it is raised in the HEALTHY state too, where no banner renders at all. Visible only in situ — the warning comes from the enterprise module, so it is absent from every fixture in the frontend tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… before it (Abilityai/trinity-enterprise#454) `TeardownResult` rendered `result.warnings` unfiltered, so the server's prefix warning — which ends "Uncheck anything that does not belong before confirming" — appeared under Notes on the RESULT, beneath a report of what had already been removed. Advice for a decision already taken, in the opt-OUT wording the preview stopped using. Only that line is dropped. Every other warning is a statement of fact about what happened (agents excluded, members with no container, ephemeral members with no recovery window) and still reads correctly after the event. The preview gained this filter in 581a531; this component did not, because nothing renders either of them in CI and the preview is the one a source guard had been written against. Found by tearing a system down for real on a live stack. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…name (#2914) Assigning a library skill whose name matched a `.claude/skills/<name>/` the agent wrote itself overwrote the agent's copy, gitignored and untracked the directory, and buried an `unmanaged_dir_overwritten` warning in the assignment response. The library and the agent's repo share one flat namespace on the agent side, so a name match is not proof of the same skill — the library silently won. The inject path now refuses a directory that exists without the platform's `.trinity-skill.json` marker before a byte is staged: no archive, no restore, no `.gitignore` line, no untracking, not listed under CLAUDE.md Platform Skills. `force` (manual Sync) does not override it; a platform-managed dir keeps upgrading in place; an unreadable probe keeps the pre-existing fail-open direction. The per-skill status is `conflict` — neither `injected` nor `failed` (overall `success` stays true so the fleet re-inject does not alarm on a standing conflict every auto-sync); `skills_conflict` + `conflicts[]` carry it, and `deliver_assigned` reports `conflict` / `partial` + `conflicts[]`. The verdict is recorded on the assignment row (`agent_skills.delivery_status`, SQLite `agent_skills_delivery_status` + Alembic `0065`), carried across the bulk-replace PUT for retained names, cleared by the next injection where the name lands, and gone on unassign. The Skills tab reads it off the rows so the badge, the explanation (which skill, the agent's own copy is what runs) and an inline "Unassign library skill" action show on a fresh load; `inject()` re-reads the rows so a resolved conflict clears without a reload. MCP passes the delivery block through, `sync_agent_skills` names conflicts on its success branch, and `get_agent_skills` carries `delivery_status` per row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ce-text specs over components (#2918) Two PRs ejected from the 2026-09-20 merge train and three from the previous one landed safety-critical UI logic — a fleet-delete confirmation's `canRemove`, a modal's Esc/focus-trap contract — in the one tier with no executable coverage: regexes over the component's source, which an inverted predicate passes byte-identically. Each justified itself with "this repo's vitest is node-only, nothing can mount", which has been false since `vitest.config.js` grew `plugins: [vue()]` + jsdom + @vue/test-utils for a per-file `// @vitest-environment jsdom` opt-in (precedent: portalThemeSwitch.spec.js). The capability was never the gap; its discoverability at the decision point was. Three things close it: 1. The design-system contract (read before any src/frontend change) gains a "Testing a component — the harness mounts" section and a PR self-check item naming the opt-in, the copyable precedent, and the bar: source-text- only coverage of a predicate that gates a destructive verb, a keyboard contract or a store write is not acceptable. design-system.md §9 and docs/testing/STRATEGY.md carry the same rule. 2. The methodology names the tell (.claude submodule bump → trinity-dev 5b09829 + 437674f): /validate-pr §5.4, /review Step 2.5 and the /implement checklist state the verdict rule and the escape, and merge-train lists the new baseline among the regenerable ratchets. 3. A ratchet, the same shape as raw colours and loading gates: scripts/scan-source-text-specs.mjs counts, per spec, non-import `.vue` literals under components/ or views/ in a spec that calls readFileSync; source-text-baseline.json freezes today's 86 specs / 271 reads; tests/unit/sourceTextRatchet.spec.js fails when a spec grows, when a NEW spec reads SFC source without a `@source-text-pin: <reason>` marker (the reviewable escape for an AST-shaped call-site guard or a parity table), or when an entry is stale. The failure message names the harness and the precedent — the point-of-decision hint the issue asked for. Negative- controlled: a new unpinned spec fails, the same spec pinned passes, and a +1 read in a baseline spec fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…; delivery copy and draft fixes (#2914) Merge-train review on #2920: marker-absence was read as "the agent authored this" and refused, but the platform itself wrote marker-less directories on two live paths — the pre-#384 legacy fallback (SKILL.md only) and a restore that dropped the meta member — so each would have turned its own package into a permanent, unrepairable conflict on the next sync (AC#6 violated). Both are closed at the write: `_legacy_fallback` writes the marker beside SKILL.md (manifest = what was actually written), and `_restore_skill` writes the marker back directly when the agent's `restored` list lacks it (`marker_written_directly`). If the marker cannot be written the injection is reported `failed` (`marker_not_written`; the legacy write is rolled back) rather than a half-managed directory. Five tests cover the two paths, their failure arms, and the legacy-marker round trip that closes AC#6. Also from the review: `deliveryText` grammar ("its own a skill") with a real plural arm; the `not_delivered` arm now names conflicts beside the failure it retries; the Skills tab resets its draft only when the assigned SET changes, so a Sync (which re-reads the rows for the verdict) no longer wipes unsaved ticks; the conflict copy says "a directory the platform did not create — usually a skill it authored" rather than asserting authorship. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…urn (#1923) Six bespoke overlays each re-implemented the same markup and each omitted the same two behaviours: Esc did nothing, and Tab walked out behind the overlay. Verified still true on `dev` before starting — **zero** `Escape` references across all six files (the `@keydown` hits there are Enter-to-submit), no shell primitive, no focus trap anywhere. `BaseModal` owns exactly four things — overlay, Esc, focus, scroll lock — and nothing about content, so adopting it is deleting two wrapper divs rather than rewriting a dialog. It teleports to `<body>`: several of these modals are declared inside panels that establish a stacking context, where a `z-50` overlay nested in one renders BEHIND its siblings. **The decidable half is a separate pure module, and that is the point.** This repo's vitest runs `environment: 'node'` with no jsdom/happy-dom and no `@vue/test-utils` — I checked, because a focus trap written entirely inside an SFC would be a rule no unit test could reach, and source-text assertions would prove only that it had been TYPED. `utils/focusTrap.js` holds every decision as a function over plain data (tabbable filtering, the Tab wrap, the dismiss-key predicate, safe-action selection, backdrop identity) with 25 tests. What that leaves uncovered is the wiring itself — listener attachment, the focus() calls — which needs a browser and belongs to e2e. Said plainly rather than implied by a green suite. Three decisions worth naming: * initial focus goes to the SAFE action, and destructiveness is DECLARED (`data-destructive`) rather than guessed from label text, which would be wrong in every language but English. A dialog that opens with Delete focused turns a reflexive Enter into data loss. * backdrop dismissal compares identity against the overlay node, not a rectangle — a rectangle test mis-fires for a select popup or date picker rendered at the document root and closes the modal under the user. * a modified Escape (Ctrl/Cmd/Alt/Shift) does not dismiss; that is a browser or OS gesture, not an intent to close. Migrated in this commit: `SystemViewEditor.vue` and `NavBar.vue`'s build-info modal. The remaining four files are enumerated in the PR with their exact overlay bounds; they are 80-230 line tag surgeries and are deliberately left for a reviewed pass rather than done blind in one go. One acceptance-criteria item is stale: `views/Agents.vue` was deleted by ent#260 (the Agents page folded into the Dashboard list view), so its bulk-tag popover no longer exists. Related to #1923 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ
…y, and a mounted spec for the wiring (#1923) Merge-train review on #2778, twice: the shell's rules were tested, its wiring was not, and the docblocks justified that with a claim that was false on this head — the repo mounts components (per-file jsdom opt-in, @vue/test-utils, #2918). Two defects sat in exactly that untested wiring. 1. The scroll lock was global state written per instance: `immediate: true` ran the else-branch on every mount and `onBeforeUnmount` cleared unconditionally, so a nested dialog (the #2780 shape — a ConfirmDialog inside another modal's slot) unlocked the page the moment the outer opened, and any modal's unmount clobbered another component's lock. Now `utils/focusTrap.js::createScrollLock` is a ref count over one value (first holder saves and hides, last release restores) and every BaseModal shares `bodyScrollLock`, taking and releasing exactly its own count. 2. The overlay had no `tabindex="-1"`, so `overlay.focus()` was a no-op and Esc did nothing once focus left a control (a click on dialog text lands focus on the nearest focusable ancestor, which was <body>). `tests/unit/baseModal.spec.js` mounts the shell: initial focus on the safe control, Tab wraps, Esc emits close and focus returns, the overlay is focusable, a modal with no tabbable child is still dismissable, backdrop semantics, the lock is taken/restored, two nested modals share one lock, and unmount releases only what the instance held. Mutation-checked: gutting @keydown, the focus return, the tabindex or the lock each turns it red. Also: the false "node-only vitest" claims are rewritten in all three docblocks; the overlay's p-4 now IS the 16px phone gutter (the two adopted panels drop their mx-4, which had doubled it to 32); the design-system docs name BaseModal as the modal shell. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…md (#2914) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…a process-wide caplog, and a stale release allowlist Three failure classes have been redding `backend-unit-test` on dev and the regression diff on unrelated PRs (#2920, #2924, #2927, the 09-18 train): 1. `test_subprocess_pgroup` — `signal_guard.guarded_killpg` walked the group's members, then read each member's cgroup; the harness parent exiting in between (that IS the scenario under test) made `_cgroup_of` answer None, which read as "outside the session cgroup", and a kill of a group that was entirely ours a millisecond earlier was refused. Membership is now decided per LIVE member: a pid that vanished (or is a zombie) is not a member. A live pid whose cgroup cannot be read stays foreign — fail closed, unchanged. Two tests pin both halves; reverting the guard turns the race test red. 2. `test_2789…test_retry_budget_is_logged_with_its_cause` asserted over EVERY caplog record in the process, so a background task left by an earlier test (order-dependent under a random seed) logging an unrelated ERROR read as `['ERROR', 'WARNING'] == ['WARNING']`. It now asserts over the module's own logger. 3. `test_2814_workflow_trigger_parity` — every ACCEPTED_UNTIL_RELEASE entry is declared on `main` since the v0.9.5 cut and the guard has said "prune these" on every run since. Pruned, as the guard was designed to demand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ived `mine()` was introduced to stop a background task's unrelated record from reading as this test's own, and applied at four sites. Six reads of the process-wide `caplog.records` survived in the same function, after the last `mine()` call — including `:523`, the exact shape the fix was written for, and two `assert not caplog.records` that any stray record from any logger reddens. Verified by negative control: with an unrelated ERROR emitted inside the third phase's `caplog.at_level` window, the pre-fix assertions fail at `assert "30s already spent" in caplog.records[0].message`; with `mine()` they pass. `caplog.records` now appears once, in `mine()` itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…cstring `_is_gone` returned True for every OSError, so a LIVE pid whose `/proc/<pid>/status` cannot be read (EACCES under a `hidepid=` mount, a malformed line) was classified as gone, dropped from `live`, and the `killpg` proceeded. The docstring two lines above states the opposite contract: "a LIVE pid whose cgroup we cannot read is treated as foreign (fail closed)". Only the vanished-pid case is `gone` — FileNotFoundError. Every other OSError/IndexError now keeps the pid in `live` so the cgroup check can refuse the kill. This guard replaces os.kill/os.killpg for the whole unit suite and exists because a mis-fire SIGKILLed a developer's desktop twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`SkillsPanel.vue` had no spec at all, so the ~60 lines of conflict UI and the fix for the regression this PR itself introduced were both inert: disabling the whole conflict block AND reverting the draft-wipe fix left the suite byte-identically green. `inject()` re-reads the assignment rows (the conflict verdict rides them), so the panel sees a new `assigned` array after every Sync even when the set is unchanged. The shipped defect was a deep watch on that array wiping unsaved ticks; the fix watches the set identity. Nothing executed it. Two tests, mounted (the ent#625 / #2918 harness), and negative-controlled in both directions so neither can be satisfied by the other's cheat: revert to `watch(() => store.assigned, resetDraft, {deep:true})` -> "keeps an unsaved tick when Sync re-reads the SAME set" FAILS delete the watcher outright ("never reset" also passes test 1) -> "still follows a real change to the assignment set" FAILS Full frontend suite: 142 files, 3246 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # docs/memory/learnings.md
…rors are one rule `teardownDefaultSelection` pre-ticked `evidence !== 'prefix'`. `evidence` is free-form text off the wire and the consumer DELETES, so a member arriving with the field absent, null, or carrying a future fourth value was pre-ticked by a denylist that had never heard of it. It was worse than one predicate: the polarity was hand-mirrored in THREE places, two of them in the template — the store's default, the "matched by name only" BaseBadge, and the explanation beneath it. The badge and the note tested `=== 'prefix'`, which is not the complement of `!== 'prefix'` for an unrecognised value, so such a member was pre-ticked AND unbadged AND unexplained: swept into a delete with no signal at all. All three now read one exported predicate, `isConfirmedMember`, an allowlist of positive confirmation (`tag` / `both` — what `system_service.py` actually emits). `teardownUnconfirmedMembers` is its complement by subtraction, so the pair partitions the roster by construction. Four cases + a partition property added to the spec that already executes this rule. Negative-controlled: restoring the denylist turns the three unrecognised-evidence cases red. The source guard's assertion moved with it — it pinned the literal `m.evidence === 'prefix'`, i.e. the implementation spelling, and now pins the shared predicate plus the absence of a raw comparison. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ming no mount harness exists Two things, one file. **The pin.** #2922 lands a ratchet that holds any spec with no baseline entry to ZERO source-text reads. This spec is new, so it fails that ratchet — the two PRs are individually green and jointly red, which is invisible to either one alone. It is a legitimate pin rather than a missing mount: the defect is a cascade tie broken by stylesheet EMISSION ORDER, and neither jsdom nor @vue/test-utils evaluates a stylesheet, so a mount would report the class list it was handed and prove nothing. That is exactly the case the marker exists to make visible to review. **The false rationale.** The docblock said vitest "runs `environment: 'node'` here with no mount harness". `environment: 'node'` is the DEFAULT only; a spec opts into jsdom per file and mounts, which 22 specs already do. Saying the harness does not exist is the claim #2918 was filed about, and repeating it in a new file is how the belief survives. The harness exists and is reachable — this spec stays at source level because a cascade is invisible to it, which is a different and true reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… target param The comment justified the `baselined` row by saying `agents` is absent from AGENT_TARGET_PARAMS, so a `none` row "would have passed the shape check and said something false". `agents` is in that set (`access.ts`), and `policyFor` THROWS on a `none` row whose parameters name an agent — the server refuses to boot. The row is right; its stated reason was the one thing about it that was wrong, and it understated why the row matters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 21, 2026
Closed
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.
Integration surface for #2935, #2922, #2778, #2920, #2756. Never merged — members merge individually once this is green. No closing keywords here on purpose; the members carry those.
Assembled from each member's head with
--no-ff, so a red job bisects to a PR rather than a commit. One member-vs-member collision, resolved on the train becausedevis not what it conflicts with: #2920 and #2756 both append todocs/memory/learnings.md(kept both; 244 entries ondev-> 248 here).Why this batch is worth a full-suite run — two things are true only of the combination:
baseButtonVariantBorder.spec.js, exactly that shape. Neither PR can see it alone. Fixed on feat(systems): the public half of system teardown — a structured membership read, a license-blind MCP proxy, and a gated remove panel (abilityai/trinity-enterprise#454) #2756's side with a@source-text-pin(its defect is a cascade tie broken by stylesheet emission order — no mount can observe it). Ratchet now passes 8/8 on this tree.dev's own standing red (test_2814…::test_every_accepted_entry_names_a_real_divergence,1 failed / 16751 passedat01162d54) and thetest_subprocess_pgrouprace that hit feat(systems): the public half of system teardown — a structured membership read, a license-blind MCP proxy, and a gated remove panel (abilityai/trinity-enterprise#454) #2756'sregression diffin 1 of 3 seeds while it was queued. Both are in scope here for the first time.Local pre-flight on this exact tree: frontend
148 files / 3335 passed·check_alembic_heads.py->66 revisions, 1 head (0065_agent_skills_delivery_status)· mcp-server456/456on #2756's head.Mechanical fixes pushed to member branches during assembly (each announced on its PR): #2935 six surviving process-wide
caplog.recordsreads +_is_gonenow fails closed; #2920 a mounted spec pinning the draft-wipe regression (negative-controlled both ways); #2756devmerge, the destructive default flipped from a denylist to an allowlist across its three mirrors, the pin above, two false "no mount harness exists" rationales, and a falseaccess.tscomment.Ejected from this train: #2936 (2 new high-severity CodeQL alerts + an Alembic fork with #2920, both off
0064) and #2728 (103 commits behind, 4 red, forks againstdev's own0059).