From f2068cf71735cc839791a75e0e765a3f6cad9ccc Mon Sep 17 00:00:00 2001 From: Phil Merrell Date: Sun, 6 Sep 2026 20:14:01 -0600 Subject: [PATCH] Release/1.19.0 (#991) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(marketplace): submitting an agent makes it public, with a notice 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 * feat(marketplace): let admins read, test-drive and decline a submission 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 * fix(marketplace): make the review test drive big enough to use 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 * feat(kb): managed knowledge base migration — inert behind flags, upgrade 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. * fix(kb): pass the Environment tag value instead of guessing it (#885) 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. * feat(kb): ship the real kb-migration Lambda image 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). * fix(kb): give the dispatcher the worker name it actually reads 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. * fix(kb): grant the tag permissions provisioning and reconciliation need 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. * fix(kb): wait for the knowledge base to be ACTIVE before its data source 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. * fix(marketplace): stop the review flow reporting failures twice 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 * fix(marketplace): report a refused decision beside the decision buttons 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 * chore(kaizen): weekly research scan 2026-08-28 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) * chore(kaizen): weekly review prep 2026-08-28 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) * fix(fine-tuning): read every dataset format the UI accepts 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 * fix(fine-tuning): make the admin cost dashboard count real spend 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 * fix(fine-tuning): wire the runtime flags that gate the feature 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 * fix(fine-tuning): reject instance types we have no price for `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 * fix(chat): let arrow keys walk the @-mention menu The textarea binds keyup to the caret-move handler so a click or an arrow key re-derives the `@…` token, and `syncMentionToken` ended by resetting `mentionActiveIndex` to 0. Arrow keys are preventDefault'ed in `onKeyDown`, but their *keyup* still fired that resync — so every ArrowDown moved the highlight down and immediately dragged it back to the first row, making the menu impossible to walk. Reset the highlight only when the token itself changed (query or start), so typing still restarts at the first row while bare caret events leave the selection where the user put it. Also scroll the active row into view: the list scrolls at eight rows (`max-h-72`) and the parent owns the keyboard, so arrowing past the fold moved a highlight nobody could see. Adds the composer's first spec, covering the keyboard path end to end. Co-Authored-By: Claude Opus 5 * feat(chat): set @-mentions apart in the user's message A mention rendered as plain prose inside the message bubble, so nothing told the reader that `@Brand Deck Builder` was an address rather than something they happened to type. Render mention runs `font-semibold text-white` against the bubble's `text-white/90` body. Matching is driven by the known agent names from `AgentMentionService` rather than a `@\w+` pattern: agent names contain spaces, so a word pattern would bold only the first word, and it would also bold `@here`, npm scopes and email addresses. The list is the same session-cached one the composer's `@` menu already warms, so this costs nothing on the render path; the component calls `load()` so a cold reload straight into a thread still bolds, and text renders plain until the names arrive. The stored message is untouched — the literal `@Name` is still exactly what was sent. Co-Authored-By: Claude Opus 5 * fix(kb): record the knowledge base id before anything else can fail The retry after the ACTIVE fix did not reach the data source. It could not: ConflictException: KnowledgeBase with name dev-boisestateai-v2-kb-ast-1a90784a7f18 already exists. `awsKbId` was only ever written by `attach_aws_ids`, which runs after BOTH AWS creates succeed. So a failure between them left a record with no identifier, and every later attempt re-entered the create path and was refused, permanently, because the name was taken. The record was unrecoverable by any number of retries. The `clientToken` does not cover this, and this commit corrects that claim wherever it appears — including in a message I added two commits ago. AWS idempotency tokens expire within minutes; a retry an hour later is a genuinely new request that collides on the unique name. Two changes: * `records.attach_knowledge_base_id` persists `awsKbId` the moment the create returns, guarded on `attribute_not_exists(awsKbId)` so a straggler cannot overwrite a newer attempt's identifier. Everything after that point is resumable. * Provisioning adopts by name on a name-collision `ConflictException`. This is what recovers records already stuck in that state, including the one in dev — without it the only recourse is deleting the knowledge base by hand. Adoption is safe because the name is derived from `app_kb_id`, so a collision can only be this knowledge base's own earlier attempt. The status conflict and the name conflict share the `ConflictException` code, so they are told apart by message. WHY TWO TESTS CERTIFIED THE BUG `test_the_record_survives_as_a_discoverable_retry_anchor` asserted `"awsKbId" not in anchor` — the missing write, encoded as a requirement — and `test_the_retry_does_not_create_a_second_knowledge_base` asserted the retry re-issues the create and relied on the fake deduplicating it. `FakeBedrockAgent` modelled `clientToken` dedup as permanent and did not model name uniqueness at all, so both passed against a design that could not recover. The fake now enforces name uniqueness, treats tokens as expired by default, and answers `list_knowledge_bases`. Both tests are rewritten to assert what actually makes the window survivable, and a third covers the stuck-record state directly. Mutations verified caught: dropping the immediate persist, and dropping adopt-by-name. Tests: 2,465 passed across shared, lambdas, property, architecture and supply_chain. * fix(kb): drop the embedding pin, defer verify, and complete a migration in dev A migration now runs shadow -> verify -> promote -> retain and serves from the managed backend. Three further defects, all found by driving the state machine locally against dev instead of through a deploy cycle. THE EMBEDDING PIN AND MANAGED RERANKING ARE MUTUALLY EXCLUSIVE (Req 8.5 amended) Req 8.5 pinned titan-embed-text-v2:0 via `embeddingModelType: CUSTOM`; Req 11.2 requires `rerankingModelType: MANAGED`. AWS rejects the combination, and §13 had measured the two separately, never together. Measured all four: CUSTOM + MANAGED -> ValidationException CUSTOM + NONE -> ok, scores 1.00/0.982/0.952 (flat) default + MANAGED -> ok, scores 0.413/0.199 (separated) default + NONE -> ok The pin loses, for a better reason than "it buys little": it protected a failure mode that cannot occur here. On S3 Vectors *we* embed the question, so the query model must match the index. Managed retrieval sends text and managed ingestion sends text — we never produce a vector, so Bedrock embeds both sides and consistency is its invariant. §13 measured the pin as worth nothing (9/9 identical) against reranking being "what makes a small context cap defensible". `embeddingModelId`/`embeddingDimensions` are no longer recorded either: with no pin, nothing here knows what Bedrock chose, and a field naming Titan on a knowledge base embedded with something else is worse than an absent one. VERIFY FAILED A GOOD MIGRATION FOR BEING ASKED TOO EARLY The canary returned nothing because the freshly-ingested document was not yet queryable, and that was terminal. Measured ~45 s from ingest to retrievable on a fresh knowledge base, against the docstring's 0.75-1.03 s (a warm figure). Now defers via `records.defer_verify`, bounded at MAX_VERIFY_ATTEMPTS, so latency reads as latency and only a corpus that never answers fails. ADOPTION TOOK A KNOWLEDGE BASE THAT WAS BEING DELETED Found while recreating one locally: the delete had not finished, adopt-by-name took the DELETING knowledge base, and the ACTIVE wait then refused it. Adoption now skips terminal statuses — the name is about to free up, so a fresh create is right. ALSO A test that only passed while MANAGED_KB_SERVICE_ROLE_ARN was absent now deletes it explicitly; the variable is needed in backend/src/.env for the local driver. `scripts/local-dev/run-kb-migration.py` drives the state machine in-process against dev. Three of the last five defects would have been minutes each with it. HANDOFF.md is rewritten: it previously said "Nothing deployed", which has been untrue since 1.16.0 shipped the feature to production behind flags. Tests: 6,817 backend passed (5 pre-existing Strands failures), 626 infra. * fix(kb): grant bedrock:StartIngestionJob, which authorizes direct ingestion A document uploaded to a promoted knowledge base in dev went to `failed` with: AccessDeniedException ... IngestKnowledgeBaseDocuments ... not authorized to perform: bedrock:StartIngestionJob on resource: knowledge-base/M8WQZVQJ8X AWS authorizes `IngestKnowledgeBaseDocuments` under the adjacent action name `bedrock:StartIngestionJob`; both appear in one statement in AWS's direct-ingestion prerequisites. The grant carried only the name matching the API call, so it reviewed as complete, deployed clean, and failed on the first real upload — the same shape as the missing `bedrock:TagResource`. The worker had the identical gap. It receives the same grant and calls the same API, so the first migration driven by the deployed dispatcher would have failed identically. It stayed invisible because every migration so far was driven by scripts/local-dev/run-kb-migration.py under an SSO identity broader than either Lambda role. The action is easy to mistake for a mistake: Requirement 9.2 forbids *calling* StartIngestionJob (0.1 RPS account-wide, one document per ten seconds) and nothing does. Holding it is authorization, not invocation. A docblock and a separately-named test carry that reason so the obvious cleanup fails a test that explains itself. `bedrock:ListKnowledgeBaseDocuments` is in AWS's example policy and left out on purpose: no code path calls it. Guards: three tests, including one asserting both the worker and the ingestion-consumer roles carry the action. Mutation-tested — removing the action fails exactly four tests, each named for the reason. * docs(kb): record the StartIngestionJob defect, two open findings, and retract a false alarm Retracts the "known unknown" that an earlier revision listed as the top open risk. `document_id` and `relevance` were reported empty from the facade after promotion, with the inference that the status filter was either not running on the managed path or losing the join key. Both inferences were wrong. Measured against the live dev knowledge base: the raw Retrieve carries `location.customDocumentLocation.id`, `_to_chunk` produces it, and the facade exposes it at `metadata.document_id` with relevance as its exact negation in `distance`. The probe had read two top-level keys that have never existed — `git log -L` on the formatted_results block confirms the four-key shape back to the function's first commit — so the observation said nothing about either backend. The filter is live and did join: it matched DOC-ae5cc5434f2d and kept both chunks because that document reads `complete`. Adds defect 31, the missing `bedrock:StartIngestionJob` grant, fixed in fdf15d21, and generalizes it in §3: three times now a Bedrock grant listing exactly the API the code calls has deployed clean and failed on first real use, because AWS checks an adjacent action name. Also records the structural consequence for §2's local driver — an SSO identity is broader than every Lambda role, so that driver cannot find this class of defect at all. Adds two findings that came out of checking the false alarm, both still open: - 32. Routing exclusivity is enforced only on the consumer's side. The legacy handler has no engine gate in either copy and its S3 notification is still live, so a document added to a promoted knowledge base is indexed twice. Visible in real data on DOC-dc8b65658e29. Every exclusivity test covers the side that works. - 33. `_filter_vectors_by_document_status` opens with `if not doc_ids: return vectors` — one fail-open path in a function that fails closed everywhere else. Also corrects the stale claims that the worker image is undeployed and that the tree has uncommitted work, records what is verified working in dev today (including that a born-managed knowledge base is not implemented — `newDefault` has zero readers), refreshes the jest count to 634, and fixes a duplicate defect number 24 by renumbering the run to 25–33. All §5.x cross-references are internal to this file and were updated with it. * fix(kb): the legacy pipeline stands down for a promoted knowledge base Routing exclusivity was only ever enforced on one side. The managed ingestion consumer returns immediately for a legacy document, but the legacy pipeline had no engine gate at all, and its `s3:ObjectCreated` notification is still live alongside the consumer's EventBridge rule. So every document added to a promoted knowledge base was handled twice, and design.md §537's "a document is indexed on exactly one backend" was untrue. The duplicate vectors are the cheap half of the problem. The expensive half is that two writers owned one `status` field and the last one won by luck. Both outcomes were observed in dev: * `DOC-b5d5d8019f44` was marked `complete` by the legacy pipeline at +30 s while the managed knowledge base — which is what actually serves this assistant — could not answer for it until +95 s. Sixty-five seconds of "your document is ready" followed by an answer that does not mention it. That is precisely the failure ingestion_consumer.py's docstring says it polls to prevent, defeated by a second writer nobody had gated. * `DOC-d637491d6cb1`, an image-only 4-year flowchart, failed Docling outright (`Docling produced zero chunks`) and was marked `failed`, while Bedrock's image extraction indexed it successfully and served it. It only ended up reading `complete` because the managed consumer happened to finish second. Reverse the finishing order — purely a function of parse time — and a good, retrievable document reads `failed` permanently, with no retry endpoint to recover it (task 14.4 is still open). So `handler.py` now resolves the engine before writing any status and returns early for `managed`, doing no parse, no embed and no status write. Keyed on the ENGINE, not on the presence of a KB record and not on `migrationState`. During `shadow` and `verify` the legacy path is still authoritative and must keep working (Requirements 16.1, 16.6); only `promote` writes `retrievalEngine`. Two tests pin that, including one for a record in `shadow`. An unreadable record resolves to legacy on purpose, because the two errors are not symmetric: wrong towards legacy costs a duplicate index while the consumer still drives the document to a correct terminal state, whereas wrong towards skipping leaves an upload un-ingested with no error and no way out but re-uploading. Same convention as resolver.load_record. Delegates to `records.resolve_engine` rather than reading the attribute inline, so "absence means legacy" keeps exactly one definition — the same call the consumer makes for the mirror-image decision. That needs `apis/shared/kb_backend/` in the rag-ingestion image, which is cheap: the package's module-level imports are stdlib only, enforced by test_kb_backend_boundary.py. Added to the Dockerfile and to build-one.sh's SOURCE_DIRS so the content hash moves with the gate. No IAM or environment change: the Lambda already carries DYNAMODB_ASSISTANTS_TABLE_NAME and already holds dynamodb:GetItem (verified against the deployed function and role in dev). The bootstrap copy at infrastructure/bootstrap-assets/rag-ingestion/handler.py needs no gate — it is a 33-line no-op placeholder that indexes nothing. Guards: 8 tests in tests/ingestion/test_ingestion_engine_gate.py. Three mutations verified caught, each by correctly-named tests and each parsed before running so a syntax error could not masquerade as a detection: - gate removed -> the 3 "promoted KB is left alone" tests - fail-open inverted -> the unreadable-record test - Dockerfile COPY dropped -> test_lambda_image_imports[rag-ingestion] Backend suite 6,825 passed; the 5 failures are the pre-existing Strands SDK contract tests documented in HANDOFF §2. * fix(kb): propagate document deletion to the managed knowledge base The mirror of the ingestion gate in the previous commit. Ingestion now routes by engine; deletion did not route at all. `cleanup_service` removed the legacy S3 Vectors copy and the `DOC#` row and never touched the managed knowledge base, so on a promoted knowledge base a deleted document stayed indexed forever. The managed delete path existed — `kb_backend/tombstones.py` — but its only callers are in the reconciler, which is report-only with `reconcilerArmed` off. Three consequences, none of which raised anything: * Storage was paid for indefinitely, at $5.00/GB-month against S3 Vectors' ~$0.15 — orphans on the expensive engine. * Retrieval quietly degraded. The status filter runs AFTER retrieval, so each orphan consumed a slot in `top_k` and was then dropped: a query could return five chunks and the model see two, with nothing logged. * The only thing preventing deleted content from being served was the fail-closed status filter, which has exactly one fail-open branch (`if not doc_ids: return vectors`). That line became load-bearing in a way it was never designed to be. `cleanup_document_resources` gains a third phase, engine-gated, conjoined into `all_succeeded` so a failure blocks the hard delete. The asymmetry with the ingestion gate is deliberate. There, an unreadable KB record resolves to legacy, because being wrong costs a duplicate index while the consumer still finishes the document. Here it must FAIL: the `DOC#` row is what the status filter joins against, so reporting success on a failed managed delete would remove the row *and* leave the content — the one combination that turns a storage leak into a disclosure. `ManagedKbNotProvisioned` is the exception, treated as terminal success: nothing was ever indexed, so there is nothing to remove and no reason to retry. IAM — the part with no code to give it away ------------------------------------------- Neither the app-api task role nor the kb-sync worker could delete from a managed knowledge base, so the code above would have deployed clean and failed on first use. New `grantManagedKbDocumentDeletion`, deliberately NOT `grantDirectIngestion`: these callers only ever remove documents, so a bug in the delete path cannot add content and a bug in the ingest path cannot remove it. It includes `bedrock:StartIngestionJob` and that again looks wrong. AWS groups the whole `KnowledgeBaseDocs` family with it in one statement, and this feature has already shipped a grant naming only the matching API and failing at runtime. Granting it costs nothing here, since `DeleteKnowledgeBaseDocuments` is itself the destructive verb. kb-sync also needed the image change: its worker calls `cleanup_document_resources` after soft-deleting a document whose upstream source has vanished, so `apis/shared/kb_backend/` is added to Dockerfile.kb-sync and to build-one.sh's kb-sync SOURCE_DIRS alongside the rag-ingestion entry. Pre-existing cleanup tests now declare their engine --------------------------------------------------- Ten blocks across tests/routes/test_cleanup_service.py and tests/property/test_pbt_cleanup_service.py patched `records.get_kb_record -> None`. They describe a legacy knowledge base — the state of every assistant predating this feature — and previously said nothing about the engine because there was no third phase to say it to. Left alone they reached a real DynamoDB read. Guards: 7 tests in tests/documents/test_managed_delete_propagation.py, plus grant-shape tests in managed-kb.test.ts and wiring assertions on the real roles in kb-sync.test.ts and platform-stack.test.ts — the latter because a grant proven on a fake role says nothing about the identity that runs the code, which is how two earlier defects on this feature shipped. Four mutations verified caught, each by correctly-named tests: - managed phase dropped from the conjunction -> test_a_failed_managed_delete_blocks_the_hard_delete - every KB treated as legacy -> 3 tests incl. test_the_document_is_deleted_from_the_managed_kb - unreadable record reports success -> test_an_unreadable_record_fails_rather_than_assuming_legacy - both grant attachments removed -> KbSyncConstruct + PlatformStack wiring tests Backend 6,832 passed; infra 640 passed. The 5 backend failures are the pre-existing Strands SDK contract tests documented in HANDOFF §2. * docs(kb): four more defects from one evening, plus two read-only diagnostics Records findings 32–36 and the tooling that found them. All but one are fixed in PR #900; §5.33 is the only one from 2026-08-31 still open. The four new entries share a root cause worth stating once: **the two engines were never made exclusive.** The ingestion consumer stands down for a legacy document; nothing made the legacy pipeline stand down for a managed one, and nothing propagated a deletion to the managed engine at all. Double-indexing (32), a status field with two owners (34), and an orphaned managed corpus (36) are three costumes on that one cause. Item 34 is the one to read. Two writers owned `status`, so "ready" was decided by whichever finished last. Observed both ways within an hour: a PDF marked `complete` 65 s before the managed KB could answer for it, and an image-only PDF marked `failed` while the managed KB served it correctly — the latter only ended up correct because the consumer happened to finish second. The generalisable lesson is that any field two components can write needs a stated owner, and nobody chose this race; it appeared because a writer was added beside an old one and the question was never asked. Item 35 is not a defect but a measured capability difference: Bedrock's image extraction works. A pure-diagram curriculum flowchart is permanently `failed` on legacy (`docling_processor.py` sets `do_ocr=False`) and retrievable in 94.5 s on managed, with the vision model's own description in the chunks. Also records that the old bundled `aws` CLI silently omits `mediaExtractionConfiguration` from `get-data-source`, so a field can look unset when it is not — do not conclude from CLI output alone. Item 36 documents the deliberate asymmetry between the two gates, which is the part most likely to be "cleaned up" by someone later: on ingest an unreadable KB record resolves to legacy, on delete it must fail. Same question, opposite answers, because the `DOC#` row is what the fail-closed status filter joins against. Corrects one claim in item 32: an earlier revision said the missing gate affected "both copies" of the handler including the bootstrap asset. The bootstrap copy is a 33-line no-op placeholder that indexes nothing and needs no gate. Adds two read-only diagnostics, documented in §2 with the numbers they produced: - `kb-doc-timings.py` — per-document ingestion timing and which engine did the work, derived from the record rather than guessed. Legacy 30 s vs managed 95 s on the same 132 KB PDF; `INDEXED → retrievable` 0.9 s, independently confirming the evaluation's 0.75–1.03 s. - `kb-compare-engines.py` — one query through both engines, exploiting the `retain` window where both indexes still hold the corpus. Marks each chunk against MAX_CONTEXT_CHARS, which is what makes the comparison honest: ~2,000 characters reach the model, so one or two chunks, so precision@1 is nearly everything. On `CS434` legacy's five chunks sit within 0.056 of each other and its top chunk does not contain the string; managed separates by 0.4988 with the literal match first. Notes that #900 ends the both-indexes trick, so a future A/B needs two assistants or documents predating promotion. Also adds an "engine visibility" item to the work queue: nothing logs which engine served a query, so "is the new one working?" is currently only answerable from the KB record — worth closing before a wide rollout given that this feature's risk profile is silent regressions. * fix(kb): wait for indexing to finish, and stop re-ingesting while it runs A 1.5 MB PDF uploaded to a promoted knowledge base in dev sat at `uploading` indefinitely with a fully retrievable copy in the knowledge base. Three separate bugs stacked up. 1. The poll window was sized against the wrong measurement --------------------------------------------------------- The consumer ingested and then polled a *retrieval* for 30 s. Its own header justifies that window with "Bedrock reports INDEXED up to a second before it can be retrieved — measured at 0.75–1.03 s", but the poll starts the moment the ingest call returns, so it actually has to cover `ingest -> INDEXED -> retrievable`. §5.1 of the evaluation measured PDF ingestion at 37–264 s; this file took 5 m 30 s. The budget was smaller than the documented lower bound. Identical in shape to §5.30, where `verify` failed good migrations against that same 0.75–1.03 s figure. That fix never reached this component, which inherited the constant. 2. Every redelivery re-ingested, restarting the work it was waiting for ---------------------------------------------------------------------- `IngestKnowledgeBaseDocuments` is fire-and-forget, and nothing asked Bedrock what it already knew, so the consumer could not tell "not indexed yet" from "never submitted". Each of the three deliveries re-submitted the document. It reached INDEXED 54 s after the final attempt had been dead-lettered. `handle_object` now probes `GetKnowledgeBaseDocuments` first and branches on the real `DocumentStatus` enum, taken from the packaged service model: STARTING/PENDING/IN_PROGRESS means do not re-ingest; FAILED is terminal; PARTIALLY_INDEXED counts as usable, because the document IS retrievable and failing it would hide content the user can see. A probe failure reads as NOT_FOUND — no evidence of prior work — so it never blocks ingestion. 3. `indexedAt` was fabricated ----------------------------- `indexed_at = _now_iso()` ran immediately after the ingest call returned, so the field recorded when we asked, presented as when indexing finished — minutes apart for this document. It is now Bedrock's own `updatedAt`. The pre-existing test asserted only that the key existed and was truthy, which a fabricated value satisfies; that is why this survived. It also means my earlier claim that a measured 0.9 s gap "confirmed the 0.75–1.03 s figure" was wrong: that gap was ingest-return to retrievable, not INDEXED to retrievable. Why the wait is in-invocation and not more retries -------------------------------------------------- I first raised a RetryPolicy on the EventBridge target. That does nothing, and the construct now says so instead: for a Lambda target EventBridge hands the event off and the function's OWN async retry config governs — `retryAttempts: 2`, which is Lambda's hard maximum and exactly the 1 + 2 attempts seen in the logs. Redelivery therefore spans a few minutes and cannot be extended, so INDEXED_POLL_TIMEOUT_SECONDS is 600 s: covers the measured tail, leaves 5 minutes under the 15-minute Lambda timeout. Small documents still complete in one invocation, so the fast path is unchanged. Guards: 8 new tests in test_kb_ingestion_consumer.py, and a cross-language test in test_kb_migration_env_contract.py that parses the Lambda timeout out of the CDK construct and asserts the poll budget fits inside it with headroom — the two live in different languages with no compiler between them. `_FakeBackend` now models document STATUS, not just ingest calls. A fake that reported instant success is what let a 30 s window look adequate for work that takes minutes — the same failure as §5.28, where the fake modelled `clientToken` dedup as permanent. Mutations verified caught, each by correctly-named tests: in-flight guard removed, `indexedAt` back to the local clock, FAILED treated as retryable, budget raised past the Lambda timeout, budget dropped back to 25 s. Backend 2,510 passed across the affected areas; infra 640 passed. * fix(security): scope admin skills routes to the catalog; close AST attribute-chain bypass Two independent defects that chained into attacker-authored OS command execution inside another principal's chat session. 1. Cross-owner write on the admin per-object skill route. GET /admin/skills/ narrows to owner_id == "system", but every per-object route read the row by id with no predicate, so an actor holding only admin.skills could read and rewrite a private, user-owned skill's instructions — instruction-trusted content that steers its owner's agent — while the owner was 403 on the same route. SkillCatalogService now exposes get_catalog_skill() / require_catalog_skill(); the predicate is applied in the service for update_skill, delete_skill, list_resources and the role-grant methods, and at the route layer for GET and the reference-file routes. Non-catalog rows now return 404 with the same message template as a nonexistent id, so the surface no longer confirms that another user's private skill exists (the old "is user-authored" role-grant error was itself an existence oracle). 2. AST allowlist bypass in the code-execution sandbox policy. visit_Attribute checked only for dunders, so an allowlisted module that itself imports a host module re-exported it: pd.io.common.os.popen(...) reached the full os module with no import statement in the submitted source. Attribute nodes are now checked against the same denylist as bare names, ImportFrom members are checked too (from pandas.io.common import os as o bound the real module under an innocuous name), the missing host modules are covered (builtins, gc, runpy, posix, nt, codeop, pdb, bdb, timeit, webbrowser), and the adjacent deserialization sinks are closed — a pickle payload can arrive as a bytes literal through io.BytesIO, so read_pickle/to_pickle/read_hdf/to_hdf/load are refused, as are process-spawn entry points as a second net. Documented consequence of the attribute rule: a column colliding with a denied name must use df['open'], and a lazily-loaded submodule must be imported directly (from scipy.signal import butter). Both were already true for the bare-name form. Verification: the disclosed payload run through both policy versions — pre-fix ACCEPTED, post-fix REJECTED (forbidden attribute: popen). Full backend suite in a clean worktree: 32 failed / 6776 passed at develop, 32 failed / 6804 passed with this change (same pre-existing artifact_render and crawl_repository failures, +28 new tests, zero new failures). No new ruff or mypy findings. Not addressed here: the sandbox policy remains a denylist over a large library surface, and instruction-trusted content still reaches other principals by design through invoke-through on a shared Agent. Capability reduction inside the execution environment is the durable control and is separate work. * fix(auth): bind BFF OAuth state to the requesting browser Fixes a High-severity OIDC login CSRF / session fixation hole in the BFF auth flow (finding f-8c4f312a). `GET /auth/login` minted a `state`, stored it server-side, and redirected to the Cognito Hosted UI without issuing any browser-side material — no state cookie, no PKCE, no nonce. `state` is a public value: it travels in a 302 Location and anyone can mint one anonymously. `GET /auth/callback` nonetheless treated "this state exists in the store" as proof the request continued a login *this* browser started, so it accepted any (code, state) pair from any browser. Exploit: an attacker mints a state anonymously, authenticates at the IdP themselves to get a real authorization code, then lures a victim to /auth/callback?code=&state=. The victim's browser is silently issued a live session for the attacker's account — reported against a `system_admin` identity — and everything the victim then does (conversations, uploads, memory entries, third-party connector consent) lands inside an account the attacker still holds credentials to. Browser binding is the fix: - /auth/login mints a 32-byte secret, returns it in a new `__Host-bff_oauth_state` cookie (HttpOnly, Secure, Path=/, no Domain, SameSite=lax), and commits only its SHA-256 digest to the state row. - /auth/callback recomputes the digest from the cookie and refuses the exchange unless it matches, via `secrets.compare_digest`. The attacker's cookie is in the attacker's jar, so the victim fails closed before the code ever reaches the token endpoint. Three deliberate choices: - The cookie is checked *before* the state-store lookup, so a probe from a crafted link can't burn a user's in-flight state. - `SameSite=lax` is required, not a compromise: the IdP returns the user via a top-level cross-site GET, which `strict` would withhold. - A state row carrying no digest fails closed. Those exist only mid-deploy; honouring them would keep the hole open for the whole rollout window, which is exactly when an attacker holding a pre-minted state would strike. Cost is one retry for logins spanning the deploy. Also adds PKCE (S256) and OIDC nonce verification end-to-end: the verifier and nonce live in the state row and never reach the browser; the verifier is sent to /oauth2/token and the nonce is compared against the ID-token claim. Both are defense in depth (code interception, token substitution) — PKCE alone would NOT have closed this finding, since the attacker drives the BFF-minted authorize URL and the stored verifier matches their code. No Origin / Sec-Fetch-Site check was added, despite the report suggesting one: a legitimate arrival at /auth/callback *is* a top-level cross-site GET carrying `Sec-Fetch-Site: cross-site` and no `Origin`, so it is indistinguishable from the attacker's link. Rejecting on those headers would break every login and stop nothing. A test pins that those headers are still accepted and that binding carries the rejection. Verification: the new tests were confirmed to catch the vulnerability, not just the implementation — neutering the three enforcement conditions fails 12 of them, including the full-chain test. Full backend suite is 6840 passed / 35 failed, with the 35 byte-identical to a baseline captured with this work stashed (pre-existing, in web_sources and artifact_render). Tests: - test_login_csrf_regression.py (new): replays the reported chain against the real /auth/login with no hand-seeded state, covers the free-retry observation and forged cookies, plus a positive control that the originating browser still completes login. - test_callback.py: browser-binding, PKCE and nonce enforcement; seeded state rows now carry the binding digest. - test_login.py: cookie attributes, digest-only storage, per-request uniqueness, secret absent from the redirect URL, S256 challenge derivation, nonce/state agreement. Incidental: annotate `_SAMESITE` as `Literal["lax"]`, clearing five pre-existing mypy arg-type errors on Starlette's `samesite` parameter. * fix(skills): close privilege-escalating stored XSS in skill resources An authenticated, zero-privilege user could plant JavaScript that later executed on the SPA's own origin inside a system_admin's authenticated browser session (finding f-317ac252). Two control failures chained: 1. WRITE — the skill-resource upload routes persisted the client-supplied multipart Content-Type verbatim with no allowlist, and permitted an .html filename, so an ordinary user could store bytes labelled text/html. 2. READ — the read routes reflected that stored type as the response media type with `Content-Disposition: inline`, and the CloudFront /api/* behavior carried no response-headers policy, so responses shipped without nosniff and without a CSP (GET / had both). Because app-api is served from the same origin as the Angular SPA, the uploaded file parsed as a top-level HTML document and its inline