Re base branding branch - #923
Closed
ofilson wants to merge 4 commits into
Closed
Conversation
* feat: add a loading state to the Agent Designer form
The Agents edit view painted an empty form while the bindable palettes and
the agent record were still in flight, then visibly rewrote itself as each
response landed — no signal to the user that anything was loading. The
`loadingAgent` signal already existed but nothing in the template read it.
- Add `loadingPalettes` (the RBAC-filtered palettes are fetched on every
entry, create and edit) and a `loading` computed that ORs it with
`loadingAgent`. Both fetches feed sections of the same form, so the
editor stays behind a skeleton until both settle.
- Render a pulsing skeleton on the form's own section rhythm (persona card
→ model grid → pill rows) so the real form lands roughly in place, with
role="status" / aria-live / aria-busy and an sr-only "Loading agent…".
- Hold the live preview back too — it resolves from the saved record, so
mounting it against a half-hydrated form flashed an empty agent card.
- Disable Save while loading, and gate Share on the record's permission
instead of the default 'owner' guess (it flashed in, then could vanish).
- Wrap `loadPalettes` in try/catch: it was an unhandled rejection before,
and a failure must not strand the page on its skeleton.
Tests cover the skeleton rendering in place of the form, Save being
disabled, and the page staying on the skeleton until BOTH fetches settle.
The existing suite needed a microtask-flush helper — `whenStable()` was
resolving ahead of the load promises' `.finally` handlers.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs: add Agent Marketplace spec, superseding the Agent Directory spec
Specifies a browsable store over the existing Agent record: a Discover
page, a per-agent detail page, square icons, @-mention, role-seeded
default pins, and the admin console that curates all of it.
Carries forward D1 (one noun), D2 (sparse directory index), D3 (listing
separate from visibility) and D6 (pin, never fork) from the directory
spec, and reverses two of its decisions:
- Publication is admin-gated (author submits, system_admin approves)
rather than self-service with takedown as the remedy. Granular
curator permissions are a follow-up, not v1 scope.
- Role-seeded pins resolve live rather than being materialized per
user, which removes a fan-out over every member of a role at the
cost of the "seed new members only" option.
Also settles publisher identity (an admin-managed PublisherProfile,
display-only and never an access gate, with institution publishing so
official agents don't carry a staff member's name) and the split
between admin-editable presentation and author-owned behavior.
docs/specs/agent-directory.md keeps its reasoning for the carried
decisions and gains a banner pointing here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): Agent Marketplace phase 1 — listing lifecycle, sparse GSI5, admin review
Implements phase 1 of docs/specs/agent-marketplace.md: authors can submit an Agent
for review, admins can approve / request changes / take down, and nothing is
user-visible until the Discover page lands in phase 2.
Backend
- AgentListing block + pure state machine (assistants/listing.py). Approval is the
only edge into `published`, asserted exhaustively over every state pair.
- Sparse AgentDirectoryIndex (GSI5): GSI5_PK=LISTED#{category},
GSI5_SK=CREATED#{created_at}, written only while published. State and index keys
move in one update_item so they cannot disagree.
- Publisher profiles (D12) with per-user eligibility and auto-created individual
profiles on first submission. publisherId is display-only and appears in no
access check; ownerId still governs edit rights and Skills v2 invoke-through.
- Submit/review/takedown API under /agents and /admin/agents. D7 runs at
submission: skill exposure is enumerated, and a memory_space binding blocks
submission with a 400 naming the space.
- PATCH /admin/agents/{id}/listing accepts presentation fields only; behavior
fields are refused at the model boundary and each edit is logged for the author.
- AGENT_MARKETPLACE_ENABLED, default on with a kill switch, app-api only.
Closes a latent hazard found on the way: Assistant is extra="allow" and reads
hydrate from the raw item, so GSI keys and the listing block round-trip as model
fields and _update_assistant_cloud would rewrite them. Both are now immutable
there, otherwise a stale author edit racing a takedown silently republishes a
pulled agent. Two tests reproduce that race and fail without the fix.
Backfill is deliberately empty: no existing record gets a listing block, and none
is ever inferred from visibility == 'PUBLIC' (D3).
Frontend
- Admin Review queue and Listings pages under a new Agent Marketplace nav group,
matching the design mockup's structure and takedown copy.
Categories are a seeded constant this phase; phase 2 swaps the source for
admin-managed records with no change to the stored shape. D14's RBAC capability
is deliberately not built — see the note in feature_flags.py.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(marketplace): settle the icon size ceiling at 400 KB
D5 said 256 KB while the design mockup's uploader said 400 KB. Resolved in
favour of 400 KB so phase 4 does not have to re-open it. Still well under the
400 KB DynamoDB item limit that motivated storing the icon by S3 key rather
than inline on the record.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(marketplace): add D15 — user problem reports into the admin queue
Users can report a problem with a published Agent; the report lands in the admin
console as a second work stream beside submissions.
Deliberately not a review: private to admins, never rendered to another browsing
user, and never an input to ranking. That keeps the 'no ratings, reviews or
comments' non-goal intact while giving the store a maintenance signal, and it
avoids the failure where report volume becomes a way to bury someone's Agent.
Five decisions: reports are triaged rather than auto-forwarded (the existing
request-changes / takedown reason stays the author-facing channel); the reporter
is visible to admins and never to the author; only published Agents are
reportable; one open report per reporter per Agent; and reports get their own
open → resolved | dismissed lifecycle that never mutates listing.state.
Storage is child rows under the Agent plus a sparse GSI6 open-report index,
following the GSI5 precedent. Sequenced as phase 8 because the report action
lives on the detail page (phase 3); independent of phases 4-7 otherwise.
Also marks phases 0-1 shipped and 2 in progress.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): phase 2 — store browse API, Discover page, categories admin
Implements phase 2 of docs/specs/agent-marketplace.md.
Backend
- GET /agents/store[?category=&cursor=] and GET /agents/store/front. Browse is a
pure GSI5 query: it cannot return an unpublished agent because an unpublished
agent has no key in the index. No filtering on listing.state anywhere on the
read path — the sparseness is the safety property.
- AgentListingResponse is a deliberately narrow projection: agentId, name,
tagline, emoji, iconUrl, publisher, category. No instructions, no binding refs,
no owner. A test pins the exact key set, since this payload goes to every
browsing user.
- Cursor pagination within a category (one partition). Browse-all merges across
categories newest-first and returns no cursor rather than a misleading partial
one; a malformed cursor degrades to the first page instead of 500ing.
- Categories become admin-managed records (D10) replacing the phase 1 constant,
with CRUD under /admin/agents/categories. Seeded idempotently on first read so
no environment is ever category-less.
The category id is immutable because it is half of GSI5_PK = LISTED#{category};
renaming would strand every listing in a partition browse no longer queries. So
rename changes the label only, the seeded ids are the exact phase 1 strings (no
migration, no GSI rewrite), and delete is refused while listings still reference
the category — disable instead, which drops it from the pickers while its agents
keep working.
Frontend
- Discover page under a new /agents hub tab strip: per-category shelves of
icon/name/one-line rows, client-side search over what is loaded, empty
categories auto-hidden.
- Admin Categories page: add, rename, enable/disable, delete.
Not user-visible yet: the /agents sidenav entry is already preview-gated to
system-admins, so Discover inherits that gate. No CDK change — GSI5 shipped in
phase 1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): phase 3 — detail page, runnability, and the instructions gate
Detail page + runnability + the instructions gate, per the Phase 3 line of
docs/specs/agent-marketplace.md.
The instructions gate (⚠️ behaviour change)
-------------------------------------------
GET /agents/{id} returned `instructions` to any PUBLIC viewer. That was bounded
while PUBLIC meant "anyone with the link"; once a store puts the link in front of
the institution it is not. It is now gated to permission in ("owner", "editor")
and dropped otherwise — the routes already serve the model with
response_model_exclude_none, so a viewer's payload simply omits the key.
No existing test asserted the field was present on a read (it appeared only in
`_make_assistant` defaults and two POST bodies), so nothing needed correcting;
test_agent_detail.py pins the new behaviour per permission level, including a
whole-payload check so a future field that echoes the prompt cannot re-open it.
Detail read
-----------
`capabilities[]` of {label, kind} — display names, never ids or refs — plus
`modelLabel`, `publisher` and `categoryLabel`. Labels resolve against the
*unfiltered* catalog on purpose: D6 has to name a capability the viewer lacks,
which a viewer-filtered lookup would silently drop. `publisher`/`categoryLabel`
exist because the stored listing holds ids: `listing.publisherId` is an internal
reference (and D12 display-only — it stays out of every access check) and
`listing.category` is an id an admin may since have renamed.
Also fixes a Phase 1 gap: `listing`, `tagline` and `iconKey` were declared on
AgentResponse but `compat.to_agent_view` never projected them, so `listing` was
always absent. An unsubmitted agent's payload is unchanged (all None, excluded).
Runnability (D6)
----------------
GET /agents/{id}/runnability composes bindable_catalog.list_bindable per kind and
diffs — no sixth access service (Designer D4). Two rules are load-bearing:
* knowledge_base is never gated. compat synthesizes a KB binding on every legacy
agent while list_bindable returns [] for that kind, so a naive diff marks the
entire back catalogue blocked.
* `limits` requires an explicitly optional binding. agent_binding_resolver is
block-with-message for every kind it resolves, so a gap only degrades when the
binding declares config.optional == true; anything else missing is `blocked`.
Telling a user an agent "runs with limits" when the next turn will raise would
be a preview that lies. No surface writes `optional` yet — it is the seam for
when the resolver grows a degrade path.
SPA
---
/agents/:id — 84px icon, publisher with the verified mark, tagline, Start chat;
a hero band carrying the @Agent prompt; About; Try asking; a Details panel; and
"What it can access" with the D6 availability line. Discover rows now route
through. Identity and runnability load separately so the page paints before the
slower catalog fan-out settles.
Out of scope and deliberately not rendered: "Add to my agents" (pins, phase 5),
icons (4), report a problem (8). The hero band and the starters are display-only
because the composer has no prompt-prefill entry point — same reasoning that kept
the store-front star and the mockup's Preview button out of phases 1–2.
Deploy: backend.yml only. No new index (GSI5 shipped in phase 1), no new env var
or IAM — runnability composes services app-api already calls.
Tests: 5014 backend passed; 1492 SPA passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): the author's Submit UI + a D7 preflight
The Phasing table never assigned the author's half of the publication flow.
POST /agents/{id}/listing/submit and DELETE /agents/{id}/listing shipped in
phase 1 and the whole reviewer console shipped with them, but nothing in the
SPA called either route — so no agent could reach the store without a
hand-rolled request. This is the missing caller.
Preflight (D7.1)
----------------
GET /agents/{id}/listing/preflight runs the submit path's own two checks
without transitioning, because D7.1 asks the *dialog* to enumerate the
exposed skills and phase 1 could only answer in the submit response. Telling
an author which of their skills just became world-readable after they
committed is a weak form of informed consent.
`_memory_space_block` splits into a `_reason` half that both callers share,
so the block the dialog shows and the block the write enforces cannot drift.
Owner-only, like every other author path: skill exposure is a statement about
the owner's publication, and it is not an editor's to see.
SPA
---
* Submit dialog — category picker, optional note, the D7.1 skill list by name,
and D7.2 rendered as a disabled Submit with the backend's message (which
names the space) rather than a 400 after the click.
* Listing-state badges on the author's own cards, with the reviewer's note
inline and the D13 admin-edit trail ("An admin updated the category on
Jul 24"). `adminEdits` was already on the list read; nothing new was needed
server-side to surface it.
* Withdraw / unpublish, whose confirmation says plainly that it revokes
nothing retroactively (D7.3) — pins keep working, conversations underway
keep running, and the direct link still resolves.
Three details worth naming
--------------------------
* The listing-state vocabulary moves to `agents/models/store.model.ts` and the
admin feature re-exports it. A reviewer and an author looking at the same
listing must read the same word for it.
* `reviewNote` carries two voices — submission writes the author's note there,
a review overwrites it with the reviewer's — so only states a reviewer must
have driven are attributed to one. `private` is reached from both and stays
neutral rather than guessing.
* Publication controls hide entirely until the kill-switch probe answers, so
`AGENT_MARKETPLACE_ENABLED=false` shows no control instead of one that 404s.
Badges are exempt: an existing listing is worth naming either way.
Backend 5051 passed; SPA 1519 passed. Verified live against dev-ai: the skill
disclosure, the memory-space block (named "Oliver's Brain"), submit → In
review, and withdraw → Private, surviving a reload.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): agent icons — upload, storage and URL resolution
Phase 4's backend half (D5). `iconKey` has been on the record since phase 1 —
declared, admin-PATCH-writable, projected by compat — with nothing to populate
it. This is the write path, and the read path that makes it renderable.
Storage
-------
Bytes to the existing assistants asset bucket (`{prefix}-rag-documents`, via
`S3_ASSISTANTS_DOCUMENTS_BUCKET_NAME`), under the `assistants/{agent_id}/`
prefix documents already use. **The key goes on the record, never the image** —
the MCP App icon lesson, except here the 400 KB ceiling *is* the icon limit, so
inlining would hit the DynamoDB item limit by design rather than by accident.
The key is content-addressed (`{sha256[:16]}.{png|jpg}`), which buys idempotent
re-uploads and gives the cache version away for free: the digest is the ETag and
the `?v=` on `iconUrl`.
Validation always re-encodes, even at exactly 512×512. That is not redundant —
re-encoding is what strips EXIF, and an icon cropped from a phone photo would
otherwise publish its GPS coordinates to the institution. The encode ladder
(JPEG quality steps; opaque PNG → JPEG; transparent PNG → quantized) exists for
the already-512, near-the-ceiling input; writing its test found that Pillow will
only quantize RGBA with FASTOCTREE, so the alpha-preserving rung was broken.
iconUrl: a stable path, not a presigned URL
-------------------------------------------
`iconUrl` resolves to `/agents/{id}/icon?v={digest}` and a new GET route serves
the bytes with `Cache-Control: immutable` + ETag. Presigning would hand out a
different string on every response, so a browsing user re-downloads every shelf
icon on every page view and the JSON stops being cacheable — for an asset whose
whole job is to be fetched repeatedly. A CloudFront path would need its own
origin and behavior over the documents bucket, i.e. a CDK deploy for something
the existing same-origin `/api/*` behavior already reaches; the URL is a stable
path, so adding that behavior later breaks no contract. **No platform.yml
deploy** — the bucket exists and app-api's task role already holds Get/Put/
Delete on it.
Two authorization rules, deliberately different
-----------------------------------------------
Writing an icon is editing the agent: owner or editor, matching `PUT /agents/{id}`.
Reading one follows the shelf: a published agent's icon is readable by any
authenticated user, because the store read already hands that agent's name,
tagline and emoji to every browsing user — and gating on the record would render
a broken tile on the shelf of any published agent still marked PRIVATE.
Also here: `DELETE /agents/{id}/icon`, which the spec's API table doesn't list.
Admin PATCH can only *replace* `iconKey`, so without it an author who uploads an
off-brand icon has no way back to the generated fallback the store is designed
around. A missing object answers 404 rather than 500, so a key that outlived its
bytes degrades to that same fallback instead of a broken image.
`write_icon_key` is a direct attribute write rather than a `update_assistant`
call: clearing has to REMOVE (the generic update builds a SET-only expression
from `exclude_none`, where None means "leave alone"), and an author may icon an
agent that has never been submitted and so has no listing block to ride on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): the agent icon, at all four store sizes
The SPA half of Phase 4 (D5). One component draws an Agent's square identity
everywhere the store shows one, and one dialog puts an icon on it.
app-agent-icon
--------------
The uploaded icon when there is one, the **generated gradient** when there is
not — and the gradient is the designed default, not a placeholder waiting for
Phase N. Most Agents will never have an uploaded icon, and an app store where a
third of the tiles are a bare emoji on grey reads as unfinished.
The gradient is a curated set of twelve pairs indexed by an FNV-1a hash of the
agent id, not hue arithmetic on the hash: free-running hue math reliably draws a
few muddy or low-contrast tiles, and for a default this common every draw has to
look deliberate. The hash lives in one exported function so the same Agent is
the same tile in the store, in My Agents and in the admin queue — three surfaces
each hashing their own way is three different agents to a user.
Two ratios needed tuning rather than inheriting from the type scale, checked by
rendering all twelve at every size: the corner radius steps with the box (a
constant `rounded-2xl` is a circle at 28px), and the glyph holds ~50% of the
tile (stepping `text-xl` → `text-4xl` grows the glyph slower than the box, so
the emoji drifted from 54% of a 28px tile to 43% of an 84px one — the same icon
looking emptier the larger it got).
`iconUrl` arrives as a relative path and the API base is prefixed here, so the
container never needs to know its public origin. A load failure falls back to
the gradient: the backend answers 404 for a key that outlived its object, and
that path has to land somewhere composed rather than on a broken tile.
All four sizes are live: 84 detail, 52 My Agents card, 40 browse row, 28 the
admin tables' compact tile. The 28px sidebar and @ menu D5 names are Phases 5
and 7 — the component is ready for them, but inventing the surfaces here is not
this phase's work. The admin tile becomes a thin adapter over it, so a reviewer
judges the icon the shelf will actually render.
The upload dialog
-----------------
Previewing all four sizes is the reason this is a dialog and not a bare file
input. An icon is authored at a size nobody browses at: it reads beautifully at
84px and turns to mud at 28px, where fine detail and thin type vanish into four
hundred pixels. So the strip renders every store size at true scale, live,
before the upload — and shows the same strip with the gradient when you remove
one, making "revert to the default" something you can see rather than risk.
Client-side checks mirror the server's limits so a screenshot or a 4:3 photo is
answered instantly instead of after a round trip; the server re-derives all of
them and does the part a browser cannot, re-encoding to strip EXIF.
The Icon action sits on the My Agents card next to Submit, gated owner-or-editor
rather than owner-only: an icon is presentation (D13), and an editor who may
rewrite the instructions is not someone to stop at the avatar. Publishing stays
owner-only, so those buttons moved to their own per-button gate.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(marketplace): record the icon routes and phase status
The two icon routes that were not in the spec's API table: `DELETE`, without
which an author cannot undo an upload (admin PATCH only replaces `iconKey`), and
the `GET` that makes `iconUrl` a stable cacheable path instead of a presigned URL
that changes on every read.
Also corrects the phasing block, which still called shipped phase 2 "in progress".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): pins and the curated store front
Phase 5 of docs/specs/agent-marketplace.md — the user's own pin state (D8,
D9 user side) and the Featured row admins own (D10).
**Pins.** One item per user on the existing user-settings table,
`PK=USER#{id}, SK=PINNED_AGENTS`, holding the pin list and — from day one,
before anything reads it — the dismissal tombstones. Role-seeded pins
(Phase 6) resolve live rather than materializing, so an unpin that does not
remember itself is an unpin the next request undoes; writing tombstones now
means that resolver inherits a real history instead of treating every
existing user as having dismissed nothing. Pin and dismiss are one toggle:
re-pinning clears the tombstone, because a pin and a tombstone coexisting is
a state the Phase 6 resolver would have to guess about.
`source` and `locked` ride on every pin row already, so Phase 6 adds rows to
the response rather than changing a shape the SPA renders.
Pinning is gated on the caller being able to *reach* the Agent, not on it
being published — publication decides what the store offers, and D11 scopes
the `@` menu to "your own and pinned Agents", which presumes an author can
pin their own work. The read applies the same gate on every request through
`get_assistant_with_access_check`, so a pin never becomes a grant, and a
denied row is dropped from the response while the stored pin is left alone:
visibility is reversible and a read is not the place to garbage-collect
someone's shelf. A pin also survives a takedown (D2: delisting is not
revocation), so the projection tolerates an Agent with no listing block.
**Store front.** `PK=AGENT_STOREFRONT, SK=CONFIG` holds an ordered array
rather than per-item `order` fields — the row is short and reordering has to
be atomic. It is the store's only ranking lever (browse is newest-first;
there is no popularity sort), so the admin surface is an explicit ordered
list with move controls and a staged Save. Only published agents may be
featured, and the PUT names any id that is not. Membership is deliberately
not self-healing: a taken-down agent leaves what the store renders but keeps
its slot in the config, because a GET that pruned would rewrite an admin's
curation and make a reversed takedown silently cost the agent its place.
No CDK change: both items live on tables that already exist, with no new
index and no new env var.
- `GET /agents/pins`, `POST`/`DELETE /agents/{id}/pin`
- `GET`/`PUT /admin/agents/storefront`
- Pinned tab, the `+` on every shelf row, Add on the detail page, the
featured row and a pinned strip on Discover, and the admin Store Front
surface (four of D10's seven now built)
* feat(marketplace): default pins by role, resolved live
Phase 6 of the Agent Marketplace (D9). An admin seeds default pinned Agents
per AppRole, so a role's members start with a useful sidebar instead of an
empty one. The effective shelf is now
(⋃ role pins) − dismissed(unlocked only) ∪ own pins
resolved per request. No fan-out, no materialized copy per member — which is
why the Phase 5 dismissal tombstones had to exist before this landed, and why
`source`/`locked` were already on the row.
Storage mirrors the grant items (PK=ROLE#{id}, SK=AGENT_PIN#{agent_id}) but
lives in its own module: a pin is not a permission, so it stays out of
EffectivePermissions, out of _compute_effective_permissions, and off the model
call path. It does not inherit through inheritsFrom.
⚠️ Fixes a live trap on the way through. AppRoleRepository._delete_mapping_items
deleted every non-DEFINITION item under the role's partition, and update_role
calls it before rebuilding from the AppRole record — which does not carry pins.
Any edit to a role's name or grants would have silently emptied its seed list.
It now deletes by prefix, with AGENT_PIN# included only for delete_role.
Admin console (D10's seventh surface) warns rather than blocks, on two axes with
two different owners: reachability (a PRIVATE agent resolves to nothing for
members — the author fixes it) and the D9.5 diff against the role's granted
model/tools/skills (the admin fixes it). memory_space is reported as a note,
never as present or missing, because a role cannot grant one. `default` is
labelled as the substitute it is (D9.6), and the save dialog says plainly that
removing a seed unpins for everyone who has not pinned it themselves.
Deploy: backend.yml + frontend-deploy.yml only. No CDK — the pin items sit on
the existing app-roles table beside TOOL_GRANT#/MODEL_GRANT#, with no new index
or env var, and app-api's task role already holds Query/BatchWriteItem on it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): @-mention hands one turn to an Agent
Phase 7 of the Agent Marketplace (D11). Typing `@` in the composer offers the
user's own Agents plus everything pinned — including role-seeded pins from
phase 6 — grouped, with the publisher as secondary text and "Browse all
agents →" as the last row. Picking one hands THAT TURN to the Agent's model,
tools and skills without leaving the thread.
⚠️ The invocation path forbade exactly this. `chat/routes.py` 400s on both
"Cannot change assistants mid-session" and "Assistants can only be attached to
new sessions", and the SPA treats the Agent as session-wide (URL query param →
session preferences → self-heal on reload). A mention is the case both rules
were written to reject.
So a mention is a turn-scoped Agent, not a binding:
- it rides the existing `rag_assistant_id` with a new `agent_mention: true`
beside it, rather than a second id field — otherwise every downstream step
(RAG, binding resolution, memory injection, the resume snapshot) would need
teaching about a second way to name the Agent running the turn;
- the flag is a *binding* signal, never an authorization one:
`get_assistant_with_access_check` still gates the Agent, so a forged flag
buys nothing but a skipped write;
- validation and persistence move together via `binds_conversation`
(`inference_api/chat/agent_binding_policy.py`, the `system_prompt_resolver`
precedent — a three-line rule inside a thousand-line route only gets tested
if it lives where a test can reach it). Splitting them gives two silent bugs:
validate-only refuses the *second* mention in a thread, persist-only lets one
`@` annex the conversation.
💰 A mention costs two prompt-cache prefix re-writes — the mention turn swaps
the system prompt and toolConfig, the next plain turn swaps them back. Bounded
(twice per mention, never per turn) and bought deliberately; the spec records
the arithmetic and the lever if it ever stops being worth it.
Composer details: one mention per turn (a second has nothing to mean, and
suppressing the menu stops it re-opening when the caret lands back inside an
already-committed `@Name`); rows commit on mousedown so the textarea never
loses the caret being edited around; the menu owns Enter/arrows/Escape while
open so a send never fires out from under it; and it stays shut entirely when
there is nothing to offer, so `@` in a sentence is just an `@`.
Deploy: backend.yml (inference-api image → AgentCore Runtime) +
frontend-deploy.yml. No CDK — one optional request field and no new route,
table, index or env var. app-api relays the body verbatim, so the BFF needed
no change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): problem reports (D15, phase 8)
Users can report a problem with a published Agent from its detail page; the
report lands in a new admin Reports queue beside submissions.
A report is a private message to the curator. It is never rendered to another
browsing user and never feeds usageCount, the store front, or any ordering —
report volume influencing placement would make reporting a way to bury a
competitor's Agent. The reporter is visible to the admin and never to the
author (D15.2), and resolving a report never changes listing.state (D15.5): if
one warrants delisting, that is the existing takedown path and a separate
recorded act.
Storage is child rows under the Agent with a sparse GSI6 written only while
open, so triage removes a report from the queue by clearing its key rather than
by a reader remembering to filter. Two things the spec left contradictory or
open, both recorded in its Phase 8 notes:
- The sketched sort key embedded created_at, which cannot satisfy D15.4's own
"conditional write on a deterministic report_id, not a second index" — you
cannot conditionally update a key you must read first. The key is now
REPORT#{sha256(agent:reporter)} and the chronology lives in GSI6_SK.
- _delete_assistant_cloud deleted only the METADATA item, so an orphaned open
report would have kept its index key and haunted the queue forever. The
shared delete now sweeps child reports, and the queue flags a row whose Agent
is gone rather than dropping one it still counts.
Requires a platform.yml deploy for the new GSI6 (PlatformStack/RagDataConstruct)
before backend.yml; app-api's task role already grants the table's index/*.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(marketplace): the agent detail page never loaded its agent
`AgentDetailPage` called `load()` from its constructor, and `load()` reads
`this.id()` — an `input.required<string>()` bound by `withComponentInputBinding()`.
The router sets that input *after* construction, so the read threw NG0950 before
`AgentApiService.getAgent()` was ever called.
The throw landed inside `load()`'s own try/catch, which turned it into
`this.error.set('Failed to load this agent.')`. So the page rendered a plausible
error banner with **no HTTP request behind it** — no console error, no failed
request, nothing to grep for. `/agents/:id` has been in this state since phase 3
(66f38346); the detail page has never successfully loaded in a browser.
Moved both init calls to `ngOnInit`, which runs after input binding.
Found while running the phase 5-7 dev smoke test, where the detail page is the
only surface carrying the pin control. There is no `agent-detail.page.spec.ts` at
all, which is why CI never caught it — a regression spec is tracked separately.
Note that a spec asserting this must set the input the way the router does (after
creation, e.g. via `RouterTestingHarness`); passing `id` at construction time
reproduces neither the bug nor the fix.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(marketplace): pin the agent detail page's load path
`AgentDetailPage` had no spec, which is the only reason the phase 3 bug
(`load()` in the constructor, fixed in f60951b1) reached users: the page
rendered "Failed to load this agent." with no HTTP request behind it, and
nothing in CI looked.
Five tests over the load path. The two that would have caught it assert that
`AgentApiService.getAgent` is called with the bound id, and that the error
banner is absent on the happy path — deliberately not assertions about the
rendered agent, since a zero-request page is what the bug actually produced.
⚠️ The component is routed through `RouterTestingHarness` with
`provideRouter(routes, withComponentInputBinding())` rather than constructed
directly. That is load-bearing, not ceremony: `id` is an `input.required` the
router sets *after* construction, so handing it in at construction time makes
every assertion here pass against the broken code too. Verified both ways —
red against the constructor form (`getAgent` called 0 times), green against
the fix.
Services are stubbed via DI tokens rather than `vi.mock`, per the repo
convention on cross-spec mock pollution.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(marketplace): pin the agent detail page's load path
`AgentDetailPage` had no spec, which is the only reason the phase 3 bug
(`load()` in the constructor, fixed in f60951b1) reached users: the page
rendered "Failed to load this agent." with no HTTP request behind it, and
nothing in CI looked.
Five tests over the load path. The two that would have caught it assert that
`AgentApiService.getAgent` is called with the bound id, and that the error
banner is absent on the happy path — deliberately not assertions about the
rendered agent, since a zero-request page is what the bug actually produced.
⚠️ The component is routed through `RouterTestingHarness` with
`provideRouter(routes, withComponentInputBinding())` rather than constructed
directly. That is load-bearing, not ceremony: `id` is an `input.required` the
router sets *after* construction, so handing it in at construction time makes
every assertion here pass against the broken code too. Verified both ways —
red against the constructor form (`getAgent` called 0 times), green against
the fix.
Services are stubbed via DI tokens rather than `vi.mock`, per the repo
convention on cross-spec mock pollution.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(inference): pin the #741 conversation fork with a strict xfail
An `@`-mention turn and the plain turns around it run on two different cached
`Agent` instances, and neither sees the other's messages. The user sees one
continuous thread — the SPA renders from the persisted store — but the model
answers "NOT IN HISTORY" about a turn on screen. Measured on dev session
e5e8b259-1780-4179-8ebe-38c57d3709a5.
The test models the round trip that produces it: plain → mention → plain, with a
`store` standing in for AgentCore Memory. The distinction it has to preserve is
why the bug is invisible in production logs — the mention turn is a cache MISS,
so it restores and legitimately sees prior history; the plain turn after it is a
cache HIT, restores nothing, and is stale. A first draft asserted the mention
turn was empty, which fails for the wrong reason and would have passed once
anyone made a fresh agent restore.
Asserts on the conversation rather than instance identity, so it stays valid
whichever fix wins: reuse one instance, hand the message list between instances,
or re-restore on a stale hit.
`strict=True` so it fails loudly the moment the fix makes it pass.
Refs #741
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(inference): one session is one conversation, whatever agent runs a turn
The agent cache keys on *configuration* — system prompt, tools, model, skills —
which is right: those need different `Agent` objects. The conversation is not
configuration, and nothing enforced that.
So an `@`-mention forked the thread. The mention turn missed the cache, built a
second agent, restored history and looked fine; the next plain turn reverted the
key and cache-*hit* the original instance, whose in-memory list still ended
before the mention. `initialize()` never re-runs on a hit, so the stale list won
silently: the model answered "NOT IN HISTORY" about a turn the user could see on
screen, because the SPA renders from the persisted store. Symmetric, too — a
second mention could not see the plain turns in between.
`_adopt_session_conversation` points a newly built agent at the list its session
is already using.
⚠️ Shared by reference, deliberately. Copying would fix only the direction that
already works — the miss. The turn that goes stale is a cache *hit*, where
nothing runs and there is nothing to copy. Aliasing is what makes the mention
turn's appends visible to the instance the next turn hits. Safe because every
site that rebinds `agent.messages` (document stripping, content-block sanitizing,
compaction slicing, pairing repair) lives inside
`TurnBasedSessionManager.initialize()` and runs before adoption; after
construction the list is only appended to.
Re-restoring on a stale hit was the alternative and is worse: restored history
passes through the sanitizers and pairing repair while accumulated history does
not, so one conversation can serialize two ways depending on the path — a prefix
byte change on an arbitrary turn, which is what the prompt-cache contract
forbids. Aliasing re-serializes nothing, so the cached prefix is untouched.
Adoption runs before the `extra_tools` early return: an uncached agent still
takes a turn in the thread and must not fork it. A length guard keeps a live
instance that trails Memory from dragging a newer restored history backwards;
the reverse comparison would be wrong, since compaction legitimately shortens a
restored list.
Concurrency is covered by the single-flight session lease — one turn per session
at a time. Separate replicas share no cache, so cross-process divergence is
unchanged by this either way.
Fixes #741
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(marketplace): a mention costs one prefix re-write, not two
The phase 7 notes predicted "two prompt-cache prefix re-writes ... roughly
$0.25 per mention round-trip". Measured on dev, the outbound leg is a genuine
full re-write but the swap-back is a cache HIT: base toolConfig and system
prompt revert byte-identically and read from the still-live pre-mention entry,
so only the mention exchange is written as a delta.
mention miss_avoidable read 0 write 4301
next plain turn hit read 2720 write 138
At a 50k prefix and Sonnet 5's $2.30/MTok write premium that is ~$0.12 per
mention, about half the original figure.
Two caveats now recorded with the number, because both change it materially:
the swap-back only hits inside the ~5-minute TTL, and an Agent that pins its
own model lands in a different cache namespace entirely, where it can hit
nothing and its write is never re-read.
⚠️ Also records why the number was wrong twice over. The first measurement was
taken against #741, where the swap-back looked *cheaper* than the truth (write
71, not 138) because the stale agent was omitting the mention exchange from the
prefix — a broken run flattering us while the spec's prediction erred the other
way. Re-measure after any invocation-path change rather than trusting either.
Refs #741
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(observability): measure the cache TTL from the same-prefix predecessor
`classify_cache_status` decided "avoidable" vs "TTL expired" using the gap to
the immediately-previous call row. That is the wrong clock the moment two
prefixes interleave in one session — which is exactly what an `@`-mention does.
The entry a call could have HIT belongs to the last call with the *same*
prefix, and that is not always the last call.
Measured on dev during the #741 verification: a plain turn 266s after a mention
was classified `miss_avoidable` and booked $0.006256 of waste, but the entry it
needed was written 308s earlier, past the 300s TTL. The re-write was
unavoidable. In that six-turn session three rows were `miss_avoidable` and none
were the bug the metric exists to catch.
The fingerprints needed to do this correctly were already on the row. The
lookup now reads a small window of recent rows instead of one, and classifies
against the newest row sharing this call's toolConfig + system-prompt hashes.
No new data, no schema migration — only a wider read of rows already indexed.
Three behaviours worth keeping:
- When no same-prefix predecessor is in the window we classify as
`miss_ttl_expired`, not `miss_avoidable`. Deliberately the conservative
direction: under-reporting waste keeps the metric trustworthy, whereas crying
wolf is what made it useless.
- Calls with no fingerprints (hook disabled, non-Bedrock provider) fall back to
the previous call exactly as before, so nothing regresses where the fix
cannot apply.
- `previous_cached_prefix_tokens` still comes from the previous call when there
is no match, so the below-threshold `first_write` guard keeps working.
`cacheGapSeconds` keeps its original meaning — plain chronology, which existing
consumers read. `cachePrefixGapSeconds` is added only when the deciding call was
an older one, so a status that looks inconsistent with the visible gap explains
itself rather than reading as a bug. Surfaced through the admin costs API and
rendered beside the gap on the session anatomy page.
Fixes #753
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): GA the Agent store — the nav gate was the only closed door
D14 called for the kill switch plus an `agent-marketplace` RBAC capability
that 404s the routes for ungranted roles. The capability was never built, and
should not be: the admin roles UI builds `grantedTools` from the tool catalog
with no free-text entry, so a feature-capability id cannot be granted from the
UI at all — only by hand-writing DynamoDB items. That is why the `skills` gate
was removed and why `scheduled-runs` 403'd in prod and was dropped. Building it
a third time ships a gate nobody can open.
What was left behind was worse than either end state. One template condition,
`@if (showAgents() && isAdmin())`, hid the nav entry — while `/agents/discover`,
`/agents/:id` and `/agents/pinned` carried `authGuard` only, the composer
`@`-mention menu had no admin check at all, and a role-seeded pin (D9) pushed
Agents into a member's Pinned tab unprompted. The store was reachable by any
authenticated user through three doors while hidden behind the one we
controlled; nobody could answer "who can see this?" without reading four files.
So: drop `isAdmin()` from the nav condition and make the kill switch the only
lever. The Preview badge stays until Assistant deprecation (#746) lands, since
until then the sidenav ships both nouns.
The regression spec renders the real template rather than reading `showAgents()`
— the bug lived only in the `@if`, so a spec asserting the computed passes
against both the gated and the GA'd code. Verified to fail against the old
condition and only against it.
Fixes #745
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): mark published Agents whose behavior changed after approval
Three individually-reasonable v1 non-goals compose into a governance hole none
of them was evaluated against: D2 does not re-review edits, there is no agent
versioning, and D9 lets an admin *lock* a role-seeded pin so members cannot
remove it. So an admin locks an Agent into every member of a role's sidebar,
the author rewrites its instructions, and the new behavior is live immediately
for everyone — no re-review, no history, no notification, and no opt-out for
the affected user, whose own dismissal loses to the lock by design.
Approval now records a SHA-256 of the instructions it approved, and the admin
Listings table marks any published listing whose instructions no longer match.
This is not a gate — it is the curator's reason to look.
The marker reports two claims and deliberately does not merge them:
instructions behavior definitely changed measured (hash mismatch)
edited record changed, cause unknown inferred (updatedAt > reviewedAt)
The issue proposed the timestamp comparison alone as a pure read. It is not
sufficient on its own: an admin's own D13 presentation edit bumps updatedAt
without touching reviewedAt, so a timestamp-only marker fires on the admin's
own typo fix, and a governance marker that cries wolf is one that gets
learned-ignored. But the hash alone would be blind on every listing approved
before it shipped — precisely the already-published, possibly-locked back
catalogue this is about. So: hash when present, timestamp as the explicitly
weaker fallback, styled and worded so the two never read alike.
The baseline is a hash *of* the instructions, so it rides the same viewer gate.
`_agent_response` already drops `instructions` for anyone below editor; it now
drops the hash with it. Not reversible on its own, but it would confirm a
guessed prompt for anyone who could produce one, which is what that gate exists
to prevent. The admin listings projection is unaffected.
Both new suites were verified to discriminate: with a timestamp-only
derivation the admin-edit test fails with 'edited' is None, and without the
viewer gate both hash-exposure tests fail while owner/editor keep passing.
Fixes #744
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(spa): stop cross-file state leaks that made ng test flaky
On any `ng test` run exactly one randomly-chosen spec file failed; the
next run passed it and failed a different one instead.
The unit-test builder hardcodes `isolate: false` in its vitest runner, so
every spec file assigned to a worker shares one module registry, one
jsdom, and one timer implementation. Worker assignment is timing
dependent, so which specs share state changes run to run — hence the
random victim.
The trigger was leaked fake timers. `vi.restoreAllMocks()` restores
neither fake timers nor `vi.stubGlobal`, and several specs relied on it
for both. A spec left the clock frozen and the next spec in that worker
to await anything real-timer-driven — a RouterTestingHarness navigation,
an HttpTestingController round-trip — died with "Test timed out in
5000ms", which is how every observed failure presented.
Fixed at the source in seven specs:
sidenav, toast, citation-display fake timers never restored
model.service stubGlobal needs unstubAllGlobals
session.service window.location + document.cookie
replaced and never put back
admin-cost-state, admin.guard unrestored spies on
document.createElement and console
Added a backstop afterEach in test-setup.ts (useRealTimers +
unstubAllGlobals + unstubAllEnvs) so a spec added later cannot
reintroduce the class of bug, and global-hygiene.spec.ts as a canary
asserting the shared jsdom is intact. A global restoreAllMocks was
deliberately not added — it would reset vi.fn() implementations and
break specs that build mocks outside beforeEach.
Also fixes a second, unrelated flake found while verifying: app.spec.ts
loads the root component through a runtime dynamic import that measures
~4s against vitest's 5000ms default, leaving ~20% headroom before load
pushes it over. Raised to 30s, matching the existing precedent in
shared-view.page.spec.ts.
Verified with a harness that builds the full spec set and varies only
execution order (subsetting with --include changes bundling and masks
the bug): 16 seeded orderings including the four that previously failed,
5 plain runs, and 3 runs under 8 CPU hogs on 10 cores — all green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(agents): redirect the Assistant editor to the Designer, one noun in the nav
Designer Phase 5, step 3 and the user-facing half of step 4. Marketplace D1 is
"one noun: Agent", and the sidenav has been shipping both.
Steps 1-2 were already done: `compat.to_agent_view` renders a legacy Assistant
*as* an Agent, and the Designer passed parity some time ago — it carries model
config, tools, skills, memory spaces, tags and visibility on top of everything
the old editor had, and reuses that editor's own knowledge-base section and
share dialog for the rest. The old editor had strictly less to offer for the
same record.
`/assistants`, `/assistants/new` and `/assistants/:id/edit` become `redirectTo`
entries rather than deletions. Those paths are in bookmarks, in the "edit" link
of every old chat session, and in links people shared with each other. The ids
are identical on both sides — nothing was migrated — so a redirect lands on
exactly what the old URL opened. Deleting them would 404 all of that for no gain.
The nav ships one entry, and the Agents "Preview" badge came off with the second
noun it existed to disambiguate.
The term pass is deliberately not a find-and-replace. "You are a helpful
assistant that…" stays as the instructions placeholder: that is the conventional
system-prompt idiom, and rewriting it to "agent" would be worse prompt guidance,
not better terminology. What changed is the words naming our own product concept
— nav, session indicator, share dialog, settings.
⚠️ This changes what AGENTS_API_ENABLED means. While both nouns shipped, off
degraded to the Assistants editor; there is nothing left to fall back to, so off
now means no authoring surface at all. It is an outage switch, not a feature
toggle. Recorded in the flag docstring and the spec rather than silently
changed — worth a deliberate decision about whether the flag should survive.
The old editor's components are now unreachable but not yet deleted; retiring
them (and the docs pass) is the rest of step 4, kept separate so this flip stays
revertible in one line.
Refs #746
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(agents): retire the Assistant editor, keep what the Agent surface consumes
Designer Phase 5, step 4 — the rest of #746. #758 made these pages unreachable;
this deletes them.
Gone: assistants.page, assistant-form.page, assistant-list, assistant-preview,
and the assistant-form barrel.
`assistants/` itself stays, and is no longer a feature — it is the set of pieces
the Agent surface consumes: the share dialog, the assistant card, the
knowledge-base dialogs and services, PreviewChatService, and the models. Each was
verified to have a live inbound reference from `agents/`, `session/` or
`knowledge-base/` before being kept. It keeps the name because the record is
still an Assistant on the wire — ids are identical and nothing was migrated, so
renaming the folder would drag the API contract's vocabulary with it. The README
is rewritten from a stale description of deleted files into that consumer map,
and says plainly that new UI belongs under `agents/`.
The backend `/assistants/*` surface is untouched and not deprecated: `test-chat`
and the document sub-routes are called by the Designer's own preview pane and
knowledge-base section.
Deleting `assistant-preview.component.spec.ts` would have quietly dropped
coverage from a live surface, because `agent-preview` — the component that
replaced it — had no spec at all. Ported the tests that still apply, and pinned
the one behavior that differs: the Agent preview sends no live instructions and
opts out of client-side prompt and tool injection, because an Agent resolves
those server-side from the saved record and a long persona would exceed the
system_prompt cap (422). That looks like a bug until you know why, so it now has
a test with the reason attached. Also pinned the model lock, including its
release on destroy — the lock lives in the root ModelService, so leaking it would
follow the user out of the Designer into an ordinary chat.
The docs pass found less than expected and deliberately changed nothing in
docs/SSE_ERROR_MESSAGING.md, the testing report, or the root README: every
"assistant" there is the model's message role, not our product concept.
Closes #746
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(compaction): re-read compaction state per turn — two managers, one row
Same root cause as #741, different blast radius. The agent cache keys on
configuration, so an `@`-mention turn builds a second `Agent`, and each `Agent`
builds its own `TurnBasedSessionManager`. Both write the same DynamoDB session
row and neither knows the other exists.
`update_after_turn` loaded persisted state only when `initialize()` had skipped
the load. `initialize()` never re-runs on a cache *hit* — the same property that
forked the conversation in #741 — so a hit turn kept whatever state its instance
was built with and then saved it straight over the newer checkpoint and
truncation anchor a sibling had just written. Every exit path of
`update_after_turn` saves, including the "nothing to do" ones, so the clobber did
not need compaction to actually fire on that turn.
This is a cost bug before a correctness one. The truncation anchor is what keeps
the restored prefix byte-stable; moving it backwards re-truncates messages
previously sent whole, which rewrites a 35k-150k-token prefix at the $2.50/MTok
write premium on a turn where nothing about the conversation appeared to change.
Fix is direction (2) from the issue: re-read on every turn. Not the shared-object
approach #750 used for the message list — a sibling may live in another replica,
where aliasing reaches nothing. The read is one GSI query and
`_save_compaction_state` already does an identical one immediately after, so it
roughly doubles a cost that is already noise next to a model call. The
single-flight session lease serializes turns per session, so read-modify-write is
safe. Conditional writes (direction 3) would only re-cover what the lease covers.
State never moves backwards. A load failure is indistinguishable from "nothing
persisted" — both return a default `CompactionState` — so adopting the result
blindly would let one transient DynamoDB error zero a real checkpoint, which is
the clobber this exists to prevent. Adopt only when the persisted record is at
least as far along, and carry the anchor forward even at an equal checkpoint,
since the anchor also advances on prompt-cache expiry.
Removed `_compaction_state_loaded`: nothing branches on it now, and its comment
described a lazy-load that no longer exists. Its one real job — stopping the
failed-compaction path from persisting defaults over a live checkpoint — is now
done by the monotonic guard.
The integration-style test the issue asked for drives two managers against one
shared fake row and asserts the checkpoint never regresses; it fails with
`assert 0 == 8` against the old code, which is the bug verbatim.
Fixes #751
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(agents): retire the unreachable `limits` runnability state
D6 specified three runnability states — ready, limits (degraded), blocked —
and the middle one could never occur. It degraded only when a binding declared
`config.optional == true`; that key was read in exactly one place and written
nowhere. No API accepted it and the Designer had no control for it, so every
gap already resolved to `blocked` and the model was two states pretending to be
three.
The deciding reason is not that it was dormant, though. Building toward it
would have contradicted `agent-designer.md` D5, whose Non-goals say "No
**downgrade** on missing capability (block-only v1)" and call downgrade "a later
opt-in". Block-only is what `agent_binding_resolver` actually implements — it
raises for model, tool, skill and memory_space alike, and `optional` appears
nowhere in it. Making the preview offer a third outcome would have required
teaching the resolver to skip bindings, which is a real product decision about
whether an Agent may silently run degraded, not a checkbox. Two specs disagreed;
this settles them on the one that shipped.
So: `RunnabilityState` is `ready | blocked`, `MissingCapability.optional` and
`_is_optional` are gone, and the detail page and admin default-pins page stop
rendering a distinction they could never draw.
The `optional` flag is not merely unused now — it is inert, and a test pins
that. A hand-edited or future record could still carry it, and it must not
resurrect a state the runtime cannot honour.
D6 is revised rather than quietly trimmed: it records that there were three
states, why the middle one went, and that if downgrade is ever taken up it
starts at the resolver, not at the preview layer.
Fixes #747
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): make locked seeds cost something to an admin, not to a cap
Resolves the D9 "locked-pin ceiling" open question. No cap; friction.
Phase 6 sharpened the concern the question was written about. A locked seed
genuinely cannot be dismissed — verified on dev, a lock beats a user's own
tombstone, which is right, because a lock a user could dismiss would be
pointless. So an admin choosing between "seed" and "seed locked" has no reason
not to lock: locking guarantees the rollout lands and the cost falls on someone
else's sidebar. Left alone, the dominant strategy is to lock everything and
Pinned stops being the user's shelf.
A cap does not work here, and the reason is structural rather than a matter of
picking the right number. Pins merge as a union across every role a user matches
and a lock from any one of them wins (D9.2), so a per-role cap of two does not
mean a member sees two locked Agents — someone in five roles sees ten. Capping
the union instead is not implementable: role membership resolves per user from
Entra claims at request time, so which roles co-occur on one person is unknowable
when an admin saves a seed list. Enforcing at read time would be worse than no
cap, silently dropping an admin's lock for some users — a rollout that looks like
it landed and did not.
So the console shows what it can actually know. A running locked count on the
seed header, a warning past a threshold, and `lockedElsewhere` — how many locked
seeds every *other* role holds, which is the one fact an admin cannot work out
from their own page and the only honest way to show the union. Advisory
throughout: `count_locked_outside` swallows a failed role listing rather than
taking the pins page down with it.
No new failure mode, nothing to migrate for roles already over any line, and a
threshold that moves without breaking a saved list. If over-locking shows up,
revisit with the counts this makes visible.
Fixes #748
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(agents): settle the Agent specs, and make the tagline decision true
Closes the spec-hygiene issue. Four items, and the last one needed code.
**Phasing table** — already fixed while shipping #746; verified, no change.
**Quota attribution** — resolved, in code rather than by inference. The invoker
pays: `check_quota(user=current_user, …)` at chat/routes.py:1319 with no
author-side lookup anywhere on the invocation path, and cost rows land on the
same person (`PK = USER#{user_id}`, metadata.py:157). Both file:lines are in the
spec so nobody re-derives it.
**Review SLA** — committed and split by queue: two business days for
submissions, same day for an `inappropriate` report, weekly for the rest. Phase
8 added reports as a second queue and they are not the same clock — a submission
is a person waiting to publish, a report is mostly maintenance signal. D2 had
been asserting "two business days" in its body while Open Questions said the
commitment was never made; that disagreement is gone.
**`tagline` backfill** — derive and let the author edit, at submission. Writing
that down alone would have re-opened the gap this issue exists to close, because
implementing it turned up a second problem the question never mentioned: tagline
was *author-owned on the wire and unsettable in the UI*. The API accepted it and
the model comment said the author owns it, but no Designer control ever wrote
one — the same dormant-field shape as the `limits` state in #747. So submission
is now where it gets set, prefilled by `deriveTagline` from the description's
first clause. The derivation is not the clever part; putting the shelf row in
front of the author at the one moment they are looking at what the store will
say is. A bad line is then one edit away instead of a surprise after publication.
Also fixed the same trap on the specs' own front pages: both still read
"Status: Draft / proposal" while describing shipped code. That is precisely the
`agent-directory.md` failure this issue cites — a document whose face does not
say what it is. Both now state what is implemented and point at the revisions
recorded in place (D6, D9.7, D14).
Open questions on the marketplace spec are now empty.
Fixes #749
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(costs): tell a deliberate @-mention swap from a cache regression
An `@`-mention hands one turn to a different Agent (Marketplace D11), which
swaps the system prompt and toolConfig and so genuinely re-writes the prompt
cache. That classifies as `miss_avoidable`, correctly — the tokens really were
spent at the write premium. The problem was that nothing on the `C#` row said
the turn was a mention, so a deliberate swap looked exactly like the
nondeterministic-ordering regression the fingerprints exist to catch: both
present as `toolConfigHash` and `systemPromptHash` flipping together. Expected
traffic was diluting the one signal that should never move.
A dimension, not a reclassification. The row records `turnAgentId` and, where
it differs from the previous call's, `agentSwitched`. `cacheStatus` and
`wastedUsd` are untouched, and the session rollup *splits* rather than deducts:
`agentSwitchMissCount` / `agentSwitchUsd` are a subset of `avoidableMissCount` /
`wastedUsd`. Hiding the spend would understate what mentions cost, which is a
thing worth being able to measure on purpose; subtracting gives unexplained
waste, which is the number a regression moves.
Three notes on how rather than what:
The agent id is threaded per turn from the route rather than read off the agent
object. The agent is cached and shared across turns, so per-turn state must
never live on it — that is exactly what forked history in #741 and compaction
state in #751.
`agentSwitched` is derived at write time because `_derive_cache_observability`
is the only place already holding the predecessor row. It compares against the
*previous call*, not the same-prefix match the TTL clock uses: the question is
whether the Agent changed from one turn to the next, and a same-prefix row is by
construction one where it did not.
EMF gains `AgentSwitchMiss` as its own metric rather than a dimension on
`AvoidableMiss`. The namespace deliberately has no dimensions — fleet-wide sums
are the alarm target — so a dimension would multiply metric streams per Agent.
`turnAgentId` rides as a property, queryable without that cost.
The admin anatomy page keeps the red count whole and names the explained part
underneath, so the number an admin reads as alarming only ever means
unexplained. Older rows carry neither attribute and read as unswitched; there is
no backfill and none is needed.
Fixes #756
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(agents): replace the assistant card with an agent launch card
The card that greeted you when an Agent opened was the last pre-Marketplace
surface, and it was out of continuity on every axis that matters.
The one that matters most: it hashed the **first letter of the name** into its
own 26-entry pastel palette, while every other surface hashes `agentId` through
FNV-1a into the twelve curated gradients. The same Agent was therefore drawn two
different ways either side of a tap — exactly what the note on `hashAgentId`
exists to prevent. Two of those 26 (`J`, `P`) were also near-white under white
text, an AA failure rather than a taste call.
The replacement is the store's detail page folded to chat width, in the same
reading order the user just tapped through: tile, name, tagline, publisher, what
it can reach, what to ask. Starters stay buttons — unlike the detail page's
read-only list, the chat has real composer prefill behind `starterSelected`.
- Tile is `app-agent-icon` at 52px, so the store, My Agents, the chat header and
this card all d…
* fix(marketplace): the agent detail page never loaded its agent
`AgentDetailPage` called `load()` from its constructor, and `load()` reads
`this.id()` — an `input.required<string>()` bound by `withComponentInputBinding()`.
The router sets that input *after* construction, so the read threw NG0950 before
`AgentApiService.getAgent()` was ever called.
The throw landed inside `load()`'s own try/catch, which turned it into
`this.error.set('Failed to load this agent.')`. So the page rendered a plausible
error banner with **no HTTP request behind it** — no console error, no failed
request, nothing to grep for. `/agents/:id` has been in this state since phase 3
(66f38346); the detail page has never successfully loaded in a browser.
Moved both init calls to `ngOnInit`, which runs after input binding.
Found while running the phase 5-7 dev smoke test, where the detail page is the
only surface carrying the pin control. There is no `agent-detail.page.spec.ts` at
all, which is why CI never caught it — a regression spec is tracked separately.
Note that a spec asserting this must set the input the way the router does (after
creation, e.g. via `RouterTestingHarness`); passing `id` at construction time
reproduces neither the bug nor the fix.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(marketplace): pin the agent detail page's load path
`AgentDetailPage` had no spec, which is the only reason the phase 3 bug
(`load()` in the constructor, fixed in f60951b1) reached users: the page
rendered "Failed to load this agent." with no HTTP request behind it, and
nothing in CI looked.
Five tests over the load path. The two that would have caught it assert that
`AgentApiService.getAgent` is called with the bound id, and that the error
banner is absent on the happy path — deliberately not assertions about the
rendered agent, since a zero-request page is what the bug actually produced.
⚠️ The component is routed through `RouterTestingHarness` with
`provideRouter(routes, withComponentInputBinding())` rather than constructed
directly. That is load-bearing, not ceremony: `id` is an `input.required` the
router sets *after* construction, so handing it in at construction time makes
every assertion here pass against the broken code too. Verified both ways —
red against the constructor form (`getAgent` called 0 times), green against
the fix.
Services are stubbed via DI tokens rather than `vi.mock`, per the repo
convention on cross-spec mock pollution.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(marketplace): pin the agent detail page's load path
`AgentDetailPage` had no spec, which is the only reason the phase 3 bug
(`load()` in the constructor, fixed in f60951b1) reached users: the page
rendered "Failed to load this agent." with no HTTP request behind it, and
nothing in CI looked.
Five tests over the load path. The two that would have caught it assert that
`AgentApiService.getAgent` is called with the bound id, and that the error
banner is absent on the happy path — deliberately not assertions about the
rendered agent, since a zero-request page is what the bug actually produced.
⚠️ The component is routed through `RouterTestingHarness` with
`provideRouter(routes, withComponentInputBinding())` rather than constructed
directly. That is load-bearing, not ceremony: `id` is an `input.required` the
router sets *after* construction, so handing it in at construction time makes
every assertion here pass against the broken code too. Verified both ways —
red against the constructor form (`getAgent` called 0 times), green against
the fix.
Services are stubbed via DI tokens rather than `vi.mock`, per the repo
convention on cross-spec mock pollution.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(inference): pin the #741 conversation fork with a strict xfail
An `@`-mention turn and the plain turns around it run on two different cached
`Agent` instances, and neither sees the other's messages. The user sees one
continuous thread — the SPA renders from the persisted store — but the model
answers "NOT IN HISTORY" about a turn on screen. Measured on dev session
e5e8b259-1780-4179-8ebe-38c57d3709a5.
The test models the round trip that produces it: plain → mention → plain, with a
`store` standing in for AgentCore Memory. The distinction it has to preserve is
why the bug is invisible in production logs — the mention turn is a cache MISS,
so it restores and legitimately sees prior history; the plain turn after it is a
cache HIT, restores nothing, and is stale. A first draft asserted the mention
turn was empty, which fails for the wrong reason and would have passed once
anyone made a fresh agent restore.
Asserts on the conversation rather than instance identity, so it stays valid
whichever fix wins: reuse one instance, hand the message list between instances,
or re-restore on a stale hit.
`strict=True` so it fails loudly the moment the fix makes it pass.
Refs #741
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(inference): one session is one conversation, whatever agent runs a turn
The agent cache keys on *configuration* — system prompt, tools, model, skills —
which is right: those need different `Agent` objects. The conversation is not
configuration, and nothing enforced that.
So an `@`-mention forked the thread. The mention turn missed the cache, built a
second agent, restored history and looked fine; the next plain turn reverted the
key and cache-*hit* the original instance, whose in-memory list still ended
before the mention. `initialize()` never re-runs on a hit, so the stale list won
silently: the model answered "NOT IN HISTORY" about a turn the user could see on
screen, because the SPA renders from the persisted store. Symmetric, too — a
second mention could not see the plain turns in between.
`_adopt_session_conversation` points a newly built agent at the list its session
is already using.
⚠️ Shared by reference, deliberately. Copying would fix only the direction that
already works — the miss. The turn that goes stale is a cache *hit*, where
nothing runs and there is nothing to copy. Aliasing is what makes the mention
turn's appends visible to the instance the next turn hits. Safe because every
site that rebinds `agent.messages` (document stripping, content-block sanitizing,
compaction slicing, pairing repair) lives inside
`TurnBasedSessionManager.initialize()` and runs before adoption; after
construction the list is only appended to.
Re-restoring on a stale hit was the alternative and is worse: restored history
passes through the sanitizers and pairing repair while accumulated history does
not, so one conversation can serialize two ways depending on the path — a prefix
byte change on an arbitrary turn, which is what the prompt-cache contract
forbids. Aliasing re-serializes nothing, so the cached prefix is untouched.
Adoption runs before the `extra_tools` early return: an uncached agent still
takes a turn in the thread and must not fork it. A length guard keeps a live
instance that trails Memory from dragging a newer restored history backwards;
the reverse comparison would be wrong, since compaction legitimately shortens a
restored list.
Concurrency is covered by the single-flight session lease — one turn per session
at a time. Separate replicas share no cache, so cross-process divergence is
unchanged by this either way.
Fixes #741
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(marketplace): a mention costs one prefix re-write, not two
The phase 7 notes predicted "two prompt-cache prefix re-writes ... roughly
$0.25 per mention round-trip". Measured on dev, the outbound leg is a genuine
full re-write but the swap-back is a cache HIT: base toolConfig and system
prompt revert byte-identically and read from the still-live pre-mention entry,
so only the mention exchange is written as a delta.
mention miss_avoidable read 0 write 4301
next plain turn hit read 2720 write 138
At a 50k prefix and Sonnet 5's $2.30/MTok write premium that is ~$0.12 per
mention, about half the original figure.
Two caveats now recorded with the number, because both change it materially:
the swap-back only hits inside the ~5-minute TTL, and an Agent that pins its
own model lands in a different cache namespace entirely, where it can hit
nothing and its write is never re-read.
⚠️ Also records why the number was wrong twice over. The first measurement was
taken against #741, where the swap-back looked *cheaper* than the truth (write
71, not 138) because the stale agent was omitting the mention exchange from the
prefix — a broken run flattering us while the spec's prediction erred the other
way. Re-measure after any invocation-path change rather than trusting either.
Refs #741
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(observability): measure the cache TTL from the same-prefix predecessor
`classify_cache_status` decided "avoidable" vs "TTL expired" using the gap to
the immediately-previous call row. That is the wrong clock the moment two
prefixes interleave in one session — which is exactly what an `@`-mention does.
The entry a call could have HIT belongs to the last call with the *same*
prefix, and that is not always the last call.
Measured on dev during the #741 verification: a plain turn 266s after a mention
was classified `miss_avoidable` and booked $0.006256 of waste, but the entry it
needed was written 308s earlier, past the 300s TTL. The re-write was
unavoidable. In that six-turn session three rows were `miss_avoidable` and none
were the bug the metric exists to catch.
The fingerprints needed to do this correctly were already on the row. The
lookup now reads a small window of recent rows instead of one, and classifies
against the newest row sharing this call's toolConfig + system-prompt hashes.
No new data, no schema migration — only a wider read of rows already indexed.
Three behaviours worth keeping:
- When no same-prefix predecessor is in the window we classify as
`miss_ttl_expired`, not `miss_avoidable`. Deliberately the conservative
direction: under-reporting waste keeps the metric trustworthy, whereas crying
wolf is what made it useless.
- Calls with no fingerprints (hook disabled, non-Bedrock provider) fall back to
the previous call exactly as before, so nothing regresses where the fix
cannot apply.
- `previous_cached_prefix_tokens` still comes from the previous call when there
is no match, so the below-threshold `first_write` guard keeps working.
`cacheGapSeconds` keeps its original meaning — plain chronology, which existing
consumers read. `cachePrefixGapSeconds` is added only when the deciding call was
an older one, so a status that looks inconsistent with the visible gap explains
itself rather than reading as a bug. Surfaced through the admin costs API and
rendered beside the gap on the session anatomy page.
Fixes #753
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): GA the Agent store — the nav gate was the only closed door
D14 called for the kill switch plus an `agent-marketplace` RBAC capability
that 404s the routes for ungranted roles. The capability was never built, and
should not be: the admin roles UI builds `grantedTools` from the tool catalog
with no free-text entry, so a feature-capability id cannot be granted from the
UI at all — only by hand-writing DynamoDB items. That is why the `skills` gate
was removed and why `scheduled-runs` 403'd in prod and was dropped. Building it
a third time ships a gate nobody can open.
What was left behind was worse than either end state. One template condition,
`@if (showAgents() && isAdmin())`, hid the nav entry — while `/agents/discover`,
`/agents/:id` and `/agents/pinned` carried `authGuard` only, the composer
`@`-mention menu had no admin check at all, and a role-seeded pin (D9) pushed
Agents into a member's Pinned tab unprompted. The store was reachable by any
authenticated user through three doors while hidden behind the one we
controlled; nobody could answer "who can see this?" without reading four files.
So: drop `isAdmin()` from the nav condition and make the kill switch the only
lever. The Preview badge stays until Assistant deprecation (#746) lands, since
until then the sidenav ships both nouns.
The regression spec renders the real template rather than reading `showAgents()`
— the bug lived only in the `@if`, so a spec asserting the computed passes
against both the gated and the GA'd code. Verified to fail against the old
condition and only against it.
Fixes #745
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): mark published Agents whose behavior changed after approval
Three individually-reasonable v1 non-goals compose into a governance hole none
of them was evaluated against: D2 does not re-review edits, there is no agent
versioning, and D9 lets an admin *lock* a role-seeded pin so members cannot
remove it. So an admin locks an Agent into every member of a role's sidebar,
the author rewrites its instructions, and the new behavior is live immediately
for everyone — no re-review, no history, no notification, and no opt-out for
the affected user, whose own dismissal loses to the lock by design.
Approval now records a SHA-256 of the instructions it approved, and the admin
Listings table marks any published listing whose instructions no longer match.
This is not a gate — it is the curator's reason to look.
The marker reports two claims and deliberately does not merge them:
instructions behavior definitely changed measured (hash mismatch)
edited record changed, cause unknown inferred (updatedAt > reviewedAt)
The issue proposed the timestamp comparison alone as a pure read. It is not
sufficient on its own: an admin's own D13 presentation edit bumps updatedAt
without touching reviewedAt, so a timestamp-only marker fires on the admin's
own typo fix, and a governance marker that cries wolf is one that gets
learned-ignored. But the hash alone would be blind on every listing approved
before it shipped — precisely the already-published, possibly-locked back
catalogue this is about. So: hash when present, timestamp as the explicitly
weaker fallback, styled and worded so the two never read alike.
The baseline is a hash *of* the instructions, so it rides the same viewer gate.
`_agent_response` already drops `instructions` for anyone below editor; it now
drops the hash with it. Not reversible on its own, but it would confirm a
guessed prompt for anyone who could produce one, which is what that gate exists
to prevent. The admin listings projection is unaffected.
Both new suites were verified to discriminate: with a timestamp-only
derivation the admin-edit test fails with 'edited' is None, and without the
viewer gate both hash-exposure tests fail while owner/editor keep passing.
Fixes #744
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(spa): stop cross-file state leaks that made ng test flaky
On any `ng test` run exactly one randomly-chosen spec file failed; the
next run passed it and failed a different one instead.
The unit-test builder hardcodes `isolate: false` in its vitest runner, so
every spec file assigned to a worker shares one module registry, one
jsdom, and one timer implementation. Worker assignment is timing
dependent, so which specs share state changes run to run — hence the
random victim.
The trigger was leaked fake timers. `vi.restoreAllMocks()` restores
neither fake timers nor `vi.stubGlobal`, and several specs relied on it
for both. A spec left the clock frozen and the next spec in that worker
to await anything real-timer-driven — a RouterTestingHarness navigation,
an HttpTestingController round-trip — died with "Test timed out in
5000ms", which is how every observed failure presented.
Fixed at the source in seven specs:
sidenav, toast, citation-display fake timers never restored
model.service stubGlobal needs unstubAllGlobals
session.service window.location + document.cookie
replaced and never put back
admin-cost-state, admin.guard unrestored spies on
document.createElement and console
Added a backstop afterEach in test-setup.ts (useRealTimers +
unstubAllGlobals + unstubAllEnvs) so a spec added later cannot
reintroduce the class of bug, and global-hygiene.spec.ts as a canary
asserting the shared jsdom is intact. A global restoreAllMocks was
deliberately not added — it would reset vi.fn() implementations and
break specs that build mocks outside beforeEach.
Also fixes a second, unrelated flake found while verifying: app.spec.ts
loads the root component through a runtime dynamic import that measures
~4s against vitest's 5000ms default, leaving ~20% headroom before load
pushes it over. Raised to 30s, matching the existing precedent in
shared-view.page.spec.ts.
Verified with a harness that builds the full spec set and varies only
execution order (subsetting with --include changes bundling and masks
the bug): 16 seeded orderings including the four that previously failed,
5 plain runs, and 3 runs under 8 CPU hogs on 10 cores — all green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(agents): redirect the Assistant editor to the Designer, one noun in the nav
Designer Phase 5, step 3 and the user-facing half of step 4. Marketplace D1 is
"one noun: Agent", and the sidenav has been shipping both.
Steps 1-2 were already done: `compat.to_agent_view` renders a legacy Assistant
*as* an Agent, and the Designer passed parity some time ago — it carries model
config, tools, skills, memory spaces, tags and visibility on top of everything
the old editor had, and reuses that editor's own knowledge-base section and
share dialog for the rest. The old editor had strictly less to offer for the
same record.
`/assistants`, `/assistants/new` and `/assistants/:id/edit` become `redirectTo`
entries rather than deletions. Those paths are in bookmarks, in the "edit" link
of every old chat session, and in links people shared with each other. The ids
are identical on both sides — nothing was migrated — so a redirect lands on
exactly what the old URL opened. Deleting them would 404 all of that for no gain.
The nav ships one entry, and the Agents "Preview" badge came off with the second
noun it existed to disambiguate.
The term pass is deliberately not a find-and-replace. "You are a helpful
assistant that…" stays as the instructions placeholder: that is the conventional
system-prompt idiom, and rewriting it to "agent" would be worse prompt guidance,
not better terminology. What changed is the words naming our own product concept
— nav, session indicator, share dialog, settings.
⚠️ This changes what AGENTS_API_ENABLED means. While both nouns shipped, off
degraded to the Assistants editor; there is nothing left to fall back to, so off
now means no authoring surface at all. It is an outage switch, not a feature
toggle. Recorded in the flag docstring and the spec rather than silently
changed — worth a deliberate decision about whether the flag should survive.
The old editor's components are now unreachable but not yet deleted; retiring
them (and the docs pass) is the rest of step 4, kept separate so this flip stays
revertible in one line.
Refs #746
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(agents): retire the Assistant editor, keep what the Agent surface consumes
Designer Phase 5, step 4 — the rest of #746. #758 made these pages unreachable;
this deletes them.
Gone: assistants.page, assistant-form.page, assistant-list, assistant-preview,
and the assistant-form barrel.
`assistants/` itself stays, and is no longer a feature — it is the set of pieces
the Agent surface consumes: the share dialog, the assistant card, the
knowledge-base dialogs and services, PreviewChatService, and the models. Each was
verified to have a live inbound reference from `agents/`, `session/` or
`knowledge-base/` before being kept. It keeps the name because the record is
still an Assistant on the wire — ids are identical and nothing was migrated, so
renaming the folder would drag the API contract's vocabulary with it. The README
is rewritten from a stale description of deleted files into that consumer map,
and says plainly that new UI belongs under `agents/`.
The backend `/assistants/*` surface is untouched and not deprecated: `test-chat`
and the document sub-routes are called by the Designer's own preview pane and
knowledge-base section.
Deleting `assistant-preview.component.spec.ts` would have quietly dropped
coverage from a live surface, because `agent-preview` — the component that
replaced it — had no spec at all. Ported the tests that still apply, and pinned
the one behavior that differs: the Agent preview sends no live instructions and
opts out of client-side prompt and tool injection, because an Agent resolves
those server-side from the saved record and a long persona would exceed the
system_prompt cap (422). That looks like a bug until you know why, so it now has
a test with the reason attached. Also pinned the model lock, including its
release on destroy — the lock lives in the root ModelService, so leaking it would
follow the user out of the Designer into an ordinary chat.
The docs pass found less than expected and deliberately changed nothing in
docs/SSE_ERROR_MESSAGING.md, the testing report, or the root README: every
"assistant" there is the model's message role, not our product concept.
Closes #746
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(compaction): re-read compaction state per turn — two managers, one row
Same root cause as #741, different blast radius. The agent cache keys on
configuration, so an `@`-mention turn builds a second `Agent`, and each `Agent`
builds its own `TurnBasedSessionManager`. Both write the same DynamoDB session
row and neither knows the other exists.
`update_after_turn` loaded persisted state only when `initialize()` had skipped
the load. `initialize()` never re-runs on a cache *hit* — the same property that
forked the conversation in #741 — so a hit turn kept whatever state its instance
was built with and then saved it straight over the newer checkpoint and
truncation anchor a sibling had just written. Every exit path of
`update_after_turn` saves, including the "nothing to do" ones, so the clobber did
not need compaction to actually fire on that turn.
This is a cost bug before a correctness one. The truncation anchor is what keeps
the restored prefix byte-stable; moving it backwards re-truncates messages
previously sent whole, which rewrites a 35k-150k-token prefix at the $2.50/MTok
write premium on a turn where nothing about the conversation appeared to change.
Fix is direction (2) from the issue: re-read on every turn. Not the shared-object
approach #750 used for the message list — a sibling may live in another replica,
where aliasing reaches nothing. The read is one GSI query and
`_save_compaction_state` already does an identical one immediately after, so it
roughly doubles a cost that is already noise next to a model call. The
single-flight session lease serializes turns per session, so read-modify-write is
safe. Conditional writes (direction 3) would only re-cover what the lease covers.
State never moves backwards. A load failure is indistinguishable from "nothing
persisted" — both return a default `CompactionState` — so adopting the result
blindly would let one transient DynamoDB error zero a real checkpoint, which is
the clobber this exists to prevent. Adopt only when the persisted record is at
least as far along, and carry the anchor forward even at an equal checkpoint,
since the anchor also advances on prompt-cache expiry.
Removed `_compaction_state_loaded`: nothing branches on it now, and its comment
described a lazy-load that no longer exists. Its one real job — stopping the
failed-compaction path from persisting defaults over a live checkpoint — is now
done by the monotonic guard.
The integration-style test the issue asked for drives two managers against one
shared fake row and asserts the checkpoint never regresses; it fails with
`assert 0 == 8` against the old code, which is the bug verbatim.
Fixes #751
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(agents): retire the unreachable `limits` runnability state
D6 specified three runnability states — ready, limits (degraded), blocked —
and the middle one could never occur. It degraded only when a binding declared
`config.optional == true`; that key was read in exactly one place and written
nowhere. No API accepted it and the Designer had no control for it, so every
gap already resolved to `blocked` and the model was two states pretending to be
three.
The deciding reason is not that it was dormant, though. Building toward it
would have contradicted `agent-designer.md` D5, whose Non-goals say "No
**downgrade** on missing capability (block-only v1)" and call downgrade "a later
opt-in". Block-only is what `agent_binding_resolver` actually implements — it
raises for model, tool, skill and memory_space alike, and `optional` appears
nowhere in it. Making the preview offer a third outcome would have required
teaching the resolver to skip bindings, which is a real product decision about
whether an Agent may silently run degraded, not a checkbox. Two specs disagreed;
this settles them on the one that shipped.
So: `RunnabilityState` is `ready | blocked`, `MissingCapability.optional` and
`_is_optional` are gone, and the detail page and admin default-pins page stop
rendering a distinction they could never draw.
The `optional` flag is not merely unused now — it is inert, and a test pins
that. A hand-edited or future record could still carry it, and it must not
resurrect a state the runtime cannot honour.
D6 is revised rather than quietly trimmed: it records that there were three
states, why the middle one went, and that if downgrade is ever taken up it
starts at the resolver, not at the preview layer.
Fixes #747
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): make locked seeds cost something to an admin, not to a cap
Resolves the D9 "locked-pin ceiling" open question. No cap; friction.
Phase 6 sharpened the concern the question was written about. A locked seed
genuinely cannot be dismissed — verified on dev, a lock beats a user's own
tombstone, which is right, because a lock a user could dismiss would be
pointless. So an admin choosing between "seed" and "seed locked" has no reason
not to lock: locking guarantees the rollout lands and the cost falls on someone
else's sidebar. Left alone, the dominant strategy is to lock everything and
Pinned stops being the user's shelf.
A cap does not work here, and the reason is structural rather than a matter of
picking the right number. Pins merge as a union across every role a user matches
and a lock from any one of them wins (D9.2), so a per-role cap of two does not
mean a member sees two locked Agents — someone in five roles sees ten. Capping
the union instead is not implementable: role membership resolves per user from
Entra claims at request time, so which roles co-occur on one person is unknowable
when an admin saves a seed list. Enforcing at read time would be worse than no
cap, silently dropping an admin's lock for some users — a rollout that looks like
it landed and did not.
So the console shows what it can actually know. A running locked count on the
seed header, a warning past a threshold, and `lockedElsewhere` — how many locked
seeds every *other* role holds, which is the one fact an admin cannot work out
from their own page and the only honest way to show the union. Advisory
throughout: `count_locked_outside` swallows a failed role listing rather than
taking the pins page down with it.
No new failure mode, nothing to migrate for roles already over any line, and a
threshold that moves without breaking a saved list. If over-locking shows up,
revisit with the counts this makes visible.
Fixes #748
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(agents): settle the Agent specs, and make the tagline decision true
Closes the spec-hygiene issue. Four items, and the last one needed code.
**Phasing table** — already fixed while shipping #746; verified, no change.
**Quota attribution** — resolved, in code rather than by inference. The invoker
pays: `check_quota(user=current_user, …)` at chat/routes.py:1319 with no
author-side lookup anywhere on the invocation path, and cost rows land on the
same person (`PK = USER#{user_id}`, metadata.py:157). Both file:lines are in the
spec so nobody re-derives it.
**Review SLA** — committed and split by queue: two business days for
submissions, same day for an `inappropriate` report, weekly for the rest. Phase
8 added reports as a second queue and they are not the same clock — a submission
is a person waiting to publish, a report is mostly maintenance signal. D2 had
been asserting "two business days" in its body while Open Questions said the
commitment was never made; that disagreement is gone.
**`tagline` backfill** — derive and let the author edit, at submission. Writing
that down alone would have re-opened the gap this issue exists to close, because
implementing it turned up a second problem the question never mentioned: tagline
was *author-owned on the wire and unsettable in the UI*. The API accepted it and
the model comment said the author owns it, but no Designer control ever wrote
one — the same dormant-field shape as the `limits` state in #747. So submission
is now where it gets set, prefilled by `deriveTagline` from the description's
first clause. The derivation is not the clever part; putting the shelf row in
front of the author at the one moment they are looking at what the store will
say is. A bad line is then one edit away instead of a surprise after publication.
Also fixed the same trap on the specs' own front pages: both still read
"Status: Draft / proposal" while describing shipped code. That is precisely the
`agent-directory.md` failure this issue cites — a document whose face does not
say what it is. Both now state what is implemented and point at the revisions
recorded in place (D6, D9.7, D14).
Open questions on the marketplace spec are now empty.
Fixes #749
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(costs): tell a deliberate @-mention swap from a cache regression
An `@`-mention hands one turn to a different Agent (Marketplace D11), which
swaps the system prompt and toolConfig and so genuinely re-writes the prompt
cache. That classifies as `miss_avoidable`, correctly — the tokens really were
spent at the write premium. The problem was that nothing on the `C#` row said
the turn was a mention, so a deliberate swap looked exactly like the
nondeterministic-ordering regression the fingerprints exist to catch: both
present as `toolConfigHash` and `systemPromptHash` flipping together. Expected
traffic was diluting the one signal that should never move.
A dimension, not a reclassification. The row records `turnAgentId` and, where
it differs from the previous call's, `agentSwitched`. `cacheStatus` and
`wastedUsd` are untouched, and the session rollup *splits* rather than deducts:
`agentSwitchMissCount` / `agentSwitchUsd` are a subset of `avoidableMissCount` /
`wastedUsd`. Hiding the spend would understate what mentions cost, which is a
thing worth being able to measure on purpose; subtracting gives unexplained
waste, which is the number a regression moves.
Three notes on how rather than what:
The agent id is threaded per turn from the route rather than read off the agent
object. The agent is cached and shared across turns, so per-turn state must
never live on it — that is exactly what forked history in #741 and compaction
state in #751.
`agentSwitched` is derived at write time because `_derive_cache_observability`
is the only place already holding the predecessor row. It compares against the
*previous call*, not the same-prefix match the TTL clock uses: the question is
whether the Agent changed from one turn to the next, and a same-prefix row is by
construction one where it did not.
EMF gains `AgentSwitchMiss` as its own metric rather than a dimension on
`AvoidableMiss`. The namespace deliberately has no dimensions — fleet-wide sums
are the alarm target — so a dimension would multiply metric streams per Agent.
`turnAgentId` rides as a property, queryable without that cost.
The admin anatomy page keeps the red count whole and names the explained part
underneath, so the number an admin reads as alarming only ever means
unexplained. Older rows carry neither attribute and read as unswitched; there is
no backfill and none is needed.
Fixes #756
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(agents): replace the assistant card with an agent launch card
The card that greeted you when an Agent opened was the last pre-Marketplace
surface, and it was out of continuity on every axis that matters.
The one that matters most: it hashed the **first letter of the name** into its
own 26-entry pastel palette, while every other surface hashes `agentId` through
FNV-1a into the twelve curated gradients. The same Agent was therefore drawn two
different ways either side of a tap — exactly what the note on `hashAgentId`
exists to prevent. Two of those 26 (`J`, `P`) were also near-white under white
text, an AA failure rather than a taste call.
The replacement is the store's detail page folded to chat width, in the same
reading order the user just tapped through: tile, name, tagline, publisher, what
it can reach, what to ask. Starters stay buttons — unlike the detail page's
read-only list, the chat has real composer prefill behind `starterSelected`.
- Tile is `app-agent-icon` at 52px, so the store, My Agents, the chat header and
this card all draw the same artwork.
- Reads the `Agent` shape, not `Assistant`: tagline, publisher, category and
capabilities exist only there. No new fetch — `session.page` already loaded it
to lock the chat-input pickers. Falls back to the `Assistant` shape so the card
still paints before the Agent resolves, or with `/agents` disabled.
- D6's run line, from a best-effort `getRunnability` after the Agent load, and
advisory exactly as it is on the detail page.
- Add/Added and the detail link are gated on a published listing. A private agent
has no detail page and nothing to add.
- The close control moved into the card's header row; it used to be an absolutely
positioned white button floating over a gradient banner that no longer exists.
`models/runnability.ts` gives the D6 sentence one source — the detail page reads
from it too, so the two surfaces can't drift into two phrasings of one state.
`assistants/components/assistant-card.component.ts` is deleted; no consumers
remain and the folder's README consumer map is updated. `assistants/` itself
stays — the share dialog, KB dialogs and services still live there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(agents): make Discover read like a storefront
Discover was an h1, a search field, then category headings over a two-column
list of 40px text rows — the information architecture of a store with the layout
of a directory. Featured, which the code itself calls the marketplace's only
ranking lever, rendered as a small card in a row of small cards.
- **Spotlight** (`app-agent-spotlight`): `featured[0]` gets the front door, with
84px artwork, Start chat and Add. The band is tinted by the agent's own tile
gradient via `gradientFor(agentId)`, so the storefront changes character with
what is on it and can never clash with the artwork sitting in it.
- **Store tile** (`app-agent-store-tile`) replaces the shelf row here. Artwork to
52px and the `+` is **always visible** — the row had it at `opacity-0
group-hover:opacity-100`, which on a touch device means the store's primary
verb does not exist. Still no model chip, no counts, no runnability badge (D4).
- **Category chips** filter the loaded shelves; tapping the active chip clears it,
so there is no separate reset. Hidden while searching — search already crosses
every category, and two filters would offer two answers to one question.
- **Your agents** becomes a rail of tiles rather than name pills.
Two judgement calls worth reviewing:
**The scrim over the spotlight gradient is load-bearing, not decoration.** The
twelve palette entries were tuned to keep a white-ish *emoji* legible at tile
size; two of them (amber→orange, lime→green) put white *body text* under 4.5:1.
A fixed scrim makes the contrast a property of the component rather than of which
agent an admin happened to feature, so no future palette entry can silently break
the band. There is a spec asserting it is always present.
**Shelves stayed a 3-up grid, deliberately not a horizontally-scrolling rail.**
The rail is the app-store shape and it is wrong at this corpus size: one holding
three items reads as a broken carousel, and with `GSI5_SK` on `created_at` there
is no ranking to justify hiding items off-screen. "Your agents" *is* a rail
because that list is genuinely unbounded and ordered by the person reading it.
`featured.slice(1)` renders as an "Also featured" shelf rather than folding into
the categories: shelves page at 12, so an older featured agent could fall off the
shelf it belongs to and vanish silently.
`app-agent-listing-row` is untouched and still serves the Pinned tab.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(agents): add a list view to My Agents
A grid/list toggle in the page header, remembered per device via
`LocalSettingsService`. Grid stays the default: it is the view that shows an
agent's artwork at a size you recognise, and most people have few enough agents
that density is not yet the problem.
Both views render the same agents with the same controls. The toggle changes
density, never what is available — a view that quietly dropped the publication
controls would make "which layout am I in?" a question an author has to answer
before they can find out why their submission came back.
That constraint is why `app-listing-status` gained a `part` input. The list row
needs the state badge inline beside the name and the reviewer's note on its own
line below, which the stacked card treatment cannot give it. It is a **split, not
a filter**: no caller may render `'badge'` and drop the note. `'all'` remains the
default, so the grid card is unchanged.
Rows are one line each until an agent has something to say — a review note, or
the marketplace lifecycle controls — and those rows earn their extra height.
The toggle is a `radiogroup` rather than two independent toggle buttons: it is
one setting with two values, and a screen reader should hear it that way. It is
hidden entirely when there are no agents, so it never sits above an empty state.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(admin): spec delegated admin scopes
Admin access is a single bit today: require_admin is
require_app_roles("system_admin"), guarding 112 handlers across 15 admin
router packages, with the SPA gating only the /admin parent route.
Proposes a fourth AppRole grant axis (grantedAdminScopes) scoped 1:1 with
the admin router packages, led by the escalation analysis:
- admin.roles and admin.auth_providers are permanently non-delegable —
IdP claim mapping controls which AppRoles resolve, so delegating it is
role administration by another route.
- Scopes live on the AppRole so the only surface that can write them is
the non-delegable roles admin.
- Closed code-defined registry, never grantedTools — the roles UI has no
free-text entry, which is what made the skills and scheduled-runs
capability gates inoperable.
- No wildcard, no inheritance, and no write-through to a protected or
scope-bearing role from a resource surface.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(admin): resolve open questions on delegated admin scopes
- Audit logging is in scope, as PR-5. Notes that admin_service already
emits structured records on every mutation, so PR-5 promotes existing
emission points rather than adding a new instrumentation pass.
- The ~5-minute scope-revocation lag is accepted; it matches the window
that already applies to removing system_admin.
- No read/write split in v1.
- No example delegated role in the seeder.
Also moves grantedAdminScopes on the role create/update bodies from PR-3
into PR-1, so the axis round-trips end to end in one change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(rbac): add delegated admin scopes as an AppRole grant axis
PR-1 of docs/specs/granular-admin-permissions.md. Data path only — no
authorization check reads admin scopes yet, so behavior is unchanged.
Adds `granted_admin_scopes` to AppRole alongside the existing tools,
models, and skills axes, resolved through EffectivePermissions and
UserEffectivePermissions into the per-user merge.
The registry in rbac/admin_scopes.py is closed and code-defined rather
than catalog-derived. The roles admin UI builds its grant controls from
resource catalogs with no free-text entry, so a capability id that is
not a catalog entry cannot be granted from the UI at all — that is what
made the earlier skills and scheduled-runs capability gates inoperable.
Escalation guards:
- admin.roles and admin.auth_providers are non-delegable. Editing a role
grants arbitrary permissions; editing IdP claim mapping decides which
roles resolve at all. validate_admin_scopes rejects both, at the
service layer, so the rule holds for the REST API and scripts alike.
- No wildcard on this axis. Full admin stays spelled system_admin, and a
stray "*" resolves to an unknown scope that matches nothing.
- Scopes do not inherit. A child role picks up a parent's tools but none
of its admin power.
- system_admin cannot carry scopes; update_role's existing protected
field stripping already covers it, now asserted.
Persisted as a plain attribute on the DEFINITION item rather than as
mapping items with a GSI: nothing needs the reverse lookup, and every
extra prefix is another case _delete_mapping_items has to know about.
Also replaces a SimpleNamespace stand-in for EffectivePermissions in the
prompt-cache determinism test with the real dataclass — a hand-rolled
stub silently loses any field added to the type it imitates.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(inference): restore chat — stream_response never accepted turn_agent_id
Every chat turn on dev returned AgentCore 424. The container was 500ing before
the model was ever reached:
TypeError: StreamCoordinator.stream_response() got an unexpected keyword
argument 'turn_agent_id'
#756 (d64d4208) threaded `turn_agent_id` through `_store_message_metadata`,
`ChatAgent.stream_async`, `base_agent` and `voice_agent`, and forwarded it from
inside `stream_response`'s own body — but never added it to `stream_response`'s
signature. `ChatAgent` passes the kwarg on *every* turn, not only mention turns,
so this was a total outage rather than a mention-path bug: verified on dev with
an Agent attached and with zero tools and no Agent, same TypeError both times.
One line to fix; the body already referenced the name.
The interesting part is why CI was green. The existing coordinator stubs take a
bare `**kwargs`, so they accept arguments the real coordinator rejects — a stub
shaped like the *caller* cannot fail on caller/callee drift. The new tests bind
against `inspect.signature(StreamCoordinator.stream_response)` instead, which
reproduces the production TypeError in-process. All four fail against the
unfixed coordinator.
The last of them asserts that *no* kwarg `ChatAgent` forwards is unknown to the
coordinator, so the next parameter added to this seam is covered without anyone
remembering to write a case for it.
Fixes #756 regression
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(timestamps): stop emitting `+00:00Z`, which no browser can parse
`datetime.now(timezone.utc)` is tz-aware, so `.isoformat()` already renders the
offset as `+00:00`. Appending `"Z"` produced
2026-07-27T05:09:55.853557+00:00Z
— an offset *and* a Z — which is not valid ISO 8601. `new Date()` returns
`Invalid Date` for it, and every SPA formatter falls back silently, so the bug
looked like missing data rather than a bug:
* the agent detail page showed "Last updated —" on an agent edited minutes ago
* admin Reports showed "recently" for every report ever filed
Found while smoke-testing the Agent epic on dev. It is NOT an epic regression —
it is long-standing, and the epic's new date-rendering surfaces are simply the
first place anyone read one of these values. Three call sites had already
discovered it independently and written local workarounds (`sync_policies`,
`users/sync`, `users/repository`), which is the clearest sign it needed one
shared implementation rather than a convention to remember.
`apis.shared.timestamps` is that implementation, promoted from the existing
`sync_policies._iso`. 52 writer sites now call `utc_now_iso()` / `to_iso()`; the
two duplicate `_iso` definitions collapse onto it.
**The part that would have bitten quietly.** Readers used
datetime.fromisoformat(value.rstrip("Z"))
which only worked *because* the writer was broken: stripping `Z` off `…+00:00Z`
leaves a valid offset (aware), but stripping it off a correct `…Z` leaves a bare
naive datetime. Fixing the writers alone silently flipped round-tripped values
from aware to naive — `test_skills_models.py::test_round_trip_preserves_fields`
caught it, and any later comparison against `datetime.now(timezone.utc)` would
have raised `TypeError`. All 11 such readers now use `from_iso`.
**No backfill.** Rows already written keep the old spelling — `createdAt` is
never rewritten — so the SPA normalizes on read instead (`parseIso` in
`utils/date.ts`, applied at 30 parse sites). That fixes historical rows too,
without rewriting values embedded in GSI sort keys (`GSI5_SK =
CREATED#{created_at}`). Mixed spellings compare safely: the suffix differs only
*after* the full date-time-microseconds, so distinct instants still order
correctly.
Two tests grep the tree so neither idiom can come back — verified failing by
reintroducing each.
Verification: ruff at baseline (164, unchanged); backend 5268 passed, 3 skipped;
SPA 147 files / 1644 tests passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(rbac): enforce delegated admin scopes on admin routes
PR-2 of docs/specs/granular-admin-permissions.md. Makes the scopes added
in PR-1 actually govern access. Behavior for system_admin is unchanged:
the superuser satisfies every scope implicitly.
Adds `require_admin_scope(scope)` and migrates 13 admin router packages
to it, one scope per package, so the permission boundary is the package
boundary. Each package names its dependency after its area
(`require_tools_admin`, …), mirroring the pre-existing
`require_marketplace_admin`, so tests have a stable public handle.
`roles/` and `auth_providers/` keep bare `require_admin` and now carry
module docstrings explaining why they can never be delegated — the
second is the non-obvious one: whoever controls IdP claim mapping
controls which AppRoles resolve at all.
Write-through guard (spec I3): the tool/model/skill role pickers write
into role records, all landing in `AppRoleAdminService.update_role`. If
the target role is protected or carries admin scopes, the actor must
hold system_admin. The check lives in `update_role` rather than the
three callers so a future resource surface with a role picker inherits
it. Raises `RoleMutationForbidden` -> 403 via a new app-level handler,
not ValueError -> 400, which would misreport a denied escalation as a
bad request.
`_assert_actor_may_mutate` resolves permissions through a service built
from this instance's own repository and cache rather than the global
singleton, which would otherwise bypass an injected repository and issue
real DynamoDB calls from unit tests.
tests/architecture/test_admin_scope_coverage.py walks the mounted admin
router and fails if any route lacks an authorization dependency, if a
scoped package reverts to bare require_admin, or if a registry scope
governs nothing. Both failure modes were verified to actually fail.
Test helper `override_admin_auth` replaces per-test
`dependency_overrides[require_admin]` across 16 files. It reads the
dependencies off the app rather than importing them, because
test_skills_feature_flag.py reloads the admin routes module and rebuilds
those objects — an import-based list silently stops matching and every
request 401s. It deliberately does not override
`require_marketplace_admin`, which also enforces the marketplace kill
switch.
Also rewrites two stale docs that describe an RBAC API that never
existed (`require_roles`, `require_faculty`, "Admin or SuperAdmin") and
five endpoints absent from this module.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): tell the reviewer when a listed Agent is unopenable
Publication and access are separate axes (D3) and the store only guards one:
`GET /agents/store` is a pure sparse-GSI5 read with no access check, while
`GET /agents/{id}` enforces `get_assistant_with_access_check`. Nothing in the
listing lifecycle touches `visibility`, so an approved PRIVATE or SHARED Agent
gets a shelf tile that everyone can see and only its author can open.
Found on dev, where *both* live listings were in that state — one PRIVATE, one
SHARED — and one of them was a `status=DRAFT` record still named "Untitled
Agent" sitting on the Teaching shelf. The admin copy half-knew ("members can
only open one they could reach on their own"), but every guard was on listing
*state*, never on visibility, and neither the author at submit nor the reviewer
at approve was told.
`reachability` projects `visibility` onto "who can open this" and rides the two
surfaces where someone can act on it: the review queue and the author's submit
preflight. Derived on every read, never stored — `visibility` can change at any
time and a cached copy would be wrong exactly when it mattered.
The two audiences get different words from one shared helper, so they cannot
drift: the author is told how to fix it ("set Visibility to Public"), the
reviewer is not — telling a reviewer to widen someone else's access is the
`allowedAppRoles` trap wearing a different hat.
Advisory throughout. Approve is never disabled, and a test asserts that.
⚠️ Three alternatives were considered and rejected; D3.1 records why so they are
not re-proposed. Blocking submission forbids the legitimate SHARED-to-a-team
publication. Filtering at browse turns the deliberately-pure GSI5 read into N
access checks per page view. Auto-setting PUBLIC on approve is D3's own
prohibition re-entering through the back door.
Verification: ruff at baseline (164); backend 5259 passed, 3 skipped; SPA 148
files / 1646 tests passed. Both new SPA specs verified non-vacuous — neutering
the helper fails 8 of them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(rbac): expose admin scopes over the API
PR-3 of docs/specs/granular-admin-permissions.md. Serves the scope
registry and the caller's own scopes, so PR-4 has something to build the
SPA against. No behavior change: nothing consumes these yet.
- GET /admin/roles/admin-scopes returns the closed registry that feeds
the role form's scope picker. Non-delegable scopes are included with
`delegable: false` rather than filtered out, so the picker can show
them as unavailable instead of leaving an admin wondering why two
areas are missing. Lives on the roles router, which is non-delegable
— granting scopes is a system_admin-only act.
- /users/me/permissions gains `adminScopes`, and `skills`, which
UserEffectivePermissions has carried for months without it ever
reaching the response — the field-added-to-model,
forgotten-in-the-response-shape bug the spec flagged. Both default to
[] so an older client is unaffected.
The registry handler must be declared above `/{role_id}`: FastAPI
matches in declaration order and a single-segment literal loses to a
single-segment path parameter declared first. The symptom is a 404, not
an import-time error, so a declaration-order test guards it — verified
to fail when the handler is moved.
Frontend `UserPermissions` gains both fields to keep the cross-package
contract honest. The guard and nav still gate on system_admin; wiring
them up is PR-4.
Also corrects spec §6.2, which described the dependency as `_require`
across 15 files — it shipped as `require_<area>_admin` across 13 — and
records what PR-2 and PR-3 added beyond the plan.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(gateway): migrate inbound auth to Cognito JWT (CUSTOM_JWT)
AgentCore outbound on-behalf-of token exchange can only exchange a *user*
subject token, and the Gateway's IAM/SigV4 inbound auth never carried one.
Switch the Gateway's inbound authorizer to CUSTOM_JWT trusting the platform
Cognito user pool, so the agent's per-user access token reaches the Gateway
and becomes exchangeable for a target-specific token downstream.
One Gateway remains the endstate. AgentCore permits exactly one inbound
authorizer per Gateway (`authorizerType` is a scalar), but outbound
credentials are per-target — so this single Gateway still fronts both
IAM-invoked Lambda targets (arxiv, policy-search) and future
OAuth/token-exchange targets. Splitting Gateways by backend type would
split along the wrong seam.
- `config.gateway.inboundAuth` ('jwt' default | 'iam') with env/context
override and synth-time enum validation. 'iam' is a code-free rollback:
both AuthorizerType and AuthorizerConfiguration are CloudFormation
"no interruption" updates, so flipping does not replace the Gateway or
orphan its registered targets (confirmed via cdk diff change set).
- Cognito user pool + BFF app client passed as construct refs, not SSM:
the pool is a sibling in this stack and CFN resolves SSM template
parameters before any stack resource exists.
- Authorizer validates `allowedClients`, NOT `allowedAudience` — Cognito
*access* tokens carry `client_id` and no `aud` claim, so an audience
check can never match and would 401 every call.
- Construct throws at synth when 'jwt' is selected without Cognito refs,
rather than deploying a Gateway that rejects everything.
- AGENTCORE_GATEWAY_INBOUND_AUTH threaded to the inference runtime from the
same config value, so agent data-plane auth and the deployed authorizer
cannot drift.
Tests: 5 new Gateway construct tests (JWT default, client_id-not-audience,
discovery URL, iam rollback, throw-without-refs). New `mockCognitoRefs`
helper uses `from*` imports so it adds zero resources and existing
resourceCountIs assertions stay meaningful. 480/480 jest, tsc clean.
Refs docs/specs/AGENTCORE_GATEWAY_TOKEN_EXCHANGE_PLAN.md (Phase 1A)
* feat(gateway): send the signed-in user's token to the JWT Gateway
Pairs with the CUSTOM_JWT inbound authorizer: the agent now presents the
signed-in user's Cognito access token as a Bearer credential instead of
SigV4-signing Gateway calls with the task's IAM identity. `self.auth_token`
already held that token, so this threads it through rather than adding a
new credential source.
Both halves must deploy together — once the authorizer flips, SigV4 is
rejected, and until it flips a bearer token is.
- `_build_gateway_auth()` selects bearer vs SigV4 from
AGENTCORE_GATEWAY_INBOUND_AUTH, which CDK sets from the same
`config.gateway.inboundAuth` that builds the authorizer — so the two
cannot drift. Unrecognized values fall back to 'jwt' with a warning
rather than silently dropping user auth.
- Reuses the existing OAuthBearerAuth from integrations/oauth_auth.py
instead of adding a second bearer implementation.
- Tokens are bound per client instance, never module-level, so one user's
credential cannot leak into another user's Gateway client.
- Missing token in jwt mode raises GatewayAuthError; GatewayIntegration
catches it and degrades to no Gateway tools rather than failing the turn
(every call would 401 anyway). Error text points at the headless-grant
path, the likely cause for a scheduled run.
Verified no non-user caller loses access: scheduled/headless runs already
mint a real Cognito access token via CognitoRefreshBearerAuth (and
run_agent_headless requires it to start today), and the API-key path calls
Bedrock Converse directly without touching the Gateway.
Tests: 11 new — auth-mode resolution, the actual Authorization header
value, missing/empty token rejection, iam rollback ignoring the token,
cross-user token isolation, and graceful degradation. Full backend suite
5419 passed.
Refs docs/specs/AGENTCORE_GATEWAY_TOKEN_EXCHANGE_PLAN.md (Phase 1B)
* docs(gateway): warn forks about SigV4 callers before the JWT switch
The Gateway inbound-auth default flips to `jwt`, which means the Gateway
stops accepting SigV4. In this repo the agent is the only data-plane caller,
so the migration is self-contained — but a fork that added its own Lambda,
scheduled job, or service calling the Gateway with SigV4 would start getting
401s with nothing in the code to warn them.
Documents `CDK_GATEWAY_INBOUND_AUTH` in the per-environment overrides table
and adds a "Gateway inbound authentication" section covering the upgrade
check, the `iam` escape hatch, and the two things that are *not* affected
(registered targets keep working; the authorizer swap never replaces the
Gateway). Also notes the infra+backend deploy-together requirement.
Docs-only. Astro build clean (51 pages).
* fix(gateway): default inbound auth to iam — the authorizer is immutable
PlatformStack failed deploying #778 to dev:
Authorizer type cannot be updated for an existing gateway
(Service: BedrockAgentCoreControl, Status Code: 400)
AgentCore will not change a Gateway's authorizerType after creation. The stack
rolled back cleanly and the Gateway kept AWS_IAM, READY, and both targets — but
the migration as designed cannot work in place.
Neither pre-deploy check caught this. The CloudFormation resource reference
documents AuthorizerType as "Update requires: No interruption", and `cdk diff`
via a real change set reported an in-place [~] modify. Both describe CFN's
plan, not the AgentCore service's validation. A change set is not a deploy test.
Two failures to fix, not one:
1. The backend half shipped while the infra half rolled back, so the runtime
had no AGENTCORE_GATEWAY_INBOUND_AUTH — and the agent's default was 'jwt'.
That pointed the new agent at bearer auth against an AWS_IAM Gateway, 401ing
every Gateway tool call. Both defaults are now 'iam': an absent value means
"behave like the Gateway that is actually deployed", which is the only safe
direction when the two halves can land independently.
2. CDK_GATEWAY_INBOUND_AUTH existed only in config.ts — step 1 of the repo's
7-step config pattern. The documented escape hatch was unreachable from CI.
Now exported and validated in load-env.sh, passed as context (synth.sh and
deploy.sh already use build_cdk_context_params), and carried in platform.yml's
job env. backend.yml runs no CDK, so it needs nothing.
Also corrects every place that asserted the opposite: the GatewayConfig doc, the
construct's inline comment and class doc, the plan's Phase 0/1A, and the public
environments.md page (now a :::danger: covering the real error and why the
pre-deploy signals mislead).
The single-Gateway endstate still holds. What changes is the mechanism: reaching
CUSTOM_JWT needs a new Gateway plus target re-registration and a cutover, not a
config flip. Targets are managed out-of-band by app-api's GatewayTargetService
and the mcp-servers repo, so that needs its own design — tracked in the plan.
Verified: synth against dev now emits AuthorizerType AWS_IAM matching the live
Gateway, and `cdk diff` shows no authorizer change, so the deploy proceeds.
tsc clean; infra 481 jest; 58 backend gateway + supply-chain tests.
* feat(rbac): gate the admin console on delegated scopes
PR-4 of docs/specs/granular-admin-permissions.md, the last one. Makes
the scopes reachable through the UI: a delegated admin can now be
granted areas from the role form, enter the console, and see only what
they hold. Full admins see no change — `hasAdminScope` short-circuits
on system_admin, so the console is identical to before.
- UserService gains `adminScopes`, `hasAdminScope()`, and
`canAccessAdmin`. `isAdmin` keeps its exact previous meaning and still
gates genuinely superuser-only surfaces.
- `adminGuard` gates the /admin shell on `canAccessAdmin` rather than
`isAdmin`; a new `adminScopeGuard` gates each page on its
`data.scope`. A route with no scope is DENIED, not allowed — failing
open there would hand every delegated admin an unvetted surface.
- The /admin landing was a static `redirectTo: 'costs'`, which would
drop a skills-only admin on a page they cannot open. It is now a guard
that resolves the first area the user can actually reach.
- Nav filters by scope and drops emptied groups. The marketplace badge
fetch is gated on `admin.marketplace`; unconditional, it was a
guaranteed 403 on every navigation for admins without it.
- The role form grows an Admin Access picker, grouped by the same
headings as the admin nav and fed by GET /admin/roles/admin-scopes.
Non-delegable areas render disabled with a lock rather than being
omitted, so it is visible that roles and auth providers are withheld
by design rather than missing by accident.
`admin-scope-wiring.spec.ts` is the SPA counterpart to the backend's
architecture test: it fails if an admin route lacks a scope or guard, if
a scope is not in the registry, or if a nav entry's scope disagrees with
its route's. Verified to fail on an unscoped route (3 tests catch it).
The sidenav specs stubbed UserService with `isAdmin` only, so moving the
admin entry point to `canAccessAdmin` broke five of them — stubs updated
rather than the source reverted.
SPA: 1675 tests pass, AOT build clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(marketplace): require PUBLIC visibility to publish an agent
The store gates browse on `listing.state` alone, while pinning gates on
`visibility`. An agent could therefore be published while still SHARED or
PRIVATE: a tile everyone saw and only the author could open. Two users hit
this during the dev demo — `POST /agents/{id}/pin` returned a bare
"Agent not found" for a tile the store had just offered them.
The marketplace is public-only. Sharing an agent with named coworkers is a
separate mechanism, and a listing carries no audience of its own, so a
published non-PUBLIC agent is incoherent state rather than a team listing.
- Block submission unless the agent is PUBLIC, surfaced by `preflight_listing`
(the dialog already renders a block reason and hides the form) and enforced
by `submit_listing`. A refusal, not a silent widening: publication must not
be a side door that changes who can reach an agent.
- Re-check at approval. `visibility` can be narrowed between submitting and
being reviewed, so the submit-time gate says nothing about approval time.
- Answer a pin denial on an already-published agent with a legible 403 instead
of collapsing to 404. The store advertised that id, so its existence is not
a secret; the collapse still applies to everything else, and the extra
lookup is best-effort so it can never escalate a 404 into a 500.
`_reachability` stays: an agent published as PUBLIC and narrowed afterwards is
the case no gate can catch. Its comment — and the SPA's — claimed publishing a
SHARED agent to a team was legitimate, which is what made this look intended.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): let authors go public from the submit dialog
Requiring PUBLIC to publish left the *common* path a dead end. Every agent is
created PRIVATE, so a first-time author opened Submit to a red block telling
them to go set visibility on the agent editor and come back — two screens for
one decision, and worse than the amber warning it replaced.
Consent now lives where the decision is made. The submit dialog shows a
checkbox ("Make this agent public"), `makePublic` rides the submit request, and
`write_listing` widens visibility in the same write as the listing. One write
matters: two could leave an agent listed but unreachable, which is the exact
state this whole gate exists to prevent.
It stays consent rather than a side door. The box starts unticked, Submit is
disabled until it is ticked, and the flag defaults to false — so a direct API
caller who omits it is refused exactly as before, and an already-public agent
never has its visibility rewritten.
`blockReason` and `requiresPublic` are now separate signals. A block means
"leave the dialog and fix someth…
Minor release on knowledge bases, marketplace governance, and fine-tuning. - Bedrock Managed Knowledge Base migration lands inert behind nine CDK_MANAGED_KB_* flags (all explicitly false in prod). Adds a Bedrock service role, four Lambdas on one image, a report-only reconciler, four alarms, and the sparse KbWorkIndex GSI. - Marketplace admins can read, test-drive and decline a submission; new terminal 'rejected' listing state. - Fine-tuning wiring restored (FINE_TUNING_ENABLED was never set on the container); access stays whitelist-only at quota 0. Three billing/format bugs fixed. - Live RAG, ungated: queries clamp at 10,000 chars, document-status filter fails CLOSED. - Chat: arrow keys walk the @-mention menu; mentions render as an address. - RBAC: JWT role mappings accept IdP group names containing spaces.
* feat(agents): run the approved snapshot, not the author's draft
PR-3 of the agent version-snapshots epic. This is the part that makes the
feature a control rather than a display fix: a user who pinned an approved
Agent now runs the reviewed configuration, not whatever the author's draft
says today.
The invocation change is one line at chat/routes.py, because PR-1 built the
round trip for it — a version deserializes back into the same `Assistant`
that `resolve_agent_invocation` already takes, so binding resolution, the
system prompt and the harness are all untouched:
assistant, resolved_version = await resolve_invocation_agent(assistant, user_id)
- **`version_resolution.py`** holds the policy: the published snapshot for
everyone, the draft for the owner, the live record when nothing is
published. Separate from `versions.py` (pure) and `version_repository.py`
(persistence) because deciding which version a person runs is policy, and
policy reads better as one short function with the whole table in view.
- **Owner identity, not edit access.** An editor can change an Agent's
instructions but does not get to *run* the unpublished result — otherwise a
share grant is a way around review.
- **A missing published snapshot raises** (503 at the route) rather than
falling back to the draft. The fallback would serve unreviewed instructions
to a pinned user at exactly the moment something is already wrong. The owner
is unaffected: they never read the version, so a broken snapshot cannot lock
them out of their own Agent.
Purely additive: three files, and no existing behavior changes except the one
swap above.
**Deliberately not implementing spec §4.2 (version in the agent cache key).**
The spec says promoting a version would keep serving the old system prompt
from a warm agent. That is not true in this codebase: the cache key is built
from construction *values*, and everything a version changes about behavior
already reaches it — instructions via `system_prompt`, tool bindings via
`enabled_tools`, skills via `skills_hash`/`agent_type`, the model via
`model_id`, and a memory binding by skipping the cache entirely. Promoting a
version already misses.
Adding the number would buy no discrimination and would cost real safety. The
resume path rebuilds its cache key from `PausedTurnSnapshot`, so a new key
element the snapshot did not carry orphans the paused agent — an OAuth-consent
or tool-approval pause on any published Agent would fail to resume with "must
resume from interrupt". `service.py` warns about precisely that desync. An
earlier revision of this PR added the key element, hit that bug, and threaded
`agent_version` through the snapshot and `stream_coordinator` to fix it; the
honest fix was to not add the element. The reasoning is recorded inline so the
next reader of §4.2 does not re-introduce it.
Version snapshots also *improve* prompt-cache stability, which is the opposite
of the risk §4.2 implies: a published Agent's system prompt now changes only
at approval, where before it changed on every author save mid-conversation.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(marketplace): reconcile version-snapshots spec with what shipped
Doc only. PR-1 (#784) and PR-2 (#787) are merged and PR-3 (#789) is open, and
building them proved parts of the spec wrong. Corrected inline with the
original claim preserved, rather than silently rewritten — a spec that quietly
agrees with the code teaches nobody why.
**§4.2 was wrong and is the substantive fix.** It said the agent cache key
"must include the resolved version, or promoting a new version will keep
serving the old system prompt from a warm agent." Not true here: `_agent_cache`
keys on construction *values*, and everything a version changes about behavior
already reaches the key — instructions via `system_prompt`, tool bindings via
`enabled_tools`, skills via `skills_hash`/`agent_type`, the model via
`model_id`, a memory binding by skipping the cache entirely. A promotion
already misses.
Acting on it also introduces a bug: the resume path rebuilds its key from
`PausedTurnSnapshot`, so a new key element the snapshot does not carry orphans
the paused agent and breaks OAuth-consent / tool-approval resumes. PR-3 added
the element, hit that, and threaded `agent_version` through three more files to
fix it before reverting. The section now says do not re-implement it.
The section also had the risk backwards: snapshots *improve* Bedrock
prompt-cache stability, since a published Agent's prompt now changes only at
approval instead of on every author save mid-conversation.
**§3.3** — real key prefixes (`AST#`/`METADATA`, not `AGENT#`/`PROFILE`);
`submittedVersion`; placement lives in the index key so an immutable snapshot
is never rewritten on recategorization; and the fail-closed write ordering that
replaced the atomicity lost when the index moved off the Agent row.
**§7** — per-PR status, the note that D13 admin edits had to cut a version
(§6.2's first option was not optional once the store renders snapshots), and a
warning that `develop` is currently in the PR-2-without-PR-3 half-state.
**§8** — two new open items: making the fail-closed ordering structural with a
transaction, and the pre-existing MCP-app dispatch call sites that build agents
from `input_data` rather than the resolved assistant.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): withdrawal becomes a request, and delete respects it
PR-4 of the agent version-snapshots epic (§5). Closes the last unilateral
removal path: an author could pull an approved Agent out of the store, or
hard-delete it outright, with no admin ever seeing it.
D2 makes publication stop for a human. Un-publication did not — `published →
private` was in the author's own hands, and `delete_assistant` guarded on
ownership alone. Both are now admin-visible acts.
**`withdrawal_requested` is a LIVE state, and that is the crux.** The author
asks; the listing stays on the shelf until an admin decides. Clearing the store
index the moment they asked would hand them exactly the unilateral delisting
this state exists to prevent — and it makes the decline path free, because
nothing was undone: no key to restore, no version to re-promote.
That forced a distinction the code did not have. `is_published` (exactly
`published`) is now separate from `is_listed` (`published` or
`withdrawal_requested`), and three of the four existing `is_published` call
sites actually meant "live in the store" and moved. A test asserts the two
predicates disagree on exactly one state, so if they ever coincide again the
guarantee has silently broken.
- **One endpoint, two acts.** `DELETE /agents/{id}/listing` branches on state:
a request when the listing is live, immediate when it is not. The author's
intent is identical either way, and making them pick the right verb for their
listing's state is asking them to know the state machine.
- **`POST /admin/agents/{id}/withdrawal`** takes `grant`/`decline`. Deliberately
not folded into `/review`, where "approve" already means "publish" — one
endpoint with four decision values makes an accidental unpublication a
one-character mistake.
- **Requests land in the existing review queue** and the nav badge counts them
(§5.1). A second queue is one an admin has to remember exists.
- **Delete is refused unless the listing is `private` or absent** (§5.2), with a
message naming the way out. `taken_down` is covered deliberately: an author
must not delete their way out of a takedown record.
Two things found while building it:
`AUTHOR_TARGET_STATES` was **dead** — declared and never read, so the mechanism
§5.1 names ("`published → private` leaves AUTHOR_TARGET_STATES") enforced
nothing. It is load-bearing now via `assert_author_target`, because the
transition table alone cannot say "this edge is legal but only for an admin",
and `withdrawal_requested → private` is exactly that.
The `/assistants/{id}` delete path soft-deletes documents and removes sync
policies **before** the record delete, so a refusal discovered at the record
write would leave the Agent gutted and still in the store — worse than either
outcome alone. Hence `assert_deletable` at step 0, sharing one rule function
with the delete itself, and a test that the two agree.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(agents): delete a deleted Agent's version snapshots
Follow-up to the version-snapshots epic (#784, #787, #789, #791), found by
checking the seams between PRs that shipped in parallel and never saw each
other.
**The leak.** `VERSION#` child rows arrived in PR-2 and were never wired into
the delete path, which cleans up `REPORT#` rows and the metadata row and
nothing else. So deleting an Agent left its entire snapshot history in the
table permanently.
`reports.py` states the principle outright — child rows live under the Agent's
partition "precisely so they never outlive what they concern" — and versions
quietly violated it.
Storage-only, no correctness impact, and it would never have surfaced on its
own: PR-4 refuses to delete anything but a `private` listing, and a private
listing's versions carry no store key, so nothing would ever render them. That
is exactly what makes it worth catching deliberately rather than waiting for a
bill.
Immutability is about *rewriting*, not retention — a version may never be
changed, and it is meaningless once the Agent it snapshots is gone. Nothing
audits it either: §8 flags "versions referenced by an audit record should
survive" as a question for a retention policy that does not exist yet, and
there is no such reference today.
**Also: two tests for a seam neither PR could have covered.** PR-3
(invocation) and PR-4 (withdrawal) were built in parallel and merged
independently, so nothing exercised `withdrawal_requested` through
`resolve_invocation_agent`. The behavior is already correct — a pending
withdrawal keeps `publishedVersion`, so everyone but the owner still runs the
approved snapshot, which is right because a request is not a removal — but it
was untested, and it is precisely the kind of thing that regresses silently.
Backend 5508 passed / 3 skipped (5504 on develop before this, +4 new).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): show a reviewer what a submission changed
PR-5 of the agent version-snapshots epic (§6.1). The reviewer's actual
question is "what changed since I approved this?", and until now they could
not see it: a submission arrived in the queue with no reference to what it
replaces, so a typo fix and a full instruction rewrite looked identical and
both got the same careful read.
`GET /admin/agents/{id}/diff` returns the pending version against the
published one — changed fields with before/after values, a unified diff of
the instructions, and a `behaviorChanged` flag. The review queue grows a
collapsed "What changed" control per row: behavior changes badge amber,
presentation changes grey, and the instructions diff renders red/green
beneath.
The asymmetry is the point. A tagline fix should be approvable at a glance;
an instruction rewrite should be impossible to miss.
- **The diff is computed server-side** with `difflib`. A client-side library
would render more prettily, but it means a new SPA dependency (this repo
pins exactly and does not add packages casually) and a second implementation
of "did this change" that can disagree with the field-level answer.
- **Collapsed and fetched on expand.** The queue is a list of decisions;
pre-loading a diff per row would pull every pending agent's full
instructions down to render a control nobody opened. Asserted by test,
because it is a property a template edit can silently lose.
- **`DIFF_FIELD_ORDER` is asserted against the snapshot field lists at import
time.** The worst failure available here is a reviewer told "nothing
changed" about something that did, which is exactly what a field added to
`AgentVersion` and forgotten here would produce.
- **"First submission" is a distinct signal**, never an empty change list.
They are opposite claims: one means "read all of this", the other means
"approve it". A dangling `publishedVersion` falls back to the same
rendering — "there is nothing live to compare against" is true, "the author
rewrote everything" would not be.
- **Absent is not empty.** `bindings: null` (synthesize the legacy KB binding)
differs from `[]` (binds nothing), and a laxer comparison would report no
change while the agent quietly lost its knowledge base.
- **Record metadata is never a change.** `version`/`createdAt`/`createdBy`
differ on every snapshot and would bury the fields that matter.
Backend 5553 passed / 3 skipped. SPA 1691 passed across 153 files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(tools): stop warning that context-bound tools are missing
ToolFilter knows three tool classes — registry, gateway, external MCP —
and warns on anything else. Context-bound tools are a fourth: they need
request scope (session/user/assistant) baked in at construction, so
inference_api builds them per invocation and passes them as extra_tools,
which BaseAgent appends *after* filtering. The filter was never taught
about them, so every enabled one logged "not found in registry or
catalog, skipping" — and then worked fine anyway.
In prod that is ~2,500 false warnings a day (create_artifact 1,744,
analyze_spreadsheet 1,690, list_spreadsheets 1,690 over 48h), which
drowns the one signal that branch exists to give: a genuinely stale tool
id pinned in a saved session's enabledTools. It also cost real time
during an incident triage, where it is the first WARNING on a failing
turn and points nowhere.
The message was also wrong on its face — the filter never consults the
catalog, and both spreadsheet ids are in it (tool_catalog.py).
Move the per-family gate ids to apis/shared/tools/injected.py so the
route (deciding what to build) and the filter (classifying) read one
definition, and classify them as a known class. These are gate keys, not
tool names — workspace_files provisions list/read/write — so the set
can't be derived from what the factories return. get_statistics gains an
injected_tools bucket instead of over-counting unknown_tools.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(admin): surface the error banner after a failed category change
Pre-existing, found while building the Publishers page against the same
pattern. `mutate()` set the error message and then called `reload()`, which
clears the banner on entry — so the message was wiped before it could render.
The visible effect: an admin deleting a category that still has listings in it
got a silent no-op. The backend returns a 409 whose message explains the
refusal and suggests disabling instead, and none of it ever reached the screen.
The reasonable conclusion is that the button is broken.
Fixed by capturing the message, reloading, then setting it — with the ordering
trap named in a comment, because the obvious order is the wrong one and the
next copy of this pattern will reach for it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): admin page for publisher profiles
PR-6 of the agent version-snapshots epic (§6.3), and the last phase. Purely a
UI gap over a finished backend: full publisher CRUD plus the eligibility
allowlist has been live at `/admin/agents/publishers` since the marketplace
shipped, with no way to reach it — so a profile like "Registrar" or
"Communications & Marketing" could only be created by calling the API directly.
Adds the page, its route under the `admin.marketplace` scope, a Publishers nav
entry, and the SPA service methods (create / update / delete / eligibility).
No backend change.
The page's job is to make three otherwise-surprising rules legible:
- **⚠️ Attribution is a name, not a permission.** Publisher never grants access
to anything, and `ownerName` remains who actually owns the agent and whose
skills resolve when it runs. Stated at the top and again next to `verified`,
which renders as a check mark and is the field most likely to be mistaken for
a grant.
- **The id is fixed at creation.** Listings store it, so renaming a publisher
would strand every attribution pointing at it. The row shows the id and says
so — the same rule categories follow, surfaced the same way.
- **Disable, don't delete.** Deleting is refused (409) while listings are
attributed to the publisher; disabling drops it from the submit picker while
existing attributions keep rendering, which is nearly always what was meant.
`department` is the default kind because it is what an admin adding a publisher
almost always wants; individual profiles are auto-created from an author's
display name on first submission, which the empty state says rather than
leaving "no publishers yet" reading as broken.
SPA 1693 passed across 153 files. Backend unchanged at 5508.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(api-keys): resolve the key owner's real roles for RBAC
/chat/api-converse built its User with a hardcoded `roles=["user"]`
placeholder. No AppRole maps the JWT role `user`, so permission
resolution matched nothing, fell back to the `default` role — which
grants no models in prod — and every request 403'd with "Access denied
to model: <id>" regardless of the caller's actual grants.
An API key record stores only key_id/user_id/name, never roles, so the
owner's roles are now read back from the Users table per request: the
same record the cookie-session path enriches from, holding the IdP roles
parsed from the Entra ID token at BFF callback. Email and name come from
the profile too, so quota tier and cost attribution stop being charged
against a synthetic `{user_id}@api-key` identity.
Fails closed — a key whose owner has no profile row is refused rather
than silently degraded to `default`.
Also fixes a cache collision this exposed. `resolve_user_permissions`
derives its result purely from `user.roles` but cached under
`user:{user_id}` alone, so the API-key and cookie paths — same subject,
different roles — shared one entry: whichever resolved first served the
other for the whole 5-minute TTL. That made the bug intermittent and let
an API-key request strip a live SPA session's grants. The key is now
`user:{user_id}:{roles_fingerprint}`, and invalidate_user prefix-deletes
every role-set entry.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(marketplace): close four version-snapshot gaps found in E2E testing
End-to-end testing of the version-snapshots epic (#784–#795) against dev
surfaced five reported issues. Four were real; the fifth turned out to be
deliberate behaviour and is documented rather than changed.
**The detail page served the draft, not the snapshot.** `GET /agents/{id}`
had no version overlay, so a published Agent whose author kept editing had a
store tile rendered from the approved snapshot and a detail page rendered from
the unreviewed draft — different name, different summary, and `capabilities`
resolved from the draft's bindings, so the page advertised tools the published
version did not have. Invocation ran the snapshot regardless, which made it a
lie rather than a preview. `resolve_display_agent` now answers this the same
way invocation does, with one deliberate difference: editors keep seeing the
draft, because the Agent Designer loads its form from this endpoint and serving
an editor the snapshot would make their next save revert the owner's draft.
**Withdrawal requests could not be granted.** `POST /admin/agents/{id}/withdrawal`
had no SPA caller. The request appeared in the review queue indistinguishable
from a submission, so "Request changes" 400'd on an illegal transition and
"Approve" silently *declined* it by re-publishing. The queue now labels the row,
says the listing is still live while the admin decides, and offers Take it down /
Keep published against the endpoint built for them. The author-side control said
"Unpublish" and promised removal from the store; it is now "Request withdrawal"
and says an admin decides first.
**The review diff was unreachable.** Every UI route to a resubmission cleared
`publishedVersion` first, so `diff_pending_version` always returned
`first_submission` — the "what changed since I approved this" comparison never
rendered. `published → changes_requested` is the one transition that preserves
it, and nothing exposed it; the Listings page now does.
**Publisher delete was unguarded.** `publishers.page.ts` documents a 409 refusal
while listings are attributed, and its error handling was already written to
surface it, but nothing enforced it — deleting an in-use profile silently
unattributed every listing naming it, live ones included, with no surface to
repair them and no confirmation first. Adds `publisher_in_use` (mirroring
`category_in_use`) and a confirmation dialog.
Also: `is_on_shelf` names the "in the store right now" question, because
`is_listed` answers by state name and a published listing sent back for changes
keeps serving — the featured row and browse disagreed about it. And
`AdminListingRow.withdrawalRequestedAt` is what lets the queue tell a withdrawal
request from a submission at all.
**Not changed:** requesting changes on a live listing does not unpublish it.
That is deliberate and asserted by test — the approved version keeps serving
until one replaces it, and takedown is the operation that pulls something down.
One consequence is left open with a note rather than patched: an author can take
such a listing private alone, and closing that needs a product decision, since
the transition table cannot allow `changes_requested → withdrawal_requested`
without opening a route to `published` for something never approved.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): version rollback, and close the withdrawal origin gap
Finishes the version-snapshots epic: the two §8 open items get decisions, and the
behavioural gap #799 deliberately left open gets closed now that the product call
behind it is made.
**An author can no longer pull a live listing alone.** A listing that was published
and then sent back for changes keeps serving (`review_listing` does not unpublish),
but sits in `changes_requested` — and `withdraw_listing` asked `is_listed`, which
answers by state name, so it read that listing as not-live and took it straight to
`private`. #799 documented this rather than fixing it, because the transition table
could not allow `changes_requested → withdrawal_requested` without also opening
`in_review → changes_requested → withdrawal_requested → published`, a route to
published for something never approved.
`AgentListing.withdrawalFrom` is what makes the edge safe: a declined withdrawal
returns to the state it came *from*, so a request that entered from
`changes_requested` can only go back there. Approval stays the only door into the
store, and `test_approval_is_the_only_door_into_the_store` now asserts that
structurally — decline targets must equal the set of states that can enter.
⚠️ Moving that decision onto `is_on_shelf` opened a hole the existing suite caught:
`withdrawal_requested → private` is a legal edge (it is how an admin *grants* a
withdrawal) and `private` is an author target, so a pending request with no
published pointer resolved to `private` and the author granted their own
withdrawal. `withdraw_listing` now refuses a second request explicitly rather than
relying on the table to imply it.
**Rollback shipped** (§8, requested): `GET /admin/agents/{id}/versions` +
`POST /admin/agents/{id}/rollback`, with a picker on the admin Listings page. Three
constraints: only from `published`, or it is a second door past review; a reason is
required and lands on the author's card, as a takedown's does; and no version is
cut — it is a pointer move over immutable records, so rolling forward is the same
operation. Reuses `_publish_version`, inheriting the new-key-first ordering.
**Retention decided: deliberately unbounded.** At a few hundred agents with a
handful of KB-sized snapshots each the storage is immaterial, and every alternative
costs more than it saves — a TTL deletes invisibly and cannot exempt versions an
audit record points at, a keep-last-N prune destroys the older half of a listing's
approval history. Rollback makes this stronger, not weaker: an old version is now
something an admin can put back, so deleting one costs a recovery path.
**Also fixed:** the store read logged a full pydantic traceback per legacy row per
browse — listings published before PR-2 still carry GSI5 keys on their `METADATA`
item, so the query returns Agent rows that cannot be an `AgentVersion`. Skipped by
sort key now. That skip exposed a latent misalignment: `browse_all` paired items to
responses positionally, so any dropped row slid every later response onto the
previous row's sort key; `_project_with_keys` builds the pairing where the drop
happens. And the review diff no longer tells a reviewer looking at an in-review row
that there is "nothing awaiting review" when the real cause is a pre-snapshot
submission.
**Not changed:** the resubmit dialog's category default. #799 reported it as
resetting to the first option; it does not — `ngOnInit` preselects the listing's
category and clears it only when that shelf has closed. Three regression tests
pin the behaviour that was previously untested.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(marketplace): keep the version picker reachable after a rollback
#800 shipped rollback with the property that makes it safe to use — no version is
cut, so the snapshot you rolled off is still there and putting it back is the same
pointer move. The dialog said so. The Listings page then hid the only control that
could do it.
`canRollBack` asked `publishedVersion > 1`. That reads "are we serving above the
first version?", which gives the same answer as "does a second version exist?" right
up until someone rolls back — and the opposite one afterwards. A listing rolled back
to v1 with v2–v5 intact looked identical to one that had only ever had v1, so the
button vanished in the one state that most needs it. The endpoint accepts the
forward move; verified against dev, the UI simply had no way to ask for it. A
rollback you cannot undo is a worse rollback.
The row now carries `latestVersion`, derived from `max(publishedVersion,
submittedVersion)`. Both counters only move *up* when a snapshot is cut and
`submittedVersion` survives the pointer moving down, so the max is ≥ 2 exactly when
a second version exists. It can understate the true highest — an admin presentation
edit bumps only `publishedVersion`, so a later rollback leaves it one short — which
is harmless because nothing reads the number itself, only whether it is above one.
No extra read: a page of rows must not fetch every agent's history, and anything
needing the real list already calls `list_agent_versions`.
Copy follows the behaviour rather than the majority case. "Roll back" named the
opposite of what the control does on the sequel to every rollback, so the button is
"Change version", the dialog is "Publish a different version", and its reason
prompt asks why you are changing the published version. The picker was already
correct — it offers everything not currently live, in either direction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(marketplace): show the reviewer why a diff is missing
#800 taught the diff endpoint to tell a reviewer the real cause when a submission
predates version snapshots, instead of claiming there was nothing awaiting review.
The message never arrived: `ReviewDiffComponent.load` caught with a bare `catch` and
replaced every failure with "Could not load what changed. Try again, or review the
agent directly."
So the reviewer was told to retry, and retrying could never work — the row is old,
not broken. Verified against dev: the backend returns the specific text and names
the fix ("ask the author to resubmit — that captures one on the way in"), and the
UI discarded it at the boundary.
The `detail` is now preferred when the server sent one. The generic line stays as
the fallback for the case it was written for: a real transport failure, where there
is no `detail` and "try again" is exactly the right advice.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* style(sidenav): put the New badge on the true brand ember
The badge was already on `secondary` tokens but read as amber. The scale
derives every step from `#d64309` by moving lightness alone and holding
chroma, which walks the colour out of gamut in both directions: the 50 step
clipped to a pale yellow, and 600 clips toward pure red. Only 500 is a
literal hex, and it is the value the New Session `+` above is drawn in — so
the badge now fills with it and matches, sampled byte-for-byte.
White on 500 measures exactly 4.50:1, clearing the AA floor for 10px text
with nothing to spare; noted at the call site so a future darkening of the
text or lightening of the fill gets re-measured rather than assumed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(agents): make the share dialog the one surface for reach
Who can open an agent was answered in three unrelated places: a people list
and a link in the share dialog, publication controls on the agent list's
cards, and nothing tying them together. They are one question asked at three
widths, so the dialog now reads as a ladder — People with access → General
access (the link) → Marketplace (found without one).
Cards go back to being index entries: icon, name, one line, four verbs. The
model chip, the tool/skill/memory counts and the whole publication rail came
off; they turned a page you scan into a page you read. The preview panel's
capability strip goes for the same reason — it restated the form sitting one
column to its left, so it could only ever agree or be wrong.
Two bugs this pairing would otherwise have introduced:
* Publishing from inside the dialog flips visibility to PUBLIC mid-session,
while the save path derives PRIVATE/SHARED from the people list. Deriving
over a fresh PUBLIC would narrow a published agent out from under its own
listing — the store keeps serving the tile while every visitor 404s on
open. Visibility is now a signal publication writes to, and the derivation
refuses to touch PUBLIC. Both branches are tested.
* The old dialog's PUBLIC branch showed only a URL, so the owner of a public
agent could not see, let alone revoke, who else held editor. Shares now
load for every visibility.
Also here:
* The dialog moves to `agents/components/`. It needs the listing service and
submit dialog from `agents/`, and all three callers already point
agents → assistants; leaving it put would have closed a cycle. It fetches
its own listing rather than taking one, so every caller gets the
marketplace section without threading it through, and the narrower data
type drops the `as unknown as` casts all three carried.
* The two "Search users" / "Add by email" tabs collapse into one field.
Asking someone to classify what they are about to type before typing it
bought nothing: names hit the directory, and anything that parses as an
address — including a comma-separated run — offers to add outright. Runs
are all-or-nothing, because a partial accept reads as "added" while the
dropped people never hear about it.
* The store icon moves to Persona, beside the emoji. Publishing is
owner-only but the icon is presentation, which D13 lets an editor set;
folding it into an owner-only dialog would have quietly revoked that.
* Dialog rebuilt on the canonical tokens (`add-curated-model-dialog`):
sectioned header/body/footer, `bg-gray-900/40` backdrop, and `max-h-[90vh]`
with a scrolling body — the old one had no height containment and ran off
a short viewport.
* Model selection is a stroke, not a `primary-50` fill. Dark mode steps to
`primary-300`: measured against the dark card the brand navy is a 1.45:1
stroke and `primary-400` only 2.24:1, both under the 3:1 floor for a
non-text indicator (WCAG 1.4.11). The fill had been hiding that.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(admin): show which roles carry delegated admin power
The roles list rendered JWT mappings and tool grants on each card but
nothing about admin scopes, so a system admin scanning the page could not
tell which roles carry admin power without opening each role's edit form.
That cuts against the invariant the feature rests on — granting a scope is
a `system_admin`-only act, and one worth being able to watch.
Adds an amber badge on the role title and an "Admin Access" cell to the
details grid, with labels resolved from the scope registry.
`system_admin` is special-cased: it holds every scope implicitly and so
carries an empty `grantedAdminScopes`. Rendering that as "no admin access"
would be exactly backwards, so it badges as "Full Admin" / "All Areas".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(rbac): durable audit trail for role mutations (PR-5)
`admin_service` has emitted structured log records on every role mutation
since before delegated admin scopes existed. Those are log lines: no
retention guarantee, no before/after values, and nothing an admin can read
from the console. With one superuser that was tolerable. Now that admin
power can be delegated, "which admin granted this, and when?" is a question
the platform has to be able to answer.
Adds `apis/shared/audit` (record, repository, service), an `audit-log`
DynamoDB table with a one-year TTL, a read-only `/admin/audit` API, and an
Audit Log console page under Identity & Access.
Three decisions worth review:
- **A record is a diff, not a snapshot.** `before`/`after` carry only the
fields a mutation changed. The diff is taken against a pre-mutation
deepcopy, because `update_role` writes onto the fetched role in place —
and against the object rather than the request body, because the role
form posts every field on every save.
- **Four of the eight emission points get no record of their own.** The
tool and skill grant helpers, and the write-through path the admin pages
use, all build an `AppRoleUpdate` and delegate to `update_role`. Emitting
from both would write two rows per mutation. `ROLE_UPDATED` already
carries the exact grant-list before/after.
- **Refused mutations are recorded.** `app_role.mutation_denied` fires when
the write-through guard raises. It is the only record where nothing
changed and the only place an attempted escalation is visible.
Reads are `system_admin`-only via a non-delegable `admin.audit` scope, and
the API exposes no mutating routes — records age out via TTL and no other
way.
An audit write never fails the mutation it describes, and a missing table
name means no sink rather than an error: the table ships in platform.yml
while this code ships in backend.yml, and backend deploys don't run CDK.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(admin): don't claim a month is empty when the read failed
The audit page rendered "Nothing recorded in <month>" underneath the
failure banner, which tells an admin the month is empty when we do not
actually know that. Gate the empty state on a successful read.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(routes): stop rebuilding the admin app on every Hypothesis example
`test_non_admin_roles_get_403` constructed a FastAPI app and called
`include_router(admin_router)` inside the example body — ~50ms on average
and occasionally over 100ms for 130 routes, run 100 times per test against
Hypothesis's default 200ms deadline.
The test passed with no margin and failed as DeadlineExceeded whenever the
full suite loaded the machine, which reads as an auth regression rather
than the timing artifact it is. Adding any router to the admin surface
makes it marginally likelier to trip; this change removes the sensitivity
rather than absorbing it.
Nothing about the property depends on a fresh app — the generated roles
reach the route only through the session dependency — so the app is built
once and the override reads the current user from a holder each example
rewrites. Same 100 examples, same assertion, same real dependency chain.
Per-example runtime: ~0-150ms → ~0-1ms.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(tools): RFC 8693 token exchange for per-user calls to existing APIs
Lets a tool call an API that trusts a DIFFERENT issuer than this platform, as the
signed-in user. The runtime trades the user's Cognito access token for one the
target already accepts, so an existing internal API serves agent requests with no
change on its side.
Opt-in and additive. Unconfigured, a deployment gets no new resources, no new IAM
permissions, no new environment variables, no required config, and no behaviour
change — verified by asserting a default synth contains zero occurrences of the
feature. A fork that never registers an external MCP server, or that wants SigV4
for all MCP traffic, is unaffected.
Why the runtime and not AgentCore Gateway
-----------------------------------------
Gateway cannot express this. Its outbound OAuth credential provider supports only
CLIENT_CREDENTIALS and AUTHORIZATION_CODE — `OAuthGrantType` in the
bedrock-agentcore-control API model has exactly those two values, and the string
"TOKEN_EXCHANGE" appears nowhere in the model (checked across three installed SDK
versions). A Gateway with AWS_IAM inbound auth also never receives the user's
token, so it would have nothing to exchange. Note this means the repo's
GatewayOAuthGrantType.TOKEN_EXCHANGE enum value and its _GRANT_TYPE_TO_AWS
mapping are dead — selecting it would fail API validation. Left in place and
filed as a follow-up rather than widening this diff.
Infrastructure (all conditional on config.tokenExchange)
- TokenExchangeConfig; optional on AppConfig so a fork constructing AppConfig
by hand need not know the feature exists
- TokenExchangeSecretConstruct — this deployment's confidential-client secret.
Separate from the token service's own copy because they are different AWS
accounts. CloudFormation seeds a random value; the agreed credential is
written over it out of band and survives later deploys
- Runtime env vars + a GetSecretValue grant, both spread in only when configured
Runtime
- integrations/token_exchange.py — exchange client with a per-(user, audience)
cache keyed so tokens can never be shared across users, TTL from expires_in
less a 30s skew, thread-locked because concurrent turns for one user would
otherwise race
- external_mcp_client.py — third per-user auth branch, ahead of forward_auth
because forwarding a raw Cognito token to such an API is a silent 401
- oauth_auth.py — OAuthBearerAuth.async_auth_flow. Required, not cosmetic:
token_provider was documented as called synchronously, so an async provider
would have put a coroutine object in the Authorization header. This is on the
shared path for all OAuth MCP servers; sync providers take identical logic
Admin surface
- token_exchange_audience on ToolDefinition and the admin request/response
models, plus the form field. Deliberately NOT a new MCPAuthType: that
dropdown describes how to reach the server, and its bearer-token option means
a static bearer, not per-user delegation
- Three per-user modes now exist and share one Authorization header, so
_validate_auth_config rejects more than one; the form also clears the others
so a valid form cannot produce a 400
Verified end-to-end against dev before this branch existed: exchange returns 200,
the issued token is signature-valid to the target API, GET /directory/me returns
the caller's own record (identity propagation, not just authentication), and a
role registered in the token service reaches a role-gated endpoint that
previously returned 403.
Known limitations, documented in code rather than discovered later:
- Registration-time discovery does not exchange. The exchange client lives in
agents/, which app_api may not import (enforced boundary), so discovery runs
unauthenticated. Fine for servers that do not gate tools/list; the form says
so. Fixing it means an exchange client in apis/shared
- Revocation does not propagate. Validation on the token service side is
offline, so a revoked-but-unexpired subject token is still exchangeable.
Exposure is bounded by the exchange's own lifetime cap
Tests: infrastructure 483 pass (incl. a default-off assertion and an opt-in one),
backend 5624 pass, frontend tsc clean. The admin/tools frontend suite has 21
pre-existing failures (Angular JIT/@angular/compiler setup) — confirmed identical
with these changes stashed, so that suite cannot currently guard this form.
* fix(tools): normalise token_exchange_audience so whitespace cannot silently
disable delegated identity
A pasted audience reached DynamoDB with a leading space:
' cc5aa8a0-90f7-427a-bf9d-60678a980215' (37 chars, not 36)
The token service compares the requested audience against its per-client
allowlist with an ordinal comparison, so it refused every exchange with
"audience is not permitted for this client".
The reason this needs fixing in code rather than just in the data: the tool still
appeared to work. The endpoint it called (GET /directory/search) allows anonymous
access, so the runtime's exchange failure meant the request simply went
unauthenticated and returned plausible results. Eight refusals in the token
service log; nothing wrong in the agent's answer. A silent downgrade from
delegated user identity to anonymous is the worst failure mode this feature has,
and one invisible space was enough to cause it.
Adds a before-validator on ToolDefinition.token_exchange_audience that strips
surrounding whitespace and maps blank to None. Blank -> None matters as much as
the strip: an empty string reads as "exchange configured" and would send an empty
audience, failing at the token service rather than leaving the feature off.
Tests cover leading/trailing/tab/newline whitespace, blank and whitespace-only
becoming None, absent staying None, a valid value untouched, and survival through
the DynamoDB round trip — normalisation has to hold in the stored form, since that
is what the runtime reads back.
Two related notes for whoever hits this next:
- Fixing the stored value alone was not enough in dev. The runtime caches MCP
clients keyed on the tool's updatedAt, and the token-provider closure captures
the audience at client construction, so a corrected record is ignored until the
version changes.
- No agent-visible Directory tool required authentication at the time, which is
why this hid. mcp-servers#24 adds directory_me for exactly that reason: it
cannot succeed without a real user token, so a broken exchange fails loudly.
Backend suite: 5635 passed.
* feat(docs-site): redesign landing page and add product roadmap
Emulates the OpenWork-style editorial layout (numbered sections, big
display headlines, app-window mockups, status-pill roadmap) while
keeping the site's existing frosted-glass Boise-blue identity.
- Landing page: hero, vendor-independence manifesto (anchored on the
"keep your harness separate from the model" argument), six numbered
feature sections grounded in shipped features, deploy terminal mock,
roadmap teaser, FAQ, and final CTA
- New /roadmap page: Live/Partial/Building/Next/Exploring status cards
reflecting actual project state across six themed sections
- ScreenshotPlaceholder component: window-chrome drop zones tagged
data-screenshot-placeholder, to be replaced with real captures
- Model support copy corrected to Strands Agents providers (not just
Bedrock), with a link to the Strands provider list
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(tools): enforce auth-mode validation on update, not just create
_validate_auth_config enforces two rules: at most one per-user auth mode
(forward_auth_token / requires_oauth_provider / token_exchange_audience — all
three compete for the single Authorization header), and MCP auth type 'none' when
a mode owns that header.
update_tool never considered token_exchange_audience: it was absent from
needs_existing, from the condition that triggers pre-validation, and from the
preview ToolDefinition passed to the validator. So creating a tool with an
audience was checked and editing one to add an audience was not.
Reachable through the admin UI. An edit could leave a tool with an audience plus
forward_auth_token, or an audience with MCP auth type aws-iam.
Not a credential leak, and worth being precise about why: at runtime the
exchange branch is evaluated before forward_auth, so the exchanged token wins and
the raw Cognito token is never sent; with aws-iam, bearer takes precedence over
SigV4 and the request reaches an IAM-expecting endpoint unsigned. Both outcomes
are a broken tool, not a disclosed secret.
The bug is the inconsistency. A validation rule that holds on create and not on
update is worse than no rule, because the UI implies the configuration was
checked. It is also the same failure shape as the whitespace bug this branch
already fixes: the tool ends up silently not working rather than erroring where
the mistake was made.
Six tests, and they were confirmed to fail without the fix — four of them do
(DID NOT RAISE), including the mirror case where forward_auth_token is added to a
tool that already has an audience, which shows the gap ran in both directions. The
two that pass either way are the positive cases: a valid audience update, and
clearing an audience. Clearing must never be blocked or a misconfigured tool
could not be repaired.
Backend suite: 5641 passed.
* feat(spa): host-gate the assistants explainer and answer the legacy site
The /assistants explainer said "nothing was lost — everything you built is
under Agents". That is only true for people who built on this version of the
site. Anyone who built an assistant on the previous boisestate.ai, now parked
at legacy.boisestate.ai, gets nothing back from that sentence: that generation
of assistant is not compatible with this one, none of it came across, and
re-creating it here is manual work.
So the hero now answers both, and every unqualified "everything you built is
here" below it is scoped to this version ("here", "on this site"). A
reassurance a reader cannot trust is worth less than no reassurance — someone
who follows "nothing to do" and finds an empty list stops believing the parts
of the page that are true for them.
The legacy half is deliberately two sentences in the hero rather than a section
of its own: where their assistants are, and that moving them is by hand. The
link reads as legacy.boisestate.ai and points at ?segment=my, so the sentence
stays plain while the click lands on their own list.
Also promotes the marketplace, since "put yours up" is the one thing this page
can ask for: a three-step section that names who acts at each stage — you
submit, an admin reviews, it goes live in Discover — with the published-snapshot
rule stated, so an author knows when it stops being theirs to change. Every
claim is asserted against the shipped flow (submit-listing-dialog,
LISTING_STATE_LABELS, PUBLISHED_VERSION_TOOLTIP), because the fastest way to
make a governed process feel arbitrary is to describe it inaccurately.
⚠️ TEMPORARY: the page and its sidenav entry are scoped to the production apex
(plus localhost, as a bench) via LEGACY_MIGRATION_HOST. Two enforcement points
that cannot disagree — legacyMigrationHostGuard restores the old silent
redirect onto /agents off-host, and the nav entry hides — because a nav entry
that hides while the URL still renders is not a scope, it is a hidden page.
This is scoping, not access control: the page is copy and two outbound links.
All of it comes out when the legacy site is retired.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(spa): drop "shelf" and "reviewer" from marketplace copy
"Shelf" is the marketplace metaphor talking. It only parses for someone already
picturing a store, and the reader who most needs these hints — a first-time
author deciding whether to publish — is the one who is not. Replaced with the
literal words the UI already uses: a category you choose, Discover where it
lands, a listing.
"Reviewer" becomes "admin" for the same reason in reverse: anonymous review
("a reviewer may move it", "note to the reviewer") makes an accountable process
sound like a committee behind a curtain, while "admin" is a role users here
already know exists. The submit dialog's own header always said "An admin
reviews this…", so the rest of the dialog now agrees with it.
Covers every user-visible instance across both audiences — the submit dialog's
Category/Tagline hints, its note field and publisher footnote, the withdrawal
confirmation in share-agent-dialog, the reachability warning shown to
reviewers, and the Categories, Publishers and Default pins admin pages.
Shelf survives in code — CategoryShelf, the shelves signal, visibleShelves and
the internal comments — because that is the domain model's name, not copy a
user reads. Renaming it would touch the store service, Discover and their specs
for no visible gain.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(marketplace): give feedback on an agent from the conversation
Reporting an Agent existed since Phase 8, but only on the store's detail
page — the wrong place to put it. The moment someone has something to say
about an Agent is the moment it just answered them, not a later trip back
to the tile they launched it from. This puts the same intake at the foot
of a conversation with a published Agent.
Deliberately the same record, endpoint and admin queue rather than a
parallel "feedback" object: a user does not know whether what they hit is
a defect or a missing capability, and a second intake form would only
mis-sort the ones who guess wrong.
Three additions to the existing report:
- `suggestion` reason, for the "it should also do X" that a conversation
produces and a store page does not. Sorts last in the severity sweep —
it is the one reason that is not a defect, so it must never displace a
complaint.
- Opt-in `sessionId`, so a curator can look up what actually happened.
Verified server-side against the caller before it is stored: the id
arrives in the request body, so without the check anyone could hand an
admin a pointer to somebody else's conversation and the queue would
present it as context the reporter chose to share. A session that does
not check out is dropped, not rejected — the feedback is the payload,
the attachment is context, and failing the submission to protect a
nicety loses the thing the user came to say. Amending with the box
unticked removes the stored reference; consent that cannot be taken
back is not consent.
- The admin queue renders the attached conversation as a reference, not a
link. There is no admin transcript reader yet, and a link that 404s for
everyone but the reporter would be worse than an id they can quote.
No new admin scope: `admin.marketplace` already governs reports, and the
route-coverage test enforces one scope per admin router.
Also collapses the duplicated `ReportReason` union — the admin model
restated it, and the two copies drifted the moment `suggestion` landed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(marketplace): let the feedback dialog scroll instead of clipping
The panel had no height cap, so on a short window it grew past the
viewport and simply clipped — the title went off the top and Send went
off the bottom, with nothing to scroll to reach either.
It got away with this while it was four reasons and no checkbox. Adding
the suggestion reason and the conversation opt-in pushed it over.
Adopts the pattern its three sibling dialogs in this folder already use:
capped panel, column flex, scrolling body, header and footer pinned.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(marketplace): close the feedback dialog on a backdrop click
The dialog had a backdrop with a click handler that could never fire. The
backdrop is painted underneath the panel's container, and that container
is itself inset-0 — so it covers the backdrop completely and swallows
every click aimed at it. Clicking outside the panel did nothing.
Moves dismissal onto the container that actually receives the click, and
guards it on mousedown as well: without that, selecting text in the note
and releasing outside the panel fires a click on the container (both ends
of the drag resolve to that common ancestor) and would throw away an
unsent report.
Verified in the browser — outside click closes, inside click and
drag-out-from-panel do not.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* style(marketplace): right-align and quiet the feedback link
Centred under the last answer, the link read as a call to action. It is a
footnote to the conversation, so it moves to the right and drops to
small type with no hover fill.
⚠️ The subtlety is size and weight, not contrast. The obvious way to play
it down is a lighter grey, but gray-400 on white is ~2.8:1 and fails AA
for body text — so the resting colour stays gray-500 (~4.8:1).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(spa): make backdrop-click dismiss work in every dialog
Every dialog in the app declared backdrop-click-to-dismiss and none of
them had it. The markup is two siblings — a backdrop, then a full-screen
centring container holding the panel — and the container is itself
inset-0, so it covers the backdrop completely and swallows every click
aimed at it. The (click) on the backdrop read perfectly and could never
fire. 25 dialogs, all of them, for as long as they have existed.
Extracts the fix from the feedback dialog into DialogDismissDirective and
puts it on the container that actually receives the click, in all of
them. The backdrop keeps its looks and loses its dead handler.
The mousedown guard travels with it, and matters more the more dialogs
have forms: selecting text inside a panel and releasing outside it
delivers a click to the container, because the browser dispatches to the
nearest common ancestor of press and release. Ungated, dragging to select
a word would throw away whatever the user had typed. So dismissal now
requires the press *and* the release to land on the container.
Out of scope, and not broken: image-lightbox, markdown-preview-modal and
the api-keys confirm use a single-element overlay where the backdrop IS
the container, so their handlers already fire.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* style(spa): match the Knowledge base section to its sibling cards
The Knowledge base section rendered as a grey inset (bg-gray-100/60, no
shadow) with a larger icon-less heading, while the Skills and Memory
spaces sections directly above it are white cards with an icon + text-sm/6
title. On the Edit Agent page the three sit in a row, so the odd one out
read as a different kind of surface rather than a peer.
Adopt the sibling tokens: bg-white + shadow-xs + border-gray-200/80 (and
the dark-mode equivalents), and an icon-led text-sm/6 heading using
heroBookOpen in amber to stay distinct from tools blue / skills purple /
memory emerald. The nested document lists keep their own borders, so they
stay delineated against the now-white surface.
Verified on the agent form: computed background, border and shadow are
identical to the Memory spaces card, and all three headings resolve to
14px/24px/600.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(marketplace): give the reachability warning a row it can be read in (#776)
Shipped in #773 inside the card's identity column, where it competed for
width with the agent name and the two action buttons. Measured on dev: the
sentence got **163px of a 528px card and wrapped across five lines**.
A warning the reviewer has to work to read is one they will skip, which
defeats the entire point of surfacing it — and this is the signal that stands
between "approved" and a shelf tile that 404s for everyone but its author.
Moved to its own full-width row beneath the decision row, separated by a hairline
rule. Same measurement after: **494px, two lines**.
Structural only — no wording, no logic, no gating change. Approve stays enabled,
and the existing specs (which assert on text, not layout) pass untouched.
Verified by applying the same restructure to the live dev DOM before committing,
against a real PRIVATE submission in the queue.
SPA: 149 files / 1656 tests passed.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(security): scrub user-controlled values out of log records
Closes the ten `py/log-injection` findings CodeQL raised against the 1.12.0
delta, plus three adjacent findings that turned out to be real defects rather
than dead code.
## Log injection
`scrub_log` already exists (`apis/shared/security/log_sanitize.py`) and is used
in six files; the marketplace, audit and role-pin code shipped without adopting
it. Path parameters (`agent_id`, `role_id`, `target_id`, `report_id`,
`actor_user_id`) and exception text now go through it before reaching a log
message. A percent-encoded `%0A` in a path segment survives URL decoding, and
most of these sites sit in `except` blocks where the value was never validated,
so a forged log line was reachable.
Scope note: only the message f-strings are scrubbed, not the `extra={...}`
structured fields CodeQL also flags. `logging.basicConfig` formats records as
`%(asctime)s - %(name)s - %(levelname)s - %(message)s`, so `extra` is never
rendered — scrubbing it would corrupt the stored value for any future handler
that does read it, and buy nothing against injection.
`tool_filter.py` had the flagged warning twice, in two near-identical methods;
CodeQL reported one. Both are fixed — leaving the other would be arbitrary.
## Missing icon registration (found via "unused import")
`pinned.page.ts` imported `provideIcons` and `heroBookmark`, called neither, and
its empty state renders `<ng-icon name="heroBookmark" />`. ng-icons resolves that
name from a registration, and the SPA registers per-component — there is no
root-level `provideIcons` in `app.config.ts` or `main.ts`, and its sibling
`discover.page.ts` registers its own icons the same way. The glyph therefore had
nothing to resolve to. The fix adds the registration rather than deleting the
imports, which is what the finding literally suggested.
## Documented rule vs enforced rule
`_ROLE_GATED_KINDS` was flagged as an unused global. The rule it describes *was*
enforced, but by a literal `(("tool","tools"),("skill","skills"))` re-stated at
the loop — two objects that could drift, which is exactly what "unused constant"
looks like from outside. The constant now carries the pairs and the loop reads
it.
## Genuinely unused imports
`computed` (listings.page.ts), `beforeEach` (admin-scope-wiring.spec.ts), and
`parseIso` (sync-policy-control.component.ts) — the last shadowed by a private
method of the same name. The local method is a broader regex variant of the
shared util and predates it; consolidating the two is left alone deliberately.
## Not fixed
The four `Unnecessary lambda` findings (`default_factory=lambda: utc_now_iso()`),
six `assert`-with-side-effect findings in `test_agent_icons.py` (only observable
under `python -O`, which tests never run with), and one superfluous-arguments
finding in a spec. All cosmetic.
## Tests
New `tests/security/test_log_sanitize.py` — 16 tests. The helper is now
load-bearing at ten more call sites and had no coverage: terminator escaping,
control-character stripping, non-string inputs, unicode preservation, and
idempotence.
Backend 5713 passed / 3 skipped. SPA 162 files / 1791 tests passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(spa): stop the Memory Spaces kill switch from announcing itself
While MEMORY_SPACES_ENABLED is off the backend 404s the whole /memory/spaces
surface on purpose, so the feature can be hidden without being removed.
MemorySpaceService already reads that 404 as "feature unavailable", clears its
state and drops the nav entry.
But error.interceptor toasts every non-401 unless the request opts out, and
MemorySpaceApiService never did — so the surface hid itself and then popped a
dialog naming an endpoint the user is not meant to know about. Seen in
production once the dark-stop finally deployed.
Set SUPPRESS_ERROR_TOAST on every Memory Spaces request, matching the shared
per-request options idiom in FileSourceService. Suppression costs nothing for
genuine failures: MemorySpaceService translates every non-404 into its own
error$ signal, which the pages already render inline and in context.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(assistants): degrade user-facing GSI reads to empty when the index is missing
The agent store browse (GSI5 / AgentDirectoryIndex) and the admin problem-report
queue (GSI6 / AgentReportsIndex) let botocore's missing-index error propagate, so
an absent index surfaced to users as a 500.
An absent index is a legitimate transient deploy state, not a programming error:
CloudFormation reports success while a GSI is still CREATING, deploys can roll
back, and platform.yml and backend.yml are separate workflows that can land out
of order. All three converged on 2026-08-01 — release 1.12.0's CloudFormation
deploy rolled back on an unrelated DynamoDB limit while the backend and frontend
shipped anyway, and the agent store returned 500 to every user for the two
releases it took to repair the infrastructure. That was the release that opened
the store's navigation gate to GA.
Both reads now catch the missing-index case specifically, log it at WARNING with
the index name, and return an empty result with no cursor.
The match is narrow on purpose. Catching every ValidationException would hide
malformed key conditions and bad cursors behind a permanently empty surface;
catching every ClientError would turn throttling into "there is nothing here".
Both the error code and the message shape must line up. Two codes are accepted
because the implementations disagree: real DynamoDB raises ValidationException,
moto raises ResourceNotFoundException.
Audit of the other GSIs on this table:
- SharedWithIndex and OwnerStatusIndex (assistants service) already returned
empty on any ClientError, so both already degraded. SharedWithIndex's
"not deployed yet" branch matched ResourceNotFoundException, which real
DynamoDB never raises for a missing index — so it only ever fired under moto
and production logged at ERROR instead. Both now route through the shared
helper for a log that names the index. No behaviour change.
- DueSyncIndex (KB sync dispatcher) is deliberately left loud: a background sweep
that silently returns nothing means scheduled syncs stop with only a warning,
where a raised error is visible as a Lambda failure.
- VisibilityStatusIndex has no read path — keys are written, nothing queries it.
Writes and admin mutations are untouched and still fail loudly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(release): pre-merge guard for DynamoDB's one-GSI-per-UpdateTable limit
Release 1.12.0 added two GSIs to the existing `{prefix}-rag-assistants` table
in a single CloudFormation update. DynamoDB's UpdateTable API permits exactly
one GSI creation or deletion per call, so the update failed and CloudFormation
rolled back every other resource in the deploy with it — including a brand-new
audit-log table. backend.yml and frontend-deploy.yml succeeded independently,
leaving production running new code against old infrastructure with the agent
store returning 500. Recovery took two more patch releases (1.12.1, 1.12.2).
The limit applies to UpdateTable only — CreateTable accepts any number of
indexes, which is why the new audit-log table's two GSIs were never a problem.
The right question is therefore "does any EXISTING table gain more than one
GSI?", not "are there new indexes?".
Dev could not have caught this. The two indexes reached develop in separate
merges (02d0f2e9, a128831d), so dev got a platform deploy for each and never
saw them collapse into one update. Only an environment that jumps a whole
release at once is exposed.
- SKILL.md: new §1 prerequisite covering the limit, why an incremental
environment cannot reveal it, how to split the release, and the recovery
mechanics — each half needs its own patch release because version-check.yml
has no skip path, and PR 2 must NOT be stacked on PR 1 (after a squash merge
the merge-base does not move, the two commits cancel in the three-dot diff,
and the restore silently no-ops). Sections renumbered 1-7 -> 2-8.
- infrastructure/gsi-in…
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if await bump_last_used_at(input_data.rag_assistant_id): | ||
| await resume_inactive_policies(input_data.rag_assistant_id) | ||
| except Exception as bump_err: | ||
| logger.warning(f"lastUsedAt bump failed for assistant {input_data.rag_assistant_id}: {bump_err}") |
|
|
||
| from apis.shared.assistants.listing import is_on_shelf | ||
|
|
||
| logger = logging.getLogger(__name__) |
| rather than reporting one. What an adapter *must* guarantee is that its | ||
| ``relevance`` values agree with the order it returns. | ||
| """ | ||
| ... |
|
|
||
| async def ingest(self, kb_ref: str, source: DocumentSource) -> None: | ||
| """Index ``source`` into the knowledge base.""" | ||
| ... |
|
|
||
| async def delete_document(self, kb_ref: str, document_id: str) -> None: | ||
| """Remove every trace of ``document_id`` from the knowledge base.""" | ||
| ... |
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
4 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.
No description provided.