Repository navigation
Conversation
`pull_coordination_service.apply_task_result` is a CAS-won terminal writer and carried every OTHER terminal hook — the #1578 completion event, the #1804 activity close — but no SUB-003 hook. So a pull-owned turn the provider refused: * recorded no `subscription_rate_limit_events` row, so the subscription was never skip-listed and no usage card or pressure badge counted it; * never called `handle_subscription_failure`, so the agent stayed pinned to the subscription that had just refused it; * landed FAILED, where `redelivery_governor` treats `billing` as a correlated code — so the re-delivery that might have recovered it is the thing most likely to be paused. Everything the push path gained in #441/#471/#792, and everything #2638 added on top, was unreachable from a pull-dispatched turn. Inert today (`PULL_MODE_PILOT_AGENTS` gates the path and nobody is piloted), which is why it needed a test rather than a soak: piloting a subscription-backed agent would have silently removed SUB-003 from that agent with nothing saying so. Three properties are load-bearing. **The vocabularies do not line up.** The worker's typed `error_code` calls the quota class `billing` (`result_callback._STATUS_MAP` maps an agent 429 to it); this subsystem calls it `rate_limit`. `_switch_failure_kind` is the map, and it is an ALLOWLIST — a code it has not heard of switches nothing, because "switch unless the code looks benign" would churn an agent through every subscription it owns the first time a worker reports an unfamiliar crash class. **CAS-won branch only**, beside the two hooks it sits with. A replayed terminal short-circuits above the write and a late one loses the CAS, so neither spends a second switch (the #1083 rule). Past that gate, `handle_subscription_failure` owns the rest: it records the event unconditionally, then takes the #799 per-agent `agent_switch_lock` and re-reads under it, so two failures racing on one agent still switch once. **A SUCCESS terminal is excluded, on the merits.** The gate is `error_code` rather than the FAILED/CANCELLED split — a worker that labels a quota refusal `cancelled` still refused for a quota reason — but a turn the provider SERVED is evidence the subscription works, so a stray `error_code` riding a success must not move the agent off it. It is also the one branch where the sink never binds `err_text`, so an ungated hook would raise NameError and turn a committed, billed terminal into a 500. Re-delivery, stated explicitly per AC #3: this hook re-delivers nothing. A switched agent's next re-delivery is expected to succeed IF the lease reaper re-queues the row (under `MAX_REDELIVERY`) and the #1085 governor is not paused — and `billing` is exactly the class the governor counts, so a fleet-wide quota event can legitimately hold re-delivery back even after a successful switch. The switch is not wasted then; the next scheduled or claimed turn benefits. The sink is sync and its caller async, so the call goes through `subscription_auto_switch.spawn_subscription_failure` — the same wrapper shape as `spawn_task_terminal_event` and `spawn_close_execution_activity`: one coroutine (not a wrapper around a pre-built one, which leaves two objects to close and still warns "never awaited"), a strong reference held until it finishes, a raising switch logged and swallowed, and no-running-loop treated as a clean skip. Still NOT wired on the pull sink, named so it is not mistaken for done: the #526 AUTH dispatch breaker and the governor's own `record_terminal_failure`. `apply_result` has both; neither is a SUB-003 concern. 23 tests, mutation-checked: a pass-through failure-kind map, hoisting the call above `if won:`, and dropping the SUCCESS exclusion each turn the suite red. Plus a structural AST pin that the one call site stays inside the CAS gate. Related to #2643. Related to #1081. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP
…2624) Both Workspace chat surfaces scrolled to the bottom on EVERY arrival. `scrollDown()` was an unconditional `scrollTop = scrollHeight` and every path called it: the room's 3s poll, an agent reply settling, a cancelled turn settling, the history load, the send. Scroll up to re-read an earlier answer and the next message yanked you back down — in a room, from any participant, every three seconds, so reading anything but the tail was close to impossible. The design-system contract already said it (principle 5: updates preserve scroll, selection, focus, and expansion). What was missing is the distinction between an ARRIVAL and an INTENT: * an arrival — the poll fetching somebody else's message, a reply settling, a stream ending — follows only if the reader was already following; * an intent — sending, opening a thread, clicking jump-to-latest — always pins and re-arms, whatever the prior position. Hence two entry points (`onArrive` / `pinToBottom`) rather than a flag threaded through one `scrollDown()`. The rule lives in ONE composable, `useStickToBottom`, because the two components own the same `flex-1 min-h-0 overflow-y-auto` container and had two copies of the same bug. `chat/ChatMessages.vue` still carries the unconditional pattern behind its `autoScroll` prop — a different surface, out of this issue's scope, and it can adopt the composable unchanged. Decisions worth keeping: * **64px, not exact.** `scrollHeight` EXCLUDES the border under `box-sizing: border-box` (the gotcha `PortalRoom.vue` already documents), and fractional layout leaves a sub-pixel gap when the reader IS at the bottom. It is also roughly "a line off the bottom", which is what a reader would call still following. * **A missing element answers "following".** Before first paint, and on a thread too short to overflow, the reader is at the bottom; false would open every thread detached and badge its first reply as unread. * **A thread switch re-arms WITHOUT scrolling.** The outgoing element is about to be replaced; the incoming thread's load pins it. * **The affordance needs detached AND behind.** Scrolled up with nothing new below is nothing missed. It says what was missed ("3 new messages"), since a bare arrow cannot tell a reader whether to go. One defect the unit tests structurally could not find, and the e2e did: **pinning takes two passes, one frame apart.** `nextTick` covers the component's own patch, but a child patching on a later tick — the loading skeleton swapping out, markdown rendering a long thread — grows the transcript after the first measurement and the browser clamps the assignment to the height it had then. Measured in a real browser: a 40-message thread opened 20px above its newest message, stably, every time. Verified: 2537 frontend unit tests (21 new over the composable), and the e2e run against a real Chromium — the two room cases and the open-at-the-bottom case pass, and reverting `onArrive` to the unconditional scroll turns the arrival case red. Related to #2624 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP
…ft (ent#556) The Workspace's brand corner was a hand-drawn outline `<svg>` and the bare word "Workspace", in two places that had to agree and had no shared source: the signed-in shell (`PortalSidebar.vue`) and the sign-in screen (`views/Portal.vue`). The real marks already shipped and were already used by `NavBar` and `PublicChat`; the Workspace never adopted them. Both surfaces now render ONE component, `PortalBrand.vue`. "Both surfaces carry the same mark and wording" is an acceptance criterion, and two copies of the markup is exactly how that stops being true. The name is a value in `portalBrand.js`, not a literal in two templates — and it is deliberately the PRODUCT name, not an instance name. The platform does resolve a per-instance label (`services/instance_identity.py`), and a company running Trinity could plausibly want its own name in that corner; but this surface's audience is the operator's own organisation (ent#78, ruled 2026-09-06) and naming the product is the point of the ask. One constant with one reader is what keeps the white-labelling call cheap later. Details that are load-bearing rather than tidiness: * **The light/dark swap follows `NavBar`** rather than inventing a second pattern — a dark mark on a dark ground is the one failure this must not have. Verified in a real browser at both themes. * **Not a dead affordance, and not a platform route.** The shell links to the Workspace root; the sign-in screen renders the mark INERT, because a client session holds no `users` row and the reader is already at the root. `:is` picks `router-link` or `div`. * **The accessible name is on the wrapper, the images are decorative.** A screen reader says the product once — and still says it when the wordmark truncates away at a narrow sidebar. (This is why the images carry `alt=""` rather than `alt="Trinity"`: with the name beside them, alt text would double-read.) * **`truncate min-w-0` on the wordmark, `shrink-0` on the mark.** The sidebar resizes to `SIDEBAR_MIN` (200px, ent#492) and this row also carries the ask and unread badges; without them the name pushes the badges out of the band. * **The mark keeps the 24px box the icon it replaces used**, so the `h-14` band ent#547 is shortening does not grow. Measured at 56px. Tab titles: the platform renders `Trinity — <label>` (#1418), so a Workspace tab already reads "Trinity — Workspace". That single format is kept rather than given a second one for one surface. What is added is identifiability where the ROUTE knows its subject: `/workspace/a/:agentName` now titles `Workspace · <agent>` through the existing `agentTabTitle`, so an operator sees the display name and a client falls back to the slug. Threads and rooms cannot do this — their subject is component state, not a route param. The generic icon stays where it is still a legitimate EMPTY-STATE illustration (the `Portal.vue` stage zones). It was a brand mark only in the two blocks this replaces. Verified: 2534 frontend unit tests (18 new source-guard cases, mutation-checked — a dark-on-dark swap and a platform-route link each turn the suite red), plus a real-browser pass over both surfaces at 1280 and 1920, light and dark, asserting exactly one mark visible and the right one per theme. Related to Abilityai/trinity-enterprise#556 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP
…2624) CI reproduced the exact defect the two-pass pin was meant to remove: a 40-message thread opened 20px above its newest message. Frame-chasing is the wrong primitive, and this is the third attempt at it: 1. one `nextTick` — short by 20px, found by the e2e locally; 2. a fixed second pass one frame later — fixed it on a fast local machine, and CI still opened the thread 20px short; 3. a settle loop re-pinning until the height stopped changing — ALSO short, because the transcript grew again after the loop had already watched it hold still for a frame. Measured: `scrollHeight` stable at 4316 from the first observation, `scrollTop` stuck at 3813, i.e. the pin had run at 4296 and nothing re-ran. There is no window that is both short enough to be free and long enough to be right, so the window is gone. A `ResizeObserver` watches the container (a resized window, a dragged column) and its content wrapper (the transcript growing) and re-pins whenever the reader is following — any cause, any delay, nothing to tune. It also makes a STREAMING reply follow for free, and it is gated on the same `following` flag as every other arrival path, so there is one rule and not two. Setting `scrollTop` changes no box's size, so it cannot feed itself. The settle loop also had a defect of its own, found while testing it: it read `scrollHeight` three times per pass, so the stability check compared a different measurement than the one it had just assigned. That code is gone rather than fixed. Verified in a real browser: the gap is 0 from the first frame, and all three e2e cases pass. 2540 unit tests, with the settle-loop cases replaced by four that pin the observer contract — it re-pins on late growth, watches both boxes, leaves a DETACHED reader alone when the transcript grows, and still pins where there is no ResizeObserver at all. One test-only note worth keeping: the observer cases `markRaw` their stand-in elements. `ref({...})` proxies a plain object, so an identity assertion fails against the raw one — Vue never proxies a real DOM element, so the raw form is the faithful stand-in. Related to #2624 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP
…n skill Submodule pointer only. Brings in trinity-dev `226f228`: DEVELOPMENT_WORKFLOW.md's P-09b listed "a member with a CRITICAL is ejected, not repaired" as load-bearing, and justified it with the blocking argument the skill explicitly rebuts. The skill replaced that rule in 9dcd4cb — ejection turns on whether a fix is mechanical or needs intent, not on severity — so the two documents disagreed on a load-bearing rule, and a reader of the workflow doc would eject a member the skill says rides. Also keeps P-09b's Phase 2 summary honest about the coverage question, cross- referenced to #2659. No product behaviour. Lane A. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… owned it `portalBrand.js` declared `WORKSPACE_ROOT = '/workspace'` while `portalUtils.js` — same directory — already exported the identical constant, consumed by `stores/clientPortal.js:546` for the sign-out target and pinned by `portalAgentPageUx.spec.js` and `workspaceSession.spec.js`. `PortalSidebar.vue` imported the new one on the line directly above its existing destructure from the module that already had it. That is the defect this PR was written to remove — its own header calls the old state "two places that had to agree and had no shared source" — fixed for the name and recreated for the route. The two specs pinned the two copies independently, so a later divergence (the sidebar mark linking one way, sign-out redirecting the other) would have shipped green. It was also a latent `SyntaxError`: adding `WORKSPACE_ROOT` to the adjacent `./portalUtils` destructure would have created a duplicate binding. The reasoning that justified the constant moves with it rather than being dropped — the brand mark is now documented at the definition as a second consumer, since that is where the next reader will look. merge-train: mechanical, per the merge-train note on the PR. Verified: npm run test:unit → 113 files / 2534 tests passed; vite build OK. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lts14MGxLdPU5tvxL6CfAo
This was referenced Sep 9, 2026
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
2 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.
DO NOT MERGE. Integration surface only — this branch exists to run the full suite over #2660, #2653, #2651 and #2652 together, which nothing else in the lifecycle does: every PR is tested against
dev, never against its siblings. The members merge individually once this is green, and this PR is then closed and its branch deleted.No closing keywords here on purpose — the members carry those, and the train must promote and close nothing.
.claudesubmodule bump — strict fast-forward, docs-onlyAssembled with
--no-ffon each member's head SHA, so a red job can be attributed to a PR rather than a commit. All four merged with zero conflicts.Why these four, and what this run is actually looking for. #2653 and #2652 both change the portal surface —
Portal.vue/PortalSidebar.vueagainstPortalConversation.vue/PortalRoom.vue/ the newPortalJumpToLatest.vue— and only a combined unit +frontend-e2erun exercises them in the same tree. Locally the merged tree gives 115 files / 2572 tests passed, so the two coexist at the unit layer;e2eis the part that needs real Chromium.Four other candidates were validated and ejected to their authors today (#2663, #2656, #2657, #2532), each with a reason on its own thread.