Release/1.15.0 - #877
Merged
Merged
Conversation
`AgentDetailPage` called `load()` from its constructor, and `load()` reads
`this.id()` — an `input.required<string>()` bound by `withComponentInputBinding()`.
The router sets that input *after* construction, so the read threw NG0950 before
`AgentApiService.getAgent()` was ever called.
The throw landed inside `load()`'s own try/catch, which turned it into
`this.error.set('Failed to load this agent.')`. So the page rendered a plausible
error banner with **no HTTP request behind it** — no console error, no failed
request, nothing to grep for. `/agents/:id` has been in this state since phase 3
(66f3834); the detail page has never successfully loaded in a browser.
Moved both init calls to `ngOnInit`, which runs after input binding.
Found while running the phase 5-7 dev smoke test, where the detail page is the
only surface carrying the pin control. There is no `agent-detail.page.spec.ts` at
all, which is why CI never caught it — a regression spec is tracked separately.
Note that a spec asserting this must set the input the way the router does (after
creation, e.g. via `RouterTestingHarness`); passing `id` at construction time
reproduces neither the bug nor the fix.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`AgentDetailPage` had no spec, which is the only reason the phase 3 bug (`load()` in the constructor, fixed in f60951b) reached users: the page rendered "Failed to load this agent." with no HTTP request behind it, and nothing in CI looked. Five tests over the load path. The two that would have caught it assert that `AgentApiService.getAgent` is called with the bound id, and that the error banner is absent on the happy path — deliberately not assertions about the rendered agent, since a zero-request page is what the bug actually produced.⚠️ The component is routed through `RouterTestingHarness` with `provideRouter(routes, withComponentInputBinding())` rather than constructed directly. That is load-bearing, not ceremony: `id` is an `input.required` the router sets *after* construction, so handing it in at construction time makes every assertion here pass against the broken code too. Verified both ways — red against the constructor form (`getAgent` called 0 times), green against the fix. Services are stubbed via DI tokens rather than `vi.mock`, per the repo convention on cross-spec mock pollution. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-page-load fix(marketplace): the agent detail page never loaded its agent
`AgentDetailPage` had no spec, which is the only reason the phase 3 bug (`load()` in the constructor, fixed in f60951b) reached users: the page rendered "Failed to load this agent." with no HTTP request behind it, and nothing in CI looked. Five tests over the load path. The two that would have caught it assert that `AgentApiService.getAgent` is called with the bound id, and that the error banner is absent on the happy path — deliberately not assertions about the rendered agent, since a zero-request page is what the bug actually produced.⚠️ The component is routed through `RouterTestingHarness` with `provideRouter(routes, withComponentInputBinding())` rather than constructed directly. That is load-bearing, not ceremony: `id` is an `input.required` the router sets *after* construction, so handing it in at construction time makes every assertion here pass against the broken code too. Verified both ways — red against the constructor form (`getAgent` called 0 times), green against the fix. Services are stubbed via DI tokens rather than `vi.mock`, per the repo convention on cross-spec mock pollution. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…l-page-spec test(marketplace): pin the agent detail page's load path
…l-page-load-spec test(marketplace): pin the agent detail page's load path
An `@`-mention turn and the plain turns around it run on two different cached `Agent` instances, and neither sees the other's messages. The user sees one continuous thread — the SPA renders from the persisted store — but the model answers "NOT IN HISTORY" about a turn on screen. Measured on dev session e5e8b259-1780-4179-8ebe-38c57d3709a5. The test models the round trip that produces it: plain → mention → plain, with a `store` standing in for AgentCore Memory. The distinction it has to preserve is why the bug is invisible in production logs — the mention turn is a cache MISS, so it restores and legitimately sees prior history; the plain turn after it is a cache HIT, restores nothing, and is stale. A first draft asserted the mention turn was empty, which fails for the wrong reason and would have passed once anyone made a fresh agent restore. Asserts on the conversation rather than instance identity, so it stays valid whichever fix wins: reuse one instance, hand the message list between instances, or re-restore on a stale hit. `strict=True` so it fails loudly the moment the fix makes it pass. Refs #741 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…a turn The agent cache keys on *configuration* — system prompt, tools, model, skills — which is right: those need different `Agent` objects. The conversation is not configuration, and nothing enforced that. So an `@`-mention forked the thread. The mention turn missed the cache, built a second agent, restored history and looked fine; the next plain turn reverted the key and cache-*hit* the original instance, whose in-memory list still ended before the mention. `initialize()` never re-runs on a hit, so the stale list won silently: the model answered "NOT IN HISTORY" about a turn the user could see on screen, because the SPA renders from the persisted store. Symmetric, too — a second mention could not see the plain turns in between. `_adopt_session_conversation` points a newly built agent at the list its session is already using.⚠️ Shared by reference, deliberately. Copying would fix only the direction that already works — the miss. The turn that goes stale is a cache *hit*, where nothing runs and there is nothing to copy. Aliasing is what makes the mention turn's appends visible to the instance the next turn hits. Safe because every site that rebinds `agent.messages` (document stripping, content-block sanitizing, compaction slicing, pairing repair) lives inside `TurnBasedSessionManager.initialize()` and runs before adoption; after construction the list is only appended to. Re-restoring on a stale hit was the alternative and is worse: restored history passes through the sanitizers and pairing repair while accumulated history does not, so one conversation can serialize two ways depending on the path — a prefix byte change on an arbitrary turn, which is what the prompt-cache contract forbids. Aliasing re-serializes nothing, so the cached prefix is untouched. Adoption runs before the `extra_tools` early return: an uncached agent still takes a turn in the thread and must not fork it. A length guard keeps a live instance that trails Memory from dragging a newer restored history backwards; the reverse comparison would be wrong, since compaction legitimately shortens a restored list. Concurrency is covered by the single-flight session lease — one turn per session at a time. Separate replicas share no cache, so cross-process divergence is unchanged by this either way. Fixes #741 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n-history-fork fix(inference): one session is one conversation, whatever agent runs a turn
The phase 7 notes predicted "two prompt-cache prefix re-writes ... roughly $0.25 per mention round-trip". Measured on dev, the outbound leg is a genuine full re-write but the swap-back is a cache HIT: base toolConfig and system prompt revert byte-identically and read from the still-live pre-mention entry, so only the mention exchange is written as a delta. mention miss_avoidable read 0 write 4301 next plain turn hit read 2720 write 138 At a 50k prefix and Sonnet 5's $2.30/MTok write premium that is ~$0.12 per mention, about half the original figure. Two caveats now recorded with the number, because both change it materially: the swap-back only hits inside the ~5-minute TTL, and an Agent that pins its own model lands in a different cache namespace entirely, where it can hit nothing and its write is never re-read.⚠️ Also records why the number was wrong twice over. The first measurement was taken against #741, where the swap-back looked *cheaper* than the truth (write 71, not 138) because the stale agent was omitting the mention exchange from the prefix — a broken run flattering us while the spec's prediction erred the other way. Re-measure after any invocation-path change rather than trusting either. Refs #741 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…he-arithmetic docs(marketplace): a mention costs one prefix re-write, not two
…essor `classify_cache_status` decided "avoidable" vs "TTL expired" using the gap to the immediately-previous call row. That is the wrong clock the moment two prefixes interleave in one session — which is exactly what an `@`-mention does. The entry a call could have HIT belongs to the last call with the *same* prefix, and that is not always the last call. Measured on dev during the #741 verification: a plain turn 266s after a mention was classified `miss_avoidable` and booked $0.006256 of waste, but the entry it needed was written 308s earlier, past the 300s TTL. The re-write was unavoidable. In that six-turn session three rows were `miss_avoidable` and none were the bug the metric exists to catch. The fingerprints needed to do this correctly were already on the row. The lookup now reads a small window of recent rows instead of one, and classifies against the newest row sharing this call's toolConfig + system-prompt hashes. No new data, no schema migration — only a wider read of rows already indexed. Three behaviours worth keeping: - When no same-prefix predecessor is in the window we classify as `miss_ttl_expired`, not `miss_avoidable`. Deliberately the conservative direction: under-reporting waste keeps the metric trustworthy, whereas crying wolf is what made it useless. - Calls with no fingerprints (hook disabled, non-Bedrock provider) fall back to the previous call exactly as before, so nothing regresses where the fix cannot apply. - `previous_cached_prefix_tokens` still comes from the previous call when there is no match, so the below-threshold `first_write` guard keeps working. `cacheGapSeconds` keeps its original meaning — plain chronology, which existing consumers read. `cachePrefixGapSeconds` is added only when the deciding call was an older one, so a status that looks inconsistent with the visible gap explains itself rather than reading as a bug. Surfaced through the admin costs API and rendered beside the gap on the session anatomy page. Fixes #753 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-ttl-clock fix(observability): measure the cache TTL from the same-prefix predecessor
…sed door D14 called for the kill switch plus an `agent-marketplace` RBAC capability that 404s the routes for ungranted roles. The capability was never built, and should not be: the admin roles UI builds `grantedTools` from the tool catalog with no free-text entry, so a feature-capability id cannot be granted from the UI at all — only by hand-writing DynamoDB items. That is why the `skills` gate was removed and why `scheduled-runs` 403'd in prod and was dropped. Building it a third time ships a gate nobody can open. What was left behind was worse than either end state. One template condition, `@if (showAgents() && isAdmin())`, hid the nav entry — while `/agents/discover`, `/agents/:id` and `/agents/pinned` carried `authGuard` only, the composer `@`-mention menu had no admin check at all, and a role-seeded pin (D9) pushed Agents into a member's Pinned tab unprompted. The store was reachable by any authenticated user through three doors while hidden behind the one we controlled; nobody could answer "who can see this?" without reading four files. So: drop `isAdmin()` from the nav condition and make the kill switch the only lever. The Preview badge stays until Assistant deprecation (#746) lands, since until then the sidenav ships both nouns. The regression spec renders the real template rather than reading `showAgents()` — the bug lived only in the `@if`, so a spec asserting the computed passes against both the gated and the GA'd code. Verified to fail against the old condition and only against it. Fixes #745 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ace-ga feat(marketplace): GA the Agent store — the nav gate was the only closed door
… approval Three individually-reasonable v1 non-goals compose into a governance hole none of them was evaluated against: D2 does not re-review edits, there is no agent versioning, and D9 lets an admin *lock* a role-seeded pin so members cannot remove it. So an admin locks an Agent into every member of a role's sidebar, the author rewrites its instructions, and the new behavior is live immediately for everyone — no re-review, no history, no notification, and no opt-out for the affected user, whose own dismissal loses to the lock by design. Approval now records a SHA-256 of the instructions it approved, and the admin Listings table marks any published listing whose instructions no longer match. This is not a gate — it is the curator's reason to look. The marker reports two claims and deliberately does not merge them: instructions behavior definitely changed measured (hash mismatch) edited record changed, cause unknown inferred (updatedAt > reviewedAt) The issue proposed the timestamp comparison alone as a pure read. It is not sufficient on its own: an admin's own D13 presentation edit bumps updatedAt without touching reviewedAt, so a timestamp-only marker fires on the admin's own typo fix, and a governance marker that cries wolf is one that gets learned-ignored. But the hash alone would be blind on every listing approved before it shipped — precisely the already-published, possibly-locked back catalogue this is about. So: hash when present, timestamp as the explicitly weaker fallback, styled and worded so the two never read alike. The baseline is a hash *of* the instructions, so it rides the same viewer gate. `_agent_response` already drops `instructions` for anyone below editor; it now drops the hash with it. Not reversible on its own, but it would confirm a guessed prompt for anyone who could produce one, which is what that gate exists to prevent. The admin listings projection is unaffected. Both new suites were verified to discriminate: with a timestamp-only derivation the admin-edit test fails with 'edited' is None, and without the viewer gate both hash-exposure tests fail while owner/editor keep passing. Fixes #744 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…drift-marker feat(marketplace): mark published Agents whose behavior changed after approval
On any `ng test` run exactly one randomly-chosen spec file failed; the
next run passed it and failed a different one instead.
The unit-test builder hardcodes `isolate: false` in its vitest runner, so
every spec file assigned to a worker shares one module registry, one
jsdom, and one timer implementation. Worker assignment is timing
dependent, so which specs share state changes run to run — hence the
random victim.
The trigger was leaked fake timers. `vi.restoreAllMocks()` restores
neither fake timers nor `vi.stubGlobal`, and several specs relied on it
for both. A spec left the clock frozen and the next spec in that worker
to await anything real-timer-driven — a RouterTestingHarness navigation,
an HttpTestingController round-trip — died with "Test timed out in
5000ms", which is how every observed failure presented.
Fixed at the source in seven specs:
sidenav, toast, citation-display fake timers never restored
model.service stubGlobal needs unstubAllGlobals
session.service window.location + document.cookie
replaced and never put back
admin-cost-state, admin.guard unrestored spies on
document.createElement and console
Added a backstop afterEach in test-setup.ts (useRealTimers +
unstubAllGlobals + unstubAllEnvs) so a spec added later cannot
reintroduce the class of bug, and global-hygiene.spec.ts as a canary
asserting the shared jsdom is intact. A global restoreAllMocks was
deliberately not added — it would reset vi.fn() implementations and
break specs that build mocks outside beforeEach.
Also fixes a second, unrelated flake found while verifying: app.spec.ts
loads the root component through a runtime dynamic import that measures
~4s against vitest's 5000ms default, leaving ~20% headroom before load
pushes it over. Raised to 30s, matching the existing precedent in
shared-view.page.spec.ts.
Verified with a harness that builds the full spec set and varies only
execution order (subsetting with --include changes bundling and masks
the bug): 16 seeded orderings including the four that previously failed,
5 plain runs, and 3 runs under 8 CPU hogs on 10 cores — all green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… in the nav Designer Phase 5, step 3 and the user-facing half of step 4. Marketplace D1 is "one noun: Agent", and the sidenav has been shipping both. Steps 1-2 were already done: `compat.to_agent_view` renders a legacy Assistant *as* an Agent, and the Designer passed parity some time ago — it carries model config, tools, skills, memory spaces, tags and visibility on top of everything the old editor had, and reuses that editor's own knowledge-base section and share dialog for the rest. The old editor had strictly less to offer for the same record. `/assistants`, `/assistants/new` and `/assistants/:id/edit` become `redirectTo` entries rather than deletions. Those paths are in bookmarks, in the "edit" link of every old chat session, and in links people shared with each other. The ids are identical on both sides — nothing was migrated — so a redirect lands on exactly what the old URL opened. Deleting them would 404 all of that for no gain. The nav ships one entry, and the Agents "Preview" badge came off with the second noun it existed to disambiguate. The term pass is deliberately not a find-and-replace. "You are a helpful assistant that…" stays as the instructions placeholder: that is the conventional system-prompt idiom, and rewriting it to "agent" would be worse prompt guidance, not better terminology. What changed is the words naming our own product concept — nav, session indicator, share dialog, settings.⚠️ This changes what AGENTS_API_ENABLED means. While both nouns shipped, off degraded to the Assistants editor; there is nothing left to fall back to, so off now means no authoring surface at all. It is an outage switch, not a feature toggle. Recorded in the flag docstring and the spec rather than silently changed — worth a deliberate decision about whether the flag should survive. The old editor's components are now unreachable but not yet deleted; retiring them (and the docs pass) is the rest of step 4, kept separate so this flip stays revertible in one line. Refs #746 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t-deprecation-redirects feat(agents): redirect the Assistant editor to the Designer, one noun in the nav
…ce consumes Designer Phase 5, step 4 — the rest of #746. #758 made these pages unreachable; this deletes them. Gone: assistants.page, assistant-form.page, assistant-list, assistant-preview, and the assistant-form barrel. `assistants/` itself stays, and is no longer a feature — it is the set of pieces the Agent surface consumes: the share dialog, the assistant card, the knowledge-base dialogs and services, PreviewChatService, and the models. Each was verified to have a live inbound reference from `agents/`, `session/` or `knowledge-base/` before being kept. It keeps the name because the record is still an Assistant on the wire — ids are identical and nothing was migrated, so renaming the folder would drag the API contract's vocabulary with it. The README is rewritten from a stale description of deleted files into that consumer map, and says plainly that new UI belongs under `agents/`. The backend `/assistants/*` surface is untouched and not deprecated: `test-chat` and the document sub-routes are called by the Designer's own preview pane and knowledge-base section. Deleting `assistant-preview.component.spec.ts` would have quietly dropped coverage from a live surface, because `agent-preview` — the component that replaced it — had no spec at all. Ported the tests that still apply, and pinned the one behavior that differs: the Agent preview sends no live instructions and opts out of client-side prompt and tool injection, because an Agent resolves those server-side from the saved record and a long persona would exceed the system_prompt cap (422). That looks like a bug until you know why, so it now has a test with the reason attached. Also pinned the model lock, including its release on destroy — the lock lives in the root ModelService, so leaking it would follow the user out of the Designer into an ordinary chat. The docs pass found less than expected and deliberately changed nothing in docs/SSE_ERROR_MESSAGING.md, the testing report, or the root README: every "assistant" there is the model's message role, not our product concept. Closes #746 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(spa): stop cross-file state leaks that made ng test flaky
…ssistant-editor chore(agents): retire the Assistant editor, keep what the Agent surface consumes
…e row Same root cause as #741, different blast radius. The agent cache keys on configuration, so an `@`-mention turn builds a second `Agent`, and each `Agent` builds its own `TurnBasedSessionManager`. Both write the same DynamoDB session row and neither knows the other exists. `update_after_turn` loaded persisted state only when `initialize()` had skipped the load. `initialize()` never re-runs on a cache *hit* — the same property that forked the conversation in #741 — so a hit turn kept whatever state its instance was built with and then saved it straight over the newer checkpoint and truncation anchor a sibling had just written. Every exit path of `update_after_turn` saves, including the "nothing to do" ones, so the clobber did not need compaction to actually fire on that turn. This is a cost bug before a correctness one. The truncation anchor is what keeps the restored prefix byte-stable; moving it backwards re-truncates messages previously sent whole, which rewrites a 35k-150k-token prefix at the $2.50/MTok write premium on a turn where nothing about the conversation appeared to change. Fix is direction (2) from the issue: re-read on every turn. Not the shared-object approach #750 used for the message list — a sibling may live in another replica, where aliasing reaches nothing. The read is one GSI query and `_save_compaction_state` already does an identical one immediately after, so it roughly doubles a cost that is already noise next to a model call. The single-flight session lease serializes turns per session, so read-modify-write is safe. Conditional writes (direction 3) would only re-cover what the lease covers. State never moves backwards. A load failure is indistinguishable from "nothing persisted" — both return a default `CompactionState` — so adopting the result blindly would let one transient DynamoDB error zero a real checkpoint, which is the clobber this exists to prevent. Adopt only when the persisted record is at least as far along, and carry the anchor forward even at an equal checkpoint, since the anchor also advances on prompt-cache expiry. Removed `_compaction_state_loaded`: nothing branches on it now, and its comment described a lazy-load that no longer exists. Its one real job — stopping the failed-compaction path from persisting defaults over a live checkpoint — is now done by the monotonic guard. The integration-style test the issue asked for drives two managers against one shared fake row and asserts the checkpoint never regresses; it fails with `assert 0 == 8` against the old code, which is the bug verbatim. Fixes #751 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tate-fork fix(compaction): re-read compaction state per turn — two managers, one row
D6 specified three runnability states — ready, limits (degraded), blocked — and the middle one could never occur. It degraded only when a binding declared `config.optional == true`; that key was read in exactly one place and written nowhere. No API accepted it and the Designer had no control for it, so every gap already resolved to `blocked` and the model was two states pretending to be three. The deciding reason is not that it was dormant, though. Building toward it would have contradicted `agent-designer.md` D5, whose Non-goals say "No **downgrade** on missing capability (block-only v1)" and call downgrade "a later opt-in". Block-only is what `agent_binding_resolver` actually implements — it raises for model, tool, skill and memory_space alike, and `optional` appears nowhere in it. Making the preview offer a third outcome would have required teaching the resolver to skip bindings, which is a real product decision about whether an Agent may silently run degraded, not a checkbox. Two specs disagreed; this settles them on the one that shipped. So: `RunnabilityState` is `ready | blocked`, `MissingCapability.optional` and `_is_optional` are gone, and the detail page and admin default-pins page stop rendering a distinction they could never draw. The `optional` flag is not merely unused now — it is inert, and a test pins that. A hand-edited or future record could still carry it, and it must not resurrect a state the runtime cannot honour. D6 is revised rather than quietly trimmed: it records that there were three states, why the middle one went, and that if downgrade is ever taken up it starts at the resolver, not at the preview layer. Fixes #747 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s-runnability fix(agents): retire the unreachable `limits` runnability state
…to a cap Resolves the D9 "locked-pin ceiling" open question. No cap; friction. Phase 6 sharpened the concern the question was written about. A locked seed genuinely cannot be dismissed — verified on dev, a lock beats a user's own tombstone, which is right, because a lock a user could dismiss would be pointless. So an admin choosing between "seed" and "seed locked" has no reason not to lock: locking guarantees the rollout lands and the cost falls on someone else's sidebar. Left alone, the dominant strategy is to lock everything and Pinned stops being the user's shelf. A cap does not work here, and the reason is structural rather than a matter of picking the right number. Pins merge as a union across every role a user matches and a lock from any one of them wins (D9.2), so a per-role cap of two does not mean a member sees two locked Agents — someone in five roles sees ten. Capping the union instead is not implementable: role membership resolves per user from Entra claims at request time, so which roles co-occur on one person is unknowable when an admin saves a seed list. Enforcing at read time would be worse than no cap, silently dropping an admin's lock for some users — a rollout that looks like it landed and did not. So the console shows what it can actually know. A running locked count on the seed header, a warning past a threshold, and `lockedElsewhere` — how many locked seeds every *other* role holds, which is the one fact an admin cannot work out from their own page and the only honest way to show the union. Advisory throughout: `count_locked_outside` swallows a failed role listing rather than taking the pins page down with it. No new failure mode, nothing to migrate for roles already over any line, and a threshold that moves without breaking a saved list. If over-locking shows up, revisit with the counts this makes visible. Fixes #748 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…in-friction feat(marketplace): make locked seeds cost something to an admin, not to a cap
Closes the spec-hygiene issue. Four items, and the last one needed code. **Phasing table** — already fixed while shipping #746; verified, no change. **Quota attribution** — resolved, in code rather than by inference. The invoker pays: `check_quota(user=current_user, …)` at chat/routes.py:1319 with no author-side lookup anywhere on the invocation path, and cost rows land on the same person (`PK = USER#{user_id}`, metadata.py:157). Both file:lines are in the spec so nobody re-derives it. **Review SLA** — committed and split by queue: two business days for submissions, same day for an `inappropriate` report, weekly for the rest. Phase 8 added reports as a second queue and they are not the same clock — a submission is a person waiting to publish, a report is mostly maintenance signal. D2 had been asserting "two business days" in its body while Open Questions said the commitment was never made; that disagreement is gone. **`tagline` backfill** — derive and let the author edit, at submission. Writing that down alone would have re-opened the gap this issue exists to close, because implementing it turned up a second problem the question never mentioned: tagline was *author-owned on the wire and unsettable in the UI*. The API accepted it and the model comment said the author owns it, but no Designer control ever wrote one — the same dormant-field shape as the `limits` state in #747. So submission is now where it gets set, prefilled by `deriveTagline` from the description's first clause. The derivation is not the clever part; putting the shelf row in front of the author at the one moment they are looking at what the store will say is. A bad line is then one edit away instead of a surprise after publication. Also fixed the same trap on the specs' own front pages: both still read "Status: Draft / proposal" while describing shipped code. That is precisely the `agent-directory.md` failure this issue cites — a document whose face does not say what it is. Both now state what is implemented and point at the revisions recorded in place (D6, D9.7, D14). Open questions on the marketplace spec are now empty. Fixes #749 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tion-attribution feat: make mid-stream disconnects attributable
…evaluation-recommendations docs: expand managed KB evaluation
The frontend allowlist omitted .pptx while the backend's ALLOWED_MIME_TYPES has included it since the PowerPoint toolset landed (65a5a6d), so decks were un-uploadable — and create_powerpoint_presentation's own error text tells users to "upload a .pptx template first", advice the UI made impossible. Adding .pptx to the frontend list alone would not work: Bedrock's Converse DocumentFormat enum has no pptx member (pdf, csv, doc, docx, xls, xlsx, html, txt, md), so a deck sent as an inline document block fails the turn with a ValidationException at any size. This adds the carve-out that makes the upload useful, mirroring the existing tabular one: - is_presentation_file() in apis/shared/files/models.py - _partition_attachments() returns a 4-tuple, diverting decks before the size gate (they never go inline, so an "oversized" note would misdescribe why they were skipped) - _build_attachment_guidance() names the deck and points at read_powerpoint_presentation, or names the toggle when the tool is off The tools needed no changes: _find_powerpoint_presentation already resolves any READY .pptx in the session, so an uploaded deck is reachable as-is. Also fixes a latent bug this exposed: validateFile accepts a file by extension when the browser reports no MIME, but uploadFile then sent `file.type || 'application/octet-stream'` — a MIME the backend allowlist rejects, 400-ing an upload the UI had just accepted. It bites decks twice, since _find_powerpoint_presentation matches the stored MIME exactly, so a deck saved as octet-stream would upload and then be invisible to the tool. resolveMimeType() now resolves from the extension, leaving validateFile as the only rejection gate. Raises the upload cap for presentations only, to 25MB (FILE_UPLOAD_MAX_SIZE_BYTES_PRESENTATION). The general 4MB limit is sized for Bedrock's inline-document budget, which no longer bounds a deck; the constraint that does is _ci_write_bytes base64-encoding the file into a single writeFiles `text` field, a MaxLenString capped at 100MB. Corporate templates with imagery routinely clear 4MB, which is what made template_name unusable. Both size gates now read max_size_for()/maxFileSizeFor() so they cannot disagree about which cap applies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…achment-carveout feat(files): accept .pptx uploads and route them to the PowerPoint tools
G3 gated every offload quality comparison on an unknown: we send no citations config in production, and the #836 validation reasoned that Bedrock's visual (page-image) PDF path is tied to citations-enabled document handling. If true, production is blind to charts today and arm A is a degraded baseline. It is not true. 14 questions over 5 documents, each asked twice — once with the document block exactly as DocumentHandler builds it, once with citations enabled and nothing else changed — on Haiku 4.5 (the dev default) and Sonnet 5: 14/14 correct in both arms on both models. The model read 631 off an unlabeled bar, found $401 in a table that exists only as pixels, and pulled PR-2291 off a rotated scan. Four of the five PDFs carry no extractable text at all, so those answers can only have come from pixels; the fifth is a text-layer canary that would have caught a broken probe. Citations turn out to be a text-layer feature, not a visual-fidelity switch. With citations explicitly enabled, exactly three responses carried citationsContent — the same three on both models — and all three draw on a text layer. The two questions answerable only from the figure in the mixed document came back uncited from the same request. So the offload baseline is full visual fidelity, uncited. The spec's "native blocks, never flattened text" rule is now measured rather than precautionary, offloading an image-only document costs no citations because there were never any, and enabling citations drops out of PR-1 into a standalone product question about attribution. That question has a real cost of its own: with citations on, the answer text moves inside citationsContent and top-level text blocks go empty, so every consumer must handle both shapes first. This probe hit it directly and scored a correct answer as a miss until the extractor was fixed. Probe is committed and self-contained — it builds its own corpus, touches no user content, and reproduces from a clean checkout. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Conflict was one ledger block in the cost-effectiveness roadmap: this branch added two rows (#836 unblocked, G3 run) while #862 rewrote the eval-harness row to record that the AgentCore Evaluations spike shrank that build. Both edits are wanted; kept all three rows. Also amends `document-offload-evaluation.md` §1 in the same commit, because #862's version of that file only reached this branch with the merge. §1 is the section that *required* the G3 probe, so it was the one place still asserting the premise the probe disproved — its "we do not know whether arm A even sees charts" now resolves to "arm A reads figures at full fidelity, uncited", with the knock-ons for the §2.3 citation family and the §2.2 corpus rule.
…ttings read Two IAM grants found missing by AccessDeniedException in the prod-ai CloudWatch logs. Both had the capability wired end-to-end except for the policy statement, so neither surfaced as a user-visible error. app-api / S3VectorsQueryAccess: The grant listed read actions only, so documents/services/cleanup_service.py exhausted its 3 retries on every vector delete and logged "Cleanup incomplete ... TTL will auto-expire". Two bulk cleanups failed outright in a single hour (0/18 and 0/1 documents), orphaning those chunks in the index where they stayed searchable until TTL. Note the batch action is DeleteVectors (plural); rag-ingestion's DeleteVector (singular) is a different action. AgentCore runtime / UserSettingsTableReadAccess: inference-agentcore-construct.ts injects DYNAMODB_USER_SETTINGS_TABLE_NAME, so UserSettingsRepository reported itself enabled, but the table was absent from the runtime role's grants. get_settings swallowed the AccessDenied into DEFAULT_SETTINGS, silently ignoring the user's saved defaultModelId and serving the system default instead. Scoped read-only on a bare ARN, following the SystemPromptsTableReadAccess precedent — the runtime never writes settings and the table has no GSIs. Adds regression tests alongside the existing SharedConversationsAccess guard, which covers the identical failure mode. Both fail without the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Generated by the kaizen-research skill. Scan window widened to 21 days (2026-07-24 → 2026-08-14) because the last two Friday runs were missed. Top 5 ideas appended to docs/kaizen/review-queue.md for the kaizen-review-prep run later this morning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ntly
An MCP server that requires auth even for `tools/list` — GitHub's
api.githubcopilot.com does — 401s the registration pre-flight whenever the
in-process `oauth_token_cache` is cold, which it always is on a fresh
microVM. `load_external_tools` caught that, logged a warning, and dropped
the tool.
Nothing recovered from there. `OAuthConsentHook` is a `BeforeToolCall`
hook, so it only runs for tools that made it into the registry; the dropped
tool never reached it, so the cache was never warmed and the drop repeated
on every turn for the life of the process. A user whose token was sitting
in the AgentCore vault the whole time lost the tool permanently, and was
told nothing — the model simply didn't have it. 20 turns hit this in prod
in 24h, the single largest source of ERROR lines in the runtime log.
On pre-flight failure for an OAuth-gated tool with a cold cache, ask the
vault directly:
* token -> warm the cache and retry the pre-flight once. This is the
consented-user path, and it is also how the tool returns by itself
after the user completes consent in the popup.
* consent URL -> AgentCore is telling us the user genuinely has not
authorized. Record it so the turn emits `oauth_required` rather than
dropping the tool with no explanation.
* hard error -> stay silent. "Couldn't ask" is not "must consent";
prompting there would nag a connected user whenever the server blips.
Only consulted when the pre-flight already failed, so the happy path costs
no extra round-trip. A warm-but-failing token is left to the hook's
existing AfterToolCall 401 refresh path.
Scopes and customParameters for the vault query move to a shared helper.
AgentCore folds both into the token-vault key, so the two callers asking
with different values would look up different vault entries and prompt a
user who is already connected — the new module exists to keep them in
agreement.
SSE contract: `oauth_required.interruptId` becomes optional. The pre-flight
flavor has no paused turn to resume. A synthetic id would be worse than
omitting it — the resume guard in inference_api/chat/routes.py 400s on
unknown ids, so the user would consent and then be shown an error. The SPA
already guards its resume call on interruptId being present; its validator
now accepts the key's absence while still rejecting an empty string.
Since these re-emit each turn until consent lands, the SPA remembers a
dismissed pre-flight provider for the tab session so declining once doesn't
mean answering the same prompt after every message. Interrupt-driven
prompts stay exempt — a paused turn must remain actionable.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tor-delete-and-user-settings fix(infra): grant app-api s3vectors:DeleteVectors and runtime user-settings read
A .pptx attachment rendered as a generic grey "FILE" chip over paragraph skeleton lines. FILE_TYPE_STYLES had no entry for the type, so it fell through to DEFAULT_STYLE — the upload allowlist gained pptx (#868) but this map never did. - Adds a PPTX style: presentation icon on an orange tint, matching PowerPoint's brand association so the chip reads at a glance - Replaces the paragraph skeleton with a mock slide deck for presentations: a 16:9 front slide with a title bar and bullet rows, two offset slides stacked behind it. The generic skeleton read as prose, which is the wrong signal for a deck - Suppresses the folded-corner and bottom-fade details for presentations — both are "sheet of paper" cues that fight the stacked-slides metaphor The mock slide is decorative. A real first-slide thumbnail needs LibreOffice (see the note in thumbnails.py); the cheaper path — unzipping ppt/slides/slide1.xml server-side for the real title — needs an endpoint and a cache, since the existing snippet route is a byte-range read and a ZIP's directory lives at the end of the file. The new spec pins FILE_TYPE_STYLES against the upload allowlist: a type missing from the map degrades silently to a grey blob rather than failing, which is how this shipped, so adding a type to one list now forces the other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An uploaded .pptx disappeared from a conversation on reload. The file was still in the session and /api/files still returned it — but its attachment card was gone. The card renders from a `fileAttachment` content block that the SPA builds client-side at send time and that is never persisted. On reload the SPA rebuilds those blocks by parsing the `[Attached files: …]` marker out of the message text and matching names against the session's file list (restoreFileAttachments in message-map.service.ts). So that marker is the only surviving link between a file and the message it was attached to. PromptBuilder derived the marker from the files it was turning into content blocks — the inline set. The presentation carve-out (#868) deliberately keeps decks out of that set, so they never reached the marker. A lone .pptx was worse still: with nothing inline, build_prompt took its `if not files: return message` early path and left no trace at all. Spreadsheets had the same gap from the tabular carve-out, so this fixes csv/xlsx cards on reload too. - build_prompt takes `attachment_names`, an authoritative marker list, defaulting to the names in `files` so existing callers are unchanged. The no-inline-files path now returns text that still carries the marker. - chat_agent.stream_async forwards it; the route derives it via _attachment_marker_names, which is the only place that sees every attachment. Oversized files are excluded — those were dropped from the turn and the guidance already explains their absence. - message_will_be_modified accounts for the marker, so displayText still holds the user's original text when nothing went inline. The marker stays at the very end of the text: the SPA's pattern is `$`-anchored, and anything appended after it makes the regex miss and the cards vanish just as completely. The new tests mirror that regex and assert it, since the two sides can drift silently. Order comes from the user's attachment order rather than a concatenation of the partition buckets, keeping the text deterministic — it reaches the cacheable prefix on later turns. Existing sessions won't heal; their messages were persisted without a marker. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
If a turn paused on an OAuth-consent or tool-approval interrupt and the user
never completed it — just typed a new message instead — Strands rejected the
new turn outright:
TypeError: prompt_type=<class 'str'> | must resume from interrupt with
list of interruptResponse's
`InterruptState.resume` refuses a plain string prompt while `activated` is
set, and that flag lives on the agent, which the cache reuses across turns.
Nothing cleared it, so every later turn in the session hit the same wall and
the user got a non-recoverable `stream_error` each time.
The "a fresh turn supersedes a paused turn" policy already exists —
`clear_paused_turn` / `clear_interrupted_turn` in chat/routes.py — but it
only clears the DynamoDB side. The live object on the cached agent was
missed. Same family as the sticky cancel flags `reset_cancellation_state`
handles, and this sits directly beside it under the same per-turn discipline.
Deactivating alone isn't enough. Strands appends the assistant `toolUse`
message before running tools and returns on interrupt without appending the
matching `toolResult`, so history ends on an unanswered tool call.
`_repair_tool_pairing` can't fix it here: it deliberately leaves a *trailing*
toolUse alone ("left for prompt-arrival handling") and doesn't count it as a
violation, so at head-of-turn it no-ops. Left as-is, Strands appends the new
user message behind the dangling toolUse and Bedrock rejects the request.
So we drop the abandoned turn back to the last completed assistant turn. That
clears the unanswered toolUse and leaves history ending on an assistant
message, so the incoming prompt keeps roles alternating. Synthesizing an
error toolResult instead would satisfy pairing but leave two consecutive user
turns — the synthetic result then the real prompt — which Bedrock rejects
just the same.
The drop is in-place. The message list is aliased across the cached agents
serving one session (#741/#750); rebinding mid-life silently breaks that
alias, which `_adopt_session_conversation`'s docstring calls out explicitly.
Resume detection mirrors `InterruptState.resume`'s own acceptance test so the
two can't disagree, with one deliberate difference: an empty list is not a
resume here. Strands tolerates `[]`, but in this codebase that's the
max_tokens "Continue" prompt, and a real resume always carries at least one
entry — treating it as a resume would leave the stale pause armed.
Tests drive the real `strands.interrupt._InterruptState`, not a stub: one
asserts the exact prompts we classify as "fresh" are exactly the ones the SDK
rejects, and an end-to-end case reproduces the production TypeError and shows
it gone after the reset. Imported by the private name on purpose, so a
strands upgrade that reshapes it fails loudly here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bedrock-agentcore and Strands bump chains both shipped in #857 (1.9.1 -> 1.21.0 at zero lag; strands 1.51.0), Nightly has been green 12 consecutive days, and the MCP 2026-07-28 spec went final -- so nine queued entries were asserting premises that are no longer true. Left open, kaizen-review-prep would have ranked them as live work. Resolved: 4x bedrock-agentcore bump, 2x Strands bump, 2x nightly CI, 2x MCP Apps spec-prep (the pair counts as two of the nine alongside the [2026-05-29] capability item). Residue carried forward rather than dropped: - #564 is still open upstream -- now its own entry, with #621/#629 flagged as related restore-path and telemetry risks. - The un-adopted Strands capabilities (continue_on_error, hook ordering, cache_tools_ttl) split into their own entry, with the decisions.md bar on a bare context_manager swap restated. - The curl-pin item down-ranked, not closed -- the deb13u* wildcard is holding, so it is correctness debt rather than active breakage. Also corrects an unverified premise: the [2026-05-29] entry called `io.modelcontextprotocol/ui` spec-canonical, which this week's scan could not confirm against the spec source. The successor entry now carries that verification as an explicit precondition. Note: kaizen-research does not normally edit `## Resolved` -- that is review-prep's job. Done here under explicit instruction; each resolved entry records that provenance. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Generated by kaizen-review-prep. Ranked agenda for the 10-15 min decision pass; queue updated with prior-review outcomes. Covers six review cycles (last review was 2026-07-03). Resolves the three stale queue entries left after commit a49d265's nine-entry hygiene pass: the hygiene entry itself, plus the two duplicate caching-audit entries now consolidated into the open [2026-07-24] one. Queue: 46 -> 34 open, no false premises.
…2026-08-14 chore(kaizen): weekly research scan 2026-08-14
…ions-probe docs: run the G3 citations probe — the premise was wrong
…lent-drop fix(mcp): recover OAuth-gated MCP tools instead of dropping them silently
…achment-card feat(files): slide-deck preview card for .pptx, and stop diverted attachments vanishing on reload
…sume-typeerror fix(streaming): abandon a stale pause instead of bricking the next turn
…26-08-14 chore(kaizen): weekly review prep 2026-08-14
`_recover_oauth_preflight` asked the AgentCore vault on *any* pre-flight failure for an OAuth-gated tool with a cold token cache. A server that is simply unreachable therefore looked identical to one refusing an unauthorized caller: the vault correctly answered "this user has no token, here is an authorization URL", the turn emitted a pre-flight `oauth_required`, and the user was shown a Connect prompt on every turn that completing consent could never satisfy. Seen in dev while verifying #872: `canvas_faculty` points at `http://localhost:8026/mcp`, unreachable from the AgentCore Runtime container, so its pre-flight fails with an httpx ConnectError rather than a 401. The same path runs in prod for any OAuth-gated MCP server during an outage. #872 guarded the vault-call-failed case ("couldn't ask" is not "must consent") but not the server-unreachable case. Classify the exception before consulting the vault: only a 401/403 counts as a possible consent gap. Connect, DNS, timeout, and 5xx failures keep the pre-recovery silent-skip behaviour. The status is not on the exception we catch. `MCPClient.load_tools()` raises `ToolProviderException` wrapping `MCPClientInitializationError` wrapping an anyio `ExceptionGroup`; the real `httpx.HTTPStatusError` is three levels down and the outermost message carries no status at all. So `_is_auth_failure` walks `__cause__`/`__context__`/`ExceptionGroup` members — the same shape `mcp_apps._is_transient_connect_error` already had to handle — and reads `.response.status_code`, with a deliberately narrow text fallback for servers that report the refusal as protocol text. Gating the whole recovery rather than just the consent-recording is safe: on a transport error the vault's token would only feed a retry that fails the same way, so the consented-user path loses nothing. Tests build the real wrapped exception chain. An existing test that used `RuntimeError("connection refused")` to reach the vault-unreachable branch now uses a 401 — under the new gate it would never have reached the vault and would have passed vacuously. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…eflight-classify fix(mcp): only treat an auth failure as an OAuth consent gap
Minor release on attachments and external tools, covering 12 PRs merged to develop since the 1.14.1 backmerge (#862–#876). * PowerPoint decks are uploadable and route to the PowerPoint toolset, with a presentation carve-out keeping them out of the inline document set Bedrock cannot encode, a slide-deck preview card, and a 25MB deck-specific upload ceiling (#868, #873) * Carved-out attachments reach the [Attached files] marker, so .pptx AND spreadsheet cards survive a reload (#873) * OAuth-gated MCP tools recover from a cold-cache pre-flight 401 instead of being dropped for the life of the process — the largest source of ERROR lines in the prod runtime log — and only a 401/403 counts as a consent gap, so an unreachable server no longer shows an unsatisfiable Connect prompt (#872, #876) * An abandoned consent pause no longer bricks every later turn in the session (#874) * Two prod IAM gaps closed: app-api s3vectors:DeleteVectors (document cleanup was orphaning vector chunks until TTL) and runtime user-settings read (saved defaultModelId was silently ignored) (#870) * Page departures attest as navigated_away, and ALB access logs are enabled, so interrupted turns can be attributed rather than bucketed as connection_lost (#864, #867) SSE contract: oauth_required.interruptId is now optional — the pre-flight flavor has no paused turn to resume. Requires a CDK deploy (platform.yml, then backend.yml, then frontend-deploy.yml). No GSI changes; gsi-inventory.json is byte-identical to main and check-gsi-update-limit.mjs passes across all 26 tables. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+434
to
+435
| f"Skipping external MCP tool {tool_id}: failed to start client " | ||
| f"with a cached {provider_id} token ({exc})" |
Comment on lines
+444
to
+446
| f"Skipping external MCP tool {tool_id}: pre-flight failed without " | ||
| f"an auth error, so this is not a {provider_id} consent gap " | ||
| f"({exc})" |
Comment on lines
+458
to
+459
| f"Skipping external MCP tool {tool_id}: failed to start client " | ||
| f"and could not resolve a {provider_id} token ({exc})" |
Comment on lines
+469
to
+471
| f"Skipping external MCP tool {tool_id}: pre-flight still failed " | ||
| f"after warming the {provider_id} token from the vault " | ||
| f"({retry_exc})" |
Comment on lines
+475
to
+476
| f"Recovered external MCP tool {tool_id}: warmed {provider_id} " | ||
| "token from the AgentCore vault after a cold-cache pre-flight failure" |
Comment on lines
+483
to
+484
| f"Skipping external MCP tool {tool_id}: no {provider_id} token and " | ||
| f"no authorization URL from AgentCore ({exc})" |
Comment on lines
+490
to
+491
| f"External MCP tool {tool_id} needs {provider_id} consent; " | ||
| "surfacing oauth_required instead of dropping it silently" |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| protocol="mcp_external", | ||
| mcp_config=SimpleNamespace( | ||
| server_url="https://api.githubcopilot.com/mcp/x/issues", | ||
| approval_required_names=lambda: set(), |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Minor release on attachments and external tools — 12 PRs merged to
developsince the 1.14.1 backmerge (#862–#876).Pre-merge checklist
check-gsi-update-limit.mjsPASSED across 26 tables;infrastructure/gsi-inventory.jsonis byte-identical tomain. No existing table gains an index, no data migration.VERSION1.14.1 → 1.15.0,sync-version.sh --checkPASSES across all manifests and lockfiles (backend, frontend, infrastructure, tui).CHANGELOG.md+RELEASE_NOTES.md, previous entries untouched.What's shipping
PowerPoint decks as attachments (#868, #873) — the backend has accepted
.pptxsince the PowerPoint toolset landed but the SPA allowlist never did, socreate_powerpoint_presentation's own error text told users to upload a template the UI made impossible. Bedrock's ConverseDocumentFormatenum has nopptxmember, so this needed a carve-out diverting decks out of the inline document set, not just an allowlist entry. Decks get a stacked-slides preview card and a 25MB ceiling.Attachment cards survive a reload (#873) — carved-out files never reached the
[Attached files: …]marker the SPA rebuilds cards from. Fixes spreadsheets too.OAuth-gated MCP tools recover from a cold token cache (#872, #876) — a server that requires auth for
tools/list401'd the pre-flight on every fresh microVM and the tool was dropped permanently with no explanation. 20 turns hit this in production in 24h, the largest single source of ERROR lines in the runtime log. The follow-up narrows the trigger so an unreachable server isn't misread as a consent gap.An abandoned consent pause no longer bricks the session (#874) —
InterruptState.activatedlived on the cached agent and nothing cleared it, so every later turn failed with a non-recoverablestream_error.Two production IAM gaps closed (#870) — app-api was missing
s3vectors:DeleteVectors, so document cleanup orphaned vector chunks until TTL; the runtime lacked user-settings read, so every user's saveddefaultModelIdwas silently ignored. Both found byAccessDeniedExceptionin the production CloudWatch logs, both now carry regression tests.Interruption attribution (#864, #867) —
navigated_awayattested from a SPApagehidehandler, plus ALB access logs, so dropped turns stop landing in the unattributableconnection_lostbucket.This release requires a CDK deploy. Run
platform.yml→backend.yml→frontend-deploy.yml.infrastructure/lib/changed in four places: a new ALB access-log bucket (SSE-S3 — KMS fails silently for ELB log delivery), two IAM grants, and theFILE_UPLOAD_MAX_SIZE_BYTES_PRESENTATIONenv var. Synthing the ALB construct now requires a concrete region.SSE contract change
oauth_required.interruptIdis now optional — the pre-flight flavor has no paused turn to resume, and a synthetic id would 400 the resume guard right after the user consents. Clients must show the Connect affordance without attempting a resume when the field is absent.🤖 Generated with Claude Code