Release/1.18.0 - #983
Merged
Merged
Conversation
…ions-probe docs: run the G3 citations probe — the premise was wrong
…lent-drop fix(mcp): recover OAuth-gated MCP tools instead of dropping them silently
…achment-card feat(files): slide-deck preview card for .pptx, and stop diverted attachments vanishing on reload
…sume-typeerror fix(streaming): abandon a stale pause instead of bricking the next turn
…26-08-14 chore(kaizen): weekly review prep 2026-08-14
`_recover_oauth_preflight` asked the AgentCore vault on *any* pre-flight failure for an OAuth-gated tool with a cold token cache. A server that is simply unreachable therefore looked identical to one refusing an unauthorized caller: the vault correctly answered "this user has no token, here is an authorization URL", the turn emitted a pre-flight `oauth_required`, and the user was shown a Connect prompt on every turn that completing consent could never satisfy. Seen in dev while verifying #872: `canvas_faculty` points at `http://localhost:8026/mcp`, unreachable from the AgentCore Runtime container, so its pre-flight fails with an httpx ConnectError rather than a 401. The same path runs in prod for any OAuth-gated MCP server during an outage. #872 guarded the vault-call-failed case ("couldn't ask" is not "must consent") but not the server-unreachable case. Classify the exception before consulting the vault: only a 401/403 counts as a possible consent gap. Connect, DNS, timeout, and 5xx failures keep the pre-recovery silent-skip behaviour. The status is not on the exception we catch. `MCPClient.load_tools()` raises `ToolProviderException` wrapping `MCPClientInitializationError` wrapping an anyio `ExceptionGroup`; the real `httpx.HTTPStatusError` is three levels down and the outermost message carries no status at all. So `_is_auth_failure` walks `__cause__`/`__context__`/`ExceptionGroup` members — the same shape `mcp_apps._is_transient_connect_error` already had to handle — and reads `.response.status_code`, with a deliberately narrow text fallback for servers that report the refusal as protocol text. Gating the whole recovery rather than just the consent-recording is safe: on a transport error the vault's token would only feed a retry that fails the same way, so the consented-user path loses nothing. Tests build the real wrapped exception chain. An existing test that used `RuntimeError("connection refused")` to reach the vault-unreachable branch now uses a 401 — under the new gate it would never have reached the vault and would have passed vacuously. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…eflight-classify fix(mcp): only treat an auth failure as an OAuth consent gap
…nto-develop-1.15.0 Backmerge main into develop (1.15.0)
…roken image Image tiles are `loading="lazy"` and their `src` is a presigned S3 GET URL minted once, when the message first renders, with a 10-minute lifetime. In a long turn the attachment scrolls out of view before the browser ever fetches it, so the lazy load fires against an already-expired signature. Nothing listened for the `<img>` error event, so the failure fell through to the browser's broken-image glyph with the filename as alt text — the component's own "Preview unavailable" state was unreachable, because from its point of view the URL fetch had succeeded. `expiresAt` was already on every one of these API responses and was not read anywhere in the SPA; no code tracked or refreshed an expiring URL. Handle the load failure at all three sites that render a presigned image: - image-attachment-group: `(error)` re-mints once and only then falls back to the error tile. `(load)` restores the retry budget so a proven URL that later expires gets its own retry, while a genuinely dead object cannot loop. Opening the lightbox refreshes a URL within 30s of expiry up front, since the lightbox has no error state of its own. - image-lightbox: new `imageError` output so the owner re-mints, plus a guard against `<img src="">` when arrowing to an image past the four visible tiles that has not loaded yet (pre-existing). - file-attachment-badge: same one-shot re-mint for PDF page-1 thumbnails, falling back to the skeleton rather than a broken image inside the card. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`PATCH /api/admin/roles/faculty` returned 400 in prod when an admin set the
JWT Role Mappings to `Faculty, PSEmeriti Entra Sync`. The second entry failed
`_JWT_MAPPING_PATTERN` (`^[A-Za-z0-9_-]{2,64}$`), and because one bad entry
rejects the whole payload, even the untouched `Faculty` mapping did not save.
The pattern's premise -- that real IdP groups never contain spaces -- is false
for Entra security groups named as display names, and Boise State does not
control those names.
Widen the pattern to `^[A-Za-z0-9_-]+(?: [A-Za-z0-9_-]+)*$`: single internal
ASCII spaces accepted, 2-64 characters, everything else unchanged. The
alternation is what keeps leading, trailing, and doubled spaces out.
Still rejected, each for a verified reason:
- Commas. The `custom:roles` claim is split on `,` in
`cognito_jwt_validator.py:127` and `bff/token_exchange.py:86`, and the admin
form field is comma-separated, so a comma-bearing name is unrepresentable.
- Edge whitespace. Both claim parsers `.strip()` every entry, so a padded
mapping could never match an incoming claim -- it would look granted and
grant nothing. Rejected rather than silently trimmed: the stored value has
to match the claim byte for byte, so normalizing behind the admin's back is
the more dangerous option.
- Non-space whitespace and invisible characters -- tab, newline, NBSP
(U+00A0), zero-width space (U+200B). JS `trim()` does not strip U+200B, so
a name pasted out of Entra or Teams can carry one into the payload.
Also replace the bare `"Invalid role configuration."` -- the message that
turned this into a CloudWatch expedition -- with one that names the offending
entry and says what is wrong with it, mirroring `validate_admin_scopes` in the
same file. Invisible characters are escaped to a visible `<U+XXXX>` token
(echoing them raw renders identically to a correct value), and the echoed
string is length-bounded and charset-escaped before it reaches the 400 body or
the log line.
Widening the charset re-opened the hole `_FORBIDDEN_PROTECTED_MAPPINGS` exists
to close: `"All Users"` and `"Authenticated Users"` are real Entra/AD display
names that previously could not be typed at all. Compare against the forbidden
set after folding case and treating space/hyphen/underscore as one separator.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-mappings-allow-spaces fix(rbac): allow spaces in JWT role mappings and name the failing entry
…review-url-expiry fix(attachments): re-mint expired preview URLs instead of showing a broken image
The submit dialog asked the author to tick "Make this agent public" and held Submit until they did. The reasoning was that widening access must be consented to, and that reasoning holds — but the control did not serve it. The store is one public shelf, so there is no submission that leaves an agent private: the box had exactly one valid answer, and a required control with one valid answer is ceremony that trains people to click past it rather than consent. Replaced with a disclosure above the form — "Submitting makes this agent public", worded for what it changes *from* (private vs shared) and saying that the widening happens now rather than at approval, which is the part an author would otherwise be surprised by. Pressing Submit under that notice is the consent. The backend gate is deliberately unchanged: `_visibility_block` still refuses a submission whose request omits `makePublic`, so a direct API caller — who sees no notice — cannot widen an agent by accident. The dialog is where the notice is shown, so the dialog is where the flag is now always set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A reviewer could name a submission but not see one. Three individually
correct access rules produced that: `instructions` is gated to
owner/editor on `GET /agents/{id}`; that read refuses a non-owner
outright on a PRIVATE agent, and a PRIVATE agent can sit in the queue;
and the review diff — the only other window onto the content — is empty
by construction on a first submission. So on the most consequential
review in the system, the admin saw a name, an author and a category.
Three additions.
**Read it.** `GET /admin/agents/{id}/submission` returns instructions,
bound capabilities by name, model, starters, publisher and reachability,
rendered by a new page at `/admin/marketplace/review/:agentId`. It serves
the frozen snapshot, never the live record: the live record is the
author's draft and they can keep editing while the row sits in the queue,
while approval promotes `submittedVersion`. Reading anything else shows
an admin one configuration and publishes another — the window
`AgentVersion` exists to close. Deliberately a separate endpoint rather
than a widened `GET /agents/{id}`, which is the store's detail read *and*
the Designer's form loader, and would serve the wrong version anyway.
**Test-drive it.** `review_preview` on the invocation payload resolves the
reviewed snapshot and bypasses the PRIVATE check, after re-resolving
`admin.marketplace` against the caller's own roles — a caller without it
is refused rather than quietly downgraded, since a silent downgrade would
run the published snapshot and report it as the version under review. The
which-version rule lives in `version_resolution.resolve_review_agent` so
the page and the test drive cannot disagree; inference-api cannot import
from app_api, and two copies of that rule is the same class of bug one
level up. Runs on a `preview-` session and skips the path's bookkeeping
writes — a reviewer poking a submission is not use.
**Decline it.** A third decision, not a harsher request-changes: an admin
who judged a submission not a fit had to publish it or say "fix this",
promising a review they did not intend to give. `rejected` is admin-only
from `in_review`, carries a required reason, and still lets the author
revise and resubmit. It is never on the shelf and has no edge to
`published`, so approval remains the only door into the store;
`rejected -> private` exists so a declined author can still delete their
own agent.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…iscloses-going-public feat(marketplace): submitting an agent makes it public, with a notice
…ace-admin-review feat(marketplace): let admins read, test-drive and decline a submission
The panel was sized by its grid cell, so its height came from whatever the prose in the left column happened to need. On a short submission that left about 120px between the warning banner and the composer — too small a window to judge an agent through, which is the one thing the panel exists for. Size it against the viewport instead, and make it sticky so it stays put while the reviewer scrolls the instructions: reading and asking are one loop, not two passes. Add an expand control that spans it across both columns for a longer session. Expanding is a class change only — the element never moves in the DOM, because remounting would destroy the component and take the reviewer's conversation with it. Also: an empty summary or system prompt now says so instead of rendering a blank box. A blank panel under a heading reads as a page that failed to load, and an agent that genuinely ships no instructions is itself a reason to decline — the two must not look alike. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…review-test-drive-size fix(marketplace): make the review test drive big enough to use
…ade UX live in dev (#884) * docs(kb): §13 benchmark outcome + production baseline measurements Records the §13 benchmark results and adds a production measurement pass that replaces the earlier hypothetical cost and volume figures. Benchmark writeup (§5.1, §11, §12, §13.5, §13.6, §14.0): - revised latency and ingest figures from the 9-question x 3-backend harness - gate outcome (CLEARED, 4 of 5 conditions), context-cap experiment, and the gaps AWS has since closed - unrelated prod/dev bugs and debris surfaced while benchmarking Production baseline, prod 897729136999 (§3.1, §3.2, §3.3, §6.4, §7.4, §13.5): - 3,886 retrievals/30d, ~26% RAG attach rate, $6.54/mo like-for-like against ~$0.09/mo today; scaling table out to 30,000 users - reconciles the expected-value trajectory ($169/mo) against §13.5's policy ceiling ($150,000/mo) so the former cannot be used to argue against the cap - $5.00/GB verified to meter raw source bytes, not post-parse text - adds the missing agentic-retrieval and Gateway line items to §3.1 - no seasonality baseline exists: all six recorded months are adoption ramp, with no summer trough (Aug pacing 1.7x Jul) - quota audit vs measured scale; query input is capped at 10,000 chars and is not adjustable, the only finding that hard-fails rather than costing money - agentic RPM is 60 per *account*, not per KB - document status distribution: 200 of 1,692 non-complete; the status filter is correct, but fails open on DynamoDB error and so can serve chunks from deleted documents - new §13.5 requirement 4: clamp query length before Retrieve (gate count 3->4) §6.5 proposes agentic retrieval as a user-triggered per-answer escalation, which keeps the 60 RPM account quota non-binding below ~30% escalation. * docs(kb): clarify attachment/KB composition; de-duplicate fail-open defect §6.3 — attachments and knowledge bases compose in one turn: - states explicitly that keeping attachments out of Managed KB does not exclude using both together; the inline document block and retrieved chunks already reach the model in the same prompt (prompt_builder, augment_prompt_with_context, merged at inference_api/chat/routes.py:2234) - adds the privacy argument for the decision, which previously rested only on latency and whole-document reasoning: an attachment ingested into a shared agent's KB becomes retrievable by every other user of that agent - flags that the 2,000-char context cap is the real constraint for this shape of task, and that §13.6's "the cap costs nothing" result covers single-fact lookups only; nominates compare-and-contrast as the question set for the multi-chunk synthesis experiment §13.6 says is missing - notes retrieval returns fragments, never a whole exemplar, with two unproven leads (GetDocumentContent; agentic retrieval, which is not cap-bound) §7.4 / §11.1 / §14.4 — the fail-open document-status filter was described three times, in three levels of detail: - §7.4 is now canonical and marked as such - §11.1 reduced to a one-line comparison against managed ACL behaviour, keeping its own distinct points (ACL awareness is not authorization; email-only identity) - §14.4 keeps the requirement and references §7.4 for the evidence - adds a scope note that was previously only implicit: the inner per-document handler already fails closed, so only the outer table-level handler fails open. The exposure window is table unavailability, not ordinary throttling — low probability, privacy-grade consequence, one-line fix. Docs only. No code or infrastructure change. * feat(kb): managed knowledge base migration — spec, schema and worker platform Introduces Amazon Bedrock Managed Knowledge Base as a second retrieval backend behind an abstraction seam. This lands the spec plus task groups 1 and 2: the additive schema, the IAM, and the worker resources. All of it is dormant. Scope is evaluation §14.7 phases 1-4 (additive schema/IAM, dark dual backends, opt-in dual-read pilot, opt-in migration with a rollback window). Phases 5-8 are a follow-up spec. Group 1 — additive schema and IAM * KbWorkIndex GSI (GSI7_PK/GSI7_SK) on the assistants table, sparse: keys exist only while a knowledge base is eligible for background work, so an ineligible or pinned KB is invisible to the dispatcher by physics rather than by filter. * GSI7_* added to the generic assistant-update immutable-field guard. * Bedrock KB service role with confused-deputy conditions (aws:SourceAccount + ArnLike AWS:SourceArn on knowledge-base/*), S3 read conditioned on aws:ResourceAccount, and bedrock:InvokeModel pinned to amazon.titan-embed-text-v2:0 only. * Caller grants split by need: provisioning CRUD, direct ingestion, and inference Retrieve, each under its own SID; iam:PassRole scoped to the service role ARN and conditioned on iam:PassedToService. Group 2 — worker resources and config * Dispatcher, worker, reconciler and ingestion consumer as four Lambdas sharing ONE Docker image, with a byte-stable bootstrap stub per the platform-as-bootstrap pattern. The real image ships via the workflow later. * Ingestion consumer at 900s with a dead-letter queue; a 50 KiB PDF was measured at 264s, so anything under 300s would turn a slow success into a retry storm. * Three independent opt-in flags, all defaulting OFF: managed-by-default, migration enabled, and reconciler armed. An unset GitHub Actions variable renders as an empty string, which parseBooleanEnv maps to false. * Per-owner byte caps and four fleet-level alarms, all NOT_BREACHING on missing data. Per-owner caps bound one user; the alarms bound the account. DEPLOY ORDER CONSTRAINT: this consumes the entire rag-assistants GSI budget for whichever release ships it. DynamoDB permits exactly one GSI create/delete per UpdateTable and CloudFormation issues one per changed table, so a release adding a second index to that table fails and rolls back — the 1.12.0 outage shape. Any other in-flight change adding a GSI to rag-assistants must ship separately. Two defects in the spec were found and corrected while implementing it: * PutMetricData was specified against AWS/Bedrock/KnowledgeBases. CloudWatch reserves every namespace beginning with "AWS" and rejects writes to them, so the grant would have deployed cleanly and published nothing. Custom metrics now target ${projectPrefix}/ManagedKb; Bedrock's own namespace is a read source. * The same grant was specified on the Bedrock service role, which Bedrock assumes and which never publishes our metrics. Removed as a dead grant. Zero behaviour change: 271 -> 307 resources with none removed, no IAM statement lost, no existing logical ID renamed, and the documents-bucket notification preserved byte-for-byte. Verified by baseline-vs-current synth. Tests: 611 infrastructure tests green. Security-relevant assertions are mutation-verified — each was confirmed to fail with its guard removed, which caught three tests that had been passing vacuously. * feat(kb): KB_Record data layer with conditional state transitions Task group 3 of the managed knowledge base migration. Adds the control-plane record for a knowledge base and the six state transitions the migration worker drives, in a new apis.shared.kb_backend package. Nothing calls it yet. Three invariants are enforced here rather than left to callers, because each one fails silently when violated: * Absence means legacy. `retrievalEngine` is only ever written as "managed"; nothing writes "s3vectors" onto a record that lacked it. That is what makes the migration zero-backfill — 1,692 existing records are already correct by having no opinion — and what makes rollback a REMOVE rather than a rewrite. * Every transition is conditional. These run from a dispatcher that fans out to concurrent workers, so a read-then-write would let two workers both believe they won. Losing a race surfaces as TransitionLost, distinct from a real error. * Sparse work keys are REMOVED on reaching a terminal state, not merely ignored. A stale GSI7 key hands the dispatcher a knowledge base nobody asked to migrate, and the dispatcher creates and deletes billed AWS resources. The package talks to DynamoDB through the raw table resource instead of importing apis.shared.assistants, whose __init__ pulls the embeddings stack in at module scope and would blow the migration Lambda's image budget — the same reason kb_sync/records.py is written this way. Module-level imports are stdlib only and boto3 is function-local. Verified empirically: importing records.py in a fresh interpreter loads no assistants or embeddings module, and does not load boto3 at all. The formal architecture test lands with task 4.8. Bug found by the tests: `total` is a DynamoDB reserved keyword, so the converged catch-up guard comparing migrationProgress.migrated to .total was rejected outright. Now aliased through ExpressionAttributeNames. It failed loudly rather than silently, but it would have broken every promotion. Tests: 26 transition tests against moto plus 9 hypothesis property tests, all mutation-verified — each of the 10 guards was removed in turn and confirmed to break a specific test. Note on that verification, because it nearly went wrong: the first mutation round reported all 10 guards caught, but four were false passes. Removing a ConditionExpression orphans the expression values it referenced, DynamoDB rejects a request carrying unused values, and the resulting ValidationException turned the HAPPY-PATH test red instead of the guard test. Re-running those four with the orphaned references stripped — so the write genuinely succeeds without its guard — put the failure on the correct tests. "A test failed" is not "the mutation was caught"; which test failed is the evidence. * refactor(kb): backend abstraction seam behind the retrieval entry point Task group 4. Puts a protocol between the retrieval entry point and the vector store so a second backend can be added later without the callers knowing. No behaviour change: same results, same ordering, same response keys, same failure modes. * protocol.py — KnowledgeBaseBackend Protocol plus a frozen Chunk whose score field is `relevance`, higher-is-better. * resolver.py — reads retrievalEngine from the KB_Record; absence resolves to the legacy backend, reusing records.resolve_engine rather than reimplementing it. * s3vectors_backend.py — the existing S3 Vectors path, moved without functional change, converting distance to relevance inside the adapter. * rag_service.py — now a facade that resolves then delegates. It also owns the parity rules (top_k, the 2,000-character cap, the document-status filter, the citation clip) so one implementation covers every backend. SCORE DIRECTION is the highest-risk detail in this feature. S3 Vectors returns cosine distance, lower-is-better; the seam speaks relevance, higher-is-better. Invert it and nothing raises: retrieval still returns five chunks, every request still succeeds, and the answers just quietly get worse. Conversion is by exact negation, done once inside the adapter, and is order-preserving and losslessly reversible — unlike 1-d or 1/(1+d). The facade still emits a `distance` key derived from relevance, because app_api/assistants/routes.py puts that value in an HTTP response body a client already reads. The public contract is unchanged. The facade's runtime signature is still (assistant_id, query, top_k=5) and it still returns text/distance/metadata/key. Both call sites — inference_api/chat/routes.py and app_api/assistants/routes.py — are byte-identical. The document-status filter's two fail-open paths are PRESERVED deliberately: the branch taken when DYNAMODB_ASSISTANTS_TABLE_NAME is unset, and the outer except. Inverting them to fail closed is Requirement 5, task group 6. Doing it here would bury a behaviour change inside a refactor. The previously-untested second path now has a test pinning current behaviour, so that inversion will be a deliberate, visible edit rather than a silent one. Tests: 6032 backend tests pass, up 34. Mutation-verified, including the two that matter most — inverting the score conversion fails four tests (ranking, agreement between backends, direction, and round-trip exactness), and adding an apis.shared.assistants import to a seam module fails the new architecture test. Each mutation was confirmed to have actually landed via diff before its result was trusted; two earlier attempts silently missed their anchor and would otherwise have been recorded as passes. * feat(kb): clamp retrieval queries and fail closed on unconfirmable status Task groups 5 and 6. Group 6 is a deliberate BEHAVIOUR CHANGE to live retrieval; group 5 is a new guard that also changes behaviour for pathologically long queries. Both are called out below. Group 5 — query clamp Managed Knowledge Base caps `Retrieve` input at 10,000 characters and the quota is not adjustable, so an over-long query is a rejected request rather than a worse answer. The clamp is applied in the facade, before dispatch, so both backends receive an identically-shaped query — clamping only the managed path would mean the two backends answered different questions whenever a query ran long, which would make a dual-read rank disagreement indistinguishable from a genuine retrieval difference. This does clamp the legacy path, which was previously unclamped: Titan v2 tolerates roughly 32,000 characters, so queries between the two ceilings used to be embedded whole. That is the intended trade — parity is worth more than the tail of a pathological query — and it is why truncation emits KbQueryClamped rather than passing silently. The clamp never raises; a long query is a fixable input, not a failed chat turn. Also removed two stale comments in bedrock_embeddings.py asserting that search queries need no length validation. True only for Titan; not true for every backend. Group 6 — fail-closed document status filter Both table-level fallbacks in _filter_vectors_by_document_status now DROP chunks instead of returning them unfiltered: the branch taken when DYNAMODB_ASSISTANTS_TABLE_NAME is unset, and the outer exception handler. The per-document handler already failed closed and is untouched — one unreadable document should cost that document, not the whole result. This supersedes reliable-document-deletion Requirement 3.4, which specified the fail-open deliberately. That requirement is annotated as superseded rather than silently contradicted. What changed is evidence, not taste: 936 retrievals in a trailing 30-day window had chunks removed by this filter, so the documents it guards are real. A lookup failure would therefore have served users content they believe they deleted, with nothing in the response indicating the check was skipped. Failures now log at ERROR and emit KbStatusFilterFailClosed, so an empty result from this path is distinguishable from "the corpus had no match". Tests: 6054 backend tests pass, up 22. Mutation-verified, and three mutations initially SURVIVED a suite that looked thorough — worth recording because each was a distinct blind spot: * Raising the cap to 32,000 went unnoticed, because every test referenced MAX_QUERY_CHARS symbolically and so followed the constant wherever it moved. Now pinned to the literal 10,000, which is a property of AWS rather than a knob. * Removing the clamp call from the facade went unnoticed: the unit properties all still passed while the clamp became dead code. Now asserted by inspecting what the backend actually received. * Pointing the metric namespace at a reserved AWS/ one went unnoticed. That is the same silently-inert-grant bug already fixed in the CDK layer; this is the backend half of the assertion. Also fixed a latent flake in the new property test: it imported the module under test inside a patched context, so the first run in a session behaved differently from every later one — failing cold and passing warm, which is the worst failure mode for a guard given CI is always cold. * feat(kb): per-owner byte cap with atomic reserve / commit / release Task group 7. Managed storage is billed at $5.00/GB-month, roughly 35x what S3 Vectors costs today. At the measured 1.13 MB average per user that is about $169/month across the fleet, but nothing structural stopped one user uploading far more — 30,000 users at 100 MB each would be 3 TB, or ~$15,000/month. This is what makes that impossible rather than merely unlikely. The obvious implementation does not work. DynamoDB condition expressions compare operands; they cannot do arithmetic, so ConditionExpression="storedBytes + reservedBytes + :n <= :cap" is rejected outright — verified: "Cannot parse condition starting at:+ reserved <= :cap". The arithmetic therefore moves to the client, where it is free: a single `totalBytes` accumulator is compared against a literal computed before the call (`cap - n`). One atomic conditional ADD, so N concurrent reservations cannot collectively overshoot. The read-compute-write alternative has a window between the read and the write, which is exactly the race a cap exists to close. Reserve / commit / release rather than a plain add, because ingestion is not instantaneous — a 50 KiB PDF measured 68-264s. Counting only on success would let a user start many concurrent uploads that are each under the cap and collectively far over it. A crash between reserve and commit leaks a reservation, which is the safe direction: it under-permits rather than over-permits. Size always comes from an S3 HEAD, never a client-reported value. Bedrock's RawDataSize is deliberately not used for enforcement: it returned 0 datapoints for a directly-ingested document during evaluation, and enforcing against a metric that is sometimes absent would fail open. Requirement 12.10 (who consumes retrieval quota) is now decided and recorded in the design. Half the answer is a fact about AWS: Managed KB's Retrieve quota is 600/min PER KNOWLEDGE BASE, where S3 Vectors is 20 rps ACCOUNT-WIDE. So the owner is the payer — and that is an improvement, not a compromise. Today one hot assistant can exhaust an account-wide ceiling and degrade retrieval for everyone; per-KB quotas make the blast radius one agent. The case to watch is a published agent, the only realistic way to approach 600/min, currently running ~26/min on a hot shared knowledge base. Infra: app-api now receives the byte cap values and the metric namespace. It enforces the cap on interactive upload, and without these it would have fallen back to module defaults and silently ignored an operator's configured limits. The namespace comes from the same helper that builds the IAM condition, so the grant and the publish cannot drift. Wiring the whole-snapshot reservation into the Migration_Worker is deferred to task group 13, where that worker is built; `reserve_snapshot` and its tests land here so the enforcement point exists before the caller does. Tests: 6073 backend tests pass, up 19. All 11 mutations verified caught, including removing the cap guard entirely, an off-by-one against the wrong bound, double-counting on commit, leaking allowance on release, and trusting a caller-supplied size. One mutation initially reported as caught was rejected on inspection: it had produced a syntax error rather than a semantic detection, so it was redone with the request left valid. * feat(kb): managed knowledge base provisioning, retrieval and direct ingestion Task group 8 — the first code that talks to Bedrock Managed Knowledge Base. Still unreachable at runtime: nothing resolves to this backend until the flags are on. Several request shapes here CONTRADICT the AWS documentation and were established by direct probing. Each is asserted by a test so nobody "corrects" them back: * CreateKnowledgeBase omits storageConfiguration entirely — there is no vector store to provision, and sending it is rejected. * The data source nests MANAGED_KNOWLEDGE_BASE_CONNECTOR with the real type inside connectorParameters. Top-level CUSTOM/S3/WEB are all rejected for a MANAGED KB. * Retrieval uses managedSearchConfiguration. vectorSearchConfiguration is rejected outright. Hybrid search is not toggleable, so nothing tries. * clientToken has a 33-character MINIMUM, and the natural "{id}-{variant}-kb" token is 31 — it fails client-side, before any request is sent. Tokens are built, not interpolated, and persisted so a retry reuses the same one. * IngestKnowledgeBaseDocuments caps at 10 documents, server-enforced; the user guide's 25 does not apply to managed knowledge bases. * StartIngestionJob is never called: 0.1 RPS account-wide, not adjustable. * "Unable to verify the specified embedding model" is retryable — pure IAM eventual consistency, observed against a model confirmed ACTIVE and invokable. Treating it as fatal would make lazy provisioning fail intermittently while blaming the model. * imageExtractionStatus is set to ENABLED explicitly; left default, the knowledge base silently indexes no image or chart content. * dataDeletionPolicy is RETAIN, the documented remedy for the DELETE_UNSUCCESSFUL state already present in the dev account. The record is written in `provisioning` BEFORE the AWS call, so a crash after CreateKnowledgeBase returns leaves a discoverable retry anchor rather than an orphan nobody is billed for quietly. A test drives that ordering by reading DynamoDB from inside the create call, and asserts a retry produces exactly one knowledge base. Managed returns relevance natively (higher is better), so unlike the S3 Vectors adapter this one applies NO conversion. A test asserts the scores pass through unchanged, because an accidental inversion here raises nothing and merely makes answers worse. Two corrections to this spec, both found by implementing it and verified against the packaged botocore service model rather than from memory: * It said managedKnowledgeBaseConfiguration={}. The shape has no required members, but its only members are the embedding pin and encryption — so a literal {} would make Requirement 8.5's pin unsatisfiable. "No required members" is not "must be empty". * It said float32. The enum value is FLOAT32; lowercase is rejected. Also bounded inline metadata attributes at 50, the service-model maximum. Caller metadata was merged unbounded, and the blast radius is disproportionate: metadata is per-document but the call is per-batch, so one over-decorated document would fail the ingestion of the nine travelling with it. Reserved keys are emitted first, because the previous plain sorted() order would have truncated away document_id — the key the status filter joins on — and every chunk of that document would then be discarded as unverifiable. Tests: 6174 backend tests pass, up 101. No test contacts live AWS: both Bedrock clients are hand-rolled fakes, DynamoDB is moto so the conditional writes execute for real, and two tests walk the test file's own AST to enforce it. All mutations caught, each verified to have landed and left the file syntactically valid first. * feat(kb): ingestion consumer with exclusive engine routing Task group 9. The Lambda that decides whether a newly uploaded document belongs to a managed knowledge base and, if so, ingests it directly. Replaces nothing yet — the bootstrap stub still ships until the real image is built. The routing is deliberately ASYMMETRIC, and that is the whole point. The legacy pipeline is driven by its own pre-existing S3 notification on the same bucket, so for a legacy document the correct behaviour here is to do nothing at all. Acting anyway would index the same bytes twice: two sets of vectors, doubled ingestion cost, and duplicate chunks competing inside one result list. None of that raises an error, which is exactly why it is tested from both sides — legacy must ingest nothing here, and an unprovisioned managed knowledge base must FAIL rather than quietly fall back and produce the dual index. Indexed is not retrievable. Bedrock reports a document INDEXED up to a second before it can actually be retrieved (measured 0.75-1.03s), so the consumer polls until a real retrieval returns the document and records indexedAt and retrievableAt as two distinct timestamps. Marking complete on INDEXED alone produces the worst kind of bug report: the UI says the upload worked, the user asks immediately, and the answer does not mention their document. A document that never becomes retrievable is left non-terminal for redelivery rather than claiming a success the user cannot observe. No in-process orchestration (Requirement 10.8): a background task is killed when the handler returns, which turns a reported success into a half-finished ingestion. Three defects found while writing this, each worth naming: * I called the backend class ManagedBackend; it is ManagedKbBackend, and it takes the App_KB_Id rather than the AWS identifiers because it resolves those itself on every operation — a rehydration cycle replaces them and a caller holding a stale pair would keep addressing a knowledge base that no longer exists. Reading the real signatures rather than assuming them turned a runtime ImportError into a compile- time fix. * The "this module does not orchestrate in process" test grepped the source for ensure_future and failed on the module's own docstring explaining why it does not use ensure_future. Now parses the AST and inspects actual calls. * The poll timeouts were default arguments, which Python binds once at import, so a test shortening the window had no effect and silently waited the full production timeout — 33 seconds of suite time for one test. They are now resolved at call time, which is both faster (2.7s) and the only way they can be overridden at all. Tests: 6200 backend tests pass, up 26. Mutation-verified: ingesting legacy documents here, silently falling back when unprovisioned, marking complete without confirming retrievability, and collapsing the two timestamps are each caught by the specific test that exists for them. * feat(kb): tombstone deletion sagas and the report-only reconciler Task group 10. This is the code that deletes billed AWS resources, so the design is built around two questions: can it leak, and can it delete the wrong thing. Deletion saga (tombstones.py) The tombstone is written BEFORE the AWS call and cleared ONLY after AWS confirms absence. That ordering is the entire mechanism — a crash mid-delete leaves a work item rather than a silent leak. Absence is established by polling ListKnowledgeBases until the name disappears, never by the delete call returning: deletion is async and took 2-6 minutes when measured, so the window tolerates 8. Tombstones carry NO TTL; letting TTL reap them would recreate exactly the silent-leak class this design closes. The service role cannot be deleted while any knowledge base still references it, and DELETE_UNSUCCESSFUL raises its own exception, emits a metric and annotates the tombstone rather than being swallowed as a completed delete. Reconciler (reconciler.py) Joins paginated, tag-filtered ListKnowledgeBases against KB_Records. Two branches matter more than the rest: * AWS-only is an orphan, and is deletable only if AWS's OWN reported createdAt is more than 24 hours old. Age-gating on when the reconciler first noticed it would mean a reconciler that had been down for a week either waits another day on everything or, inverted, deletes every in-flight create. A missing or unparseable createdAt fails closed. * Record-only NEVER deletes the record. A record with no AWS knowledge base means the vectors are gone, not the documents: the source bytes are still in S3 and the DOC# rows are still valid, so the corpus is rebuildable and the record is the only pointer to it. It is marked vectorState: missing and left alone. delete_item appears zero times in the module. It ships report-only: arming is a separate flag, an empty string reads as off, and the per-run action limit is clamped to a ceiling the environment cannot lift. The audit caught one genuinely dangerous defect, worth recording because it would have been invisible in review. lambda_handler forwarded an `armed` field from the invocation event into reconcile(), so an EventBridge target carrying a constant {"armed": true} — or any principal holding lambda:InvokeFunction — would have deleted user resources while every piece of reviewable configuration still said report-only, leaving only an Invoke in CloudTrail. The environment flag is now the only arming path, and the regression test is parametrised over the boolean case that the previous, misleadingly-named test never covered. Tests: 6301 backend tests pass, up 101. Network isolation is proven rather than asserted: the suites were run under a plugin that raises on any non-loopback socket connection, and the plugin was itself verified by pointing a throwaway test at a real AWS endpoint and watching it trip. Mutations all caught by the correct test, including age-gating on discovery time, deleting the record on record-only, clearing the tombstone before confirmation, report-only performing a delete, an empty-string flag reading as armed, and a TTL added to a tombstone. Requirements 14.1's schedule and 14.7's deployed mode need EventBridge wiring and deploy config, which belong to the platform group under the rule that backend code never deploys before the IAM and resources it requires. * docs(kb): handoff document and accurate task-list state Groups 1-10 were complete but only group 10 was ticked. Ticks 1-9 and their subtasks, leaving 26 subtasks across groups 11-15. Adds HANDOFF.md, written for a session with no memory of this work. It records the constraints that were expensive to discover and would otherwise be rediscovered the hard way: * The DynamoDB one-GSI-per-UpdateTable limit, and that this feature has consumed the rag-assistants budget for whatever release ships it. Violating it took production down in 1.12.0. * That DynamoDB cannot do arithmetic inside a ConditionExpression, which is why the byte cap uses a single accumulator compared against a client-computed literal. * That `total` and `ttl` are reserved keywords. Both bit this feature. * Why kb_backend exists as its own package with an empty __init__, and that apis.shared.embeddings is separately fine to import. * That a module constant bound as a default argument cannot be patched, which cost a 33-second test that silently ignored its own override. * The four distinct ways a mutation test can report a false pass, all four of which happened here: an anchor that never matched, orphaned expression values turning a ValidationException into a happy-path failure, a syntax error mistaken for a detection, and simply not checking which test failed. * The ten defects already found and fixed, so none is reintroduced — including the reconciler arming bypass and the reserved-namespace metric grant that would have deployed cleanly and published nothing. Also records what is deliberately not built: the feature is unreachable by design until group 14 registers the managed backend, and both the real Docker image and the reconciler's EventBridge wiring are correctly deferred to the platform group. * feat(kb): app-side authorization, IAM-enforced sharing, publication semantics Group 11 of the managed-kb-migration spec (Requirement 25 / evaluation gate §14.3). Managed KB ships two features whose names overstate what they provide, so neither is trusted as the authority: AWS's own multi-tenant guidance calls metadata filtering "filter-level (logical) isolation, not IAM-enforced", and states that ACL-aware retrieval "is not authorization" — its identity is email only, with no alias resolution, and a mismatch fails silently. On an OIDC claim-mapped platform that is a worse primitive than an explicit app-side check. `kb_access` resolves the invoking user's grant from the existing assistant permission model, and the facade refuses to contact any backend without one. The `access` parameter is required and keyword-only rather than defaulted: forgetting it is then a TypeError at the call site, while a genuine denial passes None and fails closed. Both production callers already resolved the permission a few lines earlier, so the answer is handed over rather than re-derived — the inference route now keeps the permission it was discarding. A `KbAccess` cannot be constructed with a permission outside the read set, so holding one is evidence the model was consulted; holding a string is not. The ordering is the requirement, so the tests assert on a recording backend's call list, not on the returned value: an empty list is also what an authorized user with an empty corpus gets, and a check that runs after retrieval is an audit log rather than an access control. `resource_policy` narrows a shared corpus to named infrastructure identities. The module says plainly what it is not — every user retrieves through the same runtime role, so no resource policy can distinguish user A from user B; what it buys is that a multi-user corpus stops being readable by anything in the account holding the wildcard grant this platform's own identity policy uses. Staleness is stored state, not an event: a policy attaches to the KB ARN, so any cycle producing a new awsKbId silently drops sharing, and an event hook is only as complete as the list of places that fire it (already two, with rehydration a known third). The record stores the identifier the policy was applied to and `policy_is_stale` compares; a path that forgets to re-apply is repaired by the next call instead of regressing silently. `kb_publication` keys reclaim exemption on `is_on_shelf`, not `is_listed`. An admin requesting changes on a live listing leaves it serving but moves its state out of LISTED_STATES, so by state name alone a reclaim pass would delete the corpus behind an agent users can still see in the store. Two defects found and fixed while testing: - `is_reclaim_exempt` treated an empty mapping as an unreadable record, which would have exempted every unheld knowledge base and made the predicate vacuous. `None` (absent) and `{}` (read, no holds) are now distinct. - The IAM surface had no resource-policy grant at all, so Req 25.6 would have deployed as inert code. Added as its own statement on its own role: the writer of a sharing policy must not also be able to create, delete, ingest into or retrieve from a knowledge base, and a test asserts no retrieval identity ever receives it. `GetDocumentContent` appears in the policies this caller writes but not in its own grant — writing a permission is not holding it. Nothing is deployed and nothing new is reachable: the managed backend is still unregistered, so this changes behaviour only by adding the access gate to the existing legacy path. Verified: 6,358 backend tests pass (was 6,301; same 5 pre-existing Strands SDK contract failures), 614 infra tests pass (was 611), ruff clean on every file touched. All 12 new guards mutation-tested — each was broken in turn and the specific named test that covers it failed, with the anchor match count, the mutant's parseability and the identity of the failing test all checked so a mutation could not report a false catch. * feat(kb): opt-in dual-read pilot that legacy always wins Group 12 (Requirement 18). An opted-in knowledge base has both backends answer the same query; legacy is served and the managed result is kept only as an observation, so the rollout can rest on evidence from our corpus rather than on a three-document benchmark. Latency is the constraint that shapes the design. Managed Retrieve measured a 662-695 ms p50 against legacy's 257 ms, so anything that awaited both would nearly triple the retrieval leg of every piloted turn -- while returning correct results and passing every other assertion. So the facade starts the managed read *before* awaiting legacy and detaches the comparison afterwards: a piloted turn does exactly the waiting an unpiloted one does. The test that holds this makes the managed backend sleep three seconds and asserts the facade still returns promptly, which is the only formulation that fails when the calls are gathered. Legacy is served even when it is empty. An empty legacy result is a finding, and reaching for the other engine's answer would destroy the measurement and change what users see in the same move. Details worth the reasoning: - The opt-in reads `is True`, not truthiness. A truthy string left by a hand-edited record must not enrol a knowledge base into paying for a second retrieval every turn -- the same shape as the reconciler-arming defect, where a permissive read of a flag turned a report-only job into a deleting one. - The detached tasks are held in a module-level set with a done-callback. create_task returns the only strong reference; drop it and comparisons silently stop being recorded under load, which looks like "the pilot found nothing interesting". - A legacy failure cancels the managed task the facade started. Left alone it would pay for a Retrieve nobody reads and surface as an unretrieved task exception. - Overlap is Jaccard, because it is symmetric: a ratio against one side's length reads as agreement when one backend simply returned fewer documents, which is the case most likely to occur while the managed corpus is catching up. - A document is ranked by its best chunk, not its last, or the correlation measures chunking rather than agreement. - A correlation over a single shared document is None, not 1.0, which would make a pilot on small corpora look like perfect agreement. - `emit_value` is separate from `emit_count` so the unit is a call-site decision. A latency published as Count is not merely mislabelled: CloudWatch graphs and alarms on it as a rate, invisibly, until someone reads the board. - The facade now reads the KB_Record once and passes it to the resolver, so the pilot flag costs no extra DynamoDB round trip. `load_record` collapses "absent" and "unreadable" to `{}` because this feature gives both the same answer. Still unreachable in production: the managed backend is unregistered, so `start_managed_read` returns None for every record regardless of the flag. That makes setting the attribute in the database harmless before task 14. Verified: 6,386 backend tests pass (was 6,358; same 5 pre-existing Strands SDK failures), 614 infra tests pass, ruff clean on every file touched. 10 new guards mutation-tested individually -- including the gathered-instead-of-detached mutation, the truthy-flag mutation and the last-chunk-ranking mutation, each caught by its own named test. * docs(kb): handoff reflects groups 11-12 and two new defects * feat(kb): migration dispatcher and the shadow/verify/promote/retain worker Group 13 (Requirements 15, 16, 17). One EventBridge tick reads the sparse KbWorkIndex, takes at most 20 records, and async-invokes the worker once each. The worker takes a lease, executes exactly ONE step, and returns. One step per invocation, because a 20-document text corpus is ~3 minutes but a 20-PDF corpus can exceed an hour -- per-document parse time was measured at 37-264 s and dominates everything. A worker that ran the whole machine would sometimes finish and sometimes hit its timeout, and a timeout mid-shadow is indistinguishable from a crash. Stepping means every interruption lands on a recorded state with a conditional guard in front of it. Nothing is mutated in place: the live corpus keeps serving from legacy through shadow and verify, the managed copy is built alongside, and promote is one conditional write. That is also why rollback moves no data -- the legacy index was never touched, so returning to it is an attribute REMOVE. Convergence rather than dual-write. The upload path stays authoritative; the worker snapshots the doc-id set, migrates it, then runs catch-up passes until a pass finds nothing new. Each DOC# record is re-read immediately before that document is ingested, not once per batch: a PDF batch takes minutes and the deletion this guards against is most likely to land inside exactly that window. THREE DEFECTS THE PROPERTY TEST FOUND, each fixed in the code rather than weakened in the test: 1. A resumed migration re-ingested the entire corpus. The completed-document set lived inside `migrationProgress`, which a later write replaces wholesale, so a crash near the end of 25 documents re-parsed all 25. Now a separate `migratedDocIds` string set, updated with ADD per batch -- additive, so a crash loses only the batch in flight, and immune to two workers clobbering each other. 2. `promote_engine` permitted a SECOND promotion. Every guard it had -- state, generation, converged progress -- stays true after a successful promotion, so a crash between the promotion and the state transition promoted again with a fresh promotedAt over the real cutover moment. Worse, two genuinely concurrent workers would BOTH succeed, which is what Requirement 15.10 forbids. Now guarded on `attribute_not_exists(retrievalEngine)`; rollback REMOVEs the attribute so a deliberate re-promotion still works. 3. Fixing (2) then made a resume after a successful promotion mark the migration `failed` -- a promoted knowledge base with no retention window. `run_promote` now treats "already promoted" as success and continues to retain, and re-reads before deciding so a genuine guard failure still raises. The property is also stated honestly. "Each document ingested at most once" is not achievable: a worker can die between a successful Ingest and the DynamoDB write recording it, and no transaction spans Bedrock and DynamoDB. What is asserted instead is that each document appears in the corpus exactly once (because customDocumentIdentifier makes a re-ingest a replace) and that redundant re-ingests are bounded by one batch -- which is the assertion that caught (1). Verification is a manifest of document_id + content identity, never count parity: count parity is satisfied by a corpus with the right NUMBER of wrong documents, which is exactly what a migration racing an upload and a delete produces. Plus a canary retrieval built from the corpus's own filenames, because a knowledge base can hold documents while returning nothing and a constant query like "test" can legitimately match nothing in a real corpus. Verified: 6,457 backend tests pass (was 6,386; same 5 pre-existing Strands SDK failures), 614 infra pass, ruff clean. 24 guards mutation-tested. Five of those mutations initially survived and each was a real finding: a limit assertion the final trim masked, a derivation whose test was vacuous because the priority list happened to be complete, a `match=` pattern loose enough that the wrong check satisfied it, and an `except LeaseLost: raise` that was dead code because the lease was taken outside the try. All four were fixed, not annotated. * docs(kb): handoff reflects group 13 and four more defects * docs(kb): restore defect-list ordering in the handoff * feat(kb): register the managed backend, fleet metrics, and tagged teardown Group 14's backend and infrastructure half (14.0, 14.1, 14.2, 14.6, 14.7). The frontend half -- 14.3 upgrade UX, 14.4 failed-document surfacing, 14.5 admin surface -- is untouched. SPEC GAP: nothing registered the managed backend. `register_backend` was defined in task 4.2 and called by nothing; task 8.3's note that it would do the registering was never carried out and no other task picked it up. Every one of the 15 groups could have been completed with the feature unreachable, because a promoted record raises BackendUnavailable -- a correct fail-safe and a useless signal, seen only by the single user whose knowledge base was migrated. Both adapters are now installed in the registry at import, not by a startup call somebody has to remember. That is free because their module bodies are stdlib-only with lazy clients, which the boundary test now asserts for `managed_backend` as well -- the day someone hoists a `boto3.client(...)` to module scope, every Lambda image carrying any part of the package pays for it. Registration does not make the feature live: nothing resolves to managed until a record says so, and only a promotion writes that. It also exposed a test passing for the wrong reason. `test_no_managed_call_when_no_managed_backend_is_registered` assumed no backend existed; once one did, it kept passing because a real adapter was starting and failing against no AWS. It now creates the condition it names. METRICS (14.1). KbCount / KbStorageGB / KbIdleGB via EMF, once per reconciler pass, into the same `{prefix}/ManagedKb` namespace the PutMetricData grant is conditioned on. Idleness is max(own lastRetrievedAt, bound agents' lastUsedAt), never retrieval alone: an agent can be invoked all day and retrieve nothing, because retrieval only fires when the query matches -- so a corpus judged by retrieval looks most abandoned exactly when its agent is busiest with questions its documents do not answer, and the follow-up spec's eviction pass would delete it. A knowledge base with no signal at all is reported as *unmeasured*, not idle; counting it as idle would make every freshly provisioned corpus look abandoned. The lastRetrievedAt write is throttled by a conditional freshness floor and detached from the request, and is guarded on `attribute_exists(SK)` so a metrics side effect cannot create KB_Records across the 1,692 legacy rows. Also implemented Requirement 20.13's metrics READ grant, which existed only as a comment -- so the reconciler could not have read Bedrock's own Invocations. COST ATTRIBUTION (14.2). docs/specs/managed-kb-cost-attribution.md. Keying on `AmazonBedrock` returns $0.00 successfully, which is worse than an error; keying on service code alone blends KB into the Runtime-memory line that is 73% of the AgentCore bill. Filter on usagetype. TEARDOWN (14.6, 14.7). Managed knowledge bases are runtime-created, so delete-stack does not touch them: left behind they keep billing at $5.00/GB-month while being invisible in the CloudFormation console. scripts/teardown/managed-kb.sh deletes only tag-matched ones, before any stack, because the Bedrock service role lives in PlatformStack and Bedrock needs it to perform the delete. THREE DEFECTS IN THAT SCRIPT, all found by running it rather than reading it: 1. Infinite spin. The wait loop advanced a counter by the poll interval and stopped at the timeout; at an interval of 0 the counter never advanced, so an `aws` call and a `python3` parse ran per iteration at full CPU. A test set the interval to 0 to be fast and ran for SIXTEEN HOURS. Interval clamped to >=1, non-numeric values fall back rather than erroring under `set -e`, and the loop is now bounded by an attempt budget so termination does not depend on arithmetic a later edit could break. 2. Timing-dependent false absence. `list | cut | grep -Fxq` -- grep exits on its first match and closes the pipe, so under `pipefail` the upstream's SIGPIPE (141) became the pipeline's status and the check answered "absent" about a knowledge base it had just found. Whether it happened depended on how much output was already written. Now a capture then a match, failing safe. 3. Fail-open discovery. A failed `list-knowledge-bases` was swallowed: `set -e` is suspended for a function called in a condition, so the loop continued, the parse of an empty response failed silently, and the run reported a clean teardown having deleted nothing. Every step now carries an explicit `|| return 1`, and the discovery walk refuses to continue rather than reading an unreadable account as an empty one. Verified: 6,511 backend tests (was 6,457; same 5 pre-existing Strands SDK failures), 616 infra (was 614), ruff clean on every file touched, `bash -n` clean. 24 guards mutation-tested. Four of those mutations initially survived and each was a real finding: two mutations aimed at lines the named test never reached, one test that asserted only termination where the guard controlled the *value*, and one that checked for the presence of strings a mutation preserved. The harness itself was also wrong twice -- an on-disk check that reported false failures for additive mutations, whose replacements contain their own anchors. * docs(kb): handoff reflects group 14 backend half and three more defects * fix(kb): one source of truth for the managed KB tag contract Four components in three languages have to agree on the tags that identify this platform's knowledge bases, because a tag-filtered ListKnowledgeBases is how the reconciler and teardown both scope themselves. Nothing asserted the agreement, and they had drifted three ways: - `provisioning.build_tags` wrote keys `prefix`/`env` from PROJECT_PREFIX and ENVIRONMENT -- neither of which the provisioning Lambda is given, so every knowledge base would have been tagged with hardcoded defaults regardless of project or environment. - `tombstones.project_tag_filter` was a hand-written MIRROR of that function, documented as one. A mirror is a second implementation, and the only thing keeping two implementations equal is that nobody has edited one yet. - `kb-migration-construct.ts` declared entirely different key names (ManagedKbPrefix, ...) and exported them plus the correct values as environment variables that NOTHING READ. - `scripts/teardown/managed-kb.sh` read a third pair (CDK_PROJECT_PREFIX, CDK_ENVIRONMENT) and matched on `prefix`/`env`. The failure was silent in the worst way. Writer and reconciler agreed with each other because both fell back to the same defaults, so knowledge bases would have been created, found and reconciled normally. Only teardown disagreed, and its symptom was matching nothing and reporting success -- Requirement 20.8 failing into a leak of resources billing at $5.00/GB-month with no CloudFormation console to notice them in. Two environments in one account would also both have claimed `agentcore/dev` and treated each other's corpora as their own. The construct was right; the Python was wrong and nothing bridged them. So the keys now live in `kb_backend/tags.py` as constants mirroring the construct's, the value resolution is one function with one fallback chain used by writer and reader alike, and `provisioning.build_tags` / `tombstones.project_tag_filter` are thin delegations rather than parallel implementations. The knowledge base's AWS *name* resolves through the same helper, so a name saying `prod` while the tag says `dev` is not reachable. Keys are namespaced (`ManagedKbPrefix`, not `prefix`) because generic keys collide: many accounts carry an organisation-wide cost-allocation tag literally called `env`, and if something else writes it our filter compares against a value we did not set -- a teardown that skips a knowledge base it owns. One production consequence beyond the rename: `reconciler._delete_orphan` read the `appKbId` AWS tag by its old name to anchor an orphan's tombstone, so it would have anchored on the AWS id instead. The KB_Record *attribute* is separately named `appKbId` and deliberately stays that way. Test fixtures now build tags through the canonical helper instead of spelling them out. A fixture that hardcodes tag keys keeps passing after the keys change under it, which is precisely how this drift stayed invisible. Verified: 6,530 backend tests (was 6,511; same 5 pre-existing Strands SDK failures), 617 infra (was 616), ruff clean, `bash -n` clean. 17 cross-layer drifts mutation-tested -- each component's idea of a key or a variable changed in turn, and every one is caught by the new contract test. One survivor was a real gap: the knowledge base name's prefix was untested despite the docstring claiming it could not disagree with the tag. * docs(kb): handoff records the tag contract fix * feat(kb): owner-facing upgrade surface and the enrolment path it needed Task 14.3, plus 14.4's surfacing half. Closes the last gap in the migration control path: before this, nothing wrote a record into `shadow`, so all fifteen task groups could have been finished with the feature unreachable. The task was scoped as frontend-only, which is how the gap hid — the missing piece was an HTTP surface nobody had planned. `apis/app_api/kb_upgrade/` is its own package rather than living in `kb_migration/`, because that package's four handlers share one size-constrained Lambda image and this one imports the embeddings-pulling assistants package for the permission model. Enrolment is two conditional writes, not one put. `KbRecord.to_item` does not write the GSI7 work keys — only `set_migration_state` maintains them — so the obvious single-put enrolment yields a record that reports an upgrade in progress while being invisible to the dispatcher's sweep forever. - Four endpoints under /assistants/{id}/knowledge-base/upgrade: read status, start, retry, dismiss notice. Read allows any resolved permission and reports canUpgrade=false to a viewer; the three writes require owner or editor. - Derived phases (none|available|in_progress|succeeded|failed) so the client never learns that shadow/verify/promote are all "working on it". - Two public transitions on kb_backend/records.py: retry_from_failed (one atomic write, guarded on generation AND still-failed) and dismiss_upgrade_notice. - The offer is gated on MANAGED_KB_MIGRATION_ENABLED, the dispatcher's own flag, so the offer and the capability cannot disagree. - Stranded documents are surfaced before the user commits, splitting unsupported format from processing failure against the ingestion pipeline's own extension list. `deleting` is deliberately shown though the ordinary document list hides it — 101 of the 200 affected production records sit there. - app-api-environment.ts now passes MANAGED_KB_MIGRATION_ENABLED to the task that reads it. It was absent, so the card would have rendered nothing in every deployed environment regardless of the flag. Deferred: Req 21.2's one-click document retry. Ingestion is S3-event-triggered and no reprocess endpoint exists, so it needs new backend against a live pipeline. The card directs the user to re-upload, which works today. Tests: 69 backend, 41 frontend, 6 infra. Ten backend and four other mutations verified against named tests; one survivor found an unrecognised migrationError leaking verbatim into the card.
A development deploy tagged its managed knowledge bases
`ManagedKbEnvironment=prod`, while `scripts/teardown/managed-kb.sh` looks for
`dev`. Teardown would therefore have matched nothing and reported a clean run,
leaving billed Bedrock knowledge bases behind — the same symptom as the original
tag-contract drift, one layer up and invisible to the tests written for it.
Two fallbacks that never agreed:
* `managedKbEnvironmentTagValue` → `config.tags.Environment ?? (production ?
'prod' : 'nonprod')`. `config.tags` is `{ManagedBy: 'CDK'}` — no Environment
key — and `config.production` is `true` in every environment, because
platform.yml never passed a production flag and cdk.context.json says true.
* the teardown script → `MANAGED_KB_TAG_VALUE_ENVIRONMENT` → `ENVIRONMENT` →
`CDK_ENVIRONMENT` → `dev`.
Neither is wrong on its own, which is why nothing caught it: the existing
contract tests check that all three languages agree on the tag *keys*, and that
the Python writer and filter share one fallback chain. Nothing compared the value
the CDK computes against the value the shell defaults to.
Fixed by passing the value explicitly per environment rather than by making the
fallbacks agree — a fallback correct for dev is wrong for prod and vice versa, so
the only safe version is not to rely on one.
CDK_TAG_ENVIRONMENT: development=dev, production=prod (GitHub Environment vars)
Forwarded as the FLAT dotted context key, and read as one. `--context a.b=c` sets
`context['a.b']` and does not merge into a nested object, so a nested-only read
silently ignores the operator's own flag — the trap that already cost this repo
the managed-KB byte caps and then the alarm thresholds.
`config.production` is deliberately left alone. It also governs PITR on the
artifacts table, X-Ray sampling, alarm thresholds and notifications; flipping it
for dev is a defensible change but a much wider one than a tag fix, and it should
be its own decision.
Verified by synth: with the value passed the Lambdas carry `dev`; without it,
`prod`. Four new tests guard the plumbing end to end, since an omission anywhere
in the chain silently reinstates the guess.
Replaces the bootstrap stub with the actual handlers, which is the last thing standing between an enrolled knowledge base and a completed migration. Until now PlatformStack shipped four no-op Lambdas and the dispatcher ticked into them every 15 minutes. Seven artifacts, not the five the spec anticipated — `deploy-image-lambda-one.sh` also needed per-function cases: * backend/Dockerfile.kb-migration * backend/src/apis/app_api/kb_migration/requirements.txt * the kb-migration case in scripts/build/build-one.sh * four cases in scripts/build/deploy-image-lambda-one.sh * build-kb-migration + deploy-kb-migration-code in backend.yml * entries in both hand-maintained supply-chain lists⚠️ THE boto3 PIN IS THE FEATURE, NOT HYGIENE. The Lambda base image at our pinned digest bundles boto3 1.40.4, whose packaged bedrock-agent model offers `type` enum ['VECTOR', 'KENDRA', 'SQL'] and has no `managedKnowledgeBaseConfiguration` shape at all. Measured, not assumed. Without boto3==1.43.68 installed over it, every CreateKnowledgeBase call fails with a ParamValidationError naming a parameter that looks correct in our source. Removing or downgrading the pin breaks the feature silently at runtime and nothing else in the repo would notice, so three tests now guard it — including one that asserts the *capability* (MANAGED in the enum, the embedding pin members, FLOAT32 uppercase, all four document operations) rather than just the version string. The COPY surface is five directives for a 16-module closure with boto3 as the only dependency. That is the `kb_backend` boundary paying off: its `__init__` is empty and its module scope is stdlib-only, so this image never pulls `apis.shared.assistants` and the embeddings stack behind it. No FastAPI, no pydantic — KB_Record is a dataclass for exactly this reason. Verified in the built container on linux/arm64: all four handlers import, all nine kb_backend modules import, boto3 resolves to 1.43.68, MANAGED is in the enum and all four document operations are present. Also verified by staging the Dockerfile's COPY list into a bare tree and asserting every closure module resolves from inside it and nowhere else — module-level imports alone prove little here, since most of these are function-local and only run on invocation. Three mutations confirmed caught by named tests: pin removed, pin downgraded to the base image's 1.40.4, and a COPY dropped. Tests: 6,772 backend (5 pre-existing Strands SDK failures).
…n-image feat(kb): ship the real kb-migration Lambda image
No knowledge base could migrate. The first real dispatcher tick after the image landed raised, and would have raised every 15 minutes forever: RuntimeError: KB_MIGRATION_WORKER_FUNCTION_NAME is not set The construct set `MANAGED_KB_WORKER_FUNCTION_NAME`; `dispatcher.py` reads `KB_MIGRATION_WORKER_FUNCTION_NAME` — the house convention its siblings use (`KB_SYNC_WORKER_FUNCTION_NAME`, `SCHEDULED_RUNS_WORKER_FUNCTION_NAME`). The IAM grant was already correct, so only the name was wrong. kb-sync does not have this bug for one reason: `kb-sync.test.ts` asserts the variable is present. A second mismatch found by the same sweep: the construct published `MANAGED_KB_RETENTION_WINDOW_DAYS`, which nothing reads, while `worker._retain_days()` reads `KB_MIGRATION_RETAIN_DAYS`. Requirement 15.11's configured retention window was therefore being silently replaced by the code's 30-day floor. Safe, because the floor is the required minimum — but the operator's value was going nowhere. The pre-existing infra test asserted the retention variable under the construct's own spelling, so it confirmed the construct against itself and passed while the value was dropped. Corrected rather than annotated. Fourth wiring mismatch of this shape in this feature — code that reviews cleanly, deploys cleanly, and does nothing. So the fix includes a general guard: `test_kb_migration_env_contract.py` parses every `os.environ` read in the four handlers and in `kb_backend`, and asserts the construct sets each one, with documented exemptions for Lambda-provided variables, defaulted tuning overrides, fallback links, and modules outside the handlers' import closure. It also fails on any kb-migration variable the construct publishes that nothing reads — which is precisely how this defect would have been caught before deploy. Both mutations verified caught: restoring the MANAGED_KB_ prefix on the worker name, and publishing the retention window under the unread spelling. Tests: 624 infra, 385 backend supply-chain + lambdas.
…-worker-env fix(kb): give the dispatcher the worker name it actually reads
The first migration to reach the worker failed: AccessDeniedException: not authorized to perform bedrock:TagResource on resource arn:aws:bedrock:us-west-2:...:knowledge-base/* `CreateKnowledgeBase` is called WITH tags, and AWS authorises the tagging as a separate action from the create. The grant had `bedrock:CreateKnowledgeBase`, so it reviewed as complete — the missing permission only appears at the moment a real knowledge base is created, which is the first thing nobody had done yet. Those tags are not decoration. They are what the reconciler and `scripts/teardown/managed-kb.sh` match knowledge bases on, so creating them untagged would be worse than failing to create them: an orphan nothing can find. Failing closed here is correct behaviour, it just needed the permission. Also adds `bedrock:ListTagsForResource`, missing for the same reason and with a quieter failure mode: `tombstones.iter_project_knowledge_bases` reads tags to decide what belongs to this project and fails closed on a read error, so without it every knowledge base looks untagged, matches nothing, and the daily orphan sweep reports a clean account forever. Both on `knowledge-base/*` — at create time there is no ARN to scope to, which is what the failing request was evaluated against. The existing test asserted the action list with `toEqual`, so it caught the addition and forced this to be deliberate; that whitelist is why a stray Bedrock permission cannot creep in unnoticed (spec defect 8). Two further tests pin *why* each action is needed rather than just that it is present, and both mutations are verified caught. Tests: 626 infra.
…ag-permissions fix(kb): grant the tag permissions provisioning and reconciliation need
The first knowledge base that got past IAM was created and then abandoned: ConflictException: The Knowledge Base is not in a valid status. Wait for the knowledge base to reach a valid status and try again. `CreateKnowledgeBase` returns while the knowledge base is still `CREATING`. This module's own header records 47-124 s to `ACTIVE` (n=7) — the knowledge was there, just not in the code path, which called `CreateDataSource` immediately. `ConflictException` is deliberately absent from RETRYABLE_ERROR_CODES, because a genuine conflict should fail fast, and `_call`'s backoff tops out near 60 s anyway — short of the measured upper bound. Retrying the dependent call is the wrong shape regardless: it spends attempts on a precondition instead of waiting for it. So the wait is explicit and bounded at 300 s, roughly 2.5x the observed worst case and well inside the worker's 15-minute timeout. A terminal status (FAILED/DELETING) fails on the first poll rather than waiting out the budget. Consequence worth recording: the create succeeded, the data source did not, and `attach_aws_ids` never ran — so the knowledge base existed in AWS with no record pointing at it. Two pre-existing decisions contained it. The `clientToken` is derived from `app_kb_id`, so the next attempt *adopts* that knowledge base rather than creating a second one; and tags are written at create, so the reconciler can find it either way. The failure message now says so, because an operator reading it needs to know a retry is safe. WHY NO TEST CAUGHT THIS `FakeBedrockAgent.create_knowledge_base` returned `status: "ACTIVE"`, which the real API never does. Every provisioning test therefore skipped straight past the window where this breaks. The fake now returns `CREATING` and answers `get_knowledge_base` from a configurable status sequence, so the wait is exercised rather than assumed. Five new tests cover it, including that the data source is never attempted while CREATING and that the budget is read at call time. Mutation verified: deleting the wait fails the named tests. Tests: 2,321 passed across tests/shared, tests/lambdas and tests/property.
…onversation-artifacts-ui feat(artifacts): render a shared conversation's artifacts for its recipient
…ncement form Found by browser-verifying the page in dev: every field filled, the form reporting `ng-valid`, and "Create draft" still disabled. No announcement could be authored from the UI at all. `canSubmit` is a `computed`, and a computed tracks the signals read during its *last* execution — so an early `return` shortens its dependency set. The guard chain read `isSubmitting()` and then `if (this.form.invalid) return false`, and `FormGroup.invalid` is a plain getter, not a signal. On the first evaluation the form was empty, so it returned there having tracked only `isSubmitting`. No later edit could schedule a recompute, and `isSubmitting` changes only inside `onSubmit` — which the disabled button prevented. Two changes, both load-bearing: - form validity is mirrored into a signal fed by `statusChanges`, like the other `valueChanges` mirrors already in this component; - every input is read unconditionally before being combined, so no branch can shrink the tracked dependency set again. The three new tests read `canSubmit()` while the form is still **empty**, then fill it. That ordering is the whole point: the existing 24 specs only ever read it after filling, so the computed's first evaluation saw a valid form, tracked everything, and stayed reactive — all 24 pass against the broken code. Verified by reverting the fix: the 3 new tests fail, the other 24 do not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Opening "Shared with you" with nothing shared showed "No artifacts match
your search" — with an empty search box. Found on dev the moment the
inbox flag went live.
`isFilteredEmpty` gated on the LIBRARY total, so any non-empty library
made an empty tab look like a failed search. It needed the SELECTED
TAB's count instead. The irony is that the comment above it already
warned about exactly this conflation ("'Nothing matches' is a different
message from 'you have nothing'") — tabs added a third state, "nothing
*here*", and the old gate quietly folded it into the wrong one.
So there are now three, in priority order:
isEmpty nothing anywhere "No artifacts yet" + CTA
isTabEmpty nothing in this tab names the tab
isFilteredEmpty filtered to nothing "No artifacts match your search"
`isEmpty` still wins when the library is empty outright: a per-tab
message would bury the one statement that actually matters.
Why the tests missed it: every empty-state spec asserted which ROWS
rendered, never which SENTENCE appeared when none did. The three added
here assert the sentence, including the case where blaming the search
is correct.
SPA suite: 2431 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s-form-submit-gate fix(announcements): submit button could never enable on the new-announcement form
…ty-tab-message fix(artifacts): stop blaming a search the user never made
The `banner` surface has been authorable since PR-1 and computed by the server since PR-2, but nothing in the SPA consumed `bannerItem()` — an admin who ticked "banner" got a field that did nothing, with no way to tell from the UI that the surface was unbuilt. `components/announcement-banner` renders the one banner the server picked (§D7) as a strip at the top of the shell: severity icon and colour from the `state-*` scale, the `summary` line when the author wrote one, an optional CTA, and a ✕ that records a durable `dismissed` ack. It writes `seen` on render — once per announcement per tab — which is what clears the unread dot for someone who reads the banner and never opens What's New. That write races the ✕, and deliberately relies on §D2's monotonic server-side rank rather than ordering the two client-side. Placement is a flex child of the shell's `<main>`, above the scroll container, so content reflows instead of hiding underneath. Three pieces of viewport-fixed chrome would otherwise paint over it, so the strip publishes its measured height as `--announcement-banner-height` and they offset against it: the chat topnav, the full-page empty-state overlay (which was `inset: 0`), and the two floating sidenav control clusters. The height is measured rather than hardcoded because the line wraps on narrow viewports. The voice overlay still covers it, which is right — that one is a modal. Gated on `isAuthenticated()`, not just chrome. `AnnouncementsService` loads its feed on the first read of `bannerItem()` and `resource()` loads exactly once, so mounting the banner on the login screen would fire `GET /announcements` unauthenticated, take the 401's empty-feed fallback, and never retry — announcements would be missing for the life of the tab. Found in the browser, not by a spec. Verified end to end against dev data with a local app-api: strip renders in light and dark and at 375px with no horizontal overflow, ✕ writes an ack that upgrades the existing `seen` row in place to rank 2 rather than duplicating it, the server then returns `banner: null` while the panel entry survives (§D1/§D2), and deleting the ack brings the banner back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ment-banner feat(announcements): render the banner surface (PR-4)
The interruptive surface, and the last one. `components/announcement-modal` renders the single modal the server picked; `AnnouncementModalService` decides whether interrupting is acceptable at all. The gate is the substance of this PR. The modal opens on route settle and only when there is no active stream, no pending tool-approval / OAuth-consent / MCP-App-consent prompt, no draft in a focused composer, and the route is not a minimal-chrome page. The consent checks are not redundant with the stream check: per mid-turn-steering (#934) `isLoading()` is FALSE while a turn is paused on an interrupt, so a stream-only gate would throw a dialog over an OAuth consent prompt and steal its focus. The prompt services are asked directly. Every gate input is read `untracked`. Read reactively, the effect would re-run the instant a stream ended or a consent was answered and fire a modal seconds after the user finished a thought — which §D8 forbids in as many words: a failed gate leaves the announcement eligible for the next clean load, it does not queue it. So the effect tracks only the announcement and a navigation counter, and snapshots the rest. `requiresAck` makes the confirm button the only exit: no ✕, `disableClose` on the overlay, and the in-component Escape and backdrop handlers return without writing an ack. Belt and braces on purpose — the CDK option and the guards fail independently. The button label follows the ack it writes, "I understand" → `acknowledged` and "Got it" → `dismissed`, so it cannot misdescribe the record. Started via `provideAppInitializer` rather than mounted in app.html: a CDK overlay is not a layout element, and nothing else would ever inject the service. Same shape as ThemeService. It also means this PR does not touch the app shell, so it does not conflict with PR-4. Body uses `.message-block`, not `prose` — the typography plugin is not installed, so the older user-menu-link-modal's classes are inert and strip list markers. Sanitization stays on (§D10): `admin.announcements` is delegable, so this body may be authored by a non-admin and reaches every user. Verified end to end against dev data with a local app-api. With `requiresAck`: opens on load, no ✕, Escape and backdrop clicks leave it open, and the button writes `acknowledged` at rank 3 — upgrading the `seen` row in place rather than duplicating it. Without it: ✕ and Escape both write `dismissed` at rank 2. Afterwards the server returns `modal: null` while both panel entries survive (§D1/§D2). Light, dark, and 375px with no horizontal overflow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ment-modal feat(announcements): render the modal surface with the §D8 gate (PR-5)
…end) `/stats` needs a count of acks across users, which the key shape does not support: acks live under `USER#<id>` partitions, so counting them per announcement means a GSI on `announcementId` or a scan. The spec ranks those second and third and says start with atomic counters on the announcement item (§9). This does. The counters are top-level attributes — `ackCountsR1Seen` and friends — not a nested `ackCounts` map, because DynamoDB's `ADD` only works on top-level attributes and creates a missing one as 0 in the same atomic write. A nested map needs `SET path = if_not_exists(path, :zero) + :one`, which raises ValidationException until the parent exists, so every announcement authored before this shipped would need an init-then-retry branch on the ack hot path. They count users, not clicks. `record_ack` now reads the previous rank via `ReturnValues="UPDATED_OLD"` and bumps only the ranks the write crossed, so `seen` then `dismissed` adds one to each rather than two to `seen`. They are a funnel, not a partition: acknowledged implies dismissed implies seen, so `seen >= dismissed >= acknowledged` holds without ever reading them back. Keyed by revision, because "Show again" (§D4) is a deliberate re-broadcast and rolling its acks into the previous revision's totals would inflate them and make the numbers lie about the version people actually saw. **The bug worth reading twice:** every admin mutation — `update_announcement`, `set_state`, `bump_revision` — is a full `put_item` of the `Announcement` dataclass, so any attribute the model does not carry is destroyed by it. Publishing an announcement, the most common admin action there is, silently zeroed every counter. `Announcement.ack_counts` now carries them through read → write. Four regression tests cover publish, archive, edit, and continued accrual afterwards. `targeted` is answerable only for a `"*"` audience, via a COUNT query on the users table's StatusLoginIndex. That index is projected INCLUDE without `roles`, so a role-filtered count has nothing to evaluate against, and the alternatives are worse than an honest null: replacing a GSI on the users table (CFN reports green well before an index is ACTIVE), or the scan the spec ranks last. Nor is there a membership list to count — roles arrive as JWT claims mapped at login. Null means "not estimated", never zero. Increments are best-effort by design: a second write after the ack is already durable, logged and swallowed on failure. An under-counted stat beats turning a successful acknowledgement into a 500. 18 new tests; full backend suite 2329 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Completes PR-6. The admin list now carries a reach line per announcement — "2 seen · 0 dismissed — of ~68 targeted (estimate)" — which is the point of the whole surface: it tells you whether any of this works. Rendered as a funnel, not a partition. "12 seen · 8 dismissed" means 8 of those 12, because the stored rank only ever rises through them (§D2). `acknowledged` appears only where one was actually asked for; on an announcement without `requiresAck` the number is real but meaningless, and showing a third figure that is always equal to the second reads as a bug. Two cases render nothing rather than a zero: - **A draft.** Nothing has been shown, so "0 seen" would read as "nobody engaged" instead of "not sent yet". `hasReach` gates on published/archived, which also keeps the fetch off every row an admin is still writing. - **A role-scoped audience.** `targeted` is null there — the users table's StatusLoginIndex does not project `roles` — and "of ~0" would imply nobody is targeted. It says "audience not estimated" instead. Stats are a second endpoint per announcement, so they load after the list rather than blocking it, and only for rows that have been live. The cache is keyed by **id plus revision**: "Show again" restarts the counters, so an entry from the previous revision would report stale reach for a broadcast that has only just gone out. A failed fetch is dropped from the requested set so the next pass retries, and leaves the row without a reach line rather than blanking the list — the page's actual job is CRUD. The hover text and the "(estimate)" suffix carry the §11 caveat. One more is now documented on the response model: **nothing is backfilled.** The counters are incremented by the ack write path, so acks recorded before this ships are invisible — an existing environment starts every announcement at zero on deploy day even where people have already read and dismissed it. Verified against dev, where four ack rows predate the counters and only the two written since are tallied. 7 new service specs, 8 new page specs; full frontend suite 2486 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Dismissing the banner pulled the whole view up by its height. It was a flex child of the shell's `<main>`, so appearing and disappearing reflowed everything below it — the jump was the bug, and reserving the space forever would have been a worse fix. It is now positioned `absolute` against a `relative` `<main>`: a rounded, shadowed pill floating over the content rather than a full-bleed strip displacing it. Measured before and after a dismissal, every content element — scroll container, greeting, composer — moves by exactly 0px in both axes. The overlay removes the reason anything had to know the banner's size, so this deletes more than it adds: - `--announcement-banner-height`, its `ResizeObserver`, the height signal, and the `DOCUMENT`/`ElementRef`/`DestroyRef` injections all go - `.chat-topnav-wrapper` goes back to `top: 0` - `.chat-container-empty.full-page` goes back to `inset: 0` - both floating sidenav control clusters go back to `top-4` `top-16` is the one constant that replaces all of it, and it is not arbitrary. On a chat route it lands the pill immediately below the fixed topnav — the placement §D1 asks for — and everywhere else it clears the shell's floating sidebar buttons, which sit at `top-4` and would otherwise be overlapped by a centred pill on any viewport narrow enough for the two to meet. Verified at 375px: the controls end at y=56 and the pill starts at y=64. The positioning strip spans the full content width, so it is `pointer-events-none` with `pointer-events-auto` on the pill alone — otherwise an invisible band would swallow clicks aimed at the topnav and the sidebar buttons beneath it. Verified: a click 30px outside the pill lands on the chat container, not the banner. `relative` on `<main>` is load-bearing. Without it the pill anchors to the viewport and drifts out from under the sidenav's padding transition. Browser-verified against dev data in light and dark and at 375px, with no horizontal overflow. Full frontend suite 2474 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ment-stats
feat(announcements): reach stats — counters, GET /{id}/stats, admin list (PR-6)
…-wrong blend `--rates-only` could never have produced a usable number. Three defects, all found by actually running it against dev-ai: 1. It filtered usage types on the substring `gpt-5.6`. No usage type contains a model id, so the filter matched nothing and the script reported "Cost Explorer lags ~24h" — a lag message for a search that was never going to match, which is the worst possible failure mode for a tool whose whole job is to answer "have the numbers landed yet?". 2. It multiplied every rate by 1000 to convert from 1K-token units. These models bill through AWS Marketplace in units of **1M tokens**, and Cost Explorer declares the unit in its own `Unit` field. Every derived rate was overstated 1000x. It now reads the declared unit and converts accordingly. 3. It read MONTHLY. Daily rows come back as exact round numbers; a multi-day window silently blends models into an average that looks like a rate. The blend is not hypothetical, and it is why this needed a guard rather than a fix. Marketplace usage types carry the token bucket and the service tier but never the model, so every OpenAI-family model in the account shares the same four rows — verified against USAGE_TYPE grouped by OPERATION and by BILLING_ENTITY; no finer dimension exists. August shows two distinct price cards ($5.50/$27.50 and $2.20/$11.00) and 2026-08-31 is visibly a blend of the two. A rate is therefore only a given model's rate on a day when it was the sole OpenAI-family model to run, so `--table` now reconciles against what we recorded in sessions-metadata and refuses to vouch for a number otherwise. I nearly shipped the mistake this guard prevents: a first read of Aug 20-31 gave a cache-read rate matching gpt-5.4's 0.1x to four decimals, and a reconcile then showed zero GPT calls in that window. The match was coincidence. This also closes off the spec's Option 1. The Price List API has no Marketplace service code at all (all 269 enumerated), and the Marketplace Catalog API is seller-side. These rates are not unpublished-yet; they are unpublishable through any pricing API while they bill this way, so waiting will not produce them. Bearing on the tier/long-context modelling gap PR-3 must resolve: every row ever seen in this account is `_standard` and no `-long-ctx` usage type has appeared, so a flat standard rate is correct for current traffic and a change would show up as a new usage type. That makes the gap monitorable rather than blocking. Controlled window claimed 2026-09-06 for `us.openai.gpt-5.6-sol` (dev had zero recorded calls beforehand); expected token totals are recorded in the spec so the read is a verification rather than a guess. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ment-banner-overlay fix(announcements): float the banner instead of occupying layout
…odel cards
I concluded yesterday that these rates existed in no source and had to be
derived empirically. That was wrong, and the error was one of scope: the search
ran against pricing *APIs* — Price List, then Marketplace Catalog — and stopped
there. AWS publishes them in prose on each model's card in the Bedrock User
Guide, alongside caching support, context windows, service tiers and endpoint
support. Absence from an API is not absence from the docs.
Every dev GPT-5.6 row was wrong, and every error over-charged by exactly 20%
(corrected in the dev catalog 2026-09-06T15:59Z):
sol output 26.40 -> 22.00
terra in/out/cache read/write 2.64 / 15.84 / 0.264 / 3.30
-> 2.20 / 13.20 / 0.22 / 2.75
luna in/out/cache read/write 0.264 / 1.584 / 0.0264 / 0.33
-> 0.22 / 1.32 / 0.022 / 0.275
The 1.2x is not coincidence: Terra and Luna were sourced wholesale from the
GovCloud Price List rows, which are exactly 1.2x commercial. Sol's output was
the one figure with no source at all — a 6x input ratio inferred from GovCloud,
where the real ratio is 5x. `openai.gpt-5.4` was already correct, empty
cache-write cell included, so yesterday's prod fix is confirmed by the card.
This also resolves the tier/long-context gap PR-3 was blocked on, rather than
merely downgrading it as the previous commit claimed:
- Service tiers do not apply. Every card says Priority and Flex are not
supported for these models, so the 0.5x/2x dimension does not exist here.
- Long context is real, and the spec's "2x twin" was wrong: above the 272K
threshold input is 2x but output is only 1.5x. A flat 2x would have
over-priced long-context output by a third.
- We do not reach it. All rows carry maxInputTokens 272000, pinned at the
short-context boundary, and compaction runs at 100K — so one short-context
rate is correct, and that cap is what keeps it correct.
Noted for the prod rows: `global.openai.gpt-5.6-*` prices 9.1% below the `us.*`
Geo CRIS card across every bucket, and prod already runs Claude on `global.*`.
Prod should not be a copy of the dev rows.
The empirical work in the previous commit is not wasted — it is now the audit
of these published numbers instead of the source of them, and the 2026-09-06
Sol window should reproduce 4.40 / 0.44 / 5.50 / 22.00 rather than discover it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds `CURATED_BEDROCK_RESPONSES_MODELS` behind a new "Bedrock Responses" catalog tab, so the three GPT-5.6 models are one-click-creatable instead of requiring the escape-hatch form. Rates are the published Geo CRIS short-context row from each AWS model card — Geo CRIS is the tier the `us.*` inference profiles resolve to, and these models are inference-profile-only. Two values are pinned by test because they are pricing correctness, not preference: - `supportsCaching: true`. These models cache implicitly server-side with no way to turn it off, so `false` is not a preference but a false statement, and its only effect is to clear the cache-rate fields — pricing cached tokens at $0.00 while AWS bills them in full. On a warm conversation nearly every input token is a cached one. - `maxInputTokens: 272_000`. These have a 1M window but AWS prices them on two cards: above 272K, input costs 2x and output 1.5x. A CuratedModel holds one flat rate per bucket, so this cap is what keeps that single rate honest. Raising it silently opens the second price card. Fixes the curated `openai.gpt-5.4` Mantle entry in the same pass. It inherited `mantleDefaults()`' `supportsCaching: false`, so one-click-creating it produced exactly the mis-priced row that had to be repaired by hand in prod last night. Its card publishes a cache-read rate at 0.1x input and an em dash for cache write, so caching is on with a literal 0 write rate — 0 is the correct value rather than a missing one, because it makes `compute_wasted_usd` see a non-positive premium and return $0 instead of inventing waste. The `mantleDefaults()` comment claiming Bedrock caching is model-bound to Claude+Nova was simply wrong and is corrected. `claudeRates` becomes `ratesWithDerivedCache`: the 1.25x write / 0.1x read multipliers are not Claude-specific. The GPT-5.6 cards publish the same two, and commercial Cost Explorer billing reproduces them to four decimals — two model families, two independent sources, same ratios. `supportedParams` is deliberately absent from the new entries. AWS publishes no parameter table for GPT-5.6 (`model-parameters-openai.html` covers only the open-weight gpt-oss family), and a declared spec flips the #915 guard from permissive to restrictive — so an invented one would silently block parameters the model actually accepts. Better none than a guess. Not browser-verified: the page is admin-gated against the dev backend, so an unmerged frontend change cannot be signed in to. Layout risk is low — the tab strip is `flex-wrap` and the card grid is unchanged — but the visual check is worth doing on dev after merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…erivation-method GPT-5.6: correct every rate from the model cards, curate the models (PR-3), and fix the tool that missed them
The banner sat at the top of the shell. What it announces — a new model, a new capability — is acted on in the composer, so the notice now lives where the decision is made rather than in a corner the eye has already left. It mounts from `chat-input` beside `quota-warning-banner`, which is where users already look for ambient notices. It still floats rather than stacking in flow. `bottom-full` against a `relative` chat-input host puts it clear of the quota tabs, which stay attached to the input, and keeps the property from the previous change: measured before and after a dismissal, the composer, greeting and scroll container all move by exactly 0px. Restyled to match the sibling it now sits beside — a compact shrink-to-fit pill rather than a bar spanning the composer, which also shrinks how much it overlays. **It is now a chat-view surface only.** That is the real consequence of the move and it is deliberate: What's New remains the everywhere-record, which is why `panel` is forced onto every announcement server-side. The spec's §D1 is updated to say so rather than leaving the doc describing a placement that no longer exists, and the two admin help strings that told authors "a strip below the top nav" now describe where a banner actually appears. `chat-input` is reused by the agent-preview and marketplace test-drive panes, where a platform-wide notice would read as a bug rather than an announcement. A `showAnnouncements` input gates it, following the same opt-out shape as the `show*` controls beside it: default true, explicitly false at those two call sites, and threaded through `chat-container` so its embedded mode is off too. The shell mount and its `isAuthenticated()` gate are gone with it — the composer only exists inside an authenticated chat route, so the 401-on-login hazard that gate existed for is now structural rather than guarded. Three real test failures found and fixed on the way: two `chat-container` specs stub `app-chat-input` and needed the new input added to the stub, and one of the banner's own assertions was stale after the restyle. Full frontend suite 2486 passed, only the known `submission-review` flake outstanding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The composer is not always in the same place. A conversation pins it to the bottom of the viewport, where a pill below it would be off the edge. The empty state centres it with the greeting immediately above, where a pill above it floats over that greeting — visibly so at 375px, where the greeting wraps and the pill covered its second line. So the placement follows the composer: `below` on the empty state, `above` otherwise. Derived, not measured. `isEmptyState()` is the same computed that already picks which layout branch renders — the centred composer or the bottom-pinned one — so reading it makes the two impossible to drift apart. Measuring the composer's viewport position would re-derive that same fact less reliably and would have to be recomputed on resize, on scroll, and when the artifact pane opens. The banner takes a `placement` input and swaps `bottom-full`/`mb-2` for `top-full`/`mt-2`; `chat-container` supplies it through `chat-input` alongside the `showAnnouncements` gate. Verified on the empty state at desktop and 375px: the pill sits below the composer in clear space and `coversGreeting` is false in both, where it was true before. Full frontend suite 2490 passed — a clean run, including the `submission-review` spec that has been flaking. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ment-banner-near-composer feat(announcements): move the banner to the chat composer, on whichever side it leaves free
Artifacts become a first-class, shareable user asset; the platform gains a
way to tell users what shipped; and a follow-up typed mid-turn no longer has
to interrupt the turn it was meant to influence.
- Artifact library at /artifacts with live grid previews, rename, delete and
an in-app viewer — zero new tables and zero new indexes, because
user-artifacts was already partitioned by user
- Artifact sharing with named recipients or the whole tenant, revocable, with
a session-delete cascade; sharing a conversation now shares its artifacts
- "Shared with you" inbox behind CDK_ARTIFACT_SHARE_INBOX_ENABLED (default
off); the fan-out rows write unconditionally so the flip needs no backfill
- Feature announcements: admin CRUD with a full lifecycle, role targeting,
per-user acks, What's New panel, banner, modal and reach stats
- Mid-turn steering — a follow-up injected at the next tool boundary,
append-only against the cached prefix
- Single-file SPA rebranding via brand.config.ts, with generated brand and
OKLCH-banded surface themes plus favicons
- GPT-5.6 Sol/Terra/Luna curated with corrected rates; explicit cache
breakpoints measured 57% more expensive and ship off
- 47 Dependabot alerts and 40 CodeQL findings closed
Requires a CDK deploy: new {prefix}-announcements table (no GSIs), its IAM
grants, and bedrock:CallWithBearerToken on the inference-api role. No GSI
operations on any existing table.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| * Generate a favicon.ico file from PNG data. | ||
| * ICO format can contain multiple resolutions in one file. | ||
| */ | ||
| async function generateIco(pngBuffer: Buffer): Promise<Buffer> { |
| async function generateIco(pngBuffer: Buffer): Promise<Buffer> { | ||
| // Sharp can convert to ICO, but we need to handle the conversion properly. | ||
| // For simplicity, we'll create the ICO from the 32x32 PNG | ||
| const icon32 = await sharp(pngBuffer).resize(32, 32, { fit: 'contain', background: { r: 0, g: 0, b: 0, alpha: 0 } }).toBuffer(); |
Comment on lines
+13
to
+17
| import { | ||
| CHART_CATEGORICAL_PALETTE, | ||
| getChromeColorsForMode, | ||
| getCategoricalColor, | ||
| } from '../../../shared/constants/chart-colors.constants'; |
| try { | ||
| const raw: unknown = BRAND_CONFIG; | ||
|
|
||
| if (raw === null || raw === undefined || typeof raw !== 'object') { |
Comment on lines
+2
to
+9
| import { | ||
| DEFAULT_LOGO, | ||
| DEFAULT_APP_NAME, | ||
| DEFAULT_GREETING_TEMPLATES, | ||
| DEFAULT_FALLBACK_GREETINGS, | ||
| DEFAULT_COLORS, | ||
| DEFAULT_PAGE_TITLE, | ||
| } from './brand.defaults'; |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| try { | ||
| const raw: unknown = BRAND_CONFIG; | ||
|
|
||
| if (raw === null || raw === undefined || typeof raw !== 'object') { |
| * Generate a favicon.ico file from PNG data. | ||
| * ICO format can contain multiple resolutions in one file. | ||
| */ | ||
| async function generateIco(pngBuffer: Buffer): Promise<Buffer> { |
| async function generateIco(pngBuffer: Buffer): Promise<Buffer> { | ||
| // Sharp can convert to ICO, but we need to handle the conversion properly. | ||
| // For simplicity, we'll create the ICO from the 32x32 PNG | ||
| const icon32 = await sharp(pngBuffer).resize(32, 32, { fit: 'contain', background: { r: 0, g: 0, b: 0, alpha: 0 } }).toBuffer(); |
Comment on lines
+13
to
+17
| import { | ||
| CHART_CATEGORICAL_PALETTE, | ||
| getChromeColorsForMode, | ||
| getCategoricalColor, | ||
| } from '../../../shared/constants/chart-colors.constants'; |
| async with httpx.AsyncClient(timeout=10.0) as client: | ||
| response = await client.get(f"https://huggingface.co/api/models/{hf_id}") | ||
| except httpx.HTTPError as e: | ||
| logger.warning(f"HuggingFace pre-flight unavailable for {hf_id}: {e}") |
| ) | ||
| if response.status_code >= 400: | ||
| logger.warning( | ||
| f"HuggingFace pre-flight returned {response.status_code} for {hf_id}" |
Comment on lines
+393
to
+394
| f"Clamped max runtime from {requested_seconds}s to {effective}s " | ||
| f"to fit ${remaining_usd:.2f} remaining on {instance_type}" |
| logger.warning( | ||
| "could not snapshot artifacts for session %s — sharing " | ||
| "the conversation without them", | ||
| ShareService._sanitize_id(session_id), |
| import zipfile | ||
|
|
||
| try: # package context: unit tests and the app-api container | ||
| from .. import task_types |
| try: # package context: unit tests and the app-api container | ||
| from .. import task_types | ||
| except ImportError: # pragma: no cover - flat sourcedir inside the SageMaker DLC | ||
| import task_types # type: ignore |
| ) | ||
|
|
||
| #: Task types whose upload is an archive of a manifest plus image files. | ||
| ARCHIVE_TASK_TYPES: Tuple[str, ...] = tuple( |
|
|
||
| from pydantic import BaseModel, Field, field_validator, model_validator | ||
|
|
||
| from apis.shared.timestamps import from_iso, to_iso, utc_now_iso |
… a URL The comment at this call site has claimed to "validate the custom HuggingFace model ID format (org/model or just model)" since before this release, but the code only ever checked non-empty and length <= 200. That was latent on main; the multi-modal work (#944) made it reachable by interpolating the value into a Hub request path, which CodeQL flags as a critical py/partial-ssrf. The host is hard-coded, so this was never an arbitrary-host SSRF. What it did allow was a user-supplied value carrying URL structure — dot-segments, extra slashes, a query or fragment — changing the meaning of two sinks: the Hub pre-flight path, and `model_name_or_path` as forwarded to the training container. Adds the anchored repo-id pattern the comment already promised, and applies it at both sinks rather than relying on the pre-flight branch having run first — the hyperparameter sink should be safe on its own terms. Uses `\Z`, not `$`: `$` also matches immediately before a trailing newline, so an otherwise-anchored pattern would accept "org/model\n". A test pins that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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.
Release 1.18.0
Artifacts become a first-class, shareable user asset; the platform gains a way to tell users what shipped; and a follow-up typed mid-turn no longer has to interrupt the turn it was meant to influence.
Full notes: RELEASE_NOTES.md · Log: CHANGELOG.md
What's in it
/artifacts— live grid previews, rename, delete, in-app viewer. Zero new tables and zero new indexes;user-artifactswas already partitioned by user.CDK_ARTIFACT_SHARE_INBOX_ENABLED. The fan-out rows write unconditionally, so the flip needs no backfill.brand.config.ts+ generated brand/surface themes and favicons.Pre-merge checklist
VERSIONbumped 1.17.0 → 1.18.0,sync-version.sh --checkPASS"announcements": [](a brand-new table,CreateTable, exempt). No index operations on any existing table.CDK_ARTIFACT_SHARE_INBOX_ENABLED=trueset on theproductionenvironment, so the inbox tab rides this release's CDK deployDeployment
Requires a CDK deploy. New
{prefix}-announcementstable (no GSIs) + its IAM grants, andbedrock:CallWithBearerTokenon the inference-api role. This release touchesinfrastructure/lib/constructs/**andconfig.ts, soplatform.ymltriggers automatically on the push tomain— but the platform → backend order is not enforced (shared concurrency group, no ordering guarantee). Watch both runs; ifbackend.ymlwins the slot, the announcements routes 500 on a missing table until CDK lands.Two unflagged behavior changes take effect on the first turn after deploy: an omitted
supported_paramis now treated as unsupported rather than passed through, and the prompt-cache TTL is derived from the serving model rather than assumed.Post-merge: backmerge
main→developwith a merge commit.🤖 Generated with Claude Code