Skip to content

fix(session): one platform credential, one 401 verdict, one handler (#2791) - #2811

Merged
vybe merged 5 commits into
devfrom
fix/2791-one-platform-session
Sep 16, 2026
Merged

vybe merged 5 commits into
devfrom
fix/2791-one-platform-session

Conversation

@dolho

@dolho dolho commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

What was wrong

Log out and log back in on the main app with a Workspace tab open from the previous session, and the new session dies within seconds.

That tab still holds the old JWT. Its 20 s asks poll 401s. The handler calls authStore.logout(), which does localStorage.removeItem('token') — removing the token the re-login had just written. The main tab's next request finds nothing and hard-redirects to /login.

The handler never asked whether the credential that failed was still the current one.

Underneath that, one browser held the platform JWT in two places that could disagree — the in-memory axios.defaults.headers.common['Authorization'] copy written once at login, and localStorage['token'] re-read per request by api.js — with no storage listener anywhere under src/frontend/src, and three separate 401 implementations that had each drifted. The Workspace bites hardest because it opens in its own tab (ent#456) and polls.

One source, one verdict, one handler

New src/frontend/src/utils/platformSession.js.

One source

readStoredToken() is the only reader. The axios.defaults copy is no longer written — setupAxiosAuth is a documented no-op — and main.js installs a global axios request interceptor that rebuilds the header from storage on every request.

That covers ~368 bare-axios call sites without rewriting them, which is the AC's second half ("or is provably never read in preference to the store"), and it is the stronger of the two options: a call site added tomorrow cannot forget to opt in.

An explicit header on the config still wins, and exactly one caller depends on it — the logout revoke. #2258 clears local state before the revoke, so with the defaults copy gone the revoke would have gone out unauthenticated and #187 would have silently stopped revoking anything. The token is captured before the clear and passed after it.

One verdict

sessionLostVerdict()ignore | stale | logout. Pure, so a node-env spec can reach it — which is the point, since the predicate it replaces had been hand-copied into api.js, main.js and portalHttp and drifted three ways.

situation verdict
already on /login, /setup, /m ignore
the failed token is not the stored one stale — adopt the current session, never destroy it
no stored token, on the Workspace ignore (an ordinary external client)
no stored token, anywhere else logout
on the Workspace and a portal token is live ignoreAC #5
otherwise logout

The stale arm is the reported bug. The Workspace veto is AC #5: a client whose browser holds a dead operator JWT is no longer thrown onto the operator login by initializeAuth's fetchUserProfile, which runs on every page load. It stays scoped by path as well as by portal token deliberately — off the Workspace the surface is an operator one, so an expired operator JWT still bounces there even with a stray portal token. This does not widen that.

One handler

setPlatformUnauthorizedHandler / notifyPlatformUnauthorized. main.js registers the reaction (the only module that already has both the router and the store); api.js, the global interceptor and portalHttp all report to it.

api.js no longer hard-reloads, no longer leaves auth0_user behind (AC #6), and carries no predicate of its own.

clientPortal.js keeps isPlatformSession as its local gate — not redundant with the shared verdict: it is the only thing that knows this tab's client session was suppressed (#2261's platformFallbackSuppressed), which no amount of reading localStorage reconstructs.

Cross-tab sync

A storage listener on the platform token key. A sibling logging in → adoptStoredSession() (converge, re-fetch the profile, reset profileVerified so role-gated UI stays closed until this token's profile lands — #2198's rule). A sibling logging out → applySessionEndedElsewhere(), which drops the in-memory mirror only: no second server revoke for an already-revoked token, and no write to storage, because N background tabs reacting to one event would each clear it again.

Neither branch navigates. A background tab pushing /login is the noise this issue reports; the visible tab converges through the router guard and its next request, both of which read the state these set.

A guard that was pinning nothing

workspaceSession.spec.js's "who gets bounced to /login" block defined its own shouldBounce helper — a hand-copied duplicate of the interceptors' expression. Nothing under test imported it. That is why it stayed green while the three real predicates drifted, and it would have stayed green through this change too. It now asserts the real function.

Two things found while reviewing my own diff

  • handlePlatformUnauthorized was async and awaited logout(). logout() clears local state synchronously before its first await, so the router guard is already satisfied — awaiting only held the user on a dead page for the length of the revoke, indefinitely if it hung. Now un-awaited, matching the previous timing.
  • The reaction pushes a route, and Vue Router rejects a redundant navigation. A sync try/catch cannot see that, so a second 401 arriving while /login was loading would have surfaced as an unhandled rejection in every user's console. The registry absorbs it.

Both are pinned.

Verification

src/frontend  vitest run     131 files, 2937 passed
              new: platformSessionVerdict.spec.js (13)
                   platformSessionSync.spec.js (16)
              updated: workspaceSession.spec.js, workspaceSignOut.spec.js
              green: axiosInstanceIsolation.spec.js, workspaceSessionRenewal.spec.js
              raw-color ratchet unchanged (no Vue changes)

Mutation-checked — each guard was made to fail by restoring the defect:

mutation reds
remove the stale arm 2
let the request interceptor overwrite explicit headers 1
restore api.js's own removeItem + location.href logout 2

Frontend only. No backend change, no schema change, no migration, no new endpoint, no config.

Acceptance criteria

  • One source of truth for the platform credential; the axios.defaults copy is never written (only cleared on logout, for tabs still running a pre-fix build).
  • Cross-tab sync via a storage listener, reaching the auth store and the Workspace's platform fallback.
  • A single shared handler for all three 401 sites that logs out only if the token that failed is still the stored one.
  • Finding 1 no longer reproduces — the superseded tab adopts the current session instead of deleting it.
  • A Workspace client with a dead operator JWT is not bounced on page load; workspaceSession.spec.js updated to the new rule rather than left pinning the old one.
  • The api.js 401 path no longer leaves auth0_user behind and no longer diverges from authStore.logout().
  • Unit tests pin each of the above.
  • workspace-session-signout.md and architecture/workspace.md updated for the single-source + cross-tab model.

Not verified in a browser. The issue's findings were code-read and so is this fix; the specs are node-env and cannot drive a real storage event or a real interceptor. Worth one manual pass of the finding-1 repro before merge.

Fixes #2791


🤖 Generated with Claude Code

https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf

dolho and others added 2 commits September 15, 2026 14:15
…2791)

Log out and log back in on the main app with a Workspace tab open from the
previous session, and the NEW session dies within seconds. That tab holds the
old JWT, its 20s poll 401s, and the handler calls `authStore.logout()` — which
removes `localStorage['token']`, i.e. the token the re-login had just written.
The handler never asked whether the credential that failed was still the current
one.

Underneath it, one browser held the platform JWT in two places that could
disagree (the in-memory `axios.defaults` copy vs localStorage re-read per
request), with no `storage` listener anywhere under `src/frontend/src`, and
three separate 401 implementations that had each drifted.

`utils/platformSession.js` makes all three singular.

**One source.** `readStoredToken()` is the only reader. The `axios.defaults`
copy is no longer written (`setupAxiosAuth` is a documented no-op); `main.js`
installs a global axios REQUEST interceptor that rebuilds the header per
request, so ~368 bare-`axios` call sites get the current credential without
being rewritten and a new one cannot forget to opt in. This is the AC's second
half ("or is provably never read in preference to the store") and it is the
stronger of the two.

An explicit header still wins, and exactly one caller needs that: the logout
revoke. #2258 clears local state BEFORE the revoke, so with the defaults copy
gone the revoke would have gone out unauthenticated and #187 would have silently
stopped revoking anything. The token is captured before the clear and passed
after it.

**One verdict.** `sessionLostVerdict()` → `ignore | stale | logout`, pure so a
node-env spec can reach it. `stale` — the failed token is not the stored one —
is the fix for the report: adopt the current session instead of destroying it.
The Workspace veto closes AC #5: a client whose browser holds a DEAD operator
JWT is no longer thrown onto the operator login by `initializeAuth`'s
`fetchUserProfile`. It stays scoped by path as well as by portal token, so an
expired operator JWT still bounces off an operator surface.

**One handler.** `setPlatformUnauthorizedHandler` / `notifyPlatformUnauthorized`.
`main.js` registers the reaction; `api.js`, the global interceptor and
`portalHttp` report to it. `api.js` no longer hard-reloads, no longer leaves
`auth0_user` behind, and carries no private predicate.

**Cross-tab sync.** A `storage` listener adopts a sibling's login and drops the
mirror on a sibling's logout — without a second server revoke and without
writing to storage, since N background tabs reacting to one event would each
clear it again. Neither branch navigates: a background tab pushing /login is the
noise this issue reports.

`workspaceSession.spec.js`'s predicate block asserted its own hand-copied
`shouldBounce` helper — which is why it stayed green while the three real
predicates drifted, and would have stayed green through this change too. It now
asserts the real function.

Verified: 131 files / 2937 tests pass. The three load-bearing guards were
mutation-checked — removing the `stale` arm reds 2, letting the interceptor
overwrite explicit headers reds 1, restoring `api.js`'s own logout reds 2.

Related to #2791

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf
…eletes (#2791)

Review finding on my own diff. The docblock's "why not `axios.create()`" argued
from `stores/auth.js` mutating `axios.defaults.headers.common.Authorization` at
login and deleting it at logout — the exact copy #2791 removes.

The conclusion survives the mechanism (the global is still the only thing
carrying a live credential, now because the request interceptor resolves it per
request and `create()` gives an instance its own chain the global never
reaches), which is precisely why the comment would have gone on reading as true.
A comment that describes a mechanism the code no longer has is the class this
repo's learnings ledger already records; the old reason is kept in parentheses
because it explains why the answer did not change.

Related to #2791

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf
@dolho

dolho commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

/cso + /review — self-review of this PR

cso v1.1 · --diff --code scope (merge-base eb41896f5, 11 files, +908/−103, frontend only)
/review against the same merge-base.

0 critical. 1 fixed during review. 3 informational — two of which say this PR meets its ACs less completely than the PR body claims.


/cso — security audit

Scope note

Phases 2–6 and 8 (secrets archaeology, supply chain, CI/CD, infra, webhooks, skills) have no diff surface: no dependency, workflow, Dockerfile, backend route or skill file is touched. Phases 0, 1, 7, 9–14 ran. The relevant OWASP categories are A01 (who may end a session), A02/A07 (where the credential lives and travels) and A05 (misconfiguration of the new interceptor).

S1 — Credential blast radius: unchanged. VERIFIED, not a finding

The change most worth attacking is the new global request interceptor, which attaches the platform JWT per request:

axios.interceptors.request.use((config) => {
  const headers = config.headers || {}
  if (!headers.Authorization && !headers.authorization) {
    const token = readStoredToken()
    if (token) headers.Authorization = `Bearer ${token}`
  }

Three things had to hold, and all three do:

  • It reaches no more requests than before. axios.interceptors binds the default instance — exactly the set axios.defaults.headers.common['Authorization'] already covered. Not a widening.
  • No bare-axios call targets an external origin. grep -rnoE "axios\.(get|post|put|delete|patch)\(\s*['\]https?://"oversrc/` returns nothing, so the JWT cannot ride to a third party.
  • Every axios.create() instance is accounted for. There are exactly two (api.js, portalHttp), each with its own request interceptor; portalHttp's strips whatever it inherits and rebuilds from the store. utils/boundedHttp.js deliberately uses the global rather than an instance and is therefore covered.

Explicit-header-wins is not attacker-reachable: every in-app caller passes either nothing or authStore.authHeader, which derives from the same place.

S2 — The diff was one commit away from silently disabling #187

Worth naming as the highest-value line in the PR rather than buried:

#2258 requires logout() to clear local state before the network revoke. Removing the axios.defaults copy means the revoke's credential no longer comes from memory — and storage is already empty by then. Left alone, POST /api/auth/logout would have gone out unauthenticated, and server-side token revocation would have stopped working with no error anywhere.

Fixed (token captured before the clear, passed after it) and pinned by
workspaceSignOut.spec.js plus a source guard. Mutation-checked.

S3 — profileVerified is reset on adopt: a security improvement

adoptStoredSession() sets profileVerified = false, so role-gated UI cannot render under the previous principal's verification while the new token's /api/users/me is still in flight (#2198's rule, extended to a path that did not exist before).

S4 — The AC #5 veto grants no access. Accepted trade, stated

onWorkspace && portalTokenPresent → ignore means an operator on /workspace with a dead JWT and a stale portal token is no longer bounced — their UI stays rendered on a dead session. That is a UX degradation, not a privilege one: every request still 401s. And there is still a way out, because a dead portal token drives portalHttp's own expiry path (isPlatformSession is false when a portal token is present), which surfaces the OTP form. No stranding case found.

Appendix (confidence 5 — below the 8/10 daily gate, recorded not reported)

clearStoredSession() swallows a storage failure. If removeItem throws, the token survives in storage — and since every transport now derives from storage, a "logged out" tab keeps sending it. Pre-existing for api.js, whose blast radius this widens to all bare-axios callers. Realistic only in storage-blocked contexts where removeItem throws, which I could not demonstrate. Cheapest hardening: log the failure rather than swallowing it.

No leaked credentials, no new endpoint, no new principal, no widened gate. No CRITICAL/HIGH findings.


/review — structural

Fixed during review

[F1] boundedHttp.js explained itself with the mechanism this PR deletes (Confidence 10/10) — pushed as ceb2e9384.

Its docblock argued why not axios.create():

stores/auth.js authenticates by mutating axios.defaults.headers.common.Authorization at login and deleting it at logout.

That mutation is exactly what this PR removes. The conclusion survives — the global is still the only thing carrying a live credential, now because the interceptor resolves per request and create() gives an instance its own chain the global never reaches — which is precisely why the comment would have gone on reading as true. The repo's learnings ledger already names this class; the old reason is kept in parentheses because it explains why the answer did not change.

Informational

[I1] AC #1 is met in substance, not in letter (Confidence 9/10)

The AC asks that "every transport derives Authorization from the same place on each request". Twenty-five raw reads of the credential key remain outside the single-source module, including three hand-built headers:

src/views/AgentDetail.vue:630,672,740   'Authorization': `Bearer ${localStorage.getItem('token')}`
src/stores/network.js:823               _authHeaders() → Bearer ${localStorage.getItem('token')}
src/utils/websocket.js:47, App.vue:59, stores/notifications.js ×4, …

The hazard the AC exists for is closed: these read the same key the interceptor reads, so they cannot disagree with it — the two-sources-that-drift problem is gone. But "derives from the same place" is generous for "re-implements the read", and they miss readStoredToken()'s try/catch, so they throw rather than degrade where storage access is blocked.

All 25 pre-date this PR. Raising it because the PR body claims the AC outright and the honest statement is narrower.

[I2] AC #3's "all three 401 sites" undercounts by one class (Confidence 9/10)

AgentDetail.vue issues three raw fetch() calls with hand-built auth:

const response = await fetch(`/api/agents/${agent.value.name}/autonomy`, {
  method: 'PUT',
  headers: { 'Content-Type': 'application/json',
             'Authorization': `Bearer ${localStorage.getItem('token')}` },

fetch is reached by no axios interceptor, so their 401s reach no handler at all — before this PR or after. The issue enumerated three 401 sites; there is a fourth transport with zero. Not introduced here and out of this PR's scope, but "one shared handler used by every 401 site" is not yet true and should not be read as such.

[I3] Cross-identity store residue on adopt (Confidence 7/10, pre-existing)

When adoptStoredSession() adopts a different user's token, the other Pinia stores still hold the previous principal's fetched data until each refetches. logout() does not clear them either, so an ordinary logout→login-as-someone-else has the same residue today. Not introduced by this PR; named so the new adopt path is not assumed to have fixed it.

Clean

  • Enum completenesssessionLostVerdict returns exactly ignore | stale | logout; the single consumer handles all three, else → logout. No other caller.
  • No cyclesplatformSession.js imports nothing; the handler registry is why (main.js owns the router and the store, and a direct import would be a cycle — the shape clientPortal.js already used).
  • No dead export — the setPlatformSessionLostHandler alias is still consumed by workspaceSession.spec.js, and now resolves to the shared registry rather than a second slot.
  • Frontend XSS — no new v-html; nothing user-controlled is rendered.
  • Error handlingreadStoredToken/clearStoredSession fail closed (no credential sent); notifyPlatformUnauthorized absorbs both a throwing and a rejecting reaction, the latter being Vue Router's redundant-navigation rejection.
  • Scope — no unrelated files; the git status was checked before staging after a stray-file incident on a sibling PR earlier today.

Verification

131 files, 2937 tests pass (after the F1 commit)

Mutation-checked, each restoring the defect:

mutation reds
remove the stale arm 2
interceptor overwrites explicit headers 1
restore api.js's own removeItem + location.href logout 2

Summary

Still outstanding from the PR body: the finding-1 repro has not been exercised in a browser. Node-env specs cannot drive a real storage event or a real interceptor, so the end-to-end claim rests on unit coverage of the parts.

@vybe

vybe commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

merge-train: ejected from the 2026-09-15 train — rides the next one once fixed. Nothing was pushed to this branch. This is a comment, not a review, so it is not a hold.

The design is right and the pure-function half is genuinely well tested. sessionLostVerdict, isAuthRoute, isWorkspacePath, tokenOfRequest, adoptStoredSession() and applySessionEndedElsewhere() are all really executed, and workspaceSession.spec.js:418-470 replacing its hand-copied shouldBounce with the real function is a legitimate fix. But the headline AC is falsified by a file the PR never touches.

C1 — the second credential source survives, so the new mechanism is inert

src/frontend/src/App.vue:59-64 — untouched by this PR — does exactly what the PR says happens nowhere:

axios.defaults.headers.common['Authorization'] = `Bearer ${localStorage.getItem('token')}`

It runs on every boot with a token, on every route including /workspace.

And in axios 1.19.0 (node_modules/axios/lib/core/Axios.js) config.headers = AxiosHeaders.concat(contextHeaders, headers) — where contextHeaders includes defaults.headers.common — executes before requestInterceptorChain is built. So a defaults-derived header is indistinguishable from an explicit one, and main.js:96's if (!headers.Authorization && !headers.authorization) always skips the rebuild.

Measured against real axios with a stub adapter, not reasoned about:

defaults localStorage on the wire
OLD NEW Bearer OLD-TOKEN
cleared NEW Bearer NEW-TOKEN
explicit revoke header Bearer REVOKING

Three consequences:

  1. AC Fix: Add missing Docker labels to system agent container #1 is false. The two-places-that-disagree condition persists for all ~368 bare-axios sites and boundedHttp (boundedHttp.js:49-53 uses global axios).
  2. A stuck tab with no recovery — in the exact scenario this PR targets. After a sibling re-login, adoptStoredSession() (auth.js:220) updates the Pinia mirror and calls bare-axios fetchUserProfile() (auth.js:264, no explicit header), which still goes out on the revoked token → 401 → verdict staleadoptStoredSession()this.token === token early-returns (auth.js:225) → nothing repaired, nothing retried. profileVerified stays false for the life of that tab (so bug: Agent Detail and Workspace over-fetch on load (21 redundant requests, N+1 roster) #2198 keeps role-gated UI closed) and every bare-axios call silently 401s forever. That trades a destructive bug for a silent permanent one.
  3. Sign-out does not clear the credential everywhere. applySessionEndedElsewhere() (auth.js:249) clears 5 store fields and deliberately nothing else; only the acting tab's logout() does delete axios.defaults.headers.common['Authorization'] (auth.js:553). A sibling tab keeps an in-memory copy of the revoked JWT and keeps transmitting it.

The guard that should have caught this walks one file. platformSessionSync.spec.js:145 matches axios.defaults.headers.common['Authorization'] = against AUTH (= stores/auth.js) only. Architecture Invariant #5's own stated lesson applies verbatim: "a guard that walks only one of the two trees is not a guard."

C2 — the architecture area file now asserts something false

docs/memory/architecture/workspace.md gains "The axios.defaults copy is written nowhere (only cleared on logout…)", and feature-flows/workspace-session-signout.md repeats it plus "setupAxiosAuth is a documented no-op". App.vue:64 contradicts both. That file is the read-before-you-change source of truth, so a false invariant there is the regression seed the Architecture Map exists to prevent.

The wiring that delivers AC #2/#3/#4 is pinned by regex only

Baseline is 134 files / 3029 tests. Each of these mutations leaves it fully green:

Mutation Result
main.js:55-63 — restore logout(); router.push('/login') on the stale branch (literally the reported bug) 3029 passed
main.js:98 — interceptor registered and reads the token, but never applies the header 3029 passed
main.js:133-134 — invert the storage listener (sibling login ends this tab's session) 3029 passed
delete the interceptor block outright 1 red — expect(MAIN).toContain("if (!headers.Authorization …"), a regex over stripped source

platformSessionSync.spec.js:141-216 reads main.js, api.js, auth.js and clientPortal.js as text, so the global request interceptor, the response interceptor, the cross-tab storage listener (AC #2) and handlePlatformUnauthorized (AC #3/#4) are all unexecuted. Your own mutation table is accurate for the pure function — it is the reaction implementing it that is unpinned.

What would get it on the next train

  1. Remove or neutralise the App.vue:59-64 writer. Non-trivial on purpose: that block also seeds authStore.token/isAuthenticated and gates connect(), overlapping initializeAuth — hence a design call, not something the train should do for you.
  2. Re-scope the axios.defaults guard to the whole src/frontend/src tree, not one file.
  3. Probably have adoptStoredSession/applySessionEndedElsewhere clear the legacy copy, so a stuck tab self-heals.
  4. Correct the two docs claims to match whatever (1) decides.
  5. Pin the three wiring sites with assertions that execute. vitest.config.js is environment: 'node' with no DOM, so this needs either jsdom or extracting the reaction into a pure callable the way sessionLostVerdict already is — a harness decision, which is the other reason this is yours rather than ours.

Also worth a look while you are in here:

  • W1 — the Workspace veto reads a different source of truth than the gate, narrowing ent#357. main.js:49 computes portalTokenPresent from shared localStorage (trinity.portalToken), while the portalHttp site gates on the per-tab store getter isPlatformSession (clientPortal.js:207). Cross-tab they disagree: a client signing in in tab B writes the shared key; an operator on the Workspace in tab A then gets ignore instead of the ent#357 bounce and is stranded on a dead Workspace with no login prompt. On dev that path bounced unconditionally. Note this partly reintroduces the /review I1 lesson in the comment this PR deletes — "who gets bounced is decided by the PLATFORM token, not the portal one."
  • W2platformSession.js:17 documents installCrossTabSync(); git grep finds one hit, that comment. The listener is inline at main.js:129.
  • W3 — 22 direct localStorage.getItem('token') sites remain outside readStoredToken() (network.js ×5, notifications.js ×4, AgentDetail.vue ×3, TemplateSelector.vue ×3, websocket.js, AgentTerminal.vue, AgentListPanel.vue, HostTelemetry.vue, App.vue), so "one reader" is not true yet. Several are WS/EventSource that genuinely cannot use an interceptor, but they all bypass the blocked-storage try/catch the new reader exists for.

Cleared and not re-raising: the other three 401 sites are not divergent logout handlers; there is no boot-order hole (initializeAuth's fetchUserProfile is dispatched after await detectAuthMode()); api.js's instance snapshots defaults at import time so its own interceptor is authoritative; both ratchets are untouched and green; git merge-tree against dev is clean and the suite passes on the merged tree.

Your body says "Not verified in a browser … worth one manual pass of the finding-1 repro before merge." That manual pass is exactly what surfaces C1.

@dolho

dolho commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

/review — head ceb2e938, second pass after the 2026-09-15 train ejection

Nothing pushed since the ejection; the C1 above is confirmed on this head, not re-derived:

  • src/frontend/src/App.vue:64axios.defaults.headers.common['Authorization'] = \Bearer ${token}`` still runs on boot. The PR's own docs claim this write exists nowhere.
  • src/frontend/tests/unit/platformSessionSync.spec.js:145 — the "no defaults writer" guard greps AUTH (= stores/auth.js) only. It walks one file, so it is green over the writer it exists to forbid.
  • src/frontend/src/stores/auth.js:226adoptStoredSession returns early on this.token === token, which is the stuck-tab loop: bare-axios fetchUserProfile() goes out on the defaults-derived (revoked) token → 401 → verdict stale → adopt → same token → return; nothing retried, profileVerified never flips.

Axios 1.19's Axios.js merges defaults.headers.common into config.headers before the request-interceptor chain runs, so main.js's if (!headers.Authorization …) skips exactly when the stale value is present. That makes the global request interceptor — the load-bearing half of AC #1 — inert on every boot with a token. So this is a CRITICAL and the PR is NEEDS-FIX, on the reviewer's findings, not on anything new.

What lands it (the ejection's list, prioritised)

  1. App.vue:59-64 — stop writing the default. The block also seeds authStore.token/isAuthenticated and gates connect(), so replace the header write with the store's own initializeAuth seam rather than deleting the block; the design call is whether connect() keeps reading the store or the interceptor becomes the only credential path. Either is fine; the doc claim in architecture/workspace.md and feature-flows/workspace-session-signout.md must then match what was chosen (C2).
  2. adoptStoredSession / applySessionEndedElsewheredelete axios.defaults.headers.common['Authorization'] in both, so a tab that inherited a stale default self-heals on the next storage event instead of only on its own logout().
  3. Re-scope the guard to src/frontend/src/** — a Grep over the tree for axios.defaults.headers.common['Authorization'] = with an allowlist of exactly zero writers.
  4. Pin the three wiring sites with something that executes — the reviewer's mutation table (restore logout(); router.push('/login') on the stale branch → 3029 green) is the strongest evidence in the thread. vitest is environment: 'node', so extract handlePlatformUnauthorized's reaction and the storage listener into pure callables the way sessionLostVerdict already is, and call them.

W1 (portal-token veto reads shared localStorage while the portalHttp gate reads the per-tab store) is real and belongs in the same push — it is the ent#357 cross-tab bounce narrowing, and once (1) is done it becomes the only remaining way two tabs disagree. W2/W3 are tidy-ups.

Applying status-needs-fix per #2815 (this is the PR shape that issue was filed on: a finding a day old, branch unchanged, still reading ✅ Ready). The label clears itself on the next push once #2819 lands; until then, whoever pushes the fix removes it by hand.

@vybe

vybe commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

merge-train: not on this train

This branch already carries status-needs-fix from the earlier train today, and the findings reproduce unchanged on head ceb2e938. Recording them here with fresh evidence, plus one that is new.

The design is right and the pure-function half is genuinely well tested — platformSessionVerdict.spec.js really calls sessionLostVerdict, and workspaceSession.spec.js:418-470 replacing its hand-copied shouldBounce with the real function is a proper fix to a guard that was pinning nothing.

❌ C1 — the second credential source survives, so the central mechanism is inert

src/frontend/src/App.vue:64 — untouched by this PR — runs axios.defaults.headers.common['Authorization'] = \Bearer ${token}`inonMounted`, on every boot where localStorage holds a token, on every route.

In axios 1.19.0 (lib/core/Axios.js:157) config.headers = AxiosHeaders.concat(contextHeaders, headers) executes before the interceptor chain is built at line 160, so a defaults-derived header is indistinguishable from an explicit one — and main.js:96's if (!headers.Authorization && !headers.authorization) skips precisely when the stale value is present.

Measured against real axios with a stub adapter: defaults STALE + storage NEW → the wire carries Bearer STALE-TOKEN-FROM-APP-VUE-BOOT.

119 of 367 bare-axios call sites pass no explicit header and depend solely on the new interceptor (views/Settings.vue ×17, stores/monitoring.js ×10, stores/settings.js ×10, views/PublicChat.vue ×10, stores/auth.js ×9, stores/network.js ×8, utils/boundedHttp.js ×5, …). Your own "an explicit header wins" rule makes the stale copy authoritative over storage for all of them, so AC #1 is false and AC #2 only half-lands: adoptStoredSession() converges the Pinia mirror while those 119 sites keep sending the boot-time token. setupAxiosAuth is now a no-op (auth.js:206) and neither sync action touches axios.defaults, so nothing short of a logout() in that tab or a reload can correct it — a destructive bug traded for a silent permanent one.

❌ C2 — the guard that should catch C1 walks one file

platformSessionSync.spec.js:145 matches axios\.defaults\.headers\.common\['Authorization'\]\s*= against AUTH — which is stores/auth.js only. Green today over the writer it exists to forbid. Invariant #5's own words: "a guard that walks only one of the two trees is not a guard."

❌ C3 — the docs assert a false invariant

docs/memory/architecture/workspace.md (and feature-flows/workspace-session-signout.md) now state "The axios.defaults copy is written nowhere (only cleared on logout…)" and "setupAxiosAuth is a documented no-op". App.vue:64 contradicts both. architecture/workspace.md is a read-before-you-change area file per the Architecture Map, so a false invariant there is the exact regression seed that map exists to prevent.

❌ C4 — new; contradicts a "pinned" claim in the PR body

main.js:72handlePlatformUnauthorized ends router.push('/login') without return, so it yields undefined and notifyPlatformUnauthorized's absorber (if (result && typeof result.catch === 'function')) never engages. The body lists the redundant-navigation fix as one of "two things found while reviewing my own diff" and says "Both are pinned" — it is neither. The guard passes only because it installs a synthetic handler that returns a promise. Reproduced with the production handler's shape: UNHANDLED REJECTION ESCAPED: Avoided redundant navigation to /login. Fix is return router.push('/login').

Why CI can't see any of this

main.js's interceptors, the storage listener, and handlePlatformUnauthorized are asserted by readFileSync + stripComments + toContain/regex (platformSessionSync.spec.js:141-216). vitest.config.js pins environment: 'node'; nothing mounts, no spec imports main.js as a module, and no spec reads App.vue at all. So ACs #2, #3 and #4 — the cross-tab sync, the single handler, and the fix for the reported bug — are delivered entirely by unexecuted wiring.

Mutation results on your head (baseline 131 files / 2937 tests green):

Mutation Result
main.js:60-62 — restore authStore.logout(); router.push('/login') on the stale branch (literally the bug #2791 reports) 2937 passed, 0 red
main.js:133-134 — invert the storage listener so a sibling login ends this tab's session 2937 passed, 0 red
platformSession.js — delete the stale arm from the pure verdict 2 red ✅

The live-consumer audit is clean, for what it's worth — every value you define has a real consumer. The defect is the inverse shape: live consumers with no executing test.

Warnings

  • W1 main.js:49 reads portalTokenPresent live from shared localStorage['trinity.portalToken'], while clientPortal.js:207 gates on the per-tab store getter isPlatformSession (hydrated once at init, :252). Cross-tab they disagree: a client signing in in tab B writes the shared key, and an operator on /workspace in tab A then gets ignore instead of the ent#357 bounce — stranded on a dead Workspace with no login prompt. On dev that path bounced unconditionally.
  • W2 auth.js:226adoptStoredSession early-returns on this.token === token, so a tab riding the stale defaults 401s → stale → adopt → early-return → nothing retried; profileVerified never flips.
  • W3 "one reader" isn't true yet: 22 direct localStorage.getItem('token') sites remain outside readStoredToken().
  • W4 platformSession.js:17 documents installCrossTabSync(); the only hit in the repo is that comment — the listener is inline at main.js:129.
  • W5 Body says Related to #2791; needs Fixes #2791.
  • W6 The storage listener keys only on TOKEN_KEY, so a sibling login writing token before auth0_user leaves this.user on the previous principal until fetchUserProfile() lands. Display-only (profileVerified=false gates role UI, bug: Agent Detail and Workspace over-fetch on load (21 redundant requests, N+1 roster) #2198), but real.

Security check, since the change touches session handling: Invariant #8 is not implicated — zero backend files, no dependencies.py, no require_admin/assert_admin/require_role call site, no mcp_scope handling. No access widening; these are client-side 401 reactions and the backend has already rejected the request in every path touched. The two behaviour changes (AC #5's ignore leaving a dead JWT at rest, and the stale arm adopting from localStorage) introduce no new trust.

🤖 Generated with Claude Code

dolho and others added 2 commits September 15, 2026 21:11
…ute under test, and the veto reads the per-tab store (#2791)

Merge-train review of #2811, all six ❌/W items:

C1  App.vue wrote `axios.defaults.headers.common['Authorization']` on every
    boot with a token. Axios merges that default into the request BEFORE the
    interceptor chain runs, so it arrived at the per-request rebuild looking
    explicit and won over storage for the life of the tab — the whole
    mechanism was inert, and 119 bare-axios sites kept the boot-time token
    after a sibling re-login. The write is removed (the store seed + WS
    connect stay); `adoptStoredSession` / `applySessionEndedElsewhere` delete
    any such copy as a belt, so a tab can never ride a credential storage no
    longer holds (W2).
C2  The "nobody writes the default" guard walked auth.js only. It now walks
    src/frontend/src/** and asserts zero writers; App.vue would have failed it.
C3  architecture/workspace.md + workspace-session-signout.md said the copy is
    written nowhere while App.vue wrote it. Both now describe what is true
    and why the guard is tree-wide.
C4  handlePlatformUnauthorized ended `router.push('/login')` without `return`,
    so notifyPlatformUnauthorized's absorber never engaged and a redundant
    navigation escaped as an unhandled rejection. It returns the navigation.
CI  The reaction, the storage listener and the request rebuild were inline in
    main.js and pinned by regex — restoring the reported bug on the `stale`
    branch and inverting the listener both stayed green. They are now
    `reactToPlatformUnauthorized`, `reactToStorageEvent` and
    `applyRequestCredential` in utils/platformSession.js, taking their
    collaborators as arguments; the spec EXECUTES them with fakes, and a
    five-mutation battery (stale→logout, inverted listener, header never
    applied, navigation dropped, App.vue writer back) is red on every one.
    main.js is wiring only, and the source guards assert exactly that.
W1  The Workspace veto read `portalTokenPresent` from shared localStorage
    while portalHttp gates on the per-tab store; a client signing in in
    another tab stranded an operator's Workspace tab on an expired JWT with
    `ignore`. It reads `useClientPortalStore().portalToken` now.
W3  The 22 direct `localStorage.getItem('token')` reads outside the reader
    (WS/EventSource included) go through `readStoredToken()`; a tree-wide
    guard keeps "one reader" true.
W4  The stale `installCrossTabSync()` reference is gone with the rewrite.
W6  The storage listener also hears `auth0_user`, and adopting an identical
    token refreshes the user from storage, so a sibling login's profile
    landing a tick after its token is not missed.

3066 frontend unit tests green; vite build green.

Fixes #2791

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf
@dolho dolho removed the status-needs-fix PR has an unaddressed review/validation finding; cleared by the author's next push (#2815) label Sep 15, 2026
@dolho

dolho commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

All of it addressed in 5326edb5c (on top of a clean origin/dev merge).

C1 App.vue's boot-time axios.defaults write is removed (the store seed + connect() stay — they were never the problem). Both sync actions and logout() delete the default as a belt, so even a writer that reappears cannot leave a tab riding a credential storage no longer holds — which also closes W2: a tab that hit the stale loop now converges on the next 401 instead of early-returning forever.
C2 The guard walks src/frontend/src/** and asserts zero writers; with the App.vue line put back it goes red (mutation M5 below).
C3 architecture/workspace.md and workspace-session-signout.md say what is now true — and why the guard is tree-wide, naming the App.vue writer so the next reader knows what the rule is protecting against.
C4 handlePlatformUnauthorized returns the navigation; the spec drives the real handler shape through notifyPlatformUnauthorized with a rejecting goToLogin and asserts nothing reaches unhandledRejection.
CI gap The reaction, the storage listener and the request rebuild are extracted — reactToPlatformUnauthorized(error, deps), reactToStorageEvent(event, deps), applyRequestCredential(config, readToken) in utils/platformSession.js — taking the store actions / router push / storage reader as arguments, and the spec executes them with fakes. main.js is wiring only. Your two mutations plus three more, each restored from a scratch copy:
Mutation Before Now
stale branch → logout(); goToLogin() (the reported bug) 2937 green 1 red
invert the storage listener 2937 green 3 red
interceptor reads the token, never applies the header 2937 green 1 red
handler drops the navigation (C4 regressed) 1 red
App.vue writer put back (C1 regressed) 2937 green 1 red
W1 The veto reads useClientPortalStore().portalToken — the same per-tab gate portalHttp uses — never shared localStorage. A client login in tab B no longer strands an operator on /workspace in tab A. Pinned.
W3 All 22 direct localStorage.getItem('token') reads (WS + EventSource included — they still need a reader, just not an interceptor) go through readStoredToken(); a tree-wide guard asserts the reader is the only one.
W4 installCrossTabSync reference gone with the header rewrite.
W5 Body says Fixes #2791.
W6 The listener hears auth0_user too, and adopting an identical token refreshes the user from storage.

3066 frontend unit tests green, vite build green, ratchets untouched. Removing status-needs-fix by hand this once — #2819 has landed on dev, so from here the author's push does it.

Still true, and still worth the manual pass you asked for: none of this was verified in a browser. The finding-1 repro (Workspace tab open across a logout + re-login in another tab) is the thing to click through before merge.

dolho added a commit that referenced this pull request Sep 16, 2026
…ords its mutation (#2829)

Three of five ejections on the 2026-09-15 merge train — the third train
running — were tests that prove the code was written rather than that it
runs: source-text regexes over the module under test (#2811), a bound
check at the one value where both bounds coincide (#2817), a docstring
claim about CI never negative-controlled (#2805). All green.

- docs/testing/STRATEGY.md: a new "Evidence bar for a test" section beside
  the harness bar — the three spellings, the two greps (the live-consumer
  grep is the one that decides), guard-vs-source-only with the train's own
  pair (#2819 kept, #2811 ejected, same shape), mutation as the fix
  standard, bound tests away from the coincidence — each with what
  enforces it.
- .github/pull_request_template.md: a Testing checkbox for "every new test
  executes the changed path" and a `Mutation:` line naming the test(s)
  that go red with the fix reverted ("n/a — not a fix" otherwise). The
  trailing space after the colon matches the existing `Journey Impact:`
  line — a fill-in prompt.
- docs/memory/learnings.md: the class, with the prior occurrences.

The skill half — /review Step 2.5 and /validate-pr §5.4 answered first
and in writing, /implement's two done-criteria — is trinity-dev#29.

Fixes #2829

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf
@dolho

dolho commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

/validate-pr + /review — head 5326edb5c vs dev (post-fix pass, merge-train pre-validation, lane B)

Scope: CLEAN. 8 ACs → 8 DONE. Local npm run test:unit: 136 files / 3066 passed.

The 2026-09-15 17:11 findings, checked in the head:

Item Closed Evidence
C1 App.vue axios.defaults writer App.vue:41-72import axios gone; store seed + connect() only. Tree grep axios.defaults.headers[^=]*=0; the 3 remaining hits are deletes (auth.js:226,263,569)
C2 one-file guard platformSessionSync.spec.js:65-78,324-337 walks src/frontend/src/**, asserts writers == []; App.vue line re-inserted → 1 red
C3 false docs claim architecture/workspace.md + flow now describe the head and name the App.vue writer as the guard's reason
C4 return router.push main.js:57-67 returns navigation; spec :220-239 drives it with a rejecting goToLogin, nothing reaches unhandledRejection
CI gap (regex-only wiring) ✅ reactions / ⚠️ wiring reactToPlatformUnauthorized, reactToStorageEvent, applyRequestCredential in platformSession.js:223-313, executed with fakes at spec :168-306; stalelogout() restored → 1 red. main.js wiring stays regex-pinned (:347-368) — acceptable now that it holds three one-line delegations and no decidable logic
W1 shared-storage veto main.js:51-56 reads useClientPortalStore().portalToken (per-tab)
W2 stale-loop early return auth.js:226 deletes the default before the compare; identical-token branch merges readStoredUser() (:236-239)
W3 22 direct readers tree grep localStorage.getItem('token') outside platformSession.js0; guard at spec :339-345
W4 phantom installCrossTabSync header rewritten platformSession.js:13-31
W5 Fixes #2791 body, bare form
W6 auth0_user key reactToStorageEvent hears USER_KEY (:306); spec :271-275

Critical: none. Logout revoke carries the token explicitly (auth.js:557-559); applyRequestCredential never overrides an explicit header (:225), so #187 keeps revoking; the storage listener filters storageArea (:304).

Informational

  • I1 (7/10) AC Setup improvements #5's veto ignores platformFallbackSuppressed. main.js:53 is portalTokenPresent = !!store.portalToken only. A client browser holding a dead operator JWT: App.vue:69 seeds isAuthenticated + connect()websocket.js:57-61 mints a ticket, 401s, retries every 5 s. While the portal token lives → ignore (right). Once the client session expires, endSession (clientPortal.js:515-527) nulls portalToken and sets platformFallbackSuppressed=true; the next retry → sessionLostVerdict sees storedToken + no portal token → logout/login: the client at the OTP form is bounced onto the operator login (the fix(workspace): Sign out doesn't sign out — the platform JWT silently re-authenticates on refresh #2258/fix(workspace): a client session that EXPIRES on a browser holding a platform login falls back to the platform identity #2261 class). Not a regression — dev bounced on every such 401 — but the tab's own "I am a client tab" declaration is unread. Mechanical: portalTokenPresent = !!s.portalToken || s.platformFallbackSuppressed (+ one spec case). Per-tab sessionStorage, so it cannot reopen W1.
  • I2 (7/10) workspaceSession.spec.js:451-461 "verdict does not depend on the portal token racing away" passes portalTokenPresent:false to both before and after — it cannot fail. Should assert the I1 property instead.
  • I3 (5/10) auth.js:121 reads auth0_user raw instead of readStoredUser(); the one-reader guard covers token only. Cosmetic.
  • Pre-existing, not this PR: the 5 s ticket-mint retry on a permanently-401ing JWT never gives up (websocket.js:59).

Coverage: executed, not source-text — two mutations each produced a real red.

Verdict: READY. I1 + I2 are mechanical and will be applied on this branch before the train (announced here when pushed). Still worth the manual click-through you asked for: none of this was verified in a browser.

…ter expiry (#2811) — mechanical, per the merge-train note on the PR

`handlePlatformUnauthorized` read `portalToken` alone. `endSession({expired})`
nulls that token and sets `platformFallbackSuppressed` in the same breath, so
for a client tab holding a dead operator JWT the 5 s ticket retry's next 401
fell through to `logout` and threw the client from the OTP form onto the
operator login — the #2258/#2261 bounce, reopened for exactly that instant.
`portalTokenPresent` now folds in the suppression flag (per-tab sessionStorage,
so W1 cannot return). The wiring pin follows, and the tautological
"racing away" spec case (identical inputs both sides) is replaced by one that
can fail: client tab → ignore, live or expired; operator → logout.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf
@dolho

dolho commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

merge-train: pushed to this branch — one commit, mechanical, per the review above.

  • I1 main.jsportalTokenPresent = !!portalStore.portalToken || !!portalStore.platformFallbackSuppressed, so an expired client session's tab still reads as a client tab and the next 401 on the dead operator JWT is ignore, not a bounce onto /login. Wiring pin updated.
  • I2 workspaceSession.spec.js — the identical-inputs "racing away" case is replaced by one that can fail (client tab → ignore live or expired; operator → logout).

npm run test:unit: 136 files / 3066 passed; vite build green. Nothing else touched.

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/validate-pr (Lane B) — APPROVE. Independently confirmed at head 75b1a6f: zero axios.defaults.headers writers in src/frontend/src, zero direct localStorage token readers outside platformSession.js, I1 veto reads platformFallbackSuppressed. All checks green incl. build (test:unit) and e2e. Residual: finding-1 repro not yet verified in a browser — one manual pass on dev after merge.

@vybe
vybe merged commit 3360e8a into dev Sep 16, 2026
28 checks passed
vybe added a commit that referenced this pull request Sep 16, 2026
…ords its mutation (#2829) (#2833)

Three of five ejections on the 2026-09-15 merge train — the third train
running — were tests that prove the code was written rather than that it
runs: source-text regexes over the module under test (#2811), a bound
check at the one value where both bounds coincide (#2817), a docstring
claim about CI never negative-controlled (#2805). All green.

- docs/testing/STRATEGY.md: a new "Evidence bar for a test" section beside
  the harness bar — the three spellings, the two greps (the live-consumer
  grep is the one that decides), guard-vs-source-only with the train's own
  pair (#2819 kept, #2811 ejected, same shape), mutation as the fix
  standard, bound tests away from the coincidence — each with what
  enforces it.
- .github/pull_request_template.md: a Testing checkbox for "every new test
  executes the changed path" and a `Mutation:` line naming the test(s)
  that go red with the fix reverted ("n/a — not a fix" otherwise). The
  trailing space after the colon matches the existing `Journey Impact:`
  line — a fill-in prompt.
- docs/memory/learnings.md: the class, with the prior occurrences.

The skill half — /review Step 2.5 and /validate-pr §5.4 answered first
and in writing, /implement's two done-criteria — is trinity-dev#29.

Fixes #2829


Claude-Session: https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Eugene Vyborov <1073874+vybe@users.noreply.github.com>
webmixgamer added a commit that referenced this pull request Sep 16, 2026
… a real transport, an agent key, and a counted backend (Abilityai/trinity-enterprise#628)

Review C1 on #2826: `server.ts::addToolWithAudit` composes `withAgentAccess`
around an `enforce` row's `execute` and hands the result to `withAudit`. Both
existing files proved the wrapper — `access.test.ts` calls it directly,
`tools/loops.test.ts` builds the composition by hand — and neither executed the
composition site. Build the wrapper there and discard it (`withAudit(tool.name,
tool.execute, …)`) and the suite stayed at 406 green with `run_agent_loop`
ungated: the #2811 class, the reaction tested and the wiring that calls it not.

`access-wiring.test.ts` drives the tool the way an agent does: a real
`createServer` in key mode, a real MCP client presenting an agent-scoped key
over the streamable-HTTP transport, and a stub backend that answers
`/api/mcp/validate` and the permission-edge read and COUNTS every
`POST /api/agents/<target>/loops`. Without an edge the count stays at zero and
the caller reads the denial; with an edge the loop starts; a self loop starts
without a permission read. Under the reviewer's mutation the first case is the
one red (`loopPosts` = ["bravo"], `success: true`); restored byte-identical,
409/409. Pattern: `inline-auth-transport.test.ts` (#2035).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
vybe pushed a commit that referenced this pull request Sep 16, 2026
…the gate a mechanism (Abilityai/trinity-enterprise#628) (#2826)

* fix(mcp): gate the loop tools on the agent permission edge, and make the gate a mechanism (Abilityai/trinity-enterprise#628)

`run_agent_loop` resolved its target from the caller's parameter and called no
gate, and the backend behind it resolves an agent-scoped key to its owner
(Invariant #8) — so an agent key could start a loop on any same-owner sibling
with no `agent_permissions` edge. Verified live: the loop ran on the sibling.
The per-tool gate had ten spellings across nine modules and the tool added
last called none of them, so this closes the class, not the tool.

- src/mcp-server/src/access.ts (new): ONE implementation of the agent-scope
  edge (`checkAgentEdge` — system bypasses; an agent key reaches itself and its
  permitted targets, the permitted list read fail-closed; a user key passes
  through because the backend already scopes it by role and per-user grant;
  any other scope is denied — an allowlist, #2323); `TOOL_ACCESS_POLICY`, one
  row per registered tool (enforce / in-tool / baselined:<owner> / none:<why>);
  `policyFor` (no row, an enforce on an undeclared parameter, or a none on a
  tool whose parameters name an agent throws at registration); and
  `withAgentAccess`, the enforce wrapper.
- server.ts: every tool passes `policyFor`; enforce rows are wrapped before
  `withAudit`; dynamic tools declare their policy as an argument.
- tools/loops.ts: `run_agent_loop` is an enforce row; `get_loop_status` /
  `stop_loop` resolve the loop's agent, gate, then act — a denial withholds the
  payload behind a compound uniform reason (the id was the caller's only
  input), and a failed resolve sends no stop and names the escape hatch.
- tools/chat.ts: the agent/system branch delegates to the shared gate; unknown
  scopes are denied before the user branch (closes a fall-through that
  promoted an unnamed agent key to the same-owner rule); `resolveClient` moves
  to access.ts. types.ts / client.ts: `LoopStatus`, `getLoopStatus` typed.
- .github/workflows/mcp-server-test.yml: an offline boot smoke of
  dist/server.js — bundler module resolution hides a missing `.js` until the
  container dies.
- tests: access.test.ts (createServer boots against the real table, every row
  names a tool and every tool has a row; policyFor refusals; the wrapper never
  reaches execute on a denial; the read fails closed), tools/loops.test.ts
  (all three tools through the real row and wrapper with a fake client; a
  denial means the side-effecting call never happened; the reason is
  byte-identical to chat_with_agent's), J10: the strict xfail comes off, and a
  loop-id read or stop after the edge is removed is refused without naming
  the loop's agent while the owner can still stop it.
- docs: the flow, the requirement (§38.1 said cross-agent loops were out of
  scope, the flow said "backend enforces", the code did neither), the
  architecture area file and the P-02 catalog entry now say one thing: a
  tool-surface gate at the MCP layer; the REST routes stay owner-equivalent,
  which is Abilityai/trinity-enterprise#629's ruling. The 53 `baselined` rows
  are that issue's work list. Learnings ledger +1; diff-scoped CSO report.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* test(mcp): execute the line that wires the gate — run_agent_loop over a real transport, an agent key, and a counted backend (Abilityai/trinity-enterprise#628)

Review C1 on #2826: `server.ts::addToolWithAudit` composes `withAgentAccess`
around an `enforce` row's `execute` and hands the result to `withAudit`. Both
existing files proved the wrapper — `access.test.ts` calls it directly,
`tools/loops.test.ts` builds the composition by hand — and neither executed the
composition site. Build the wrapper there and discard it (`withAudit(tool.name,
tool.execute, …)`) and the suite stayed at 406 green with `run_agent_loop`
ungated: the #2811 class, the reaction tested and the wiring that calls it not.

`access-wiring.test.ts` drives the tool the way an agent does: a real
`createServer` in key mode, a real MCP client presenting an agent-scoped key
over the streamable-HTTP transport, and a stub backend that answers
`/api/mcp/validate` and the permission-edge read and COUNTS every
`POST /api/agents/<target>/loops`. Without an edge the count stays at zero and
the caller reads the denial; with an edge the loop starts; a self loop starts
without a permission read. Under the reviewer's mutation the first case is the
one red (`loopPosts` = ["bravo"], `success: true`); restored byte-identical,
409/409. Pattern: `inline-auth-transport.test.ts` (#2035).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants