Release/1.19.0 - #991
Merged
Merged
Conversation
…-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.
Approve refused on a private agent showed the backend's message twice: inline on the page, next to the button that was just pressed, and again in the global toast in the corner. The toast is the worse copy of the two — further from the control, and it disappears on its own. Opt the review flow out via the existing SUPPRESS_ERROR_TOAST context token: the submission read, the diff, the review decision, and the withdrawal decision. Each of those already renders the backend's own message inline, and the diff's is load-bearing — it distinguishes "this submission predates snapshots" from a transport failure, which a toast would flatten into "something went wrong". Deliberately not applied service-wide, and there is a test pinning that: takedown has no inline error region on the Listings page, so the toast is its only surface and silencing it would turn a visible failure into a silent one. A call earns the opt-out by having inline UI, not by being in this service. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Verifying the toast removal on a running stack showed the inline message it was supposed to leave behind renders at the top of the page, 565px from the Approve button that produced it. The decision bar is sticky and the read above it is not, so on a submission with real instructions — this test agent had none, which is why it looked fine — the reviewer presses Approve at the bottom of a scrolled page and the explanation is off-screen above. That is the gap the global toast was covering, and removing the toast without moving the message would have turned a duplicated failure into a silent one. Split the two error regions, because they are read at different moments and from different scroll positions: a load failure is the first thing on an otherwise empty page and stays at the top; a refused decision now renders inside the sticky bar, directly above the buttons. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Generated by the kaizen-research skill. Top 5 ideas appended to docs/kaizen/review-queue.md for the kaizen-review-prep run later this morning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Generated by kaizen-review-prep. Ranked agenda for the 10-15 min decision pass; queue updated with this pass's resolutions. Executed in the queue (evidence-backed, per research/2026-08-28 Top 5 #4): - Retired the [2026-08-14] tool-mutation probe entry as "premise not substantiated" - Anthropic's caching docs contradict it and name no beta. It was last review's recommended #1. - Struck both MCP Apps verification prerequisites (both answered in favor of code we already ship) and down-ranked the entry. - Struck the stale "blocked by Strands #3758" caveat on the cookbook entry (Python-side fix shipped in 1.53.0 via #3858). Queue: 39 -> 38 open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…2026-08-28 chore(kaizen): weekly research scan 2026-08-28
…26-08-28 chore(kaizen): weekly review prep 2026-08-28
The upload page advertised "JSONL, CSV, or TXT" and accepted four extensions, but train.py only ever looked for a .csv. A JSONL dataset — the format the copy names first — uploaded fine, dispatched fine, then died on the GPU with `No CSV file found in /opt/ml/input/data/train` about five billed minutes in. The user lost the time, the quota and $0.12, and got a SageMaker AlgorithmError instead of a reason. Teach the trainer JSONL and JSON alongside CSV, and validate that the "text" and "label" columns the page promises are actually present. The reader dispatch is a table rather than an if/elif chain so the supported-format contract can be asserted without importing pandas, which exists only inside the SageMaker training container. That absence is why the loader had no test coverage and why this survived: the old tests could only reach find_csv_in_channel, never the read itself. Drop .txt rather than support it — a training record needs both a text and a label, and a newline-delimited text file cannot express the label without guessing a delimiter. It stays valid for inference input, which is unlabelled, so that page is untouched. Reject unreadable formats at /presign and again at POST /jobs. The second gate is the one that matters: it is the last point before SageMaker provisions a GPU, so a doomed dataset can no longer cost anyone five minutes of ml.g5.xlarge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The dashboard reported $0.00 and 0 jobs for every period while jobs were
being billed, with the cost sitting in plain sight on the records it was
meant to aggregate:
{"status": "COMPLETED", "billable_seconds": 300, "estimated_cost_usd": 0.1175}
{"status": "FAILED", "billable_seconds": 296, "estimated_cost_usd": 0.1159}
Three separate faults, each of which hid the next.
The StatusIndex GSI partition key is compared case-sensitively, and the
query used SageMaker's "Completed"/"Stopped" spelling. Records store
"COMPLETED"/"STOPPED" — routes.py maps between them on write — so the
query matched nothing, every time. Against real dev data: "Completed"
returned 0 rows where "COMPLETED" returned 2.
FAILED was excluded entirely. AWS bills a job that dies partway through,
so leaving it out understates spend even once the casing is right. The
user-facing quota counter already charges for failures; the admin view
now matches it.
Training and inference records share this table and this index, and only
the inference query filtered by job_type. Fixing the casing alone would
have exposed that: the training query returns the inference row, whose
record has no model_id, so _item_to_dict raises KeyError and the
dashboard 500s. Filter training on the JOB# sort-key prefix.
Adds the coverage this endpoint never had — the reason three faults sat
here undetected. Verified against dev: 1 training COMPLETED, 1 training
FAILED, 1 inference COMPLETED, each counted once, totalling 826s /
0.229h — matching the independent quota counter exactly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fine-tuning has been unreachable in every deployed environment. The tables, bucket, SageMaker execution role and IAM grants all ship unconditionally, but FINE_TUNING_ENABLED — which mounts the /fine-tuning and /admin/fine-tuning routers, and defaults to "false" in Python — was never set on the app-api container. Live deployed dev: /api/sessions returns 401, /api/fine-tuning/access returns 404. What made this hard to see is a name collision. A repo variable CDK_FINE_TUNING_ENABLED exists and reads "true", so the settings say the feature is on. But that flag gated whether the SageMaker *stack* deployed, and its consumer was deleted in the single-stack migration (#396) — "deploy-everything-always". It has had no reader since. Three links were missing, not one; wiring any subset still yields nothing. config.ts now resolves the flag, the app-api construct sets it on the container, and platform.yml forwards it — the workflow passed no fine-tuning variables at all. CDK_FINE_TUNING_CORS_ORIGINS was never forwarded either, which is why the bucket's origins have only ever come from the global CDK_CORS_ORIGINS. Same treatment for FINE_TUNING_DEFAULT_QUOTA_HOURS, whose repo variable (10) was equally stranded. Absent, it defaults to 0 = whitelist-only, so users would meet a 403 rather than the intended automatic grant — a quieter failure than the 404, and a likely next bug report. Default ON with a kill switch, following the agentMarketplace idiom: an unset Actions variable arrives as an empty string, so only the literal "false" disables. Switching it off leaves storage untouched, so no dataset or trained model is orphaned. The config fields are required rather than optional, which immediately surfaced ten hand-built fineTuning: {} literals in the CDK tests. That is the type system catching exactly the omission behind this bug, so they are filled in rather than the fields made optional. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…review-duplicate-error-toast fix(marketplace): stop the review flow reporting failures twice
…formats-costs-and-runtime-flags fix(fine-tuning): make the feature work end to end
`instance_type` arrives straight off the request body as a free-form optional string, unvalidated, on both the training and inference create paths. `calculate_cost` resolves it against INSTANCE_COST_PER_HOUR with a 0.0 fallback, so anything outside that eleven-entry map runs real GPUs and records $0.00 spend — the same invisibility the StatusIndex casing bug produced, arriving by a different route. The quota does not bound it. It meters GPU-*hours*, not dollars, so the same ten-hour allowance buys roughly $14 on the ml.g5.xlarge the catalog offers, or several hundred on a larger unlisted instance, and the admin dashboard reports zero either way. Validate the resolved instance type on both create paths and 400 with the supported list, mirroring the dataset-format guard. Validating after resolution rather than on the request field also covers a bad value reaching inference from a stored training job record. Not reachable from the SPA, which renders instance type read-only — this is an API-level hole, and worth closing before the default quota opens the endpoint beyond the current whitelist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…validate-instance-type fix(fine-tuning): reject instance types we have no price for
…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
Groundwork for serving the library's user-wide listing from an index
instead of a base-table Query. Nothing reads it yet — the query switch
ships separately, for the deploy-ordering reason below.
## Why an index at all
The base table is already partitioned by user, so this was never about
reachability: `list_for_user` can Query PK=USER#{uid} today. It is about
what that Query has to read. HEAD and version rows share the partition,
so it spans roughly 3x the rows it returns, then filters and date-sorts
them in memory — which is also what makes it unpaginable, since the page
boundary would fall in the wrong place. Only HEAD rows carry GSI2PK, so
the index holds one row per artifact, already newest-first.
## The backfill is the load-bearing half
`UserArtifactsIndex` is sparse. A HEAD row without GSI2PK is not stale
in it — it is absent from it forever, silently. The writer began
stamping the keys on 2026-09-04; every row written before that has
neither attribute, so switching the query without backfilling would drop
every older artifact from its owner's library with no error anywhere.
`backfill_artifact_user_index_keys.py` stamps them. Dry-run by default,
idempotent (`attribute_not_exists(GSI2PK)`, so it also yields to the
writer), and it never resurrects a row deleted mid-run
(`attribute_exists(SK)`).
It reads `updated_at` to build GSI2SK and never assigns it — that
attribute is embedded in both GSI sort keys and is writer-owned, the
same restraint `rename` observes. A HEAD row lacking `updated_at` is
reported by name rather than stamped with a fabricated timestamp that
would sort it wrongly forever.
Run on dev: 22 HEAD rows, 22 stamped, 0 skipped, 0 failed; re-run
reported 22 already-stamped, 0 stamped. Verified independently
afterwards: 0 rows missing GSI2PK, 0 version rows wrongly stamped, and
GSI1SK == GSI2SK on all 22 — GSI1SK is the *writer's* own
`ARTIFACT#{updated_at}#{aid}`, so that agreement checks the backfill's
format against production-written data rather than against a reading of
the code.
## Deploy notes
Adding this is one `UpdateTable`, and only one GSI may be added per
`UpdateTable` — the committed `gsi-inventory.json` gains exactly one
line here, so no release split is needed.
CFN reporting UPDATE_COMPLETE does NOT mean the index is ACTIVE;
DynamoDB backfills it asynchronously. That is why the query switch is a
separate PR: platform.yml and backend.yml share a concurrency group but
their order is not enforced, so shipping both together risks app-api
querying an index that does not exist yet.
Infrastructure: 784 passed. Backend: 186 passed across the artifact
suites, including 10 new tests for the script.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…evelop-1.18.0 # Conflicts: # backend/src/apis/app_api/fine_tuning/routes.py # backend/tests/fine_tuning/test_job_guards.py
…nto-develop-1.18.0 Backmerge/main into develop 1.18.0
`isLoading()` tested `sessionsResource.value() === undefined`, which stopped being true during the cold-start fetch. The loader short-circuits to `null` when `sessionsRequest` is false, so the resource *resolves* before any session is fetched. That path is now the ordinary one: `SessionService`'s constructor only enables loading if the BFF session is already authenticated, but the service is constructed during the APP_INITIALIZER pass — `AnnouncementModalService` -> `MessageMapService` -> `SessionService` — and Angular's `runInitializers` invokes every initializer synchronously before awaiting any of them, so `bootstrap()` is still in flight and `isAuthenticated()` is false. The auth effect enables loading afterwards, but `reload()` preserves the previous value, so `null` survives the whole real fetch. `isLoading()` read false, `groupedSessions()` was empty, and the sidebar rendered the empty state until the response landed. `isLoading()` now means "nothing to draw yet": no API response (`undefined` before the first load resolves, or the short-circuit `null`) and no locally cached rows. An empty `sessions` array is a real response and still falls through to the empty state. `error()` is checked first — reading `value()` on an errored resource throws, which the old ordering walked into. Deliberately not keyed on `status() === 'reloading'`: `refreshSessions()` reloads after send/rename/mark-unread, and that would flash the list to a skeleton every time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The "Shared with you" inbox shipped default-off and opt-in in 1.18.0, and the flag documented exactly why: "the surface it gates lands before the product decision about it does." That decision has now been made — the inbox is live — so the reason for the inversion is spent. Carrying an opt-in default past it has a real cost for a repo that is forked: every institution deploying this stack would silently lose a finished feature and have to discover a variable to get it back. Default-on is the right answer for a fork, and "false" still turns it off for anyone who wants it dark. Flipped in all four places that encode the default: - feature_flags.py: `== "true"` -> `!= "false"` (house style, matching announcements_enabled / scheduled_runs_enabled) - config.ts: the opt-in ternary -> the kill-switch ternary, `?? false` -> `?? true`, same shape as scheduledRuns - platform.yml and app-api-service-construct.ts: the comments that told a reader the opposite The empty-string case is now load-bearing in the other direction: an unset GitHub Actions variable forwards "", which must resolve to ON. That is the single most likely way to ship this flag silently disabled, so it is pinned by test on both sides of the boundary — and the backend tests are mutation-checked (reverting the flag fails exactly the two new ones). Adds CDK config coverage for this flag, which had none at all: unset, empty, "false", "true", and the cdk.json context path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-share-inbox-default-on feat(artifacts): default the share inbox on, with a kill switch
…ion-list-loading-state fix(sidenav): show the skeleton, not "No Chats Yet", while sessions load
…-user-index-backfill feat(artifacts): add UserArtifactsIndex and backfill the keys it needs
The banner pill renders no body, so its only affordances were a ✕ and an optional external CTA — which trains people to dismiss unread, with What's New (buried in the user menu) as the only other route to the content. The text is now a button that opens the announcement's own dialog, and the dialog owns the ack from there: any of its exits writes `dismissed` (or `acknowledged`) and retires the pill durably. Having read the body is a stronger signal of consumption than clicking ✕ on a one-line strip, and leaving the pill up afterwards would only ask the user to dismiss something they have already dealt with. It opens the single-announcement dialog rather than the What's New list deliberately: the pill named one thing, and handing back a list to search through is a worse answer than the thing itself. Three supporting changes: - `AnnouncementModalData.sourceSurface` (default `modal`) attributes every ack to where the gesture happened, so reach stats can tell a banner that earned a read from an interruption nobody asked for. No backend change: `AnnouncementSurface` is a plain literal and the ack route never cross-checks it against the announcement's own `surfaces`. - `AnnouncementModalService.openFor()` is the one entry point for a user-initiated open. It skips the §D8 turn-safety gate — a click is not an interruption — but still respects `openRef`, and marks the item `shown` so the auto-modal cannot re-interrupt with something already read. - A `requiresAck` announcement gets no ✕ on the pill. Suppression is rank-based and covers the banner *and* modal slots alike, so a ✕ there let a user retire a compliance notice before the blocking modal ever fired, leaving no `acknowledged` record anywhere. The text button uses an `sr-only` prefix rather than an `aria-label`: the visible line may be the summary, so a title-derived label would leave the visible words outside the accessible name (WCAG 2.5.3, Label in Name). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ment-banner-opens-detail feat(announcements): open the full announcement from the banner text
A session's AbortController was created per request but released only by
the Stop button — never on normal stream teardown. So after any turn that
finished on its own, `streamingSessionIds()` kept reporting that session
as in-flight for the life of the tab, and the page-hide attribution
handler signalled `navigated_away` for it on the next refresh, tab close,
or cross-document navigation. One departure marked every session ever
streamed in that tab.
The result is a "Response interrupted" chip plus a Continue button on a
complete answer, and — invisibly — a false `<interruption_note>` prepended
to that session's next prompt and persisted in its history, telling the
model its previous response was cut off and to pick up where it left off.
Measured in prod: 1,678 sessions carry the marker and 1,505 of them (90%)
received it more than 30s after their last message landed; 1,475 of those
are `navigated_away`. Not model-specific and not new — the oldest is
2026-08-21.
Two changes:
* `releaseAbortController` on stream teardown (identity-checked, so a
superseded stream's late close can't clear its replacement's handle).
This makes `streamingSessionIds()` mean what its docstring claims.
* `POST /sessions/{id}/interrupt` now verifies "mid-turn" against the
session's single-flight lease before recording `navigated_away`, rather
than taking the client's word for it. Old tabs run the old SPA long
after this ships. `user_stopped` is deliberately not gated: the button
only exists while streaming, and it also arms distributed cancellation,
which must still reach a turn whose lease read fails open.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One-shot cleanup for the markers the stale-controller bug left behind. Each one shows a "Response interrupted" chip plus a Continue button on a complete answer, and puts a false <interruption_note> into that session's next prompt — persisted in history, invisible in the UI. Markers self-clear only at the start of that session's next turn, so a conversation nobody returns to stays armed indefinitely. Clears only what is provably false: reason `navigated_away`, marked more than 900s after the session's last message. Gap alone is not enough — an interrupted continuation persists no assistant message, so `lastMessageAt` stays at the previous turn and a real interruption can show a modest positive gap. But the SSE stream times out at 600s, so nothing is still running 15 minutes later. `user_stopped` and `connection_lost` rows are never touched: neither comes from this bug, and clearing one would destroy a real record. Dry-run by default. Each write is conditional on the exact timestamp and reason the scan read, so a session re-marked in between is skipped rather than clobbered; re-running is a no-op. Prod dry-run (2026-09-06): 1,680 marked rows → 787 provably false, 847 left alone as ambiguous, 46 left alone as another reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…upted-turn-marker fix(sessions): stop marking completed turns as interrupted
…urn end
`displayText` is the clean copy of what the user typed, and what the UI
renders whenever the prompt sent to the model was augmented — RAG context,
attachment guidance, an MCP App context block, or the `<interruption_note>`
addressed to the model after an interrupted turn.
It was written by a single call site in the stream coordinator's success
path, as the last thing a turn does. Nothing on the Stop, disconnect, or
error paths wrote it, so any turn that didn't reach that line left the raw
augmented prompt as the only thing the UI could render. Two effects, both
reproduced on dev:
* transiently, for every augmented turn — reload while it is still
streaming and the note is in the user's own chat bubble;
* permanently, for any turn that never completes. Stop it and the `D#`
record is never written, so the note stays in the bubble on every load.
The permanent case lands where it hurts most: a turn only carries an
interruption note because the PREVIOUS turn was interrupted, so the note is
disproportionately likely to be on a turn that gets interrupted too. Not
model-specific — reproduced on GPT-5.6 Luna and Claude Haiku 4.5.
`DisplayTextHook` writes the record on `MessageAddedEvent` instead — the
moment the user's message enters history, before the model call — so every
later exit path already has it. Writing at request start instead would risk
a record keyed to an index the failed turn never filled, which a later
message would inherit; anchoring to the append keeps index and record
together.
Two subtleties the hook has to respect:
* Not every role-`user` message is the user. Tool results carry that role,
and Strands prepends a SYNTHETIC tool-result message ahead of the prompt
when history ends on a dangling `toolUse` — exactly what an interrupted
tool turn leaves behind. Content carrying `toolResult`/`toolUse` is
skipped so the arm is spent on the user's own message.
* The agent instance is cached across turns (#741/#751), so the hook is
armed unconditionally at the head of every turn, including to None.
The coordinator's write stays as a backstop for wrappers that carry no hook
(voice, tests) and for a failed hook write, gated on `wrote_this_turn` so the
normal path still makes exactly one put.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-on-interrupted-turns fix(sessions): store displayText when the user's turn lands, not at turn end
A correctness release for interrupted turns, plus the artifact share inbox coming out of the dark. Two unrelated defects made a conversation misreport its own history — a completed response could be labelled "Response interrupted" with a Continue button that would resume an already-finished answer, and an interrupted one could show the model-directed <interruption_note> inside the user's own chat bubble, permanently. Both are fixed at the source rather than at the render layer. - Minor, not patch: the share inbox flips from opt-in to default-on with a kill switch (a dark feature turned on by default is user-facing), and the announcement banner gains a new interaction. - Requires a CDK deploy: UserArtifactsIndex on the existing user-artifacts table. One index operation on one existing table — within the one-GSI-per-UpdateTable limit, so no split is needed. Wait for ACTIVE, not just UPDATE_COMPLETE. - CDK_ARTIFACT_SHARE_INBOX_ENABLED changes meaning: an environment that never set it gains the "Shared with you" tab on this deploy. - Two optional one-shot scripts ship with it, both dry-run by default. - PR #989 (the UserArtifactsIndex reader) is deliberately held for the next release: its query has no fallback, and its preconditions — index ACTIVE, then backfill run — are not enforced by the release's deploy order. 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}
| if not await is_session_lease_held(session_id, user_id): | ||
| logger.info( | ||
| "Ignoring navigated_away for session %s — no turn in flight", | ||
| session_id, |
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.
Ships
1.19.0— a correctness release for interrupted turns, plus the artifact share inbox coming out of the dark.Why minor, not patch
Two already-merged changes set the level, independent of the fixes:
ARTIFACT_SHARE_INBOX_ENABLEDfrom opt-in to default-on with a kill switch. Turning a dark feature on by default is user-facing.What's in it
<interruption_note>in the user's bubbleUserArtifactsIndex+ backfill (groundwork — nothing reads it yet)Both interrupted-turn fixes were validated on dev in the browser, on GPT-5.6 Luna and Claude Haiku 4.5, not just by unit tests.
GSI pre-merge check (§1)
Passes — no split needed.
Read by hand as well, per the skill's instruction not to let the automation replace the question — the
gsi-inventory.jsondiff is exactly one added line under one existing table:Deploy notes
UserArtifactsIndexto reportACTIVE;UPDATE_COMPLETEon the stack is not the same thing.CDK_ARTIFACT_SHARE_INBOX_ENABLEDchanges meaning — an environment that never set it gains the "Shared with you" tab. Set"false"first to keep it dark.--apply: the artifact GSI key backfill, and the false interrupted-marker cleanup (787 rows on prod at last dry-run).Deliberately held back
#989 (
serve the library listing from UserArtifactsIndex) is CI-green but is not in this release. Its query has no fallback — a missing index raisesArtifactQueryErrorand the library page fails — and its preconditions (indexACTIVE, then backfill run) aren't enforced by the release:platform.ymlandbackend.ymlhave no ordering between them, and the sparse index silently under-lists until backfilled. It ships next release, into an environment where its precondition is already true.Version
VERSION1.18.0 → 1.19.0,sync-version.sh --checkpasses across backend, frontend, infrastructure, and tui manifests plus all four lockfiles.🤖 Generated with Claude Code