From eaefde28d98f3a1f1076fed1487a9765c148827e Mon Sep 17 00:00:00 2001 From: Phil Merrell Date: Tue, 22 Sep 2026 21:18:18 -0600 Subject: [PATCH] Release/1.24.0 (#1244) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(documents): DocumentDigest built at upload and persisted on FileMetadata (offload PR-2) Stacked on PR-1. The digest a later turn will carry instead of the document's bytes (docs/specs/document-context-offload.md §4A), built once, off the model path, when a document upload completes. - apis/shared/files/document_digest.py: a deterministic outline (headings with page/paragraph/line anchors, table and figure mentions, counts, a text sample spread across the document) via pypdfium2 and the PR-1 DOCX extractor, plus a 3-5 sentence abstract from Nova Micro (DOCUMENT_DIGEST_MODEL_ID) that fails open. render_digest emits the block under a hard 1,500-token budget (sections dropped first, then the abstract; the handle always survives) and the estimate is stored as digest.tokens. - FileMetadata.digest + FileUploadRepository.update_file_digest; complete_upload schedules the build as a strong-referenced background task for document uploads only. DOCUMENT_DIGEST_ENABLED=false skips it. - Content policy: digest.abstract / digest.sections denylisted; status, format, count and tokens projected; the attachment profile reports digested count and digest tokens. DocumentDigestGenerated/Tokens/Ms EMF in AgentCoreStack/Compaction. - Nothing in the chat path reads the digest yet (PR-3). Spec: PR-2 row marked built; §8 decisions 12-15. Co-Authored-By: Claude Fable 5.1 * feat(documents): restore rehydrates stripped documents as digests (offload PR-3) Stacked on PR-2. The correctness half of docs/specs/document-context-offload.md §4E: on restore, an inline document block becomes its block (abstract, outline, upload_id handle for document_read) instead of the contentless placeholder that lost a returning user's document. - session/document_rehydration.py: match each block to the session's upload rows on the sanitized filename PromptBuilder gave it (dot -> underscore, _2/_3 duplicate suffix), byte size as tiebreak, newest first, each row claimed once; render the row's digest. Rows without a digest get an outline-only digest built from the bytes already in the restored message (no S3, no model call on the synchronous restore path) and persisted. Unmatched blocks keep the placeholder verbatim; every failure falls back to it. DOCUMENT_REHYDRATE_ENABLED=false restores the old path. - _strip_document_bytes delegates and records document_rehydrated and document_stripped separately on the ledger. - FileUploadRepository: list_session_files_sync / update_file_digest_sync bodies (initialize() runs under the agent constructor and cannot await). - Restore output is byte-stable across restores given the same rows (tested). - Spec: PR-3 row marked built; §8 decisions 16-20. Co-Authored-By: Claude Fable 5.1 * feat(documents): offload unpinned documents to their digests when the re-write is free (offload PR-4) Stacked on PR-3. The cost half of docs/specs/document-context-offload.md (§4C / §4D): once a document is no longer the active subject, its inline bytes leave the cacheable prefix and its digest (PR-2) plus the document_read handle (PR-1) take their place. - session/document_offload.py: pinning (attach turn + next; the incoming or previous prompt names the document's stem; a recent document_read result), the 5,000-token size floor, the crc32 rollout bucket (DOCUMENT_OFFLOAD_ROLLOUT_PERCENT) and DOCUMENT_OFFLOAD_ENABLED kill switch, in-place replacement, and document_read slice ageing. - TurnBasedSessionManager.apply_document_offload runs head-of-turn right after apply_pending_compaction (the compaction stack's §3.5 predicate is this spec's cache-aware rule): only on cache_expired, prefix_changed or over_ceiling; otherwise nothing moves. The replacement is PR-3's restore transformation, so the live block equals a cold restore's. Records document_offload on the ledger with cacheGapSeconds and emits DocumentOffloaded/DocumentOffloadedTokens/DocumentSlicesAged EMF. - Slices older than DOCUMENT_SLICE_MAX_TURNS are stubbed on the live path and on restore, so a warm agent and a cold restore agree. - CompactionEvent (py + TS) gains digestTokens / slices / sliceTokens. - Spec: PR-4 row marked built; §8 decisions 21-26. Co-Authored-By: Claude Fable 5.1 * feat(compaction): record the truncation anchor's cache gap on the ledger (offload PR-5) The spec's PR-5 asked for a cache-live guard on the truncation deferral (July 2026: 34 of 47 truncation events fired inside the 5-minute TTL). That defect was closed by the compaction stack before this spec's PRs began: the anchor moves only with a cut (applied paid-when-free) or in _maybe_advance_truncation_anchor behind _cache_window_expired, and the stability suite pins that a warm cache never advances it. What was missing was the proof from rows. - truncation_anchor ledger event (anchorFrom, anchorTo, cacheGapSeconds measured BEFORE the save re-stamps updated_at) and TruncationAnchorAdvanced / TruncationAnchorCacheGapSeconds EMF. - cacheGapSeconds on the restore-time `applied` event, measured at the top of _apply_compaction, so a restore-time slice or truncation inside the TTL is countable as a rebuild while the cache was live. - No behavior change. Spec: PR-5 row rewritten; §8 decision 27. Co-Authored-By: Claude Fable 5.1 * feat(feedback): content-free thumbs on assistant messages joined to cost rows (offload PR-7) A thumbs up/down on assistant messages, persisted as an F# row on the sessions-metadata table keyed (sessionId, messageId) beside the C# cost row, so the admin session profile joins the outcome signal to the call's turn class in one key lookup. Value ±1, timestamp and an optional reason code from a closed enum — never free text, by the request model and the storage layer both. - apis.shared.sessions.feedback: F#{sessionId}#{messageId} rows (documented with the C#/D# schemas in metadata.py); a second click replaces; session rollups thumbsUp/thumbsDown ADDed while COST_DIAGNOSTICS_ENABLED. - GET /sessions/{id}/messages merges the thumb onto metadata.feedback so a reload restores the pressed state. - PUT/DELETE /sessions/{id}/messages/{messageId}/feedback via get_current_user_from_session; MESSAGE_FEEDBACK_ENABLED kill switch (404). - Admin profile: feedback {up, down, byTurnClass, unjoined} + dataCoverage. feedback; byTurnClass is null ("not tracked") until the C# rows carry hasDocuments / documentDigests / documentReads (#1137). - content_policy: FEEDBACK_ROW_PROJECTION walked by the existing test; session projection gains thumbsUp/thumbsDown; call projection gains the three turn-class fields. - SPA: thumbs pair with reason chips in the message actions, optimistic with rollback; Feedback tile with down-thumb rate by turn class on the anatomy page. Co-Authored-By: Claude Fable 5.1 * test(admin): profile dataCoverage now carries feedback Co-Authored-By: Claude Fable 5.1 * feat(attachments): hold a turn's inline attachments to the Memory event quota (PR-6) The stored unit is the message, not the file: a turn's inline attachments are persisted as one AgentCore Memory event, base64-inflated 4/3 into a blob payload and bounded by the 10 MB event quota, so ~7.5 MB of raw bytes per turn is the break point. Past it create_message re-raises SessionException — a hole in history. The per-file 4 MB gate and the SPA's 5-file cap do not protect: ~1.3–1.4% of prod attachment turns exceed it, several with only 3–4 files (document-context-offload-validation.md, Claim 7). - `_apply_inline_byte_budget`: first-fit in attachment order against INLINE_ATTACHMENTS_MAX_TOTAL_BYTES (default 7.5 MB, derivation in the constant's comment); images count. Trimmed files join the existing oversized-note path with their own wording ("send in a follow-up"). - `_apply_message_file_cap`: the server side of the SPA's MAX_FILES_PER_MESSAGE, applied to both `files` and `file_upload_ids` before the S3 fetch. The resolver's silent `[:5]` becomes a note to the user; the app_api's dead `max_files_per_message` now reads the same shared constant. - One content-free EMF metric, AttachmentTurnOverQuota (Bytes) in AgentCoreStack/Compaction, plus a count-only log line. - ATTACHMENT_TURN_GUARD_ENABLED=false restores the prior behaviour. Co-Authored-By: Claude Fable 5.1 * feat(spa): share artifacts from the library and the owner view page Sharing an artifact used to be reachable only from the card inside the conversation that produced it, which meant the library at /artifacts — the page whose whole premise is that artifacts outlive their chat — had no way to share one. Both surfaces now open the same ArtifactShareModalComponent the card does, so there stays exactly one place in the app that knows what an artifact share is. The control is gated on `row.owned`, so it appears on your own rows and never on the "Shared with you" ones: a received artifact has no artifact id to share (the share id is the only handle you have on it), and re-sharing someone else's grant is not something this model supports. The view page deliberately has no ownership check of its own. It resolves its artifact out of listLibrary(), which reads the caller's own DynamoDB partition, so an id that is not yours renders "Artifact not found" before the header exists at all — the boundary is the partition key, and a comparison in the template would imply the guarantee lives in the UI. A test pins that the control is absent when nothing resolved. Both surfaces show HEAD, so they pin HEAD's version. Shares are immutable and never follow HEAD, so the version rides in each control's accessible name ("Share Quarterly plan, version 4") as well as the dialog's caption — the library prints a version badge only when it is > 1, so the control cannot rely on one being on screen. Grid footer: raise the label-collapse threshold 19rem -> 21rem. Found in the browser, not by a test. 19rem was tuned for FOUR controls and fit with zero slack: at three columns on a 1280px window the container measures exactly 306px and the labelled row needed exactly 306px. Adding Share pushed the row to 326px while leaving the container 2px ABOVE the collapse threshold, so the labels stayed visible and the Delete icon was clipped. 21rem (336px) collapses them with 10px to spare, and the labels still reappear at one column, so they are hidden where they do not fit rather than removed. Verified against real data on dev: zero overflow on every card, in both views. Co-Authored-By: Claude Opus 5 * feat(feedback): align with response-feedback spec — flag name, six reason buckets, explicit/implicit signal discriminator RESPONSE_FEEDBACK_ENABLED per spec §5; reason codes are the spec's §6 buckets (still a closed enum — free text stays out by decision, recorded in the spec's new §13); rows carry signal="explicit" and every thumb reader filters on it so the §10 implicit phase needs no backfill and never sums in. Co-Authored-By: Claude Fable 5.1 * feat(feedback): retry-with-correction — the consequence behind a thumbs down response-feedback spec §11 PR-1's other half: a down-thumb's reason row offers "Retry with that in mind", which prefills the composer with a correction template for the reason code (new ComposerDraftService; the composer consumes drafts for its session and never submits). When the user sends it, ChatRequestService hands the added user message to MessageFeedbackService.consumePendingRetry, which PUTs retryMessageId — the message's index, never its text — onto the F# row. The write is now an update_item upsert so a later re-thumb keeps the link. Open question 1 settled as a new turn rather than /steer (which targets a running turn). The admin profile reports feedback.retried and reworkUsd (spec §7 "rework cost": the thumbed call rows plus the retry turn's consecutive assistant rows). Spec §13 updated. Co-Authored-By: Claude Fable 5.1 * fix(admin-costs): expect formatGap's minute-only form in the offload title formatGap drops a zero seconds remainder, so a 420s cache gap renders as "7m", not "7m 0s". The assertion was written against the latter and was the only red check on this PR. Co-Authored-By: Claude Opus 5 * fix(admin-costs): keep DataCoverage.documents on DataCoverage The merge resolution appended develop's `documents` member after the interface's closing brace, landing it on FeedbackProfile and breaking the AOT typecheck. Co-Authored-By: Claude Opus 5 * feat(feedback): implicit signals — copy and continue (response-feedback PR-2) Spec §10: denser than thumbs, no UI cost. Copy and Continue in the message actions rail POST a kind code to /sessions/{id}/messages/{messageId}/signals (fire-and-forget, once per message per kind per page load). Rows share the F# family under their own key — F#{session}#{message}#{kind} — with signal="implicit", kind, and an ADDed count, so they never collide with the thumb and every thumb reader keeps filtering them out. The profile reports feedback.implicit as messages touched per kind, on its own line: explicit and implicit are never summed. Edit-and-resend has no SPA affordance to hook; abandonment is deferred by design (spec §13). Spec status and §13 updated. Co-Authored-By: Claude Fable 5.1 * fix(documents): count PDF pages, not bytes, in documentTokens `documentTokens` (offload spec §6.1) and the `DOCUMENT_OFFLOAD_MIN_TOKENS` floor (§4C) both read a document's weight from the compaction estimator's `bytes/4`. That is wrong for PDFs by an order of magnitude: Bedrock dual-encodes each page as an image *and* a text layer — the property §3 of the spec depends on — and the image channel has nothing to do with file size. Measured on dev (session 61de2256, 2026-09-16): a 27,578-byte, 60-page PDF produced a 109.1K-token first write against a 14.6K static prefix, so the document was ~94K tokens. `bytes/4` reported 6,894 — 13.7x low. At that error the spec's "document share of the prefix" reads 6% where the truth is ~87%, which inverts the ship/abandon call the evaluation spec §4.2 reads off these rows, and understates a measured 109.1K -> ~15K prefix drop. New `apis/shared/files/document_tokens.estimate_document_tokens`: pages x PDF_PAGE_TOKEN_ESTIMATE (default 1,500, env-tunable) for PDFs, floored by bytes/4 so a byte-heavy scan can only score higher than before; bytes/4 unchanged for formats Bedrock extracts to text. Page counting costs ~0.1ms even at 200 pages (measured), so no memoization is needed, and an unparseable PDF falls back to today's number. On the dev canary it lands at 90,000 against 94,485 measured — 5% low, deliberately, because this number gates an eviction. Wired into every consumer that reports or decides on that quantity, so the row and the offloader can never disagree: `document_context` (the `C#` row's documentTokens and documentSliceTokens), `document_offload` (Candidate.tokens, the min_tokens floor and the aged-slice ledger figure) and `document_rehydration` (document_stripped). The compaction estimator is deliberately NOT changed — it rescales to measured history and drives cut thresholds, so it needs its own decision. Co-Authored-By: Claude Opus 5 * fix(documents): couple DOCUMENT_READ_ENABLED to the paths that promise the tool The flag was read in exactly one place — `_document_tools_gate`, which decides whether `document_read` is injected. Neither the restore path nor the live offload consulted it, so pulling the one kill switch an operator would actually reach for (the tool is misbehaving) left two worse-than-before behaviours: 1. restore kept emitting ``, advertising a retrieval handle for a tool that would not be injected — the model is invited to make a call it cannot make; 2. the live offload kept evicting document bytes from the prefix with the only recovery path switched off. That is strictly worse than the pre-offload world, where live bytes never left at all. Spec §5 states the rule this violated: "a digest that points at a tool nobody has is no better than today's placeholder." Fix, split by whether dropping the bytes is optional: - **Restore has to drop them** either way (Bedrock rejects duplicate document names across a conversation), and a digest is still strictly more than the pre-PR-3 placeholder — so it still renders, but `render_digest` gains `include_handle` and the restore path passes `document_read_enabled()`. The outline and abstract survive; the false affordance does not. - **The live offload is optional**, so it stops entirely: `offload_enabled_for` now also requires `document_read_enabled()`. Both callers (the head-of-turn offload and restore-path slice ageing) already go through that single gate, so one check covers both and there is no second flag read to drift. With the flag on, every rendered byte is identical to before. Co-Authored-By: Claude Opus 5 * feat(feedback): eval sampling — down-thumbs feed AgentCore Evaluations (response-feedback PR-4) Down-thumbed turns become the evaluation harness's input queue. Thumb rows carry GSI1PK=FEEDBACK#down / GSI1SK=updatedAt on the existing UserTimestampIndex, so the fleet's recent down-thumbs are one query with no new index. POST /admin/feedback/evaluations/run judges up to N not-yet-judged ones in a background task via bedrock_agentcore.evaluation.EvaluationClient over the runtime log group (keyed by the runtime session id the chat proxy pins); GET /admin/feedback/evaluations is the queue with verdicts. Routing is spec §6: wrong → Correctness + Faithfulness, instructions → InstructionFollowing, length → Conciseness, other/none → Helpfulness; tool_failed gets no judge and is corroborated against the call's tool census (ops, not model); outdated has no judge yet. The stored verdict is per-evaluator value / rating / n / tokens — the judge's explanation quotes the conversation, so it is dropped at summarisation, refused at the storage write, and denylisted in the content policy. The profile shows feedback.evaluations. Opt-in: FEEDBACK_EVAL_SAMPLING_ENABLED defaults off (CDK CDK_FEEDBACK_EVAL_SAMPLING_ENABLED=true) because the managed judge reads the sampled conversation's spans — the scoping decision the evaluations spike says to make per environment. CDK wires the runtime log group into app-api and grants Logs Insights on it and aws/spans plus Evaluate/GetEvaluator; inert until an environment opts in. Spec §13 updated. Co-Authored-By: Claude Fable 5.1 * Agent Templates: create-form prefill + admin-managed backend (#1149) * chore(agents): integrate Round 1 template-prefill work (P1 hydration seam, P4 reconcile, P3/P5 picker+content) * feat(agents): Phase 2 template prefill — read localStorage draft in create mode, reconcile tool refs, populate form dirty with flagged/dropped notice * feat(agents): add Q&A, Roleplay and tool-notice demo templates + catalog-wide spec coverage * feat(agents): pin a concrete default model (Claude Sonnet 5) on templates so the picker prefills a selection * feat(agent-templates): admin-managed template catalog backend Backend for the Agent Templates feature per docs/specs/AGENT_TEMPLATES_SPEC.md, mirroring the system_prompts admin-catalog pattern: - shared/agent_templates: models (AgentTemplate + admin/public wire shapes), repository (DynamoDB CRUD, Scan listing), service, and an idempotent seed of three NEUTRAL fork-safe templates (no org tool refs, model_id=null). - app_api/admin/agent_templates: scope-guarded CRUD (admin.agent_templates). - app_api/agent_templates: public GET /templates returning the client TemplateCatalogEntry shape (camelCase draft + pitch), enabled-only, ordered, [] on empty. - admin.agent_templates RBAC scope (GROUP_AI_CONFIG, delegable); routers mounted; AGENT_TEMPLATES config-cache key. - CDK: AgentTemplatesTable in AdminTablesConstruct + SSM params, compute ref, app_api env var + IAM grant, platform-stack wiring + backup list. - Tests: 24 model/repo/service/seed + route tests; scope-coverage map updated. Local only, no push. Client picker still reads the hardcoded array; the data-source switch is the follow-on step. * test(infra): add AgentTemplatesTable CDK assertions Cover the AgentTemplatesTable + wiring already added in 57a7bd53 with infra CDK tests (the backend commit added none): - constructs.test.ts: AdminTablesConstruct 4->5 tables + PK/SK/no-GSI/ AWS_MANAGED assertion for the agent-templates table. - platform-stack.test.ts: shared-table count 27->28; SSM param cap 49->51 for the table name+ARN publishes. - app-api-environment.test.ts: DYNAMODB_AGENT_TEMPLATES_TABLE_NAME wiring. - security-policy.test.ts: scoped AgentTemplatesTableAccess IAM grant (never Action:* / Resource:*). Tests only, no source changes. Local, no push. * feat(agents): source template picker from live backend catalog Swap the 'Start from a template' picker from the hardcoded AGENT_TEMPLATES array to the public GET /templates/ endpoint via a new AgentTemplatesService. Remove the org-specific template DATA from agent-templates.ts (keep the TemplateDraft/TemplateCatalogEntry types + the localStorage contract key) so no Boise/higher-ed content is baked into the client (forkability). The select handoff (write draft to localStorage, route to /agents/new) and the downstream prefill/reconcile/notice path are unchanged. Picker gains loading/empty/error states. Specs updated to mock the service. * feat(admin-ui): agent templates admin console (list + form + service) Add the admin UI for the admin-managed Agent Templates catalog, consuming the /admin/agent-templates CRUD routes on the backend branch: - AdminAgentTemplatesService: list/get/create/update/delete (lazy resource). - Manage list page: name/emoji/enabled/order, enable-disable, delete, reorder (sort_order swap), mirroring manage-system-prompts + manage-models. - Create/edit form page (new / edit/:id): agent-form field set (name, emoji, description, instructions, visibility, tags, starters, model, bindings) plus catalog fields (pitch, status, sort_order). Blank model id -> platform default; model params + binding config preserved on edit. - Routes under admin/agent-templates guarded by adminScopeGuard data:{scope:'admin.agent_templates'}; nav entry in AI Configuration; admin.agent_templates added to the client scope union. - 13 vitest specs (service endpoints, list render/toggle/sort, form buildPayload/create/edit) green via the Angular builder. Local only. * feat(admin): model dropdown for agent-template form Replace the raw model-id text input in the admin Agent Templates create/edit form with a friendlier select: a 'Platform default' option (modelConfig.modelId = null, the default for new templates) plus the RBAC-filtered configured-model catalog, reused from AgentService.loadBindable('model') (GET /agents/bindable?kind=model) — the same source the create-agent form uses. On edit the saved modelId preselects; a saved id no longer in the catalog is shown as a clearly-labeled 'unavailable' option and preserved on save rather than silently dropped. Spec covers platform-default, select-a-model, and edit-preselect (valid + stale). * feat(admin): emoji picker popup for agent-template form Replace the plain emoji text input in the admin Agent Templates form with a real emoji-picker popup. Extract the create-agent builder's inline emoji field (emoji-mart in a CDK connected overlay) into a reusable ControlValueAccessor component, app-emoji-picker (src/app/shared/emoji-picker), so it drops into the reactive form via formControlName="emoji" — the form drives its value (including edit-mode patchValue) and reads selections back; clearing emits ''. agent-form is left as-is and can migrate to the shared component later. * feat(agent-templates): drop template visibility; tool-binding picker Change 1 — remove visibility as a template concept end-to-end. A template's lifecycle is enabled/disabled; visibility (PRIVATE/PUBLIC/SHARED) is an agent concept, meaningless on a template. Dropped from the backend model/repo/seed, the public GET /templates/ TemplateDraft, the admin + client TS types, and the admin form. Prefill no longer sets the create-agent form's visibility from the template — applyAgentToForm falls back to the form's own default (PRIVATE) when a draft carries none, so a created agent stays owner-private by default. Change 2 — admin tool bindings pick from the enabled tool catalog instead of raw ref text. Reuses AgentService.loadBindable('tool') (GET /agents/bindable?kind=tool), same as the model dropdown. A tool binding renders a dropdown (label shown, ref stored); a saved ref no longer in the catalog is kept as a clearly-labelled 'unavailable' option and preserved on save. Other binding kinds keep the text input. Specs updated (backend + frontend), targeted runs green. * chore(agent-templates): drop now-unused BindingPayload import in seed * test(infra): register AgentTemplatesTable in GSI inventory + table count The new AgentTemplatesTable bumps the stack's table inventory, tripping two full-suite infra guards the targeted pre-push run didn't cover: - gsi-inventory.json regenerated to include agent-templates (no GSIs). - observability-dynamodb-alarms table count 27 -> 28 (the per-table throttle alarm is auto-derived, so no construct change needed). * docs(kaizen): queue the document-offload prod readout, and mark the epic built The epic shipped (#1137–#1143, #1145) and was validated end to end (#1147 fixed the two defects that sweep found). What is left is measurement, which the Friday loop is the right home for. Three entries: - **Decide the rollout arm before the release.** `DOCUMENT_OFFLOAD_ROLLOUT_PERCENT` defaults to 100, which is the evaluation spec's own §4.2 "Ship" state, while that spec is still Draft with no harness and its §2 gate written as a veto. The bucket exists to give the evaluation concurrent B/C arms; at 100% there is no control and only a before/after across the release boundary remains. This is time-sensitive in a way the compaction veto item is not. - **The prod readout — five numbers and what each decides**, including the prediction that `document_offload` events will be ≈0 because `document_read` is itself an `extra_tool`, so restore reaches the document every turn before the live offload can. If so, all the benefit is PR-3 and PR-4's pinning is inert. Also the first down-thumb-by-turn-class read, which is what PR-7 was built for. - **Recalibration plus the four findings still unfixed** — ReDoS in pattern mode first (measured 14 s at 28 chars, exponential, model-supplied input), then fitting `PDF_PAGE_TOKEN_ESTIMATE` from prod attach turns. The originating [2026-09-16] epic entry is marked BUILT so review-prep does not re-propose the build. Co-Authored-By: Claude Opus 5 * docs(kaizen): correct the rollout-percent entry — the bucket gates PR-4 only I overstated this. `offload_enabled_for` is consulted in exactly two places: the head-of-turn live offload and restore-path slice ageing. PR-3 — the digest-on-restore that is the epic's actual behaviour change — is NOT bucketed; it rides `DOCUMENT_REHYDRATE_ENABLED`, a plain global flag, as do PR-1, PR-2 and PR-6. So the percent never was a control arm for PRs 1–3, and 'at 100% there is no control' was wrong about the epic and right only about PR-4. Since readout item 1 predicts PR-4 fires ~0 times, counting `document_offload` events at 100% answers the same question the A/B would have, more cheaply. Entry now recommends shipping at 100%, records what that gives up, and names `DOCUMENT_REHYDRATE_ENABLED=false` as the real rollback lever. Co-Authored-By: Claude Opus 5 * chore(kaizen): weekly research scan 2026-09-18 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-09-18 Generated by kaizen-review-prep. Ranked agenda for the 10-15 min decision pass; queue updated with prior-review outcomes. Built queue-first and outcome-first: this morning's kaizen-research run fired 1h41m later than last week and was still running at prep time, so there is no current research doc. The queue's Open entries are the skill's primary source, so the review degrades to that; the pipeline gap is Proposal #5. Outcome scorecard: last review converted 6 of 9 proposals, falsifying its own "named carrier" thesis. The class that has never converted is the one requiring a CLAUDE.md or skill-file edit (5th cycle). Queue: 3 items resolved by outcome (nightly both halves, rate provenance, GPT-6 Astra), 3 annotated as partially shipped, 3 new entries added. Deliberately does NOT duplicate PR #1147's three queue entries, which are the authoritative versions of the offload residue. An earlier draft of this review also recommended moving DOCUMENT_OFFLOAD_ROLLOUT_PERCENT off 100; #1147's own entry refutes that (PR-3, the epic's real behaviour change, is not bucketed at all), so the recommendation is withdrawn and the correction is recorded in both the review's Take and the queue entry. Co-Authored-By: Claude Opus 5 (1M context) * fix(documents): bound document_read pattern mode against ReDoS `_compile` passed a model-supplied regex straight to `re.compile`. A nested-quantifier pattern then runs in exponential time — measured on the real engine, `(a+)+$` against `"a"*n + "!"` takes 0.87 s at n=24, 3.5 s at 26 and **14 s at 28**. Extracted PDF lines run 60–100 characters, so such a pattern never returns. The scan runs in `asyncio.to_thread`, and CPython offers no way to cancel a running `re.search` (signals are main-thread only), so the turn hangs to the 600 s SSE timeout. Two defences, because neither is sufficient alone: 1. `catastrophic_pattern` refuses the nested-quantifier family up front — an unbounded quantifier applied to a group that itself contains one — and the pattern is searched as literal text instead. That is the same graceful degradation an invalid regex already got, and the payload carries a `pattern_note` so a literal result is never presented as the regex one. Deliberately narrow so it has no false positives: verified against 7 catastrophic shapes (all flagged) and 13 patterns a model would plausibly write — `\d+`, `(invoice|receipt)`, `(foo)+`, `\$[\d,]+\.\d{2}` — none flagged. Escapes and character classes are honoured, so `\(\w+\)+` and `[(]a+[)]+` are correctly read as literals, not groups. 2. `DOCUMENT_READ_PATTERN_BUDGET_SECONDS` (default 2.0, `0` disables) bounds the *walk* across lines and pages for anything the detector does not catch (overlapping literal alternations like `(a|a)+`, backreference blowups). It cannot interrupt one `re.search` — nothing can — so it is a backstop, not a guarantee: a scan that runs out returns the matches it already has with `timed_out` and a hint naming how far it got. The tool description now also tells the model not to nest quantifiers, so the common case is avoided rather than only corrected after the fact. A `regex` module with native `timeout=` would be the complete fix; it is installed transitively but not declared, and adding a dependency needs approval. Co-Authored-By: Claude Opus 5 * docs(kaizen): mark the document_read ReDoS fixed, and record what is left Fixed in 71b0fdfb. The residual is worth keeping in the queue rather than closing outright: a single re.search still cannot be interrupted, so the wall-clock budget is a backstop. The complete fix is the regex module's native timeout=, which is a dependency decision (installed transitively, not declared) and therefore needs approval rather than a quiet adoption. Co-Authored-By: Claude Opus 5 * fix(documents): close the last three findings from the offload validation sweep **1. The inline-attachment budget had zero headroom.** base64 of N raw bytes is `4*ceil(N/3)`, so the 7,500,000 default encoded to *exactly* 10,000,000 — the AgentCore event quota itself — before the event's JSON envelope (role, content keys, prompt text, per-file metadata, wrapper). A guard whose default sits on the break point it exists to stay under does not prevent the failure it was written for: `create_message` raises `SessionException` and leaves a hole in history. Default is now 7,000,000, which encodes to 9,333,336 and leaves ~666 KB. Still far above prod's p90 attachment cluster (2.58 MB), so no real turn is newly trimmed. The existing test pinned the old number and its own comment named the flaw ("lands on the quota, not over it") — rewritten to assert the break point is real *and* that the default stays below it. **2. The digest's 1,500-token cap was soft.** `render_digest` XML-escaped the abstract *after* slicing it to the room left, so `&` -> `&` could render up to 5x longer than the slice it was measured on — an all-`&` abstract rendered 388 tokens against a 100-token budget, and `digest.tokens` recorded the wrong number with it. Now escaped first and cut on an entity-safe boundary (`&` is never left as `&am`), with a final guard that drops the abstract rather than overshoot. Only the opening tag — the `document_read` handle — may outlive a budget too small for anything else. **3. `prefixTokens.tools` absorbed the whole document on attachment turns.** Diagnosed: `toolTokens` is a *residual* between two independently sourced numbers — `full` (Strands' projection for the upcoming request) and `no_tools` (our own CountTokens call) — so any disagreement about how a content block is counted lands wholly in it. Bedrock understands a PDF page as an image *and* a text layer; when the two sources disagree there, the document's entire weight is attributed to tools. Measured on dev (session `61de2256`): `toolTokens` of **106,756** where the real tools prefix was **12,516** — a 94,240 difference against a document measured at ~94,485, i.e. the whole document. The counting disagreement is Bedrock/Strands behaviour we do not control, so the fix is to stop publishing a number we know to be untrustworthy: the split is not computed while inline document or image bytes are in context, and is taken on a later clean turn instead. An absent `prefixTokens` reads "not tracked" (the ledger's convention); a wrong one silently corrupts every share computed from it. A digest is text, so an attachment session still gets the field from turn 2 onward, and an agent that already holds a trustworthy split keeps using it — the document then lands in `messages`, where it belongs. Co-Authored-By: Claude Opus 5 * docs(kaizen): mark the last three offload findings fixed Also records that prefixTokens.tools was NOT an off-by-one: toolTokens is a residual between two independently sourced counts, and 106,756 - 12,516 = 94,240 against a ~94,485 document — the whole document landing in the tools bucket. Carries forward the one open question (whether images disagree the same way) for the prod readout. Co-Authored-By: Claude Opus 5 * feat(feedback): fleet-level attribution — down-thumb rate by config arm (response-feedback PR-3) The config-dimension attribution spec §7 calls the prize, and the compaction spec §7.2 asked for: down-thumb rate joined against the choices we made, across the fleet rather than one conversation at a time. GET /admin/feedback/fleet?days= reads a trailing window off the FEEDBACK#down / FEEDBACK#up partitions, joins each thumb to its C# cost row, and splits it four ways: model, agent switch, distance from a compaction cut (never / same call / 1-3 / 4+), and document turn class. Both reads are capped and exceeding either is reported as coverage.truncated / sessionsOmitted, never silently under-counted. /admin/feedback renders it under the admin.costs scope. Spec §9 is enforced, not promised: every arm carries its own n, an arm under the floor returns downRate: null so there is no number to quote, and no fleet-wide rate field exists at any level — the page states the gap between the two furthest-apart reportable arms instead of a score. Open question 3 answered: the floor is 20, env-tunable via FEEDBACK_ARM_MINIMUM_N. Skills is deliberately not an arm: enabledTools lives on the S# row, not the C# row. Spec §13 updated with both deviations. Co-Authored-By: Claude Opus 5 * feat(admin-costs): down-thumb reason split on the per-session profile The fleet view (#1152) reports the reason distribution across a period; the per-session profile an admin drills into from it still did not. All six codes (`wrong`, `instructions`, `length`, `tool_failed`, `outdated`, `other`) were written to the `F#` row and round-tripped to the owner's own UI, but `FeedbackProfile` exposed only counts — so the fleet could say *how much* and the drill-down could not say *why this conversation*. `_join_feedback` now aggregates them into `reasons`, which is the field that turns "someone disliked this" into "these are tool_failed". Closed set only: an unknown code (a row from a future schema) is dropped rather than surfaced, and a reason on an up-thumb is ignored as meaningless. Scope note: this began as a pair with a fleet-level aggregate, which #1152 landed first and better — generic config arms (model / agentSwitch / callsSinceCompaction / turnClass) against my turn-class-only view, plus a coverage block, and it deliberately refuses a fleet-wide rate where mine emitted one. That work was dropped rather than duplicated; this is the half #1152 left open. SPA contract updated to match; `tsc --noEmit` clean. Also queues the live-AWS test hang found while validating: one full-suite run stalled 72 minutes on 4m26s of CPU holding four TCP connections to real AWS. The test it stopped at passes in 1.9s alone and its whole slice is green, so it is the victim of leaked state. Not reproducible on demand — one hang in seven full runs the same day — which is itself the first thing to fix. Co-Authored-By: Claude Opus 5 * docs(kaizen): diagnose the live-AWS test calls — 25 cases, hidden by fail-open A socket guard (autouse patch of socket.connect, failing any off-box destination) found 25 off-box connection attempts across 6 test files in one run, all to real us-east-1 AWS. The archetype, from test_skills_access.py: the test patches the role service, but resolve_accessible_skill_ids then calls resolve_owned_skill_ids, which builds a real DynamoDB client. Its except Exception returns [], catalog + [] == catalog, and the assertion passes either way. Fail-open production code plus a partially-mocked test equals a silently real AWS call that never fails the suite — 9196 tests pass with or without the connections. Worse than a flake: ~/.aws/config carries a [default] profile, so these resolve real credentials and are authenticated requests against real AWS from a unit test run. The 72-minute hang was just the tail case. Co-Authored-By: Claude Opus 5 * test: block off-box sockets, and fix the tests that were calling real AWS The suite was not fully mocked. A socket guard found real, authenticated calls to us-east-1 AWS from unit tests — `~/.aws/config` carries a `[default]` profile, so these resolved working credentials. One run stalled 72 minutes on 4m26s of CPU holding four ESTABLISHED connections while botocore retried. **Why it was invisible.** Service code here is deliberately fail-open — a user should not lose their session because a table blipped. So a test that mocks one dependency and misses a second gets a real client, a real request, a swallowed exception, and a green assertion. The suite reported the same 9196 passed with and without the connections. The archetype: `test_skills_access` patches the role service, then `resolve_accessible_skill_ids` calls `resolve_owned_skill_ids`, which builds a real skill-catalog repository; `catalog + [] == catalog`, so the assertion passed either way. **The guard** (`tests/conftest.py`) patches `socket.connect`/`connect_ex` and blocks any non-localhost TCP destination. It sits at the socket layer rather than in botocore because moto intercepts `before-send` itself, making handler ordering ambiguous; moto never opens a real socket, so "left the box" is an exact test. Only AF_INET/AF_INET6 **stream** sockets are guarded, so DNS and AF_UNIX are untouched. Because the raise is swallowed by the code under test, the violation is also recorded and asserted at teardown — that assert is the only thing that makes this visible at all. `AWS_TEST_ALLOW_OFF_BOX_SOCKETS=1` disables it. **Fixed** by stubbing the second, unmocked dependency each path builds: `test_skills_access` (the file's own `_owned()` helper existed and simply was not used), `test_rbac_cache_service` + `test_app_role_service` (the tool-catalog freshness snapshot), `test_kb_backend_parity` (CloudWatch `_publish`), `test_role_agent_pins` (role admin service, `ToolCatalogService.get_tool`, `MemorySpaceService.resolve_permission` — the latter two patched on the service because the code receives an already-constructed one), `test_pbt_model_access_preservation` (session metadata + the `document_read` gate), `test_resolver` (the AppRole quota branch). **Quarantined**, to be burned down: ten files under `tests/routes/`, listed in `_OFF_BOX_QUARANTINE` with the recipe. Note what quarantine does and does not do — the block applies to *every* test, so none of them can reach AWS from now on; quarantine only suppresses the teardown failure, so the debt is bounded and visible rather than load-bearing. No new offender can be added silently. Full suite: 9196 passed, 3 skipped, 0 errors with the guard enforcing. Co-Authored-By: Claude Opus 5 * docs(kaizen): guard landed; record the burn-down list and the two traps Co-Authored-By: Claude Opus 5 * docs(specs): assess the AuraX load-test A/B and plan the fixes Validates the five fixes and three residuals from the 2026-09-15 40k-user A/B against develop: four confirmed, uploads-bucket CORS is a deployment-config gap. Names the RetrieveMemoryRecords 30/s quota the report calls 'conversation memory', and the per-message retrieval that consumes it. Phased plan with a proof per item. Co-Authored-By: Claude Fable 5.1 * fix(infra): fail synth when the uploads bucket would have no CORS rule buildCorsOrigins() returning empty used to be a console.warn; the UserFilesBucket was then created without CORS and every browser upload failed at S3. A production-mirror load-test environment shipped exactly that. Throw instead, with CDK_ALLOW_NO_CORS_ORIGINS=true as the explicit opt-out for a deployment with no browser front-end. Co-Authored-By: Claude Fable 5.1 * perf(inference-api): precompile bytecode and warm the first turn at start uv sync ships no .pyc by default and /app/src was copied raw, so every fresh AgentCore micro-VM compiled the dependency tree (sympy behind the calculator tool included) on a new session's first message. Set UV_COMPILE_BYTECODE=1 for the venv and compileall /app/src in the image. A daemon-thread warm-up at lifespan start pulls the remaining lazy imports and boto service-model loads off the request path; /ping answers immediately. INFERENCE_WARMUP_ENABLED=false is the kill switch. Co-Authored-By: Claude Fable 5.1 * perf(agents): bound CountTokens on the reply path, stop swapping the model id Strands awaits one CountTokens call before every model call. Under load the count throttles before the model does, and the SDK's shared client retried it with backoff, so the reply waited out that backoff (p95 5.8s on the 40k-user run). Count through a dedicated client with one attempt and a 2s timeout: a throttle now costs one failed request before the chars/4 heuristic. Pass the base model id to the API explicitly instead of mutating config['model_id'] around the call, which was only safe while a count and a stream could never overlap on one instance. Co-Authored-By: Claude Fable 5.1 * perf(agents): memoise the attribution split per session; cache four more tool families Sessions whose injected tools keep them out of the agent cache rebuild their Agent every turn and paid the two cold-start CountTokens calls every turn on top of the SDK's own. Key the stable system/tools split by (session, prompt digest, tool-spec digest) in a bounded process memo so a rebuilt Agent adopts it. Promote Word, Excel, PowerPoint and workspace tools to the cache-eligible set: each closes over (session_id, user_id) only, exactly what the artifact arm proved safe in #839/#841. Spreadsheet analysis still bypasses until assistant_id is in the key. Co-Authored-By: Claude Fable 5.1 * test(admin-costs): pin the cache-bypass diagnosis to fixed sets, not today's promotion state The rule derives its list from KEY_DESCRIBED_INJECTED_TOOL_IDS, so the test broke the moment four more families were promoted. Test the mechanics against patched sets, and keep one assertion on the live state that names spreadsheet analysis as the only remaining bypass. Co-Authored-By: Claude Fable 5.1 * perf(agents): cache spreadsheet-analysis sessions by carrying assistant_id in the key The spreadsheet builders close over assistant_id, which the agent cache key did not carry, so every session with analyze_spreadsheet or list_spreadsheets enabled (2,669 of 3,565 prod sessions on the last read) bypassed the cache and rebuilt its Agent, session manager and AgentCore Memory restore on every turn. Add assistant_id to _create_cache_key, stamp it on the construction snapshot, carry it on PausedTurnSnapshot and replay it on resume so a paused agent is never orphaned. A snapshot written before the field existed resumes with None, misses the slot and rebuilds — the pre-existing eviction path, never a wrong hit. Promote the spreadsheet family; every enabled_tools-gated injected family is now cache-eligible. Co-Authored-By: Claude Fable 5.1 * test: burn down the off-box quarantine — no test reaches AWS, and the suite halves The guard shipped with ten `tests/routes/` files quarantined. They are fixed, the quarantine list and its bypass are deleted, and the guard is now unconditional: any test that opens a non-localhost socket fails. **The suite runs in 8:02, down from ~14-16 minutes.** That gap was the cost of the problem — connection attempts to real AWS, each with botocore's connect timeout and retries, on every run. Roughly half the suite's wall clock was waiting on calls whose results were then thrown away by a fail-open `except`. Most of the 51 cases came from a handful of shared seams, so the fixes live in `tests/routes/conftest.py` as one autouse fixture rather than per file: the inference path (session metadata, the `document_read` gate), the converse path (managed-model routing, the rate-limit window, three quota-assignment lookups), RBAC role resolution, agent-detail label resolution, and the artifact share cascade on session delete. All are fail-open infrastructure lookups that no route test asserts on; a test that cares patches its own, and that patch wins. Two per-file fixes: `test_converse_cost_accounting`'s `mock_pricing` fixture already existed and its docstring already said "without this mock the call hits DynamoDB and fails" — it was simply opt-in, and the tests that did not request it were calling the real table. It is autouse now. `test_sessions`' delete cascade needed the artifact seam above. Two order-dependence traps worth recording, both of which made these look unreproducible: `tools/freshness` memoizes on a module-level TTL, and the artifact share cascade only fires once an earlier test has left artifacts "configured" — so several of these passed in isolation and failed only in full-suite order. Full suite: 9196 passed, 3 skipped, 0 errors, no quarantine. Co-Authored-By: Claude Opus 5 * docs(kaizen): close the live-AWS entry — quarantine burned down, suite halved Co-Authored-By: Claude Opus 5 * test: warm tiktoken before the guard arms, and write the rule into CLAUDE.md CI caught what my machine could not. `csv_chunker` calls `tiktoken.get_encoding("cl100k_base")`, which downloads the BPE vocabulary from an external CDN on first use and caches it on disk. My cache was warm, CI's was cold, so the guard correctly blocked a real network fetch and 16 tests failed. Not an escaped AWS call — a legitimate asset fetch — but also a real network dependency of the test run, which had been invisible precisely because every developer machine has it cached. Warming it in `conftest.py` *before* the guard arms keeps the guarded window hermetic without widening the allowlist, and makes the dependency explicit rather than incidental. Best-effort: offline, the CSV chunker tests now fail on their own terms rather than on a confusing socket error. Verified against a cold `TIKTOKEN_CACHE_DIR`. Also adds a **Tests** section to CLAUDE.md, since the rule is only useful if it is where someone looks before writing the next test: no test may reach the network; why the teardown assert is load-bearing (fail-open code swallows the block, so the raise alone leaves a green suite); how to find the *second*, unmocked dependency when the guard trips; the two module-level-memoization traps that make escapes look order-dependent; and that shared stubs belong in the directory's `conftest.py`. Full suite: 9196 passed, 3 skipped, 0 errors. Co-Authored-By: Claude Opus 5 * docs(spec): plan the agent state feedback refinement Records what the live indicator can honestly say beyond "Thinking" and "Running ", and sequences it into three PRs. Two findings shape the plan. `thinking` and `tool_start` already cross the wire and are dropped on the floor — `ToolInsightService` writes every transition into a signal with zero readers, and the loader derives its label from the content stream instead. That is not a style choice: the status drain runs between yields of the agent stream, so a `tool_start` queues for exactly the silence it exists to explain. Decoupling the drain is therefore a precondition for the richer phases, not a cleanup task. Also states the non-goals, so they are not re-litigated: no "responding" phase, no "almost done thinking" (an inference wearing a fact's clothes), no invented per-tool phrasing, and no persisted durations. Co-Authored-By: Claude Opus 5 * feat(chat): report how long the model spent thinking The reasoning block's header said "Thinking" forever, including on a block that finished nine minutes ago. It now reads "Thought for 17s" once the model has stopped. The span is client-observed — the parser has no server-side measurement of thinking time — and it is stated as such in the code, because the tool durations on the rail are measured inside Strands' event loop and the two are not the same kind of number. It is sound here because the agent loop starts a new Bedrock message at every tool round trip, so a reasoning block never spans a tool call and cannot silently absorb its time. The span closes at the first non-reasoning content in the same message, with message_stop as the backstop for a cycle that reasoned and emitted nothing else. Sub-second blocks read "<1s" rather than rounding up to a second the model did not spend. Live-only by construction: the duration rides `ContentBlock`, which only the stream parser populates, so a reloaded conversation falls back to the plain header with no persisted number — the same posture as the tool rail. It sits at the block's top level rather than inside `reasoningContent`, which is Bedrock's Converse shape. docs/specs/agent-state-feedback.md PR-1. Co-Authored-By: Claude Opus 5 * feat(attachments): auto-enable Spreadsheet Analysis for a session that holds a spreadsheet CSV/XLSX never go inline (_partition_attachments) and the analysis tools are opt-in in the picker, so on the default tool set an attached spreadsheet was a dead end: the model told the user to find a sidebar toggle. A session that holds a spreadsheet — this turn's attachment, or a READY upload from an earlier turn — now gets list_spreadsheets and analyze_spreadsheet appended to the turn's effective enabled_tools, gated on the caller's RBAC grant via can_access_tool (enables, never grants). Applied on the effective list so it flows into the cache key, every builder, the attachment guidance and the paused-turn snapshot as one value, and through the same seam on the MCP App dispatch paths so they read the slot the real turns fill. Sticky per session (memoised positive answer, one query otherwise), so the key does not flip between the attach turn and the follow-up. The guidance for the remaining case — a role that does not carry the tool — no longer points at a toggle the user may not have. ATTACHMENT_TOOL_AUTOENABLE_ENABLED=false is the kill switch. Co-Authored-By: Claude Fable 5.1 * fix(agents): count the system prompt against a probe message, not an empty conversation Bedrock CountTokens rejects an empty message list ('A conversation must start with a user message' — verified against dev 2026-09-18), and Strands swallows that into the chars/4 heuristic. So the systemTokens partition of the context breakdown has been an estimate since it shipped, never the authoritative count it looked like. Count the system prompt against a fixed probe user message and subtract the probe's own weight, a per-model constant measured once per process. Cold start is now three counts (two after the first agent on a model), none of them with an empty conversation. Co-Authored-By: Claude Fable 5.1 * perf(memory): retrieve long-term memory through a bounded client, and stop re-fetching the session's own summary per message The SDK retrieves long-term memory on every user message, awaited before the model call, through its shared read/write boto client with default retries. Under load a throttled RetrieveMemoryRecords (30/s per account by default) put boto's retry backoff on first-token latency — the same mechanism as the CountTokens throttle fixed in #1154. Override retrieve_customer_context with the SDK's exact contract (namespaces, relevance filter, block prepended to the last user message) but through a dedicated one-attempt, 2s-timeout client; writes keep their retries. The third retrieval per message was the current session's own summary namespace: AgentCore's summary of THIS conversation, re-injected into every user message the model already holds the conversation for — paid as input tokens on every turn and a lookup against the quota. Off by default; MEMORY_SUMMARY_NAMESPACE_RETRIEVAL_ENABLED=true restores it. Compaction still reads summaries where they matter, at checkpoint advance. Lookups per user message: 3 → 2, none of them retried. Co-Authored-By: Claude Fable 5.1 * feat(agents): drain agent_status while the agent stream is silent The status hook records model-call and tool-call boundaries from inside Strands' event loop, which has no route to the SSE stream — so the coordinator drains it from the emit loop. That loop only regains control when the agent stream yields, and during tool execution the agent stream yields nothing. A `tool_start` therefore waited out exactly the silence it existed to explain and arrived bundled with its own `tool_end`: measured on a three-tool browse turn, the indicator read "Thinking" for all 4.5s and never named a tool. `_merge_agent_status` races the agent stream against a short timer and drains on every pass, so a transition recorded mid-silence leaves the container while the tool is still running. The in-flight `__anext__` is held across timeouts rather than re-requested — an async generator cannot have two outstanding — and cancelled when the coordinator abandons the stream, which unwinds `process_agent_stream` through the cleanup-on-cancellation path it already documents and already took. Its own kill switch, `AGENT_STATUS_LIVE_DRAIN_ENABLED`, rather than riding AGENT_STATUS_ENABLED: that flag gates what the hook records, all of it in-process, while this changes how the turn's stream is consumed, so a regression here would be a streaming bug rather than a missing status line. Off, the between-yields drain is restored exactly. With the transitions now trustworthy, the SPA reads them: - the loader prefers `agent_status` over the content stream for the running tool, keeping the content derivation as the fallback — it needs no round trip, so it is more current when it fires, and it is all that is left if the drain is killed. - a parallel batch is finally legible. `ToolInsightService` tracks the matched tool_start/tool_end pairs, so three tools in flight read "Running list_assignments and 2 more" instead of naming whichever unresolved block happened to come first. - `thinking` narrows the generic label to "Waiting for the model", which rules out the agent build and head-of-turn context work that precede it and read identically today. docs/specs/agent-state-feedback.md PR-2. Co-Authored-By: Claude Opus 5 * docs(spec): mark PR-1 shipped and record the loader-mount finding PR-1 merged as #1159. Also records what building PR-2 turned up and the plan did not anticipate: the loader stays mounted for the whole turn, so a label keyed on the `thinking` phase would contradict text already on screen. The gate that avoids it is a workaround — the real fix is that the loader should not render under a streaming answer at all, which is a separate change to when it renders rather than what it says. Co-Authored-By: Claude Opus 5 * docs: respond to the AuraX load-test report, finding by finding Records our verdict on each of the five fixes and three residuals, what we shipped for each (#1154, #1155, #1156, #1157, #1158), and what is left. Two corrections the report should carry: the 30/s quota is RetrieveMemoryRecords, not the restore path, and ListEvents has a fixed 20/s per-session cap no increase moves; and '6 lookups per message' conflates a per-cold-start restore with a per-message retrieval. Two deliberate divergences: we bounded the pre-model token count rather than backgrounding it, because the same projection drives compaction timing and therefore prompt-cache cost; and we scoped the attachment fix to spreadsheets, since documents already route through document_read. Also notes what their probe-message detail found: systemTokens had been a chars/4 heuristic since it shipped. Co-Authored-By: Claude Opus 5 * docs: scope the deployment claim to what is verified in dev Five PRs are merged; only #1154 and #1155 have a completed dev deploy. The three merged since were superseded by concurrent deploys, so the status table now separates 'verified in dev' from 'deploy pending'. Co-Authored-By: Claude Opus 5 * fix(files): start the pattern-scan budget after the document is opened `_pdf_pattern` built its `_Budget` before calling `_open_pdf`, so lazily importing the pypdfium2 native library and parsing the PDF were charged to a budget whose docstring bounds "the walk across lines and pages". With enough of the budget spent before the loop, the first `clock.out_of_time()` check breaks immediately and the tool returns `match_count: 0`, `pages_searched: 0` and `timed_out: True` — a timeout reported on a scan that never started, and an empty answer to an ordinary pattern. Measured, the open is a skim rather than a wipe-out: on a warm machine the lazy import costs 46-95 ms and the parse is lazy in pypdfium2, so it does not scale with the document (0.4 ms for 1000 pages; 68 ms for a 6 MB file with a broken xref, the worst case that forces a full rebuild). That is 2-8% of the 2.0 s default, so full exhaustion needs a much smaller configured budget or a pathological cold-start import. The ordering is still wrong: it silently shrinks the scan window by an amount that varies with container warmth, and the failure it produces is indistinguishable from "no matches". `_text_pattern` had the milder version of the same thing — `text.splitlines()` inside the budget — now also moved ahead of the clock. The docx path already extracted its paragraphs before calling `_text_pattern`, so it needed no change. This loosens total wall time by the open cost, and deliberately adds no outer bound: `_pdf_index`, `_pdf_pages` and the S3 fetch already run with no clock at all, and index mode extracts text from up to 40 pages unbudgeted, so pattern mode's new exposure is strictly smaller than what the other modes already accept. Co-Authored-By: Claude Opus 5 * fix(chat): remove the parallel-batch count as dead code PR-2 shipped "Running list_assignments and 2 more" on the reasoning that `tool_start`/`tool_end` are a matched pair, so the count is real. The count is real; the case never occurs. `agent_factory.py` pins `tool_executor=SequentialToolExecutor()` so that concurrent browser tools cannot start two Playwright sessions. A batch therefore emits strictly interleaved start/end pairs and more than one tool is never in flight. Verified on dev against a real three-tool batch: the rail showed "3 tools", the loader showed one name throughout. I reasoned from the Strands default (ConcurrentToolExecutor) without checking that we override it, which we do, with a comment saying why. `ToolInsightService.runningTools` keeps its list shape — it costs nothing, it is what a concurrent executor would need, and it remains the truthful source for the ONE running tool: a `toolUse` block starts streaming when the model begins writing a tool's arguments, while `tool_start` fires when the tool begins executing, a measured ~640ms apart. Also records the dev verification in the spec, including where the time actually goes (3.75s before the first SSE byte on a warm turn), and marks PR-3 blocked: the window it wants to narrate is one in which no SSE channel is open, so it cannot be built as written. docs/specs/agent-state-feedback.md Co-Authored-By: Claude Opus 5 * feat(browser): hand the browser to the user so they can sign in PR 1 of docs/specs/authenticated-web-assessment.md. `browse_web` can only read what a logged-out Chromium can reach, which is the one thing every target in the Library and VPAT requests fails. This adds the backend half of the fix: the agent hits a login wall, calls `request_user_login`, the turn pauses, the user signs in with their own hands, and the turn resumes on the authenticated session. Its own tool, not a `browse_web` action. RBAC granularity is exactly one tool_id — a role's `grantedTools` rolls up into `enabled_tool_ids` and there is no sub-tool gate — so an action would have shipped an interactive browser inside our AWS account to every user who can browse, students included. As a separate catalog entry it is granted to named roles, ships `enabledByDefault: false`, and costs an ungranted user zero prefix tokens. An interrupt, not an endpoint. The turn genuinely must stop, and a turn that polls for login completion burns model calls to learn nothing. `ask_user_question` already proved the pause/snapshot/resume path and Strands routes tool- and hook-raised interrupts identically, so the `PausedTurnSnapshot`, the resume route and the breadcrumb need no special case. No URL anywhere near the model. `generate_live_view_url` signs with SigV4 query auth and caps at 300 seconds, so a URL in a tool result is re-emitted truncated at the `?` (PR #1101) and dead before a human reacts either way. `assert_no_url` enforces this on the interrupt, the SSE event and the persisted row rather than trusting it, and the broken `live_view` action is removed. app-api mints one per request in PR 2. Also: the D4 projection onto the session-metadata row (app-api cannot read `agent.state`), reaper pinning bounded by an abandonment deadline so a walked-away-from takeover stops billing, and `BROWSER_TAKEOVER_ENABLED`. Two deviations from the spec sketch, recorded in it: the event carries both `sessionId` (conversation, as everywhere else) and `browserSessionId`, and the conversation id plus the D4 write are stamped on in the streaming layer so the tool stays a static registry tool. Strands re-runs an interrupted tool from the top, so `take_control` is idempotent and a marker on `agent.state` carries the descriptor across the pause — without it a resume in a container that lost the session would start a second, unauthenticated browser and report success. Co-Authored-By: Claude Opus 5 * feat(inference-api): measure where the pre-stream time goes A warm artifact turn spends 3.75s between the click and the first SSE byte — against ~3.8s for the whole streamed response — and a cold turn 6.7s before the first `agent_status` frame. That window is invisible from both ends: the client has only a generic "Thinking", and the server cannot narrate it because FastAPI flushes headers when the handler returns its StreamingResponse, which is after model resolution, quota, RAG, tool building and `get_agent`. PR-3 wants to open a channel into that window. All three ways of doing it carry real risk — up to surgery on a ~1700-line handler that owns quota, resume 400s and RAG — so this measures the split first. On a warm turn the agent cache hits, so `get_agent` is probably not the bulk of it, and narrating the wrong stage is the waste the cost-effectiveness tenet exists to catch. `TurnPrelude` records a delta per stage and logs one `turn_prelude` line per agent turn, immediately before the response is returned — the moment the client can first hear anything, which is the end of the window being measured. Deltas rather than cumulative totals, so the slowest stage is the answer rather than always the last one. Content-free, like the cost rows: session id, stage deltas, and two booleans. Costs nothing against the model: a perf_counter call per stage and one log line per turn. Nothing reaches the prompt. Best-effort throughout — a measurement must never break the turn it measures. docs/specs/agent-state-feedback.md Co-Authored-By: Claude Opus 5 * docs(spec): rescope PR-3 from the dev measurement The `turn_prelude` instrumentation from #1163 answered the question it was added for, and the answer shrinks PR-3 rather than unblocking it. Four turns on dev: a cold agent cache spends 1478ms in `agent_build` against a 2542ms prelude; warm turns spend 0-38ms there and 641-678ms total. Against a client-side click-to-first-byte of 1156ms, the warm budget is ~478ms outside the handler entirely, 484ms preamble, 154ms tools, 38ms agent build. So a warm turn does not need narrating — nothing dominates, and 41% of it is not even in the handler — while a cold agent build is the whole problem. PR-3 therefore becomes "wrap `get_agent` only": move that one call inside the stream generator and emit `Getting ready…` before it, leaving every other stage eager. The quota check and the other HTTP-error-capable guards live in `preamble`, which stays ahead of the stream, so the restructure that looked frightening mostly evaporates. Records the feasibility check still owed (nothing on the non-resume path may raise HTTPException between `get_agent` and the response return) and declines the other two options with the numbers that rule them out: the app-api variant would cover the ~478ms that is not where the pain is, and a full handler restructure buys nothing for three stages that do not need narrating. I predicted `get_agent` would not be the bulk of it. That held for warm turns and was wrong for cold ones, which is the case users feel. Co-Authored-By: Claude Opus 5 * feat(inference-api): narrate the cold agent build A cold agent-cache miss spends 1478ms inside `get_agent` against a 2542ms pre-stream window, and every millisecond of it is dead air: FastAPI flushes response headers when the handler returns its StreamingResponse, and the build is awaited before that return, so there is no channel to say anything on. A warm build is 0-38ms. The non-resume path now builds the agent inside the stream generator and emits one `agent_status` `preparing` frame — "Getting ready…" — while it runs. The response opens first, so the longest silence in the product becomes a sentence instead of a stare. Three things the plan did not anticipate: The frame is emitted only when the build is actually slow. Announcing "Getting ready" for 38ms is a flicker, and it would land AFTER the generic "Thinking" the client shows from the moment the user hits send — reading as going backwards. The generator races the build against a 250ms threshold and emits only if it is still running. Peeking the agent cache instead would mean rebuilding its key out in the route, and a key that drifts from the real one is a bug this repo has paid for. A failed build needs its own error path: the handler has already returned, so neither `except` arm can see it, and without an in-generator catch it is a silent hung stream. It now surfaces as a conversational error and the `finally` still releases the lease. The SPA validator required `typeof cycle === 'number'`, and `preparing` precedes the event loop so it has no cycle — every frame would have been dropped silently, which is the exact failure that validator exists to prevent. The relaxation is scoped to `preparing`. Resume keeps its eager build: it validates submitted interrupt ids against the rebuilt agent and 400s on a mismatch, which has to happen before any byte is sent. That guard is also the only `raise HTTPException` between the build and the response, so the deferred path has none. Kill switch `AGENT_PREPARING_PHASE_ENABLED` (default on) restores the eager build exactly. docs/specs/agent-state-feedback.md PR-3. Co-Authored-By: Claude Opus 5 * feat(browser): mint a live view for a browser takeover PR 2 of docs/specs/authenticated-web-assessment.md, backend + SPA half. PR 1 pauses the turn and hands the browser to the user; this is how the user gets to see it. `POST /sessions/{session_id}/browser/live-view` on app-api. The client sends only a conversation id — the browser session is resolved server-side from the metadata row PR 1 projects, which is what stops any signed-in user streaming any browser in the account. Ownership is the metadata read itself, since that lookup is user-scoped, so another user's conversation is indistinguishable from a missing one. Minted per request rather than once, because the URL is SigV4 query-signed with a 300-second cap: a URL minted when the turn paused is dead before a human finds their password manager, and dead again on every reload. 404 means "no viewer here", 409 means "the session ended" — the SPA says different things for each, so they are not collapsed. The IAM is deliberately narrower than the Runtime's BrowserAccess statement: ConnectBrowserLiveViewStream, UpdateBrowserStream and GetBrowserSession only. No Start/Stop and no ConnectBrowserAutomationStream — app-api is the surface reachable with a session cookie, and the agent owns the browser lifecycle. A CDK test asserts the absences, not just the presences. SPA: the `browser_login_required` validator (which rejects any event carrying a url, mirroring the backend's assert_no_url), `BrowserLoginService`, and the resume path posting `{completed: true}` / `{skipped: true}` — always an object, never null, or the interrupt re-raises forever. Two corrections to D3, recorded in the spec rather than built blind: The viewer must NOT call app-api itself. app-api's CORS is an allowlist with allow_credentials=True, so the viewer calling the route from the sandbox origin would mean adding that origin to CORS_ORIGINS — and that origin is where we frame untrusted MCP App HTML. The SPA already holds the cookie; it should mint and postMessage the URL into the frame, which needs no CORS entry and no credentials in the viewer at all. And no new distribution. A second CloudFront origin means a second subdomain, a second us-east-1 cert and a second CDK_*_CERTIFICATE_ARN — and per #396's scar tissue a configured domain with a missing cert throws at synth, so it would break deploys until ops caught up. The mcp-sandbox origin already is what D3 describes. The viewer page itself is therefore still open, and the first thing it must answer is whether DCV's WebSocket survives that origin's CSP. Co-Authored-By: Claude Opus 5 * fix(chat): move the preparing-phase delay to the client PR-3 shipped a 250ms race in the stream generator: start the agent build, wait, and emit `preparing` only if the build was still running — so a warm build (0-38ms) never flashed a label nobody can read. Verified on dev: it never fired. A 1548ms cold build, six times the threshold, emitted nothing at all. `create_agent` is synchronous (`agent_factory.py`), so the build occupies the event loop for its whole duration and `asyncio.wait` cannot fire its timeout — it returns only once the build is already done, `finished` is non-empty, and the frame is skipped. The feature was inert in production from the moment it shipped, and silently: no error, just a label that never appeared. A timer only works where the clock runs. The backend now announces the build unconditionally, and the SPA holds the phase for 250ms before rendering it — a warm build is superseded by `thinking` well before that and never reaches the screen, while a cold one shows exactly as intended. Cost is one extra SSE frame per non-resume turn. The backend test that covered the race is replaced by one asserting the race is gone: a server-side timer around a synchronous build cannot fire, so its reappearance would mean the frame had silently stopped being sent again. Also fixes a test-collection trap hit while writing this. `tests/apis/` is itself a package named `apis` and pytest puts `backend/tests` on sys.path, so a file whose first `apis.` import happens inside a fixture can bind `apis` to the TEST package and die on `apis.shared.caching`. Every other test file imports from `apis.` at module level; this one now does too. docs/specs/agent-state-feedback.md PR-3. Co-Authored-By: Claude Opus 5 * fix(spa): wait for the tool/skill lists before assembling a chat turn A message sent a second or two after page load was assembled from selection sources that had not arrived yet. `ToolService` fetches `/tools/` in its constructor and `SkillService` fetches `/skills/` lazily on the first composer focus; until either lands, `getEnabledToolIds()` / `getEnabledSkillIds()` answer with an empty array, which is indistinguishable from "the user turned everything off". So turn 1 of a fresh session went out with no skills and a short tool list, and turn 2 went out with the real ones. Both `systemPromptHash` and `toolConfigHash` changed between the two calls, so turn 2 missed the prompt cache outright and re-wrote the whole ~15k-token prefix at the cache-write premium — for nothing the user did. Measured in dev on session 8a6c5f7c: turn 1 disclosed 0 skills / 12,665 tool tokens, turn 2 disclosed 3 skills + read_skill_file / 12,328, and logged `cacheStatus=miss_ttl_expired` with cacheReadTokens 0 and cacheWriteTokens 15,085. Both services now expose `ensureLoaded()`, and `loadTools()` / `loadSkills()` join an in-flight request instead of returning early — the old `if (this._loading()) return;` guard deduped the request but resolved immediately, which is what let a caller proceed on an empty list. `ChatRequestService` awaits both before building the request, on a normal send and on a Continue. The wait sits behind the optimistic UI (the user's bubble, the streaming state and the route change all happen first) and is bounded at 4s, after which the send proceeds on whatever has loaded. A slow or broken `/tools/` or `/skills/` therefore degrades to exactly the old behaviour rather than holding the message hostage. Co-Authored-By: Claude Opus 5 * perf(backend): precompile bytecode in the app-api and Lambda images Extends the fix from 8ec2f3a2 (inference-api) to the backend images that ship raw Python source. uv skips bytecode compilation by default and the COPY'd application source is never compiled at all, so each image parsed its whole import closure at startup. app-api gets both halves of the inference-api fix: UV_COMPILE_BYTECODE=1 in the builder for the venv, and `compileall` over /app/src. It is a long-lived ECS service, so the saving is per task start (deploy, scale-out, Fargate task replacement), not per session as on AgentCore. Measured at --cpus=1 on arm64, importing apis.app_api.main in fresh containers against a twin with all .pyc stripped: 5.0-5.7s compiled vs 10.6-12.7s stripped, ~6.8s median saving. kb-sync, kb-migration and scheduled-runs get `compileall` only. They install with pip, not uv, and pip already compiles site-packages by default (verified: 1279/1279 .pyc) — only the COPY'd source was raw (0/39 on scheduled-runs). UV_COMPILE_BYTECODE would be cargo-cult in a file that never invokes uv. These matter more than their tick rate suggests: /var/task is read-only on Lambda, so a cold start can never cache the .pyc it produces, and the dispatchers run off 5- and 15-minute EventBridge ticks that mostly land cold. rag-ingestion is deliberately left out; see the PR for the measurements. Image size cost is +1-2MB on the three Lambda images. app-api grows ~285MB uncompressed (~41MB compressed of .pyc, counted twice because the existing `chown -R /app` layer duplicates the whole tree) — noted as a follow-up, not changed here. Bytecode only: inference-api's warmup.py daemon thread is specific to the AgentCore first-message path and is not ported. Co-Authored-By: Claude Opus 5 * perf(app-api): stop shipping /app twice by moving ownership into the COPYs The final stage ended with `useradd ... && chown -R appuser:appuser /app`. A recursive chown rewrites every file it touches, so Docker wrote the whole tree into a second layer: /app shipped twice. That layer was 313MB on develop and 454MB once the previous commit added bytecode — the duplicate was eating roughly half of that commit's win before the image ever reached a task. Creating appuser before the COPYs lets --chown put the ownership in the COPY itself, which costs nothing, and the recursive chown goes away. 1.39GB -> 806MB, and 214MB below develop's current 1.02GB while now carrying the bytecode. ~126MB less to pull per task start (compressed). Ownership is preserved exactly, which matters more than it looks: the lifespan in apis/app_api/main.py resolves base_dir to Path(__file__).parent.parent — /app/src/apis, NOT /app — and os.makedirs output/uploads/generated_images THERE on every startup. appuser must own the source tree, not merely read it, or the service dies at boot on a PermissionError. Verified in the built image: /app, /app/src, /app/src/apis, /app/.venv and the three /app/* dirs are all appuser:appuser exactly as before, a real lifespan startup succeeds, and the dirs land under /app/src/apis. (The /app/output, /app/uploads and /app/generated_images the Dockerfile creates are dead for that same base_dir reason; kept and kept appuser-owned so this stays a pure layer change.) compileall moves below USER so the .pyc are appuser-owned rather than root-owned. Import timing is unchanged (4.7-5.1s both ways) — this is a pull-time change, not a runtime one. Co-Authored-By: Claude Opus 5 * fix(chat): stop the preparing label stepping backwards on exit Verified on dev, the `preparing` label rendered correctly on all five turns — 2.1s of "Getting ready…" on a 2.3s cold build — and then stepped backwards every time on the way out: "Getting ready…" → "Thinking…" (~40ms) → "Waiting for the model…". The settle flag was cleared when the phase left `preparing`. That opens a propagation window in which the label computes with the phase still `preparing` and the flag already false, falls through to the not-yet-settled branch, and shows "Thinking" — the exact backwards reading this delay exists to prevent, reintroduced by the fix for it. The flag is now armed on the way IN and never cleared on the way out. Leaving it set costs nothing: the only branch that reads it is unreachable unless the phase IS `preparing`, and re-entering that phase re-arms it. A fast build is still suppressed by `onCleanup` cancelling the pending timer, which was doing that work already. A unit test cannot see the propagation window itself, so the new case pins the invariant that removes it: leaving the phase must not disarm the flag, so a late read while still `preparing` can only ever resolve to "Getting ready". docs/specs/agent-state-feedback.md PR-3. Co-Authored-By: Claude Opus 5 * docs(spec): block institutional sites per session, not a second browser Rewrites D6 and the Security controls after working backward from the actual requirement: faculty and staff run accessibility and VPAT checks behind authenticated pages, and nobody uses the same capability to have an agent submit their own coursework. That is a constraint on which SITES, not which PEOPLE, and three findings say the drafted design could not implement it. RBAC cannot express it. Effective permissions merge as a union across every matched role (rbac/service.py) with no deny, and priority decides only the quota tier. A grad assistant in both the staff and current-term student Entra groups receives the union. At a university that population is large and permanent, so the RBAC split is demoted to a rollout control and the spec now says so rather than presenting it as a boundary. An application-level URL check cannot implement it either: takeover hands the human an interactive Chromium, so a check in request_user_login sees only the starting page and the user can navigate away from it. And the allowlist-on-a-second-browser design cannot be maintained. There is no UpdateBrowser operation, and policy files are read from S3 at CreateBrowser time and frozen thereafter, so an allowlist of vendors under evaluation would need a browser replacement per VPAT review. So: one browser, a MANAGED URLBlocklist of institutional systems, applied on every StartBrowserSession — whose type enum does accept MANAGED. Per session the list is read fresh, needs no CfnBrowserCustom change, and works on the pinned CDK, which does not expose enterprisePolicies at all. MANAGED specifically, because RECOMMENDED is user-overridable and so is not a boundary against the person driving the browser. Seeds the list with boisestatecanvas.instructure.com, pending the CISO review. Also resolves two long-open risks: profiles are account-level (CreateBrowserProfile takes no browserId), so a browser replacement does not invalidate saved logins; and networkConfiguration being create-only is moot now that every property is. Adds the one that replaces them — whether MANAGED is honoured at session start or silently downgraded, which is now the load-bearing assumption and must be verified on dev before anyone is granted the tool. Re-sequences the blocklist ahead of profiles: a persisted login is standing authenticated access with no takeover in the path, so shipping profiles first would remove the one control the user actually passes through. Co-Authored-By: Claude Opus 5 * fix(infra): gate the uploads CORS guard on the filtered origin list The synth guard tested `config.corsOrigins` for raw-string truthiness while `FileUploadConstruct` attaches CORS based on `buildCorsOrigins(config).length`, which trims entries and filters blanks. Any value that is truthy but filters to empty -- `","` from a templated list's trailing comma, `" "` from a YAML value that quotes to a space, `"${UNSET_VAR},"` in CI -- passed the guard and still produced an uploads bucket with no CORS rule: exactly the silent failure the guard was added to prevent. Gate on `buildCorsOrigins(config)` instead, with no `additionalOrigins`, so the guard and the bucket read the same value. Verified at real synth: `","`, `" "` and `",,"` now fail, while a list carrying a blank alongside a real origin still synths and attaches the rule. Co-Authored-By: Claude Opus 5 * feat(browser): block institutional sites with a MANAGED Chromium policy Implements the rewritten D6. A browser takeover hands a human a fully interactive Chromium inside our AWS account, so the only thing that can stop them navigating to the LMS and having an agent act as them is Chromium refusing — a check in request_user_login sees only the page the takeover started on, and RBAC merges as a union across matched roles, so a grad assistant holding both a staff and a student role gets both roles' tools. The policy is applied on every StartBrowserSession rather than attached to the browser resource. There is no UpdateBrowser, policy files are read from S3 at API-call time and frozen thereafter, and CfnBrowserCustom (aws-cdk-lib 2.251.0) does not expose enterprisePolicies at all — so the CFN route would need a custom resource and would still replace the browser on every edit. The session API takes the same shape with type accepting MANAGED, read fresh each time. MANAGED specifically, never RECOMMENDED: Chromium treats managed policies as mandated and un-overridable, while recommended ones are user-overridable defaults, which would make this advisory against the very person it constrains. A test asserts the type. The list lives in CDK config and deploys as the S3 object, so changing what the browser may reach is a reviewed deploy rather than an in-place edit. Seeded with boisestatecanvas.instructure.com pending the CISO review. The bucket is versioned: on a security control, what was allowed on a given date is worth more than the storage. Frees an env-var slot to fit under the runtime's 50-variable ceiling, which was exactly full. ENABLE_QUOTA_ENFORCEMENT was hardcoded to 'true' while quota.py already defaults it to 'true', so the entry only restated the default — removing it leaves enforcement on, which is the remedy that test recommends. The policy location is one S3 URI rather than a bucket/key pair for the same reason. An unconfigured or malformed BROWSER_POLICY_S3 starts the session with no policy and logs an error, because silently starting an unrestricted browser is the failure mode that matters. Co-Authored-By: Claude Opus 5 * feat(chat): show how long a finished turn took When a turn ended, everything describing it disappeared: the loading line went and took the elapsed timer with it. The rail kept per-tool durations, but nothing said how long the turn took. A finished turn now carries a one-line footer — "9.6s · 4 tools" — at the END of the turn, which is what the number describes: a turn spans several assistant messages and it covers all of them plus the tools and the agent build between. This needed a new field rather than the one that looks right. `latency.endToEndLatency` prefers the provider's own API-call time on the persist path, so summing it across a turn drops tool execution and the pre-stream agent build — a turn the user watched for 9s reads as 3s. It also disagrees with itself: the LIVE metadata event's `latencyMs` IS the whole turn, so the same field shows one number during the turn and a smaller one after a refresh. `turnDurationMs` is explicit: measured from the invocation arriving to the stream ending, emitted on the live event and persisted on the turn's last assistant message, so the recap reads the same either way. A test pins that the footer ignores `endToEndLatency`, because that is the trap this field exists to avoid. Two deliberate limits. It excludes the app-api hop (~478ms measured), so it reads slightly under the user's own stopwatch — the alternative, a client-measured click-to-done, is truer to the felt wait but cannot survive a reload, which "always on" requires. And turns written before the field show nothing rather than a zero, the same rule the tool rail already follows. docs/specs/agent-state-feedback.md Co-Authored-By: Claude Opus 5 * fix(chat): measure the turn recap from the turn, not the stream The end-of-turn recap shipped reading 2.1s for a turn the user waited 7.8s for. Caught on dev. `turnDurationMs` was measured from `stream_response`'s own `stream_start_time`. That was the start of the turn until PR-3 deferred the agent build into the stream generator: the build now runs before that generator is ever iterated, so the number excludes it — and the build is precisely the wait PR-3 exists to make visible. The two changes were developed in parallel and this is where they interact. The route now hands the coordinator `TurnPrelude.started_at`, the moment the invocation reached the container, so the recap spans the whole turn: preamble, RAG, tool building, the agent build, and the model. That property is deliberately wall clock while the stage marks stay on `perf_counter`. The coordinator subtracts it from a `time.time()` reading, and mixing clock domains yields a meaningless number rather than a slightly wrong one — a `perf_counter` value would have rendered as decades. Both invariants now have a test: the start is bounded by `time.time()` either side of construction, and a wall clock stepped to zero mid-turn leaves the stage deltas non-negative. docs/specs/agent-state-feedback.md Co-Authored-By: Claude Opus 5 * refactor(backend): delete the dead local-output mechanism The services created output/, uploads/ and generated_images/ inside the source tree on every startup and served them as StaticFiles mounts, and the agents package wrote per-session tool output one directory up. None of it ran: - The only writer, `_handle_tool_storage`, fires for tool names `run_python_code` and `finalize_document`. Neither exists anywhere in the repo. - Its only caller, StreamEventFormatter, has no production callers — stream_coordinator and stream_processor never reference it. Live SSE goes through apis/shared/harness/sse.py. - Config.get_*_dir() had exactly one consumer: that unreachable writer. - The frontend references /output, /uploads and /generated_images zero times. So the mounts served empty directories nothing linked to. This also explains a bug nobody hit: the mounts resolved to /apis/output while the writer used /output, so the download_url it built pointed at a mount that could not serve it. There was no point fixing the mismatch in a mechanism that cannot run. Generated output goes to S3. Deleted: event_formatter.py, tool_result_processor.py, the whole agents/utils package, the lifespan makedirs and StaticFiles mounts in both entrypoints, and the two dedicated test files. Also removes TestSSEFormatInvariantAndJSONRoundTrip (tagged Requirements 12.9/12.10) and its now-orphaned strategy. It asserted the DEAD formatter's bare `data: ` shape; the live path emits `event: \ndata: `, so it was testing a format nothing produces. Not repointed at the live emitter because the only equivalent is `_sse`, a private helper in an app_api route module — a poor import for an agent-core property test. The wire format stays covered by tests/apis/shared/test_harness_sse.py, which round-trips both named and bare-data events. backend/tests/architecture/test_source_tree_not_written.py replaces the deleted behaviour with a guard: no writable directory derived from __file__, no makedirs under /app/src, and no StaticFiles mount returning (it would need a writable /app again AND would serve whatever it holds on an unauthenticated route). Verified to fail — naming both offending lines — when the pattern is reintroduced, and to pass on restore. Co-Authored-By: Claude Opus 5 * perf(images): ship app-api and inference-api code root-owned Both service images ended with a recursive chown over /app: RUN useradd ... && chown -R : /app That did two things, both bad. It handed the runtime user write access to the code it executes, so a process compromised at runtime could rewrite its own source or drop a backdoor into site-packages. And because a recursive chown rewrites every file it touches, Docker wrote the whole tree into a second layer — /app shipped TWICE. /app/src and /app/.venv now ship root-owned: readable and executable by the runtime user, not writable. The previous commit deleted the last thing that wrote to local disk, so there is no writable directory under /app at all — no data dir, no env var, nothing to configure. app-api 1.02GB on develop -> 808MB (and it now carries bytecode) inference-api 1.43GB on develop -> 830MB (-600MB) inference-api is the one that matters most: on AgentCore every new conversation can land on a fresh micro-VM, so that image is pulled constantly rather than once per deploy. Also removes `mkdir -p /app/output /app/uploads /app/generated_images` from both. Nothing ever read or wrote those: base_dir has always resolved under /app/src, so they were dead the day they were added. Verified in both built images: /app, /app/src and /app/.venv are root:root; no /app/var; the runtime user gets EACCES writing to /app/src and /app/.venv; a real `async with lifespan(app)` boots; bytecode is still fully baked (596/596); and agents.utils.config is confirmed gone from the image. backend/tests/supply_chain/test_image_filesystem_hardening.py guards it: no recursive chown of /app, no COPY --chown of code, and the dead top-level asset dirs stay gone. Note for forks: the images no longer let the runtime user write anywhere under /app. Nothing in this repo does — the architecture guard added in the previous commit proves it — but a fork that writes to local disk will hit EACCES. Send that output to S3, as the rest of the stack does. Co-Authored-By: Claude Opus 5 * chore(infra): retire the dead directory env vars and refresh the cap comments UPLOAD_DIR / OUTPUT_DIR / GENERATED_IMAGES_DIR were set here to /tmp/*, but the runtime never read them for anything except one log line — the directories actually resolved from __file__, inside the source tree — so they pointed operators at paths nothing used. The mechanism they described has now been deleted outright, so there is nothing left to configure. This construct sat AT the AgentCore Runtime's hard 50-variable cap, the one that broke the dev Platform Stack deploy on 2026-08-05 (`maximum size: [50], found: [51]`). Retiring three dead variables takes the worst-case config to 47/50 — confirmed against the guard's own headroom output, not by counting lines. That made both comments describing the cap wrong. The construct no longer sits at the limit, and the test's worked example ("counts 47 while prod and dev deploy 50") shifted by the same three. The construct comment also pointed at the wrong test file; the assertion lives in runtime-env-var-limit.test.ts. Co-Authored-By: Claude Opus 5 * feat(browser): add the sign-in viewer, with no third-party code PR 2 shipped the live-view route but no UI, so a paused turn showed the user nothing. This adds the half that makes the feature usable: an inline sign-in prompt, and a DCV viewer served from the sandbox origin. No React and no npm. AWS's own BrowserLiveView component cannot be built from the public registry — it imports `dcv` and `dcv-ui`, neither of which is declared in its dependencies. `dcv-ui` does not exist on npm at all, and the only `dcv` package there is an unrelated third-party Vue component library. The React path was therefore a strict superset of vendoring the SDK, not an alternative to it, and would have put ~189 packages behind a surface where a user types a password. The viewer is plain ES2017 against the official Amazon DCV Web Client SDK instead. The SDK is fetched at build time, never committed: it is EULA-licensed and this repository is public, so vendoring it would be redistribution. AWS's own instruction is to place it on a web server, which is what platform.yml now does before synth. What makes that safe is not that it comes from AWS — three pins do: the exact version, the archive SHA256, and the signing-key fingerprint. The latter two live in the script rather than being read from the network, because whoever could swap the artifact could swap the published checksum and serve a different key; importing whatever key a URL offers today is trust-on-first-use on every build. The viewer stays at the sandbox origin rather than becoming an Angular component. The original reason — AWS ships only React — is gone, but the one that matters is not: this is where a password is typed, dcv.js forwards the keystrokes, and that origin holds no session cookie. The SPA mints the URL and postMessages it in, targeted at that origin explicitly rather than '*', and the viewer accepts messages only from the origin that framed it and never calls app-api. The frame's CSP names the minted URL's own origin rather than a wildcard, which the sandbox origin's existing CloudFront function composes into connect-src. That resolves the open question of whether DCV's WebSocket survives that CSP. Also adds sandboxOrigin to browser_login_required, and widens assert_no_url for it — deliberately, with the allowed keys documented: the guard exists for presigned URLs, which are credentials in URL form, not for a deployment constant. Found while building: the iframe reference was captured on the load event, so a viewer that announced itself first had its ready message dropped. Queried via viewChild now, with a test that fails on the old behaviour. Co-Authored-By: Claude Opus 5 * fix(browser): stop sending MANAGED at session level, which broke every session Measured on dev: StartBrowserSession rejects `type: MANAGED` outright — ValidationException ... Invalid value for parameter 'type'. MANAGED is not supported for session-level policies. The API's enum advertises `['MANAGED', 'RECOMMENDED']`, so this looked supported and the spec flagged it as the load-bearing assumption to verify before granting the tool. Verifying it is what found this. It does not degrade. The call fails, so no browser session starts at all and browse_web stops working entirely for anyone with the tool. That regression reached dev only; main is on Release/1.22.0 and never had it. Switches to RECOMMENDED, which restores browsing, and says plainly what that does and does not buy: Chromium treats recommended policies as user-overridable defaults, so the blocklist now constrains the AGENT — which drives through CDP and never opens settings — but NOT a human holding the browser during a takeover, who is the threat it exists for. So request_user_login must stay ungranted until the MANAGED policy is applied at CreateBrowser. That needs a custom resource, since CfnBrowserCustom does not expose enterprisePolicies, and it reinstates replace-per-edit on the browser resource — survivable only because profiles turned out to be account-level. A test now asserts the session policy is never MANAGED, so nobody tries it again and takes browsing down to find out. Co-Authored-By: Claude Opus 5 * fix(chat): end the preparing phase explicitly "Getting ready…" rendered on agent builds of 0ms, 1ms and 40ms — the exact case the 250ms client delay exists to suppress. Measured on dev across a controlled four-turn session: turn 1 missed the agent cache and built in 1631ms, turns 2-4 hit it and built in 1ms, 40ms and 0ms, and all four showed the label. `preparing` was never a state with an end; it was just the latest event. The client inferred the build had finished from the arrival of `thinking` — but `thinking` does not arrive until the head-of-turn context work and the event loop's startup have also run, hundreds of ms later. So the settle timer kept firing on a build that was already over. The backend now emits an explicit `prepared` frame the moment the build returns, carrying its measured duration, and the SPA treats that as the end of the wait. A bare marker would have done the job; the duration also says how long the thing the user was told about actually took. The unit tests missed this because they sent `thinking` 40ms after `preparing` — a sequence the backend never emits. They now model the real one, including the long gap before the model call, and the warm case fails without this fix rather than passing for the wrong reason. That is the third defect in this mechanism found only on dev, and all three shared a root cause: treating "most recent event" as if it were "current state". docs/specs/agent-state-feedback.md Co-Authored-By: Claude Opus 5 * fix(browser): stop double-prefixing the browser policy object key The policy landed at `policies/policies/managed-policies.json` while BROWSER_POLICY_S3 pointed at `policies/managed-policies.json`. Found by listing the dev bucket while setting up the MANAGED-policy experiment. The BucketDeployment's `destinationKeyPrefix: 'policies'` compounded with a `Source.jsonData` name that already carried the prefix. Both came from the same constant conceptually but were written out twice, so nothing caught it. The failure shape is the bad one for a security control: the deploy is green, the object exists, and the policy silently resolves to a missing key. Nothing errors. Derives the key and the deployment prefix from one pair of constants so they cannot drift, and asserts the env var the backend reads names the key the deployment writes. A template test cannot see inside the asset zip, so the guard is that the two are derived rather than duplicated. Co-Authored-By: Claude Opus 5 * docs(spec): correct the turn-recap shortfall, and record the validation The spec put the recap's shortfall at "~478ms, the app-api hop". That figure came from a warm turn. Measured end to end on a cold container, the gap is ~1.5s: a turn whose client stopwatch read 7821ms reported 6291ms — short by 1530ms, or 20% of the turn. The behaviour is correct and the trade is the one "always on" already decided (a client-measured number matches the stopwatch but cannot survive a reload). The number quoted for it was simply wrong by 3x, and a reader deciding whether that trade is acceptable needs the real one. Also records the post-fix validation: the label sequence with no step backwards, and turnDurationMs at 6291ms plainly distinct from the 2364ms model latency — which is what proves it now spans the agent build rather than reporting the remainder after it. Co-Authored-By: Claude Opus 5 * docs(spec): close the agent state feedback epic Every state is shipped and observed working on dev, including the fast-build suppression that had never been seen succeed until today. Records what a turn says now, what was declined and the measurement that settled each one, and the four defects that reached dev — all of which passed CI, and three of which share a single root cause: treating "most recent event" as if it were "current state". Also records what measurement killed (#1171: the agent cache works; that data was deploy churn) and what is deliberately left: `preamble` at 450-900ms every turn, the recap's ~1.5s shortfall, and the absent footer on conversations older than the field. Co-Authored-By: Claude Opus 5 * fix(browser): grant the CALLER read on the policy object, not just the browser (#1183) Every browser session on dev was failing: ValidationException ... Access denied to S3 object - bucket: ..., key: policies/managed-policies.json. Verify that the caller has permission to access this bucket and is the bucket owner. The service reads the enterprise-policy object as **the caller** of StartBrowserSession — the AgentCore Runtime execution role — not as the browser resource's execution role. The service's own documented prerequisite names the browser role, which is what the original grant followed, so this looked right and failed closed. It also fails loudly in the worst place: a policy the caller cannot read takes down EVERY browser session, not just the policy, so browse_web stops working entirely rather than degrading to unrestricted browsing. Grants the runtime role s3:GetObject/GetObjectVersion scoped to the single policy object — it never needs anything else in that bucket. The browser role keeps its grant too, since the service documents it and it costs one read-only object. Found by reading the runtime's CloudWatch logs; the model's user-facing paraphrase said only "an S3 bucket configuration issue", which would not have located it. Co-authored-by: Claude Opus 5 * feat(session): reveal copy and thumbs on response hover The per-response action rail sat permanently visible under every assistant message, competing with the answer itself. Fade it in on hover of the response instead, matching the metadata row directly below it, which already reveals on the same `.group`. Opacity only — deliberately NOT the `invisible` + `opacity-0` pair the metadata row uses. That row holds no interactive elements; these are buttons, and `visibility: hidden` would drop them out of the tab order, so `group-focus-within` could never fire and a keyboard user would lose copy and thumbs entirely. `opacity-0` keeps them focusable at rest and the row reveals on focus. Two things stay put: - `actionsPinned()` holds the row up without a hover once it carries state the user set — a cast thumb (and its reason chips) or a just-copied checkmark — so that state doesn't vanish with the pointer. - Continue / interrupted chips are not hover-gated at all. They report that the answer is incomplete and offer the only way to finish it, which a user cannot be expected to discover by hovering. Verified against the running dev server and the real compiled stylesheet: resting opacity 0, hovering the response 1, pointer away 0, and a button focused at rest is reachable and reveals the row. Co-Authored-By: Claude Opus 5 * fix(browser): connect the live view once, not once per postMessage (#1186) The sign-in viewer never streamed on dev. It framed, then failed with "Could not start the session. It may have ended." — the `dcv.authenticate` error path, code 10, behind a WebSocket that closed with `Close received after close`. The service was not at fault. Probed against real AgentCore, the live-view auth socket opens cleanly: the SigV4 query signature minted for the base `/live-view` path DOES validate for the `/live-view/auth` sub-path (signing `/auth` itself is what 403s), the required `dcv` subprotocol negotiates, and it opens with the automation stream both ENABLED and DISABLED — so `take_control` is not implicated either. The viewer was. The SPA posts the minted URL up to three times — after minting, on the iframe's `load`, and in reply to the viewer's `ready` — because it cannot know which fires first. The viewer guarded on an established `connection`, but that is assigned only after `dcv.connect()` RESOLVES. Two posts milliseconds apart therefore both reached `dcv.authenticate`, and the second socket's open closed the first. Guard on the connection ATTEMPT instead, and release the latch on every terminal outcome so a failed mint cannot lock the viewer shut for the session. The comment in the SPA claiming the later posts were no-ops was simply wrong, and is corrected rather than deleted. Adds the first test for `live-view.js`, which had none — it runs the real file in a `vm` context against a DOM stub. Mutation-checked: reverting the guard to its original form fails 2 of the 5. Co-authored-by: Claude Opus 5 * feat(ui): give the agent-state orb a halo, and settle it on finished turns The orb had two problems at opposite ends of a turn. While the turn ran it was a 7px dot fading between 0.35 and 1 opacity, which read as blinking rather than as something happening. When the turn ended it vanished outright, taking the one mark the user had been tracking peripherally. Running: restore the expanding halo this indicator used to have and lost in 9bc9bc6b. What does NOT come back is the old scale — that ring was 36px around a 12px dot and out-shouted the sentence beside it. At 7px with a 13px halo it keeps the motion inside a footprint that stays subordinate to the text. The halo and the core are separate pseudo-elements so their transforms do not multiply, the core breathes on scale at full opacity instead of fading, and the two run out of phase. Finished: the same orb at rest on the turn recap, muted to 0.55 and followed by the loader's own bullet — "orb, 7.8s, 2 tools". Muted with opacity rather than a lighter token, because opacity blends toward the backdrop and so reads as settled in both themes, where secondary-300 would mute on white and brighten on the dark surface. Also fixes a live bug uncovered while rewriting these rules: the amber retry dot never appeared in dark mode. `.pulse-dot.is-notice` and `:host-context(.dark) .pulse-dot` carry equal specificity, and with the dark rule last it won — so a model retry painted the routine colour, in exactly the case the colour change exists to signal. Co-Authored-By: Claude Opus 5 * fix(ci): trigger the Platform Stack deploy on the assets it deploys (#1187) * fix(browser): connect the live view once, not once per postMessage The sign-in viewer never streamed on dev. It framed, then failed with "Could not start the session. It may have ended." — the `dcv.authenticate` error path, code 10, behind a WebSocket that closed with `Close received after close`. The service was not at fault. Probed against real AgentCore, the live-view auth socket opens cleanly: the SigV4 query signature minted for the base `/live-view` path DOES validate for the `/live-view/auth` sub-path (signing `/auth` itself is what 403s), the required `dcv` subprotocol negotiates, and it opens with the automation stream both ENABLED and DISABLED — so `take_control` is not implicated either. The viewer was. The SPA posts the minted URL up to three times — after minting, on the iframe's `load`, and in reply to the viewer's `ready` — because it cannot know which fires first. The viewer guarded on an established `connection`, but that is assigned only after `dcv.connect()` RESOLVES. Two posts milliseconds apart therefore both reached `dcv.authenticate`, and the second socket's open closed the first. Guard on the connection ATTEMPT instead, and release the latch on every terminal outcome so a failed mint cannot lock the viewer shut for the session. The comment in the SPA claiming the later posts were no-ops was simply wrong, and is corrected rather than deleted. Adds the first test for `live-view.js`, which had none — it runs the real file in a `vm` context against a DOM stub. Mutation-checked: reverting the guard to its original form fails 2 of the 5. Co-Authored-By: Claude Opus 5 * fix(ci): trigger the Platform Stack deploy on the assets it deploys `infrastructure/assets/**` is staged into the mcp-sandbox bucket at synth, and `scripts/build/fetch-dcv-sdk.sh` runs inside platform.yml before synth. Neither was in the workflow's `paths:` filter. So a change to either deployed NOWHERE. frontend-deploy.yml does not ship those assets, and no workflow failed — the push went green, the merge looked clean, and the old artifact stayed live. That is exactly what happened to the browser sign-in viewer fix in #1186: it merged, CI passed, and the broken viewer remained deployed until the workflow was dispatched by hand. The same hole covers a `fetch-dcv-sdk.sh` version, SHA256 or GPG-fingerprint bump — a supply-chain pin change that would silently not take effect. Tests derive the expectation from what the workflow and the CDK tree actually do (scripts it invokes; directories the constructs stage) rather than restating the `paths:` list back at itself, so a newly staged directory is covered the day it is added. Mutation-checked: removing either filter fails all three. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 * fix(browser): stop sending every SigV4 parameter twice to the live view (#1189) #1186 removed a real duplicate-connect bug but the viewer still never streamed: one `dcv.authenticate`, still failing with code 10 behind a socket that closed immediately. Measured against real AgentCore, one probe at a time: correct single query -> socket OPEN, held DOUBLED query -> HTTP 403 That is what we were sending. The SDK APPENDS `httpExtraSearchParams` to whatever URL it is given, and we handed `authenticate` the full signed URL with its query still attached AND the extras callback — so every SigV4 parameter went twice and the service refused it. `connect()` has always stripped the query for exactly this reason; its comment even says "the transport appends to whatever URL it is given". `authenticate()` was simply missed, and the failure mode (a socket that opens then closes, surfacing as "Failed to communicate with server") reads like a service problem rather than a malformed request. Also ruled out by probe, so the next person does not re-walk them: base-path SigV4 signing IS valid for the `/auth` sub-path (signing `/auth` itself 403s), the `dcv` subprotocol is requested correctly by the SDK, an `Origin` header changes nothing, and `take_control` is not implicated. Mutation-checked: restoring the signed URL fails the new assertion. Co-authored-by: Claude Opus 5 * perf(inference-api): open the preamble stage, and make a fix to it provable (#1184) * perf(inference-api): decompose the preamble stage into five sub-marks `agent-state-feedback.md` PR-3 measured the pre-stream window into four stages, fixed the one that dominated a cold turn (`agent_build`), and left `preamble` closed. On a warm turn `preamble` is the largest stage in that table — 460-494ms of a 641-678ms handler total — and it is a single mark spanning ~500 lines, so nothing says which of its stages owns the time. Split it into `preamble.ownership` / `.skills` / `.files` / `.session_state` / `.quota`, and teach `TurnPrelude.emit` to sum dotted stages back into a `groups` field. Without that sum, decomposing the stage would discard the only measurement anyone has of the window — the four-turn dev baseline is stated in terms of the single coarse number, and nothing else in the log reproduces it. `groups.preamble` now does. Measurement only: no behavior changes, and the code reading suggests but does not prove where the time goes. The accompanying spec records the hypothesis (eight serialized reads of the same session row, five of them in `session_state`) *and* what would falsify it, so the next PR is decided by the log rather than by the reading. Nothing reaches the model, the conversation, or the cacheable prefix — four extra `perf_counter()` calls and one log field per turn. Co-Authored-By: Claude Opus 5 * perf(observability): make a preamble fix provable, not just attributable PR-1 makes the preamble attributable. It does not make a fix provable, and those need different instruments: attribution needs one turn, proof needs a population. Three parts. **The field that would have lied to us.** `time_to_first_token` is the obvious instrument and the wrong one: it measures from the coordinator's `stream_start_time`, which since agent-state-feedback PR-3 runs after the preamble AND after the agent build. It is blind to the app-api hop, the whole preamble and the whole build — so a successful PR-2 moves it by exactly zero. Documented in the spec so nobody validates with it. Same trap this repo already paid for once with `latency.endToEndLatency` vs `turnDurationMs`. **Percentiles instead of grepped log lines.** `TurnPrelude.emit` now also writes one EMF record per turn into `AgentCoreStack/TurnLatency` through the existing helper — no new infrastructure, no new IAM. Metric names are derived from stage names rather than listed, so a new mark cannot silently go unmeasured. Dimension-less like every other EMF caller here; `isResume` and `deferredBuild` ride as log properties and Logs Insights does the slicing a dimension would have done badly. **Widgets on the EXISTING AgentCore dashboard, not a fourth one.** A dedicated construct was built and then removed: `observability-platform-dashboard.test.ts` pins the stack at three dashboards because CloudWatch charges $3/month beyond that. The ceiling is a deliberate cost decision with a test guarding it, and it is not worth breaking as a side effect of adding widgets. Folding in is the better design anyway — that dashboard already graphs AWS's own data-plane `Latency`, so the gap between it and `PreludeTotalMs` is an independent read on the routing overhead no server-side stage can see. Two of the widgets exist to catch our own errors rather than the platform's: a split by turn shape (a shift toward resumes would look exactly like a latency win) and an unaccounted-time panel (which says when the decomposition is incomplete). No alarms: a threshold needs a baseline and there is none yet. Inventing one is the guessing the spec exists to prevent. Kill switch `TURN_LATENCY_METRICS_ENABLED` (default on) — unlike PR-1's marks, which stay ungated, an EMF line costs log ingestion and custom-metric charges. Co-Authored-By: Claude Opus 5 * perf(observability): give turn latency its own dashboard, and buy the fourth Reverses the placement decision in the previous commit, not its substance. PR-1b put the pre-stream stage widgets on the AgentCore Runtime dashboard to stay inside CloudWatch's free three. That version worked and was still worse: this board is read *while shipping a latency change*, side by side with load-test output, and burying the stage breakdown under runtime-health widgets answering an unrelated question made it harder to use for its one job. So the fourth dashboard is bought deliberately at $3/month, against a 450-900ms wait on every turn. The pinned count in `observability-platform-dashboard.test.ts` moves to four and now reads "three free + one bought", with the argument recorded next to it — the point of pinning the number was never that three is sacred, it was that crossing the tier should be a decision somebody made rather than a side effect of adding widgets. The next one should have to argue the same way. The AgentCore Runtime board stays the companion: it graphs AWS's own data-plane `Latency`, so the gap between that and `PreludeTotalMs` is the routing overhead no server-side stage can see. Both headers point at each other and the platform dashboard links to all three drill-downs. `inference-agentcore-construct.ts` is byte-identical to its pre-PR-1b state. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 * fix(browser): sign the live-view stream socket, not just the auth socket (#1190) * fix(browser): sign the live-view stream socket, not just the auth socket `connect` reads `httpExtraSearchParams` ONLY from `observers`. We passed it at the top level, where it is silently ignored — so the stream socket opened with no query string at all, unsigned, and the service refused it. Measured by running the shipped 1.13.2 bundle against a stubbed WebSocket: top-level -> wss://.../live-view/ws (UNSIGNED) observers -> wss://.../live-view/ws?X-Amz-Algorithm= (signed) That is exactly what the browser console showed: repeated failures to `/live-view/ws` carrying no parameters. `authenticate` is the opposite — it reads the callback from the top level — which is what made the top-level form look right. The two entry points genuinely differ. `firstFrame`/`disconnect` move into `observers` with it, because the SDK honours observers or callbacks and not both. This is also what AWS's own BrowserLiveView component does; its config was read from the published package rather than guessed. Mutation-checked: restoring the top-level form fails the new assertion. Co-Authored-By: Claude Opus 5 * chore: remove a scratch probe file committed by mistake `.connprobe.mjs` was a throwaway jsdom harness used to capture the DCV SDK's WebSocket URL. Its cleanup `rm` sat in the same command line as a node process that never exited, so `git add -A` swept it into the branch. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 * perf(inference-api): read the session row once, not eight times (#1191) Measured on dev after #1184 shipped the instrumentation (docs/specs/turn-latency-preamble.md): a warm turn spends ~455ms in the preamble, and ~445ms of that is DynamoDB reading ONE item eight times. The per-read cost is the finding. `preamble.ownership` is exactly one GSI query and nothing else, and it measures 53-55ms — dead stable across turns. `preamble.files` on a turn with no attachments is also exactly one read, also 53ms. So a GSI query from inside an AgentCore Runtime container costs ~53ms, not the ~12ms an in-region figure suggests, and the spec's earlier estimate that this work would "recover roughly a quarter of the stage" was low by 4x. `load_session_meta()` performs one `SessionLookupIndex` query and returns a `SessionMetaSnapshot` carrying both answers the preamble needs: this user's META row, and whether the session belongs to someone else. Both were always in that one response — `_get_session_by_gsi` discarded the half the ownership probe needed, which is why the probe was a second query of the same index for the same key. The route reads once and threads the snapshot into `pop_pending_attachments`, `ensure_session_metadata_exists` and the four `clear_*` helpers. Every one takes `snapshot=None` by default, so non-preamble callers are untouched. Two decisions worth naming: **Explicit, not a per-request memo.** Memoising inside `_get_session_by_gsi` would be a three-line diff and the wrong shape. CLAUDE.md's "never cache session state" rule has been paid for twice (#741, #751); a snapshot a caller opts into cannot leak into a caller that needs a fresh read, and a memo can. **The snapshot object, not the row dict.** `row is None` is a real answer — "no META row yet" — not "nothing prefetched". Taking the row alone would make a brand-new session indistinguishable from an absent prefetch, so the first turn of every conversation would silently fall back to re-reading. Pinned by test. Safety: the helpers were already best-effort and fail-open; the GSI is eventually consistent so re-reading never bought consistency; the single-flight lease excludes a concurrent turn on the same session; the SK is static so the conditional writes' key cannot drift. The atomic parts stay atomic — the `ReturnValues=UPDATED_OLD` writes are untouched, and the snapshot only replaces the gate read. Expected: preamble ~455ms -> ~125ms. To be validated on dev, not assumed. The quota session-notice read (62ms) is deliberately left for PR-2b: it goes through `get_session_metadata`, which lazily backfills cost aggregates, and skipping that would quietly stop session notices on legacy rows. Co-authored-by: Claude Opus 5 * fix(sandbox): allow data:/blob: in connect-src so DCV can load its decoder (#1192) The live view died with "Display channel is not available" and a black frame. The browser said why: Connecting to 'data:application/octet-stream;base64,AGFzbQ...' violates the following Content Security Policy directive: "connect-src 'self' https://bedrock-agentcore.us-west-2.amazonaws.com ..." `AGFzbQ` is the WebAssembly magic number. The DCV Web Client SDK fetches its decoder from an inline `data:` URI, and `connect-src` was the ONLY directive here that did not allow `data:`/`blob:` — script-, style-, img-, font- and media-src all already do. This does not widen exfiltration. `data:` and `blob:` name bytes inside the document, not a network peer, so nothing can be sent anywhere through them; the remote origins a page may reach are still exactly `connectDomains`. The existing tests pinned the old directive string and failed on this change, which is what they are for — updated, keeping each one's intent. The malformed-input case asserted "nothing follows 'self'" as a proxy for "no injected domains"; it now pins the directive exactly, which tests the same property without depending on the constants being empty. Mutation-checked: reverting the directive fails 10 of 34. Co-authored-by: Claude Opus 5 * perf(quota): take the session cost from the row the preamble already read (#1193) PR-2b of docs/specs/turn-latency-preamble.md, and the eighth and last read of the session META row in the preamble — a measured 62ms. `check_quota` / `_resolve_session_notice` take `session_total_cost: Optional[float] = None`. The inference-api route supplies it from the snapshot PR-2 introduced, but ONLY when that row actually carries `totalCost`. `None` means "not known", never "zero", and that distinction is the whole safety of this path. `get_session_metadata` lazily backfills the cost aggregates for legacy rows written before write-time aggregation; treating an absent attribute as 0.0 would skip the backfill and silence the session notice on exactly the long-lived conversations it exists to catch — a latency fix quietly becoming a feature regression. A genuinely free session reports 0.0, which is falsy, so the check is `is None` rather than truthiness: a truthy test would send that session back to the read this PR removes and the bug would be invisible, because the answer would still be correct — just expensive. app-api's converse route passes nothing and keeps its read unchanged. Expected: preamble.quota 62ms -> ~0, warm preamble ~163ms -> ~100ms. To be validated on dev. Co-authored-by: Claude Opus 5 * fix(browser): grant ConnectBrowserLiveViewStream on *, as AWS requires (#1194) This is why the sign-in viewer never streamed. AWS's own service reference lists the resource types for each action: GetBrowserSession -> ['browser', 'browser-custom'] UpdateBrowserStream -> ['browser', 'browser-custom'] ConnectBrowserLiveViewStream -> [] <- none An action with NO resource types never matches a resource-scoped statement, so granting all three on the browser ARN made the third an implicit deny. `iam simulate-principal-policy` confirms it: allowed for the first two and implicitDeny for the third, from the SAME statement on the SAME ARN. It failed silently and late. `generate_live_view_url` only signs locally and calls no API, so app-api minted a URL happily; the denial appeared only when the browser opened the socket and the service closed it, surfacing as DCV auth code 10, "Failed to communicate with server" — which reads as a service fault, not a missing permission. Proven by running the real SDK against a real session outside the browser: `dcv.authenticate` SUCCEEDS with admin credentials (returns sessionId and authToken, socket closes 1000) and fails from app-api. Same SDK, same service, same session — only the signing principal differs. `*` is as narrow as this action can be expressed. The statement is split so it carries that one action and nothing else, and the resource-typed actions stay scoped to the browser ARN. app-api still cannot start, stop or drive a browser. Mutation-checked: re-scoping the connect grant fails the new assertion. Co-authored-by: Claude Opus 5 * fix(ui): show the turn recap only on the latest turn, in the loader's slot Two problems with where the recap landed, both from it being rendered inside the turn rather than in the tail. It appeared on every finished turn. The number describes what just happened, and a column of durations down the length of a conversation turns a punctuation mark into a metrics readout — every earlier turn's recap is a fact nobody asked for, competing with the answer it sits under. It is now derived once, for the last turn only. It also overlapped the live line it was meant to replace. The old guard was `streamingMessageId`, which clears at message_stop; the loader lives on `isChatLoading`, which clears later at stream close. That gap is exactly the window where both were on screen, so the finished state read as a second, different thing appearing beneath the running one rather than as the running one settling. The recap is now the loading line's `@else`, in the loader's own slot with the same padding, gated on the same signal — one slot, never two, and nothing moves when they swap. Verified across two turns in one session: 1 loader / 0 recaps while running, 0 loaders / 1 recap when done, and the first turn's mark gone the moment the second turn starts. Co-Authored-By: Claude Opus 5 * fix(frontend): send Cache-Control on deploy so a new build is actually served The SPA was synced to S3 with no Cache-Control metadata on any object, so CloudFront served index.html with no Cache-Control at all. With no explicit freshness directive a browser falls back to HEURISTIC caching — roughly 10% of the document's age when it was cached — which for a shell that had been sitting in the bucket for days means hours of reuse with no revalidation. Hashed bundle names do not save you here. The stale shell names the OLD hashes, those objects still exist, and everything loads cleanly. The failure mode is not an error: it is a deploy that appears to have silently not happened, which is exactly how it was found — the orb work in #1188 was live at the origin while the browser kept rendering the previous build. The CloudFront invalidation the deploy already runs does not help, because the stale copy is in the browser, not at the edge. The sync is now two passes: - Hashed bundles (main-ZMUT4FS2.js, chunk-4DOJ3VBG.js, styles-*.css) get `public, max-age=31536000, immutable`. Safe because the name is a function of the content. - Everything else, index.html above all, gets `no-cache` — cache it, but revalidate. That costs a 304 on a 32KB file. The pattern matches an 8-character hash rather than a `.js` suffix, on purpose: `public/` is copied verbatim and unhashed, and it holds `audio/pcm-capture.worklet.js`, which an extension rule would have pinned for a year. Verified against all 66 unhashed files in `public/` plus the five bundle names live on dev: 5 pinned, 0 leaked. Pass order is load-bearing and commented as such — the second pass sweeps the whole tree so --delete keeps pruning stale bundles, and relies on sync skipping the files the first pass just uploaded so they keep the immutable header. Co-Authored-By: Claude Opus 5 * fix(browser): size the display on firstFrame, not when connect resolves (#1196) The viewer logged an unhandled rejection on every connect: Uncaught (in promise) {code: 2, message: Display channel is not available.} `requestDisplayLayout` was called from `connect().then()`, but the display channel is not up at that point. It returns a PROMISE, so the try/catch around it never saw the failure — it escaped as an unhandled rejection, which is also why the comment claiming "not fatal" was never actually exercised. Moved into the `firstFrame` observer, which by definition fires once the display channel is live, and the returned promise is now caught so a declined layout stays a console note rather than an unhandled rejection. The stream remains usable at whatever size the server chose. Mutation-checked: calling it from `.then()` again fails the new assertion. Co-authored-by: Claude Opus 5 * perf(aws): cache the boto3 clients the session path rebuilds per call (#1198) PR-2 made this measurable. With the preamble's eight DynamoDB reads collapsed into one, `preamble.session_state` still measured 17-21ms on dev while doing no IO at all — six helpers each constructing `boto3.resource("dynamodb")` before reaching their snapshot short-circuit, at the ~1.5ms per construction measured at the start of this work. After PR-2b took the quota read, that residual is ~20% of the 95ms the warm preamble now costs. `apis/shared/aws_clients.py` caches resources and clients per (service, region) and exposes `get_dynamodb_table()`. All 29 construction sites in `sessions/metadata.py` are converted; 23 now-dead `import boto3` lines go with them. Seven of those sites used single quotes and the first pass missed them. Worth naming: a partial conversion would have left the residual half-present and read on the dashboard as "the cache underperforms" rather than "the cache was not applied" — much harder to notice than a clean miss. THE MOTO TRAP `mock_aws()` is entered per test. A client cached under one test's mock keeps pointing at a backend torn down when that test ends, so the next test talks to a dead backend — or to a live AWS endpoint. The failure is order-dependent, the same shape as the static-memo leak this repo already paid for across SPA spec files. So the cache is explicitly resettable and all three `aws` fixtures reset it on entry AND exit. An entry-only reset would still let the last test in a file leak into a file that never uses the fixture, which is why the teardown half has its own test. THE LAMBDA IMAGE CLOSURE A new `apis/shared/` module breaks the scheduled-runs image, which COPYs an explicit import closure rather than the whole package. Caught by `test_lambda_image_copies_its_full_import_closure`, which also names the fix: a COPY in `Dockerfile.scheduled-runs` AND a `MANIFESTS` entry in `build-one.sh` so the content-hash tag notices future edits. Both are here. Without the second half the image would build correctly once and then go stale silently. Also records PR-2b's dev validation: preamble 163ms -> 95ms, whole pre-stream window 349ms -> 263ms, 79% below baseline. Co-authored-by: Claude Opus 5 * fix(browser): mint one live-view URL per open, not two (#1199) Opening the sign-in viewer issued TWO `POST .../browser/live-view` about 30ms apart, measured on dev. `openViewer()` set `minting` but never checked it, so a second activation of the button re-entered it. Each mint is a live SigV4-signed credential for the browser session, which is reason enough on its own. It also produced a second `dcv.authenticate` that failed with code 10 while the first was still connecting, leaving an error in the console of an otherwise healthy stream and sending me looking for a server fault that was not there. ⚠️ Test-design note, because the first version of the second test was worthless: it clicked "Open sign-in" again and asserted the mint count had not changed — but that button no longer exists once the viewer is open, so `again?.click()` was a no-op and it passed with the guard REMOVED. It now asserts the affordance is gone, which is a true statement about the UI, and the guard itself is covered by the double-activation test. Mutation-checked: removing the guard fails that one. Co-authored-by: Claude Opus 5 * fix(browser): don't paint an error over a live stream (#1200) The SDK closes the auth WebSocket as soon as it is finished with it, and reports that close through the `error` callback EVEN THOUGH authentication succeeded. Measured on dev, with a single mint so nothing else is in play: 23:32:58.148 [connection] WARN ... <- connect() ALREADY running WebSocket .../auth failed: Close received after close dcv.authenticate failed {code: 10} 23:32:58.456 [connectionmanager] Added connection 1 23:32:58.467 [dcv::display:1] ... 23:32:59.177 [dcv::input:1] ... `connect` is running BEFORE the error arrives, so `success` fired first and the stream establishes normally afterwards. In Node the same close is a clean code 1000; Chrome surfaces it as a socket error. `#status` is an absolutely-positioned overlay covering the whole display, so the error handler was painting "Could not start the session. It may have ended." across a working stream — and clearing `starting`, so a later post could open a second one. Ignore an auth error once a session exists; a real pre-session failure still reports, and a disconnect resets the flag so a later attempt can fail loudly again. This also retires the `code: 10` I chased for several rounds as a service fault. It was never a failure at all. Mutation-checked: removing the guard fails the new assertion. Co-authored-by: Claude Opus 5 * perf(observability): decompose the agent build, and correct what PR-3 disproved (#1201) * perf(observability): decompose the agent build into sub-stages Same move that opened the preamble, applied to what is now the largest number left in the prelude. After PR-2/PR-2b/PR-3 the warm preamble is 95ms, down from 455ms. `agent_build` is ~2950ms on a cold turn against 1-47ms warm, and nothing says which part of it that is. The spec's standing hypothesis was the serial MCP `tools/list` pre-flight, and that hypothesis is UNVERIFIED in two ways worth naming: - dev loads exactly ONE external MCP server (`google_tasks`, over two days of logs), so parallelising it there would save nothing and would be unmeasurable in the only environment we can validate in; and - `AgentFactory.create_agent` receives `tools` and `session_manager` already built, so the expensive work is upstream of it in the agent's own constructor — where `SessionFactory.create_session_manager` restores conversation history from AgentCore Memory, which has never been timed. Also worth recording: the fix the spec proposed — `asyncio.gather` over the servers — would have been a NO-OP. `MCPClient.load_tools()` is `async def` with no await in its hot path: `start()` blocks on `_init_future.result()` and `_list_all_tools_sync()` blocks on `_invoke_on_background_thread(...).result()`. Every coroutine blocks the loop before it yields. That is the same shape as the starved-timer bug agent-state-feedback PR-3 already paid for, and measuring first is what caught it. Seven sub-stages — prompt, registry, session_mgr, tools, hooks, plugins, finalize — plus `agent_build.rest` for the remainder. `groups.agent_build` sums them, so the pre-split series stays comparable, reusing the grouping PR-1 built. Eight EMF metrics derive automatically from the mark names, and the dashboard gains a p90 breakdown row (p90 not p50: a warm build is ~0 across the board, so p50 would be a row of flat lines and the cold builds worth fixing live in the upper percentiles). A CONTEXTVAR, AND WHY THAT IS NOT INCONSISTENT WITH PR-2 PR-2 threaded its session snapshot explicitly and rejected an implicit memo. This does the opposite on purpose. There, implicit state going wrong meant stale session data in production — the bug CLAUDE.md names and this repo has shipped twice. Here it means a timing number is missing or mis-attributed: nothing the user sees, nothing persisted, nothing reaching the model. Against that, the explicit route costs a kwarg threaded through a type registry and three agent classes that do not share constructor signatures. Not propagated across threads, deliberately noted: contextvars do not cross into a ThreadPoolExecutor and the MCP load path crosses one, so a future caller marking from inside it would silently record nothing. Co-Authored-By: Claude Opus 5 * docs(spec): record PR-3's validation, and correct the per-read claim it disproves PR-3 took the warm preamble from 95ms to 22-37ms and the whole pre-stream window to 128-189ms — cumulatively 95% and ~75% below where this spec opened. It also disproves something this spec asserted in bold. After PR-1 it claimed a GSI query from an AgentCore Runtime container costs ~53ms, 4-5x in-region. It does not. `preamble.ownership` is exactly one query and nothing else; with a cached client it measures 6-7ms. The 53ms was ~47ms of boto3 resource construction on throttled container CPU plus ~6ms of entirely normal DynamoDB. The spec anchored on a ~1.5ms-per-construction figure measured on a laptop, which is ~47ms on the container — thirty times off — and then found a story that fit it. So PR-2 was the right fix for the wrong stated reason: it removed eight client constructions, not eight slow network calls. Recorded rather than quietly overwritten, because the reasoning error is more reusable than the result: a laptop number is not a measurement of production, and a hypothesis that keeps fitting the data can still be wrong about mechanism while right about remedy. The sub-stage marks are what made it visible — a single `preamble` number would have shown the same improvement and hidden the bad explanation. Also names the new largest warm stage: `tools` at 98-102ms, never decomposed. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 * fix(browser): broaden the Canvas blocklist entry to instructure.com (#1202) * fix(browser): block Canvas by all its hostnames, not just the vendor one The blocklist was DEFEATED on dev in about ten seconds. During a takeover a human typed `canvas.boisestate.edu` and the Canvas login page loaded normally. The list contained only `boisestatecanvas.instructure.com`. Chromium's URLBlocklist matches on HOST, not on the service behind it. Both names resolve into Instructure's 99.86.101.0/24 and serve the same LMS, so naming one of them stopped nothing that mattered — a student would use the address they already know. The enforcement mechanism was never the problem: the agent is refused with ERR_BLOCKED_BY_ADMINISTRATOR, and that holds under human control too. The DATA was wrong. instructure.com covers the host AND its subdomains, so the boisestatecanvas/.test/.beta instances are included canvas.boisestate.edu is not a subdomain of anything blocked and has to be listed on its own The general lesson, recorded in both the config and the test: when adding a site, enumerate its aliases FIRST — vanity CNAMEs, regional hosts, the mobile hostname. A list naming only the obvious host is a control that looks real and is not, and it fails silently because the blocked name still blocks. Mutation-checked: restoring the single-host list fails both guards. Co-Authored-By: Claude Opus 5 * fix(browser): don't block the sign-in page; block where it leads Correcting my own overreach. `canvas.boisestate.edu` is Boise State's sign-in / discovery page, not the Canvas app, so seeing it load during a takeover was not the bypass I called it. Blocking it would have been actively wrong: reaching a login page is exactly what an accessibility or VPAT review needs to do, which is the use case this feature exists to serve. I would have broken the primary purpose to defend against a page where no work is submitted. What survives from that alarm is a real improvement. The seed was the single host `boisestatecanvas.instructure.com`; it is now `instructure.com`, which covers that host AND its subdomains, so the `.test.` and `.beta.` Canvas instances are no longer unlisted side doors. Still unverified, and the thing that actually decides whether this control holds: signing in from the discovery page and confirming the destination is refused. The doorway being open is fine; the room must not be. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 * feat(browser): full-screen the sign-in viewer, and say it can be driven (#1203) Two problems with the inline frame, both reported from a real takeover: the remote browser is letterboxed into a message-column-width box with no room to work, and nothing tells the user they can actually drive it — it reads as a screenshot. Adds "Take over full screen", which expands the viewer over the whole conversation pane, and a plain statement while expanded: "You're driving this browser — click and type in it as you normally would." Escape or "Exit full screen" returns. ⚠️ The iframe is ONE element across both states, and the tests enforce it. Re-creating it on expand would reload it, tearing down the DCV stream and losing whatever the user had typed — a password, most likely. That is also why this is NOT a CDK Dialog as the Angular guide otherwise requires: a dialog or a CDK portal re-parents the content, and browsers reload an iframe when it moves in the DOM. Escape, the close control and the ARIA roles are wired by hand instead, which is the trade this one surface has to make. The expanded layer is role="dialog" + aria-modal only while expanded, so the inline state never reads as a modal to assistive tech. Sizing stays driven by the session viewport inline; expanded it fills the pane and letterboxes inside, so the remote display is never scaled to a shape it is not rendering. An effect leaves full screen if the deadline passes, so a lapsed window cannot strand a dead black rectangle over the conversation. ⚠️ Test-design note: the lapse test drives `lapsed` through the INPUT, not the clock. The component's 1s ticker is a real `setInterval` created in the constructor, so `vi.useFakeTimers()` installed afterwards never drives it — a timer-based version of this test passed for the wrong reason. Mutation-checked: splitting the frame into per-state iframes fails two guards. Suite 3580 pass; `ng build --configuration=production` clean, which is the check that catches what tsc alone does not here. Co-authored-by: Claude Opus 5 * docs(spec): record what the agent-build decomposition found #1201 shipped the sub-stage marks but no spec section, so the epic's headline finding existed only in a chat transcript. This writes it down. `agent_build` on a cold turn is 3286ms, and it decomposes as: tools 2039ms 62% <- the largest number in the whole turn session_mgr 830ms 25% <- AgentCore Memory restore, never timed before finalize 194ms prompt 160ms registry 50ms plugins 13ms hooks 0ms `agent_build.tools` is four times what the preamble cost at its worst and larger than everything this epic has fixed so far, combined. It points at the stage this spec always suspected — the MCP pre-flight lives in there — while disproving the fix it proposed. Dev loads exactly ONE external MCP server and the stage still costs 2039ms, so parallelising across servers cannot be the answer. `_build_filtered_tools()` also does registry filtering, gateway integration and local tool assembly, and how the 2039ms splits between them is NOT yet known; given the mechanism error recorded under PR-3, that is not a gap to fill by reasoning. Also records that the proposed `asyncio.gather` would have been a no-op — `MCPClient.load_tools()` has no await in its hot path, so every coroutine blocks the loop before it yields — and re-scopes the remaining work: PR-5 split agent_build.tools (MCP / gateway / local). Measure first; the pattern has been applied twice and overturned an assumption both times. then agent_build.session_mgr at 830ms. drop asyncio.to_thread for the DynamoDB calls. PR-2 removed seven of the eight blocking reads and PR-3 showed the cost was CPU, not IO wait, so its rationale is gone. Kept in the doc as declined-with-reasons rather than deleted. Status header now carries the epic's outcome: warm preamble 455ms -> 22-37ms (-95%), warm pre-stream window 591-656ms -> 128-189ms (-75%), cold agent_build fully attributed. Co-Authored-By: Claude Opus 5 * fix(infrastructure): make the browser URL blocklist configurable per environment The blocklist is the control that stops a human in a browser takeover reaching a system the agent must not act inside. It was hardcoded to `instructure.com` in config.ts, and `platform.yml` never forwarded CDK_BROWSER_URL_BLOCKLIST — so through the deploy pipeline the list was effectively unconfigurable, and every fork of this stack inherited one institution's LMS policy with no breadcrumb. The hosts that matter are a per-deployment policy call, not a property of this stack, so the default is now empty and the value is supplied per environment, following the `domainName` / `corsOrigins` convention. The operational guidance stays in the comment: Chromium matches on HOST, prefer the registrable domain so `.test.`/`.beta.` instances are covered, and don't block a vendor's sign-in page — reaching a login page is what a VPAT review exists to do. New `parseListEnv` treats empty as unset and falls through to context. An unset GitHub Actions variable arrives as '', and the previous `!== undefined` check would have read that as a deliberate "block nothing" the moment the workflow started forwarding it — silently overriding a configured default. Because the list now comes entirely from outside the repo, a deploy that ships an empty one has to say so: the synth log prints the list, or warns that RBAC on browse_web / request_user_login is the only remaining control. The CDK_BROWSER_URL_BLOCKLIST variable is set to `instructure.com` on both the production and development environments, so neither loses the control. Co-Authored-By: Claude Opus 5 * fix(seed): add the spreadsheet analysis tools to the bootstrap catalog `seed_bootstrap_data.py` is the only writer of the `TOOL#` rows that RBAC grants and the tool picker read, and nothing syncs it with the Python TOOL_CATALOG. `list_spreadsheets` and `analyze_spreadsheet` were catalogued but never seeded, so a freshly bootstrapped deployment got no rows and the tools could not be granted to any role — silently, because an absent row reads as "not in the catalog" rather than as an error. The live environments only have them because the rows were created by hand. Values mirror the live deployments: category `data`, enabledByDefault true, isPublic true. Default-on is safe against the injected-tool cost trap because SPREADSHEET_TOOL_IDS is in KEY_DESCRIBED_INJECTED_TOOL_IDS — the factory closes over (session_id, user_id, assistant_id), all cache-key elements, so it does not force an agent-cache bypass. Seeding skips existing rows, so this cannot touch a deployed catalog. test_seed_matches_tool_catalog pins the two lists together. It is one- directional: seeded-without-catalog is legitimate for context-bound tools, and `document_read` is excluded by name because it is gated on the session having an attachment rather than on enabled_tools, so it must never get a row. Run against the v1.22.0 seeder it fails with exactly the three ids that were missing, so it is not vacuous. Co-Authored-By: Claude Opus 5 * fix(browser): reconnect the CDP socket after a sign-in handback Handing the browser to a human disables the automation stream, and the service closes our CDP socket with a clean 1000 ("Disconnected by admin"). Re-enabling the stream does not revive it. `CdpSession` only set `_closed` in its own `close()`, and a clean peer close ends the reader's `async for` normally — no exception — so the session reported itself open while holding a dead socket. `acquire` then handed that corpse back on every later call: in prod, a successful sign-in was followed by four straight `browse_web` failures and no recovery for the life of the conversation. - `CdpSession` marks itself closed whenever the reader ends, however it ends, and fails pending commands rather than leaving them hanging. - `release_control` closes the dead socket (keeping the pool entry, so a second handback stays a local no-op) and leaves the remote browser running, so the next `acquire` reconnects to the session the user just signed into. - `acquire` refuses to reconnect or start a second browser while a human holds control — with the liveness fix in place, a takeover's dead socket would otherwise look reconnectable and orphan the browser mid-login. Co-Authored-By: Claude Opus 5 * feat(usage): surface total quota alongside usage and spend (#1210) Users could see spend and threshold warnings but never the quota denominator (issue #941). Adds a read-only GET /costs/quota-status (caller-scoped, records no enforcement events) that resolves the user's tier + current-period usage. Frontend surfaces it two ways: - A full-width quota progress bar on the Usage page, above the summary cards and under the period filter, shown for every period so past usage can be compared to the limit. Handles unlimited and no-tier-assigned states. - A hover/focus tooltip on the composer cost counter showing monthly quota used / limit / remaining. Tests: backend route test (normal / unlimited / unconfigured / no side effects); frontend specs for the service and both components. * feat(cognito): close self-signup by default via config The user pool's self-registration was hardcoded `selfSignUpEnabled: true`, the only field on `CognitoConfig` with no env/context override. Production is consequently open: anyone with any email address can register, verify themselves, and land on the `default` role — which grants a model and real spend. There is no PreSignUp trigger, so no domain gate either. The app already tried to close it. `disable_self_signup()` at the end of first-boot sets `AllowAdminCreateUserOnly=true` at runtime, but CDK renders `adminCreateUserConfig` unconditionally, so any deploy that updates the user pool pushes the property back and silently re-opens registration. A console toggle is reverted the same way. Declarative config is the only thing that holds. Promote `selfSignUpEnabled` to `CognitoConfig`, resolved from `CDK_COGNITO_SELF_SIGNUP_ENABLED` and threaded through `platform.yml`, defaulting to `false`. Default-closed rather than default-on-with-a-kill-switch is deliberate. That convention covers feature rollout, not access control, and an unset GitHub variable arrives as an empty string — which must not mean "public signup page". Nothing in the stack needs it open: `AdminCreateUser` and federated JIT provisioning both ignore this setting, so first-boot and SSO are unaffected on a fresh fork. Verified by synth in all three states (unset / empty / `=true`). Adds a regression guard asserting `AdminCreateUserConfig` is always rendered — if it ever stops being, the config value has quietly stopped being authoritative. Co-Authored-By: Claude Opus 5 * refactor(app-api): drop the runtime self-signup disable from first-boot Self-signup on the Cognito User Pool is declared by `config.cognito.selfSignUpEnabled` (infrastructure/lib/config.ts, resolved from CDK_COGNITO_SELF_SIGNUP_ENABLED, default closed) and rendered into `AdminCreateUserConfig.AllowAdminCreateUserOnly`. CDK re-asserts that property on every deploy that touches the pool, so the runtime toggle at step 5 of first-boot was either a no-op (default, closed) or actively wrong (an environment that deliberately opted into open registration had signup silently closed behind its back until the next deploy reopened it). Removes `CognitoService.disable_self_signup()` and its call site, along with the three tests that covered it, and leaves a docstring note naming the config value as the setting of record. Co-Authored-By: Claude Opus 5 * docs: correct the Bedrock Price List API guidance Both CLAUDE.md and the curated-models header warned that the Price List API holds "10 Claude SKUs, none newer than Claude 3" and told you not to re-derive Claude rates from it. Re-verified 2026-09-21 in us-west-2: that is no longer true, and the original finding was a query artifact rather than a gap in AWS's data. Two traps produced it, and both are now documented: - There are TWO offer files. `AmazonBedrockFoundationModels` (372 SKUs) carries Claude, Cohere, Palmyra, TwelveLabs, Luma and Stability; `AmazonBedrock` (1052) carries Nova, xAI, Google, DeepSeek, Qwen and Moonshot. The command the header recommended queried the first while the prose claimed authority for xAI and Google, which live in the second — so it returned nothing for exactly the vendors it named. - The `model` attribute is gone from the product schema (only regionCode, usagetype, locationType, location, servicename and operation remain), so any filter keyed on it silently returns zero. Match on `servicename` or `usagetype` instead. Every Claude row we curate is present and matches its model card to the cent, including Fable 5.1's $0.25 cache read independently confirming the 0.025x multiplier its own comment already claimed. Both usagetype shapes are live in the same file today, so the existing "match both" advice is kept and now has evidence attached. The hosted OpenAI family is still absent from both files, so its cards remain the only source. That gap is kept, but its stated cause was wrong: Claude bills through `MP:` Marketplace usagetypes and IS covered, so Marketplace billing is not the reason. Model cards stay the primary source — they carry context windows, caching support, parameter tables and cutoffs that no pricing API publishes — but rates can and should now be corroborated in both. Co-Authored-By: Claude Opus 5 * feat(models): curate Kimi K3 on bedrock-responses with per-model explicit caching Moonshot AI's Kimi K3 went GA on Bedrock 2026-09-18. It looks like an ordinary `bedrock` catalog row — bedrock-runtime, Converse ticked, 1M context, TEXT+IMAGE — and its model card rules that out on three counts, naming our framework while doing it: - Converse throws InternalServerException "when reasoning content from earlier turns is included in a multi-turn request, which affects frameworks such as LangChain and Strands Agents in their default configurations" — our agent loop on turn two. - Explicit prompt caching is "Responses and Chat Completions APIs only", so a Converse row would reach only implicit caching and our cachePoint contract would place nothing. - Converse rejects attached PDF and HTML documents, and attachments are ~31% of prod spend. bedrock-mantle is marked unsupported on its card too, which leaves bedrock-responses — the same transport, bearer mint and caching path the GPT-5.6 family already rides. Its advertised implicit caching is a cost regression ----------------------------------------------------- The card says it "supports implicit (automatic) prompt caching" by default. Measured clean-room in dev-ai (us-west-2, a unique prefix per arm so no arm could read another's entry, 10.5k prefix, 4 turns, priced at US CRIS $3.30 / $0.33 / $4.125): read write uncached 4-turn $ stock (no options) 0 42,008 28 0.17338 developer-message placement 0 42,024 28 0.17344 options, no breakpoint 23,040 0 19,008 0.07033 FULL explicit (opts + bp) 31,458 10,486 112 0.05401 gpt-5.6-luna stock (control) 31,263 10,421 8 0.05333 Stock writes the whole prefix every turn and reads one back never. The same turns UNCACHED cost $0.1387, so its default caching is 25% worse than not caching at all, for the life of every session. Full explicit saves 69% and lands on the control's efficiency. `prompt_cache_options` is the deciding variable, not breakpoint placement. That is the opposite sign to the GPT-5.6 measurement BEDROCK_RESPONSES_EXPLICIT_CACHE_ENABLED encodes (explicit measured 57% worse there), so with both models on one transport the flag had no correct value. It now takes an optional model id, and _EXPLICIT_CACHE_REQUIRED_MODELS forces the existing explicit path on for models that need it regardless of the flag — a kill switch that killed the only thing making the model affordable would not be one. No new caching code: apply_explicit_prompt_cache is untouched and the request shape is identical to GPT-5.6's under the flag. A code tuple rather than a second env var because AWS::BedrockAgentCore::Runtime caps EnvironmentVariables at 50 and inference-agentcore-construct.ts is at that cap. TTL, cache-status classification, usage normalization and cost accounting all needed no change — cache_ttl_seconds_for keys on the provider and Kimi's documented 30m retention already matches. Catalog entry ------------- `us.` not `global.`: Global CRIS is cheaper ($3.00/$15.00 vs $3.30/$16.50) but dev's Control Tower SCP p-r61tynkc denies bedrock:InvokeModel on every `global.*` profile — verified against global.anthropic.claude-haiku-4-5 and global.anthropic.claude-sonnet-5 as well, while each `us.*` sibling works. Curating the Global id would put a model in the picker that 401s on first use. maxInputTokens is 1M rather than the family's 272K pin: that pin exists because the OpenAI cards publish two price tables selected by actual token count, and Kimi publishes one per inference option, so there is no second card to fall into. Rates agree to the cent across two independent sources — the model card and the Price List API's AmazonBedrock offer file. supportedParams is measured against the live endpoint, not inherited. openaiResponsesParams declares temperature unsupported, which is a fact about GPT-5.6 and false here: - temperature accepted; 400 on 2 gives "between 0 and 1" - top_p 400 on 0.9: "only with the value 0.95" — declared unsupported, since its sole legal value is what omitting it already gives - max_output_tokens 400 on 1: "Expected a value >= 16"; the family's usual min of 1 would be a guaranteed 400 - reasoning.effort enum none/low/medium/high/xhigh/max, with unset measured at ~107 reasoning tokens against medium's ~113, so pinning medium is cost-neutral The catalog spec's OpenAI-family invariants are scoped to the OpenAI entries rather than widened to a model they were never measured against, and a change-detector fails if a future non-OpenAI entry inherits that parameter spec. Co-Authored-By: Claude Opus 5 * fix(tests): replace deprecated get_event_loop with pytest-asyncio marker `_run()` in test_analyze_policy_gate.py used `asyncio.get_event_loop().run_until_complete(coro)`. On Python 3.12 `get_event_loop()` raises `RuntimeError: There is no current event loop` when no loop is running in the thread, so the three tests passed in isolation but failed whenever an earlier test in the session left the thread without a current loop. Convert them to `@pytest.mark.asyncio` + `await`, matching the convention already used by every async-invoking test in this directory (test_find_file.py, test_list_spreadsheets.py). pytest-asyncio 1.3.0 is pinned and runs in strict mode, so the explicit marker is required. Before: 3 failed, 4533 passed (tests/shared tests/agents tests/architecture) After: 4536 passed Co-Authored-By: Claude Opus 5 * test: pin Kimi K3's measured param spec to the inference-param guard The curated spec records four 400s measured against us.moonshotai.kimi-k3, but a declared spec is only worth anything if the guard in _merge_inference_params actually enforces it before the request goes out. Nothing connected the two, so a regression in either would have been silent. Each test corresponds to a measured rejection: - top_p is stripped ("only with the value 0.95") - temperature SURVIVES — the reason this model does not reuse openaiResponsesParams, which declares it unsupported; inheriting would have stripped a parameter the model accepts - temperature is clamped ("between 0 and 1") - max_tokens clamps UP ("Expected a value >= 16"), where the family's usual min of 1 would forward a certain 400 - out-of-enum effort falls back to medium, and `none` stays reachable as the cost lever (measured at 0 reasoning tokens) Co-Authored-By: Claude Opus 5 * fix(agents): guard external-MCP loop probe against the 3.12 raise `_build_filtered_tools` probed for a running loop with `asyncio.get_event_loop()`, unlike the three sibling sync->async bridges in this file and strands_mapping.py, which wrap the same call in `except RuntimeError`. On Python 3.12 `get_event_loop()` raises instead of creating a loop when none is running, which had two consequences here: - the `else: loop.run_until_complete(...)` arm was unreachable, since a returned loop is always a running one; and - the no-loop case it existed to serve escaped as an uncaught RuntimeError, surfacing via ChatAgent._create_agent as the opaque "Error creating agent: There is no current event loop". Probe with `get_running_loop()` and branch explicitly instead: run the coroutine here when no loop is running, hand it to a worker thread when one is. Both arms now go through `_load_with_context`, so the captured oauth2 callback URL and workload token are restored either way — the old `else` arm called `load_external_tools` directly and would have lost them had it been reachable. Latent rather than live: both callers reach this through the async request path, where a loop is running and the threadpool arm was already taken. No behaviour change on that path. tests/shared tests/agents tests/architecture: 4536 passed, 0 failed. Co-Authored-By: Claude Opus 5 * docs(cache): record that a second breakpoint loses on Kimi K3 The explicit-cache comment has carried a standing hypothesis since the GPT-5.6 work: the API allows up to 4 breakpoints, so "a scheme that also marks the end of history could plausibly beat implicit." Kimi K3 makes that idea attractive, because its history genuinely is uncached — a live 5-tool turn in dev spent $0.0145 of its $0.0431 on uncached input that sits behind our single breakpoint. Tested it. It loses, and not marginally: read write uncached 5-turn $ one breakpoint (ships) 30,740 7,685 11,610 0.08016 + end-of-history bp 30,740 19,155 140 0.08962 +11.8% The second breakpoint re-writes the whole growing history every turn (1,147 -> 2,294 -> 3,441 -> 4,588) and never reads it back; cache reads stay pinned at the static prefix in both arms. Marking a boundary whose content changes every turn just moves history from the $3.30 input rate to the $4.125 write rate — 1.25x worse by construction. Two mechanics recorded alongside it, both found the hard way: - A breakpoint on an assistant `output_text` block is rejected outright ("must be attached to a content block that renders cacheable prompt content"); it has to ride user or developer input content. - Lookup did not do longest-prefix matching. Turn N+1's history contains turn N's as a prefix and nothing from the earlier entry came back. The one variant still open is named rather than left implicit: a breakpoint pinned at a FIXED history offset across turns, whose content is therefore stable enough to be read. Its ceiling is small, since the tail keeps growing uncached either way. No behaviour change — comment only. The single-breakpoint scheme we ship is the better one on this model. Co-Authored-By: Claude Opus 5 * fix(files): build Content-Disposition headers that survive non-ASCII names Three call sites interpolated a user-controlled filename straight into a `Content-Disposition` value. Add a shared builder and route all three through it. The builder emits the RFC 6266 pair: a sanitized ASCII `filename` that any client can read, plus an RFC 5987 `filename*` carrying the real UTF-8 name percent-encoded. It also splits the extension off before sanitizing, so a name that loses every character still downloads as a usable file. Severity differs per site: * `files/service.py` (preview + download presigned URLs) was the live bug. `FileMetadata.filename` is stored verbatim — the model constrains length, not charset — so a macOS screenshot, whose name carries U+202F before AM/PM, reached S3 raw. botocore signs it without complaint and S3 rejects the `response-content-disposition` it cannot encode as latin-1, which the user saw as a broken thumbnail rather than as an error anyone could read. * `skills/resource_types.py` sanitized against an ASCII allowlist, so it could not 500 — but it stripped before considering the extension, so `研究ノート.md` was served as a file literally named `md`. * `memory_spaces/routes.py` was likewise already ASCII-safe via `_safe_component`; the defect was name loss, a CJK-named space exporting under its opaque space id. `_safe_component` still names the folder *inside* the zip, which is a zip-slip control, not a header one. `admin/costs/routes.py` also interpolates, but its `period` is gated by `Query(pattern=r"^\d{4}-\d{2}$")` and so can only ever be ASCII. Verified, not assumed; left alone. Security properties at the skills site are preserved deliberately: `os.path.basename` still runs first, so a legacy `../../etc/passwd` row cannot suggest a save path; `attachment` + `nosniff` + the inert CSP are unchanged and none of them depend on the filename; and a quote or CRLF is collapsed in the ASCII part and percent-encoded (`%22`, `%0D%0A`) in `filename*`, so neither form can forge a header line. Two deltas worth noting: spaces now survive the ASCII fallback, and non-ASCII names keep their extension. Tests: unit tests for the builder, a service-level test over a real boto3 client for both presigned params (reverting the fix fails 8 of 9), and route coverage for the memory-space export. Full backend suite green. Co-Authored-By: Claude Opus 5 * docs: spec admin-managed always-on tools Proposes a catalog flag that pins a tool into every turn's effective toolset for users whose roles grant it, so a user cannot turn it off. The design turns on one fact: `enabled_tools` is client-supplied and never re-validated on the default chat path, so a locked toggle in the picker would be cosmetic. Enforcement is a server-side union at the existing `_apply_attachment_tool_autoenable` seam, which already has the right shape (enables-never-grants via `can_access_tool`, fixed order, applied before the agent cache key, default-on flag with a kill switch). Latency is a first-class constraint: the warm pre-stream window is 128-189ms after four PRs of work and `tools` is already its largest stage, so resolution adds no new I/O — the always-on id set rides as a third slot on the `freshness._get_snapshot` read that already derives two frozensets from one `list_tools()`, and the grant set is already resolved and cached by `preamble.quota`. D4 is the load-bearing decision: the exemption is "the Agent binds its own toolset", not "the Agent is user-created". An Agent with no tool bindings falls through to the user's picker and does receive the union — otherwise always-on is opt-out-by-construction, since anyone could shed a pinned tool with a trivial unbound Agent. Status: proposed, not implemented. No code changes. Co-Authored-By: Claude Opus 5 * fix(attribution): only compute the context split where the token count is real `ContextAttributionHook` derives `toolTokens` as `projected_input_tokens - count_tokens(system + messages, no tools)`. Those are the same estimator only on Bedrock Converse, where `BedrockModel` implements a native CountTokens. On `bedrock-responses` and `mantle` the model is an `OpenAIResponsesModel`, whose `count_tokens` consults the native endpoint only when `use_native_token_count` is set — and that endpoint is not served on bedrock-runtime's OpenAI surface: enabling it makes `count_tokens` return None (measured against us.moonshotai.kimi-k3, us-west-2; Strands returns None rather than raising, so it would poison the arithmetic silently). Left unset it degrades to chars/4. Subtracting a heuristic from a usage-anchored projection does not yield tool tokens. It yields tools plus the estimator disagreement, which moves with the conversation. One live Kimi K3 session reported `tools` of 13,967 then 7,145 on consecutive turns with a byte-identical `toolConfigHash` — a 2x swing on the number whose whole job is saying what fills the window. Forcing both sides through `count_tokens` makes the residual exactly stable (5,882 short, 5,882 long, +0 drift), which is what identifies the mismatch rather than the tools as the cause. So the split is skipped there, on the principle the file's own attachment guard already states: an absent `prefixTokens` reads "not tracked", which the ledger handles, while a wrong one silently corrupts every share computed from it. The guard sits before the two CountTokens calls, so it also stops paying for numbers we would discard. Scope, stated precisely because "no attribution" reads worse than it is: - Bedrock Converse models — including the default Haiku 4.5 — are UNCHANGED and keep full attribution. Admin per-conversation breakdowns stay actionable there. - On the gated models nothing else is lost. Cost, `cacheStatus`, `wastedUsd`, the chat context meter (`lastContextTokens` = input + cacheRead + cacheWrite, summed from provider-reported usage), `contextWindow`, per-call `tokenUsage` and quota are all independent of this split. Only the system/tools/messages partition is withheld. `_token_count_is_authoritative` is a capability check on the model object rather than a provider allowlist, because what actually decides this is which class implements `count_tokens` — a name list would drift from it. A model may override with a boolean `token_count_is_authoritative`, which is both the extension point for a transport that later gains a real counter and how the tests exercise both branches without pretending to be a `BedrockModel`. Queued in docs/kaizen/review-queue.md with the three upstream triggers that would let the guard be deleted, a two-call probe to re-run each cycle, and the reason the obvious fallback (both sides on `count_tokens`, published as shares) needs a UI decision rather than a silent backend swap — it relocates the error into `messages` instead of removing it. Co-Authored-By: Claude Opus 5 * docs: state backward compatibility as a requirement for always-on tools The spec carried compatibility only as a by-product — one line about `alwaysOn` being absent on existing rows, and the boolean-vs-enum decision being motivated by migration cost. Neither said the feature must be safe to deploy, and nothing recorded the debt that encoding takes on. Adds §10 with the property stated as an acceptance criterion: deploying every PR to an environment where no tool is flagged must be a no-op — byte-identical `toolConfig`, identical cache keys, no new writes — and asserted by a test in PR-1 rather than assumed. This is structural, not incidental: `_with_auto_enabled_tools` returns the same object when there is nothing to add, so `None` stays `None` and the whole downstream chain is untouched. Documents each surface (catalog rows, `MCPToolEntry`, admin API, `GET /tools`, the wire, the cache key, the flag), including two traps: the update route's `exclude_unset=True` is what makes an older admin client's omission mean "leave it alone" rather than "set False", and SPA skew must be safe in both directions since the two deploy independently. Names the two intended behaviour changes that are NOT backward compatible — an all-off picker, and template-derived unbound Agents — and records that both fire on enablement, never on deploy. Finally, a retirement plan for the two-boolean shim: fold the enum migration into the next change that already touches every `TOOL#` row, and until then treat the normalizing validator as load-bearing rather than redundant. Co-Authored-By: Claude Opus 5 * fix(documents): classify the turn's uploads before injecting document_read Attaching a file rotates `toolConfigHash` and re-writes the whole cacheable prefix at 1.25x input. Measured both arms before writing code; the answer is neither "accept it" nor "register the tool permanently". The gate short-circuited on presence, not class `_session_has_documents` opened on `if turn_upload_ids:` — "this turn attached something" — while every other consumer of an upload asks what the upload *is*. An image, a spreadsheet and a deck are all upload ids and none of them is a document: `document_read`'s own listing filters them out by `document_format_for`, so the injected tool's listing is empty by construction. Worse, the same branch calls `_remember_document_session`, so the process-level memo is poisoned and every later turn in that process carries the tool too. What prod did with that, in 19.25 hours Every `C#` row in `boisestateai-v2-sessions-metadata` from 2026-09-20T22:00 (the epic's first prod row) to 2026-09-21T17:15 — 823 call pairs, 41 `toolConfigHash` rotations, 18 of them with an unchanged `systemPromptHash` and no `agentSwitched`: - 29 sessions carried attachments. **13 held no readable document at all** (53 PNG, 2 JPEG, 6 XLSX, 5 PPTX, per the `SessionIndex` rows). - Those 13 account for **100%** of the avoidable rotation cost: $0.18 counting only rotations inside the 5-minute cache TTL, where the counterfactual read is certain; $0.78 counting every rotation. - The one rotation in a session that genuinely gained a readable document cost **$0.00** — session `1f47434a` read the shared tools+system prefix another session had already written. It also thrashed. The memo is per-process, the DynamoDB query is not, so on an image-only session the two disagree and the tool appears and vanishes as microVMs recycle: `be4f0784` ran A→B→A→B, `841d162d` and `077aa73e` ran A→B→A — 19,664 / 33,508 / 23,923 tokens of full prefix re-write per flip. Confirmed in the runtime log: the 10-minute window around `4f3edc52`'s image-only attach contains exactly one "Resolved 1 files from upload IDs" and one "Created document_read tool (session has a readable document)". The same defect orphans paused agents. The resume path calls `_document_tools_gate` with no turn signal and relies on the answer being reproducible from session state; an image-only turn answered True live (raw ids) and False on resume (the query), so the key missed the slot. Arm B — register document_read permanently — rejected on its own numbers The spec measures **664 tokens**: Bedrock `CountTokens` against `anthropic.claude-haiku-4-5`, as the marginal difference between a 6-tool `toolConfig` and the same list plus `document_read` (5,838 -> 6,502). Not read off `prefixTokens.tools`, which is a residual between two estimators (#1216). Kimi K3's tokenizer corroborates independently at 546 — dev session `dca13df7`, cache write 9,295 against the prior call's read of 8,749, with `toolConfig` the only component whose hash moved. Costed against 14 days of real prod traffic (26,395 model calls, 8.8% of them full-prefix writes, each at its own session's model rate): **$0.44/day, $161/yr**, unconditional and forever. That is more than the conservative arm it would replace, and it would advertise a tool with an empty listing to every conversation that never attaches anything. Arm C — what this change does Classify the turn's resolved uploads (`_resolved_files_include_a_document`) and pass a boolean, mirroring `_session_has_tabular`, which already takes `turn_has_tabular=bool(diverted_tabular)` rather than "the request carried files". This removes the whole measured avoidable cost, removes the flap, makes the resume gate reproducible, and leaves only the irreducible rotation — a session gaining its first readable document mid-conversation — which measured $0.00 in the same window. An image-only turn now falls through to the authoritative query instead of guessing. That query already ran on every non-attach turn of such a session, so it is not a new read. Resolution failure or a per-message-cap drop also falls through to the query, which sees the READY row: strictly better than today, never worse. Reproduce: diff `prefixFingerprints.toolConfigHash` across consecutive `C#` rows for one session (`SessionLookupIndex`, `GSI_PK = SESSION#`), then join the session's `SessionIndex` upload rows through `is_document_class`. Spec decision 2 claimed the bit "flips at most once per session" and that "the resume path recomputes the same gate"; both were false as shipped and are corrected in place, with the measurement recorded as decision 28. Backend suite: 9,513 passed, 3 skipped. Co-Authored-By: Claude Opus 5 * test(csp): pin the /api/* sandbox omission and artifact CSP parity Verifying PR #1214 on dev surfaced that app-api's route-specific CSP never reaches the browser on /api/*: ApiResponseHeadersPolicy sets its CSP with `override: true`, so RESOURCE_SECURITY_HEADERS loses its `; sandbox` suffix. A skill-resource download and /api/memory/spaces (which sets no CSP at all) return byte-identical headers. That is intentional, and stays. Two findings settle it: - The edge policy and the backend `sandbox` landed in the SAME commit (01c91e89). The construct comment already stated sandbox was excluded at the edge; only half that decision was legible from the backend side. - The backend header is not dead code. The ALB is `internetFacing: true` and a localhost SPA reaches the dev backend directly, so on non-CloudFront paths there is no edge policy and the backend header — sandbox included — is the browser's only CSP. Both layers are load-bearing, on different paths. Neither alternative is right. Dropping `override: true` would turn an origin-wide backstop into a mere default, letting any route, middleware or error handler define the CSP for the whole origin. Adding a bare `sandbox` at the edge would force an opaque origin on every /api/* response including the top-level OAuth login navigation, for ~zero gain: `default-src 'none'` already blocks all script, so there is nothing for an opaque origin to contain. (Downloads were never the risk the original comment feared — every /api/* download path uses a presigned S3 URL or a blob URL.) So no synthesized resource changes and no platform.yml deploy is needed. What was missing was anything making the split legible or enforceable: - Document the two-path split in resource_types.py and the SPA construct, each pointing at the other. - api-security-headers.test.ts asserts the edge CSP omits `sandbox` AND that Override is true, with the rationale inline, so adding sandbox means deleting a test that explains why not. - New artifacts-csp-parity.test.ts covers a real untested hazard found while checking the third construct: artifact_render mirrors the edge CSP via CSP_SCRIPT_SRC, both sides commented "must stay byte-identical", with nothing checking. Drift there is silent precisely because the edge overrides — a CDN added to one side only breaks a direct Function-URL reach. Both new assertions mutation-checked: drifting the Lambda CDN list and adding sandbox at the edge each fail as intended. Co-Authored-By: Claude Opus 5 * feat(tools): add the always-on catalog flag (admin surface only) PR-1 of docs/specs/admin-always-on-tools.md. Ships `alwaysOn` as inert data: an admin can set it, nothing reads it yet. Enforcement is PR-2. `ToolDefinition.always_on` and `MCPToolEntry.always_on` (the per-tool flag inside an MCP server, which §2.3 prefers over pinning a whole server), through the admin create/update/response models and the form. The encoding stays two booleans rather than a three-state enum, to avoid migrating every catalog row, request model and SPA model (§2.2). What pays for that is a `model_validator` forcing `enabled_by_default` true whenever `always_on` is — on read as well as write, so a hand-written DynamoDB item cannot produce the incoherent pair either. It is load-bearing until the enum migration lands (§10.3) and is commented as such. Admin form replaces the "Enabled by default" checkbox with a single three-way control, which is what makes the invalid pair unreachable from the UI. Marking a whole MCP server always-on names the tool count and blocks save behind an explicit acknowledgement; the always-on option warns when no role grants the tool yet (`isPublic` counts as a grant, matching the backend's grant set). List page gains an "Always on" chip. Backward compatibility is asserted, not assumed (§10): a catalog row without the attribute reads False, an `MCPToolEntry` dict without it reads False, and `ToolUpdateRequest` omitting it stays absent from the `exclude_unset` dump so an older admin client cannot silently clear an admin's flag. The no-op property is asserted by identity rather than equality — `_with_auto_enabled_tools` returns the object it was handed when there is nothing to add, which is what keeps `None` as `None` and leaves the cache key, freshness hash and toolConfig untouched. The §6 token-size readout is deliberately NOT in this PR: a tool's serialized schema exists only at `_build_filtered_tools()` on the inference path, so app-api has no honest number to show. Spec records the two ways to get one; a chars/4 estimate would be worse than the plain-language cost statement that shipped, because admins would budget against it. Co-Authored-By: Claude Opus 5 * chore(kaizen): retire the POC-comment loop, log four standing declines The loop both kaizen skills describe as their feedback channel has never fired: seven cycles, zero comments across #926/#1045/#1046, and the "tested outranks untested" tiebreak never once used — while 6 of 9 proposals converted through merged PRs, the channel no skill described. The 2026-09-18 review's first draft was corrected by a review-queue entry sitting on PR #1147's own unmerged branch, which is where this repo's deepest analysis actually lives. - kaizen-review-prep now opens with a scorecard of the prior review's proposals resolved against git and disk, and must diff open PR branches for docs/kaizen/ entries before ranking. - `POC findings` -> `Evidence` (measured / verified on disk / asserted); tiebreak becomes measured-over-asserted. - Pipeline spacing: the skills claimed ~2 hours between research and prep against a 2h18m scan. Both now say ~4 hours, and prep carries a preflight that must state an absent or in-flight research doc in the review header rather than degrading silently, as it did on 2026-09-18. - decisions.md: the POC loop, tool_result duration_ms (superseded by agent_status), the oauth_required standing audit, and the dormant-skill keep verdict for cors-deployment / frontend-design. - review-queue.md: four entries Open -> Resolved; the two verification rules that did not ship are re-queued on their own. The kaizen-review-prep schedule moved 0 8 * * 5 -> 0 10 * * 5 outside the repo. Parts (b) and (c) of the [2026-09-04] entry are not done. Co-Authored-By: Claude Opus 5 * docs(kaizen): the cheap quality veto has no substrate — measured reviews/2026-09-18 Proposal #3 recommended shipping "the cheap version" of the quality veto this week: a comparison of down-thumb rate across document turn classes, "available today with a query, not a build." Both halves are wrong. The query was already built — GET /admin/feedback/fleet emits arms.turnClass with the spec's own full > retrieved > digestOnly precedence. And the substrate is empty: every F# row in prod since 1.23.0 is 13 rows — one explicit thumb and twelve implicit copy signals — against a per-arm floor of 20, with three document arms to populate. Dev has one thumb in 30 days. At ~0.7 thumbs/day fleet-wide the digest-only and retrieved arms are months out, so the recommendation cannot be executed as written. The queue entry lays out the three honest branches, recommends writing the waiver into both specs, and records that the window closed when 1.23.0 shipped both stacks to prod on 2026-09-20. Co-Authored-By: Claude Opus 5 * docs(specs): waive the two quality vetoes in writing Both specs wrote their quality gate as a veto, neither harness was ever built, and 1.23.0 carried both stacks to prod on 2026-09-20 — so the window each spec named ("before prod") is closed. A gate merely unrun reads, to the next reader, exactly like a gate that passed. The waivers head compaction-model-relative-thresholds.md §5 and document-offload-evaluation.md §2, with a pointer from §4.2 recording that the epic entered that rule's own "Ship" state without the quality leg it conjoins. Each states what shipped anyway, why it was waived rather than run, that nobody has looked and a regression would currently be invisible, and three triggers that delete the waiver rather than amend it. The second reason each was waived is new and measured: the cheap fallback does not exist either. GET /admin/feedback/fleet already emits arms.turnClass with the spec's own precedence, but every F# row in prod since 1.23.0 is 13 rows of which one is a thumb, against a floor of 20 per arm and three document arms to populate. Both waivers prohibit reopening by lowering FEEDBACK_ARM_MINIMUM_N, which would manufacture a rate from single-digit n and produce the fleet-wide quality score response-feedback spec §9 forbids. Not resolved: the harness is still unbuilt, and is now the only path to an answer that does not wait on user volume. Co-Authored-By: Claude Opus 5 * fix(costs): drop a prefixTokens split larger than the prompt it sits inside `prefixTokens.tools` is a residual between two estimators — Strands' projection for the upcoming request minus our own CountTokens call without tools — so any disagreement between them lands wholly in it. `context_attribution.py` already refuses to compute the split on the two failure modes it can predict (inline attachments, OpenAI-surface providers), but nothing checked the result. Prod session 7f5f207f reported `tools = 223,782` on a row whose real prompt was 55,783 tokens: the tool schemas alone claiming 4x the prompt they are part of. The admin anatomy page renders that as a measurement, and it is exactly the number someone would size a tool budget from. The invariant is arithmetic, not a heuristic: the static prefix is a subset of the prompt, so `system + tools` can never exceed what the provider billed (`inputTokens + cacheRead + cacheWrite`). A split that violates it is dropped, following the ledger's existing convention that an absent `prefixTokens` reads "not tracked" — which every consumer, including the SPA, already handles. Applied on both sides of the import boundary from one shared rule: `agents/` before persisting a row, `app_api/` when reading rows written earlier. Nothing is backfilled. Deliberately catches only the provably-impossible case. The same user's 13,606 -> 4,624 swing on a byte-identical system prompt is also wrong, but arithmetic cannot tell it from a real tool-set change; narrowing that needs the root cause, queued for the fleet sweep. The stale split is also not invalidated — recomputing costs two CountTokens calls, and a systematic disagreement would pay them every turn, trading a wrong number for a latency regression. Verified against the real rows: row 48 (223,782 vs 55,783) drops; rows 46-47 (163,909 vs 226,370 / 254,061) are kept. Co-Authored-By: Claude Opus 5 * docs(kaizen): queue a fleet-scale anatomy sweep; recommend DECLINE on the 1h static TTL Session 7f5f207f ($21.84 / 48 Opus 5 calls, 2026-09-16→21) produced two results the dashboard's per-session view cannot surface on its own, which is the argument for sweeping anatomies at cohort scale. It spans the Release 1.23.0 prod deploy, so it is a free A/B: write:read went 1:1.18 (45 calls, 1.22.0) to 1:18.3 (3 calls, 1.23.0), with cache write falling from 65% to 13% of cost. Both defects in the old half are ones 1.23.0 fixed — the pre-9f246cb7 hourly system-prompt tick and a prefix that reached 202,512 tokens before model-relative compaction shipped. 1:18.3 would be the first healthy ratio measured here (July fleet-wide was 1:1.6-1:2), but it is three calls and one of them was warmed by a concurrent session, so it needs a real sample before anyone quotes it. It also refuted the queued PR-5 decision, which was one flag flip and a dev week from being run. Break-even for the 1h static-prefix TTL is a SHARE of cache-writing calls, not a prefix size: (2 - 1.25) / (1.25 - 0.1) = 65%. This session is at 23%, and the modelled net is -$0.78 — growing more negative as the tool prefix grows (-$2.41 at 166k), because a bigger static prefix scales both sides of the trade. That inverts the intuition the item was resting on. The share is computable from rows we already hold, so the item can close without spending the dev week. Co-Authored-By: Claude Opus 5 * feat(tools): enforce admin always-on tools on the invocation path PR-2 of docs/specs/admin-always-on-tools.md. PR-1 shipped the flag as inert data; this makes it mean something. `_apply_admin_always_on_tools` sits at the same seam as the attachment auto-enable, so the main turn and both MCP App dispatch paths compute the same effective list and therefore the same agent-cache slot. Voice gets it too (D5) by rebuilding a User from the connection's token — it has no `Depends()` and so no User in hand, and a rolesless one would resolve the `default` role's grant rather than the caller's, so it fails closed to today's behaviour when no token is present. D4 is the load-bearing decision and is asserted both ways: an Agent that binds tools is exempt (and the pinned set is never even resolved), while an Agent with NO tool bindings falls through to the picker and does get pinned. `_resolve_tools` returns None for the latter, template-derived Agents ship with empty bindings, and exempting them would make always-on opt-out-by-construction. Latency (§4): resolution adds no new I/O per candidate. The pinned set rides a third slot on `freshness._get_snapshot`, filled from the same `list_tools()` read that already derives the all-ids and public-ids sets — asserted by `list_tools.await_count == 1` across all three accessors, because "few reads" is not the property, "one" is. The grant set resolves once inside `filter_requested_tools`, which is also the predicate the picker and Agent bindings answer to, so the three surfaces cannot drift. Two things the suite caught that reading the code did not: `_always_on_ids_for` used direct attribute access for the MCP configs. It runs inside `_get_snapshot`, which fills ALL THREE slots in one pass, so an AttributeError on one odd row would have aborted the refresh and left `get_public_tool_ids` empty — quietly narrowing every RBAC check that unions the public tools. Now getattr throughout, plus per-row isolation so one bad row cannot empty the other two snapshots. 21 route tests began failing on the off-box socket guard, whose message names the hazard exactly: "a fail-open except block hides the failure but the call still happens." The resolver swallowed the error; the socket still opened. That is direct evidence the invocation path never read the tool catalog before and now does — §4.3 predicted it, this proves it. Fixed with one stub in the fixture that exists for this class of fail-open read, not by weakening the guard. Co-Authored-By: Claude Opus 5 * fix(ci): recover deploy runs evicted from the shared concurrency group platform.yml and backend.yml share `concurrency: deploy-` so a CFN deploy and the API-driven code deploys never mutate the same ECS service / AgentCore Runtime / Lambda at once. But a GitHub concurrency group is a latch with a queue depth of ONE, not a queue: "any existing pending job or workflow in the same concurrency group will be canceled and the new queued job or workflow will take its place." So a third run entering an occupied group kills whichever run was waiting, before it executes a single job. One merge is enough — a commit touching both infrastructure/ and backend/ starts two runs that compete with each other and with the previous merge's still-running deploy. Eviction is harmless when a LATER run of the SAME workflow supersedes the evicted one: both deploy the tree at HEAD and git is cumulative. It is destructive across workflows — a Backend Deploy does not run `cdk deploy`, so evicting a pending Platform Stack run skips the infrastructure change entirely. Nothing surfaces it: the PR's checks are green and the run reports `conclusion=cancelled`, which reads like a human pressing Cancel. Observed live on 2026-09-21 (merge 2601a87d on develop, run 35635692102, cancelled 4s after creation with zero jobs). It is not a one-off: every one of the 32 cancelled runs across both workflows executed zero jobs — 11 Platform Stack, 21 Backend Deploy — including one on main that a human recovered by hand 18 minutes later via workflow_dispatch. Each workflow now carries a `recover-evicted-peer` job. The run that takes the pending slot is the only party that knows the peer was dropped, so it checks from inside: peer runs that ended `cancelled` having executed zero jobs, minus those a later peer run already covers, are re-dispatched against the branch. Deploys are convergent, so one dispatch of HEAD applies every change that was dropped along the way. A cancel that left jobs behind is a person, not an eviction, and is never second-guessed; a brake caps re-dispatches so a thrashing merge train asks for a human instead. The group itself is unchanged — the mutual exclusion it provides has never failed, and every alternative serialization is a hand-rolled mutex with a TOCTOU hole where GitHub's latch has none. Also corrects both concurrency comments, which claimed the runs "queue instead", and adds scripts/ci/** to both workflows' trigger paths so a change to the guard is exercised by a real run. Co-Authored-By: Claude Opus 5 * docs(kaizen): scope the quality-veto harness Both waivers recorded today name "build the harness" as trigger 1, and neither spec ever said what the smallest useful version is. This scopes it, and finds it smaller than either assumed. The specs and the evaluations spike describe one harness driving live multi-turn sessions through the deployed runtime, for both epics. That is right for offload and substantially wrong for compaction: CompactionPolicy.resolve() and choose_checkpoint() take a config object and a message list and read no environment at call time, so both arms can be applied to the same authored transcript in-process. The cut is deterministic; only the bounded summary and the answers need a model. Correction to the spike's "extend experiment_agent_cache_arms.py": run_arm and attach_cost_rows generalize, but the arm definition does not. That script's arms differ by enabled_tools because it wanted arms with no redeploy, and run_agent_headless exposes only the InvocationRequest surface. Compaction config is from_env on the runtime, so two compaction arms on one runtime is not possible through the live path at all. Offload B-vs-C is per-session selectable via the crc32 bucket; only A-vs-B needs an env change. And the slice that needs no judge: two of three families in each spec are programmatically generatable and scorable, which is also where the statistical power lives. Slice 1 needs no blinded judge, no scrubber, no AgentCore Evaluations and no judging spend. Co-Authored-By: Claude Opus 5 * feat(tools): lock admin-pinned tools in the user's tool picker PR-3 of docs/specs/admin-always-on-tools.md. PR-2 made the backend union pinned tools into every turn; until now the picker still showed them as ordinary toggles, so a user could switch one off and watch it come back. `GET /tools` carries `alwaysOn` on the tool row and on each MCP sub-tool row, and forces the effective state on. It is the *effective* lock for that user: only set when their roles actually grant the tool, because always-on enables and never grants. A stored opt-out is kept, not honoured. It is the user's preference and applies again the moment an admin unpins — but while the pin is in force the picker shows what the turn will actually do. `save_user_preferences` drops an opt-out of a pinned tool (D6) rather than rejecting the request: the SPA sends the whole preference map on every toggle, so a 400 would let one pinned tool block every other change, turning an admin's pin into a broken settings page. Turning a pinned tool ON is kept — it agrees with the pin and is what should apply if it is ever unpinned. A server with only SOME tools pinned is deliberately not locked at the row level: the user may still turn the unpinned ones off. Switching such a server off leaves its pinned tool on, and the SPA omits it from the payload rather than sending a False the backend would drop — so the optimistic state and the saved state agree. The card renders policy, not a broken control: full opacity (a greyed switch reads as "loading"), the reason stated on the card rather than only in a `title`, which reaches neither touch users nor a screen reader browsing statically, and the reason in the accessible name. Agent and schedule forms are untouched — they define their own local toggles over their own selection state, so an Agent author stays free to exclude a pinned tool, which D4 requires. One note on the specs: the first cut passed four assertions vacuously because `loadTools` is async and the harness read `tools()` before the flush settled, so every guard returned early on a missing tool. The helper now awaits and asserts the catalog actually loaded. Co-Authored-By: Claude Opus 5 * fix(quota): stop assignment edit 400 on tier change (#718) (#1225) QuotaRepository.update_assignment setattr-ed the camelCase alias key (tierId) onto a QuotaAssignment whose field is tier_id, which raises '"QuotaAssignment" object has no field "tierId"' in Pydantic v2 and returned a 400 on every assignment tier change. Decouple the two uses of an update key: normalize each key to its field name for setattr, and to its alias for the DynamoDB UpdateExpression so stored attributes stay camelCase (tierId) with no duplicate tier_id. Tolerant of either alias- or field-name-keyed input. Adds regression tests covering the alias-key path, camelCase-only stored attrs, field-name input, and priority-driven GSI sort-key rebuild. * docs(kaizen): standing watch on Strands' first-party ContextManager Phil asked to keep eyes on the compaction features Strands delivers. The research note said the context-manager/offloading stack was "fully released in 1.56.0"; reading the package on disk corrects that — it is already in our 1.55.0 pin. 1.56 finished it (strategy presets, agent rewire, and session-manager integration #4254). Evaluating it is not gated on the bump, which is separately blocked as a paired pin. What it is: dual trigger (proactive on BeforeModelCallEvent using the projected token count already in hand, so no extra CountTokens; plus overflow recovery with retry), a composable strategy pipeline rather than a summarizer, block-level replacement that can shrink a 90k tool result without dropping the turn, and a Stash that keeps offloaded content retrievable through a tool registered once at agent construction rather than through extra_tools. Against the 2026-05-18 decline's four conditions: tool-content truncation is now built in, checkpoint persistence is partly addressed, and LTM retrieval plus the compaction SSE-once invariant are still ours. The decline stands as written; the gap is about a third of what it was. Four traps recorded, two of them blocking. It rides context_window_limit — the value Strands puts at 1M for Sonnet 4.6 where our catalog says 200k, and which is None for every non-Anthropic model we curate. And the Stash is namespaced per (session_id, agent_id), so a second agent on the same session cannot retrieve what the first offloaded, which @-mentions make a live case. Upstream #4367 is the issue to watch. Also moves the [2026-07-19] ContextOffloader spike to Resolved — its own status said ADOPTED while it sat in Open. Co-Authored-By: Claude Opus 5 * fix(models): reconcile the context window against Strands' table (#267) maxInputTokens drives two things: the SPA context badge, and — since #1125 — the model-relative compaction policy, which cuts at window * COMPACTION_CEILING_RATIO. A stale row therefore does not mis-render a badge, it compacts early: paying a prefix re-write and a summarizer call sooner than the model requires, and dropping context the model could have held. Nothing detected it, because a plausible number is indistinguishable from a correct one. Measured 2026-09-21. Claude Sonnet 4.6 and Claude Opus 4.7 both publish a 1M context window on their AWS model cards; both inherited claude4xDefaults()'s 200_000 in the curated catalog. Dev additionally carried Claude Sonnet 5 at 200_000 against a catalog that declares 1_000_000. Three enabled models cutting at 100k instead of 500k. - curated-models.ts: declare 1_000_000 on the Sonnet 4.6 and Opus 4.7 rows, and warn on the 200_000 default itself — it is correct for Haiku 4.5 only, and every other Claude model we curate is 1M. - New apis/shared/models/context_window.py: catalog value wins, Strands' table fills an absent one (#267's actual ask), and a disagreement is logged once per model id for a human. - stream_coordinator: both the badge and the compaction policy go through the one resolver, so they cannot disagree about the window. The precedence is the part worth reviewing. Taken literally #267 asks for the SDK value as a fallback, but preferring it on conflict would be actively wrong: maxInputTokens: 272_000 on the hosted OpenAI rows is a deliberate pricing cap, not a stale value — those models have a 1M window and AWS bills input at 2x above 272K, so raising it silently under-charges every long turn. A stale row and a deliberate cap are indistinguishable from inside the code, so the guard detects and reports rather than resolving. Strands' table is also Anthropic-on-Bedrock only: Astra, the GPT-5.6 family, Kimi K3 and DeepSeek all return None. The three live dev rows were corrected separately, by hand. Prod carries one stale row (Sonnet 4.6) but it is enabled=False, so nothing there is serving a wrong window today. Co-Authored-By: Claude Opus 5 * fix(admin): don't gate a single-tool server behind the always-on ack PR-4 of docs/specs/admin-always-on-tools.md: dev validation, plus the one bug it surfaced. Real rendering showed what the specs and unit tests did not — the whole-server confirmation read "I understand this pins all 1 tools." Beyond the grammar, the gate itself was wrong for that case: it exists to prevent "I meant to pin one tool and pinned thirty", and a single-tool server holds no such surprise. Both the warning and the blocking acknowledgement now apply only above one tool, and the copy names what the cost actually is. Spec gains §11 recording the validation against 9c04c3d9 on dev with `create_visualization` pinned for real. The result worth keeping: enforcement was proven with the prompt cache as an oracle rather than by asking the model to list its tools. Three sessions differing only in `enabled_tools` — one omitting the pinned tool, one including it, one with an unrelated extra tool. The omitting session wrote the cache, the including session HIT it, and the control missed. Exact-prefix-match on toolConfig means the first two carried identical tool lists, so the union demonstrably happened server-side. A model's self-report would have been a hypothesis; this is evidence. D6 was validated in exactly the scenario it was designed for: one PUT carrying an opt-out of the pinned tool AND a change to another tool returned 200, ignored the former and saved the latter. Implemented as a 400, one pinned tool would have blocked an unrelated toggle. The §4.5 latency gate passes: 275.8ms unpinned vs 278.9ms pinned, +1.1% with fully overlapping ranges, below the run-to-run variance of the unpinned arm alone. Ambient dev traffic could not answer it (n=1 after the change), so turns were generated 10 per arm. §11.3 records what was NOT validated live and why, rather than implying fuller coverage: the negative RBAC side is unreachable from a wildcard account, and both sides of the D4 boundary need purpose-built Agents. Co-Authored-By: Claude Opus 5 * feat(composer): remember unsent composer state per conversation Leaving a conversation destroyed whatever the user had composed but not sent. Navigating away, a reload, a closed tab or a session expiry bouncing through Cognito all lost it outright. The composer now parks its unsent state under the conversation it belongs to and takes it back on return: the text, the `@`-mention bound to it, the follow-ups queued behind a streaming turn, and the files attached to it. `localStorage`, one entry per conversation, so two tabs in different conversations never overwrite each other's drafts. Three decisions worth the reviewer's attention: - The draft key is the *route* conversation, never `effectiveSessionId`. A new conversation mints a staged session id the moment a file is attached, so keying off it would blank the composer exactly when someone attaches a file to text they already typed. - Only *unarmed* queued follow-ups are kept, and they come back as composer text rather than as chips. An armed entry is one the backend confirmed it holds against the running turn, so restoring it would be the duplicate. A chip promises end-of-turn delivery, and after a reload the flush is edge-triggered on a stream the new component never saw start, so a restored chip would sit there forever. - Attachment metadata is a display cache: cards paint from storage in the first frame, then reconcile against `GET /files?sessionId=` and drop whatever the server will not confirm. Ownership is enforced server-side twice over (the listing, and `FileResolver` keying on `USER#{userId}`), so nothing stored locally is trusted. Two pre-existing leaks fall out of this. Composer text carried across a `/s/A` -> `/s/B` switch, because the component instance is reused and `userInput` was never cleared; and `FileUploadService.pendingUploads` is application-wide, so attachments followed the user between conversations. Both are now scoped by the draft swap. Drafts are the user's words rather than a device preference, so their lifetime is tied to the session: claimed on bootstrap, cleared on logout, and stamped with the account id so a shared machine where the last person closed the browser instead of signing out is wiped at the next sign-in. Co-Authored-By: Claude Opus 5 * docs: record the live D4 validation, both sides Closes the largest gap §11.3 admitted: D4 had unit coverage but no live proof, and it is the load-bearing decision in the spec. Five throwaway Agents on dev (since deleted), same prompt-cache oracle. Exempt side: three Agents with identical instructions differing only in bindings. Alpha [calculator] wrote the prefix, Beta [calculator] HIT it, Gamma [calculator, create_visualization] missed. Beta is what makes the result readable — it proves identical bindings share a prefix, so Agent identity does not perturb it. Gamma then missing proves Alpha's toolConfig held only calculator: had the union reached a bound Agent, Alpha would equal Gamma and Gamma would have hit. Without Beta the test would have been vacuous, since "no hit" could equally have meant "Agents never share prefixes" and read as a pass. Fall-through side: an unbound Agent, with a UNIQUE instruction string. The first attempt reused the shared instructions and returned hit on both arms — ambiguous, because cacheStatus names no prefix and the unbound Agent's resolved set is exactly Gamma's bound set. Re-run in isolation: A ['calculator'] first_write, B ['calculator', 'create_visualization'] HIT, C ['calculator','browse_web'] first_write. B hitting A proves A already carried the pinned tool the client never sent. The method note is the part worth keeping: a bare `hit` is not evidence. It names no prefix, so a test built on it needs a positive control and isolation from unrelated sessions. The first run looked like a pass. Also records the post-deploy regression check on the single-tool ack fix: hello_world (1 tool) ungated and saving, campus_directory (3 tools) still blocking until acknowledged, with the no-role warning and cost copy both intact. Co-Authored-By: Claude Opus 5 * docs: runbook for retiring an MCP server that Agents already bind Answers the staging question directly: retirement must start by stopping new adoption while leaving grants and the catalog row untouched, because revoking access to a bound tool is a hard block rather than a degradation (`_resolve_tools` raises `AgentBindingBlockedError`, D5), and hard-deleting the row while grants survive is worse still — `can_access_tool` passes, then `external_mcp_client` silently drops the server and the Agent answers with no tools and no error. Verified against the code rather than assumed. Notable findings the runbook records: - `ToolStatus` has no readers on any enforcement, listing or runtime path. `soft_delete_tool` (status=DISABLED) is observably a no-op, and `BindableItem.meta` does not carry `status` at all, so the Agent Designer picker is structurally blind to it. - The safe divergence is display-layer, not grant-layer. Status-filtering `get_user_accessible_tools` would 403 an author editing an Agent that keeps the binding, because `binding_validation._validate_tool` reads that same function. - A published Agent runs its frozen `AgentVersion` snapshot for every non-owner invoker, so the author removing the binding fixes nobody but themselves. Admin takedown clears `publishedVersion` and is the escape hatch. - `get_roles_for_tool` is blind to wildcard grants (`GSI2PK = TOOL#*`). - `delete_tool` cascades into neither `TOOL_GRANT#` rows nor the `CAPABILITIES` sibling row; `delete_capabilities` has zero callers. - `DEFAULT_TOOLS` seeds no MCP servers, so the seeder is a resurrection risk only for built-ins — split out as its own section. Co-Authored-By: Claude Opus 5 * fix(tools): re-validate tool updates and share the union helper Three defects found by auditing the claims in the always-on spec's validation section, plus the correction to one of those claims. **The claim was wrong.** §11.1 said "the normalizing validator fired in production" because pinning a tool flipped enabledByDefault false->true. It did not. `ToolDefinition` sets no `validate_assignment` and `repository.update_tool` patches with `setattr`, so a `mode="after"` validator runs at construction and never again. The coherent value came from the admin form, whose three-way control sends both flags. **The defect underneath.** A partial `PUT {"alwaysOn": true}` persisted alwaysOn=True next to enabledByDefault=False — the incoherent pair the validator exists to prevent. Reads normalised it, so runtime behaviour was correct and nothing misbehaved, which is why it survived the original validation; raw-item consumers (backfills, exports, analytics) do not read through the model. Fixed by re-validating the patched state before serialising. One of the new tests guards the round trip itself: re-validation goes through `model_dump`, so anything the dump drops would be lost silently on every update. **Voice bypassed the seam.** It hand-rolled its own union instead of calling the shared helper — a second set of semantics to keep in step, which is exactly what the single-seam argument in §1 exists to prevent. `union_enabled_tools` moves to `apis/shared/tools/always_on.py`, routes keeps a thin alias so its call sites and tests are untouched, and a test asserts voice imports the same object rather than a copy. **Voice under-reported its toolset.** The connection log was emitted before the union, so the first line an operator reads when debugging a voice toolset was short by exactly the tools this feature adds. Resolved before the log now, and the log names the pinned count separately. Spec §11 records the correction, the general lesson ("the observed value is correct" is not evidence the mechanism you credit produced it), and a new §11.5 explaining why the negative RBAC side is unverifiable on dev specifically: three of four dev roles carry the `*` wildcard, including `default` and `developer`-via-inheritance, leaving `faculty` the only narrow role in the environment. Co-Authored-By: Claude Opus 5 * feat(tools): gate new adoption of a retiring tool in every picker Implements §7 of docs/specs/mcp-server-retirement.md — the first stage of retiring an MCP server, which must stop new adoption while leaving every existing binding working. Before this, `ToolStatus` had no readers on any path that mattered, so `status: deprecated` (and the soft-delete that sets DISABLED) was observably a no-op. The rule, stated once: a non-`active` tool can be turned OFF but not ON. That is the exact mirror of `alwaysOn`, which is on and cannot be turned off, and both guards now sit side by side in the same functions. Blocking both directions would strand every user who already has the tool — which is the opposite of what retirement asks of them. Backend: `bindable_catalog._list_tools` carries `status` on each tool item's `meta` (a free-form dict, so no contract break; a str-Enum serialises as "deprecated", which the SPA's `!== 'active'` comparison needs). `GET /tools/` already returned it on `UserToolAccess`. SPA: `isRetiring()` in tool.service is the single predicate, with a guard on every surface that can newly adopt a tool — Agent Designer chips, the Customize tools list and detail page (master switch and every sub-tool switch), the scheduled-run checkbox, and `ToolService.toggleTool`/`toggleServerTool` as the backstop the keyboard and programmatic paths land on. Retirement is a property of the server, so a retiring server that is off cannot be adopted one sub-tool at a time; narrowing one already on stays open. Nothing is filtered out of any list. `binding_validation._validate_tool` reads the same `get_user_accessible_tools` as the palette, so dropping a retiring tool from it would 403 an author editing an Agent that keeps the binding — for a tool that Agent can still run. And an author who cannot see a binding cannot remove it. The Agent Designer adds a section notice naming the retiring tools an agent still binds, since "remove it, and resubmit if published" does not fit on a chip. No grant-layer predicate moves: `_tool_grant_set`, `can_access_tool`, `get_public_tool_ids` and `_validate_tool` are untouched, and test_public_tool_grants.py still guards the picker/enforcement agreement rule. Compatibility, measured rather than assumed: prod's catalog is 33 tools, all `active`, so this deploys as a strict no-op there. Dev has two `disabled` probe servers (`pe12_probe_mcp`, `sk_hello_approval`) that stop being addable — the intended outcome, recorded in §7 with a re-runnable check for other environments. Tests: 25 new (backend projection + serialization + not-filtered-out; SPA service guards both directions, sub-tool adoption, version-skew fallback; Agent Designer selected/unselected/scoped-ref cases). Full backend suite 3982 passed; SPA 3641 passed. Co-Authored-By: Claude Opus 5 * refactor(admin): give the console the sidenav body and the full content width An admin page was squeezed between two navigations — the chat sidenav on the left, listing conversations that cannot be opened from /admin, and the console's own