6 months ago
6246a26* feat: add Amazon Bedrock as a first-class provider Wire Amazon Bedrock into the existing PI runtime path with structured auth methods, global credential storage, dynamic discovery-backed catalog state, and Bedrock-specific model fallback handling. The desktop settings surface now renders Bedrock credential forms, chat can accept custom Bedrock model IDs or ARNs, and the server/runtime path resolves saved Bedrock auth without relying on process-wide environment mutation. Constraint: Bedrock v1 had to fit the existing PI runtime architecture and the repo's fail-closed model registry without weakening validation for other providers Constraint: Bedrock auth needs more than a raw API key, so a structured provider auth config path was required instead of reusing the existing setApiKey flow Rejected: Add a fourth Bedrock-specific runtime | unnecessary because the installed PI stack already supports Bedrock Converse and could be extended safely Rejected: Use process-wide AWS env mutation for bearer tokens and IAM keys | unsafe across concurrent sessions and contrary to the Bedrock auth requirements Confidence: medium Scope-risk: broad Reversibility: messy Directive: Bedrock custom model IDs and ARNs are intentionally a narrow dynamic-provider exception; do not generalize that behavior to other providers without revisiting model validation and UI selection contracts Tested: bun run typecheck; bun run docs:generate-jsonrpc; bun run docs:check; bun test Not-tested: live AWS Bedrock discovery and inference against a real account * fix: keep Bedrock out of onboarding and stop forcing region defaults Remove Bedrock from onboarding surfaces so first-run setup stays limited to providers the onboarding UI can actually configure, and tighten Bedrock ambient auth resolution so shared AWS default-profile setups are detected without Cowork inventing a fallback region. Constraint: Onboarding still only supports API-key and OAuth flows, so Bedrock needed to stay settings-only until structured credential forms exist there Rejected: Teach onboarding the full Bedrock structured-auth flow in this follow-up | too much scope for a review fix and unnecessary once Bedrock is hidden from onboarding Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep Bedrock onboarding-hidden until onboarding can submit for structured auth methods Tested: bun test test/providers/bedrock-shared.test.ts apps/desktop/test/onboarding-ui.test.tsx; bun run typecheck; bun test Not-tested: live AWS Bedrock discovery against a shared-config default profile * fix: tighten Bedrock discovery and ambient auth detection Drop Bedrock foundation models that do not advertise streaming support before they reach the catalog, and broaden aws_default ambient-auth detection to cover IMDS-backed environments so Cowork can discover Bedrock in common EC2-style setups without a saved config entry. Constraint: Bedrock runtime is ConverseStream-only, so the catalog must not offer models that Bedrock itself reports as non-streaming Rejected: Broaden the follow-up to also honor AWS_SHARED_CREDENTIALS_FILE and AWS_CONFIG_FILE overrides | user explicitly scoped that second P2 out of this pass Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep Bedrock discovery aligned with the actual runtime transport contract; if we add non-streaming Bedrock inference later, revisit this filter together with runtime routing Tested: bun test test/providers/bedrock-shared.test.ts; bun run typecheck; bun test Not-tested: live Bedrock discovery against a real IMDS-backed EC2 instance * fix: drop unusable Bedrock derived catalog entries Skip Bedrock inference profiles and provisioned resources when their backing foundation models were filtered out as non-streaming, so the catalog stays aligned with the current stream-only Bedrock runtime contract. Constraint: Bedrock is still routed through ConverseStream today, so derived catalog entries must only survive when they resolve to a streaming-capable backing model Rejected: Surface the derived entries anyway and let runtime fail later | poor UX and the review finding was correct about the resulting dead-end selections Confidence: high Scope-risk: narrow Reversibility: clean Directive: Once Bedrock supports both Converse and ConverseStream, revisit this filter and remove it together with the stream-only runtime assumption Tested: bun test test/providers/bedrock-shared.test.ts; bun run typecheck; bun test Not-tested: live Bedrock discovery against a real account with mixed streaming and non-streaming derived resources * fix: stop passive bedrock refreshes from doing live discovery Bedrock provider status now separates cache-backed polling from explicit live discovery so onboarding and settings refresh loops stop generating repeated discovery traffic. The desktop fallback auth forms also share one Bedrock-aware definition, and Bedrock model metadata now honors the requested runtime home and env context instead of silently reading process defaults. Constraint: Provider status polling runs repeatedly during onboarding and must not issue live Bedrock discovery requests by default Constraint: Desktop fallback auth UI must stay aligned with harness-side provider auth method contracts Rejected: Leave provider status as the single live-discovery path | repeated Bedrock discovery on passive polling risks latency, throttling, and cost Rejected: Fix only the missing Bedrock fields locally in one file | duplicated fallback definitions would drift again Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep passive provider status reads cache-only; only explicit refresh and auth mutation flows should request live Bedrock discovery Tested: bun run typecheck; bun test test/providerStatus.test.ts test/providers/bedrock.test.ts test/desktop.providerDisplayNames.test.ts test/server.jsonrpc.control.test.ts apps/desktop/test/provider-actions.test.ts test/jsonrpc.codegen.test.ts; bun test Not-tested: Live AWS Bedrock API calls against a real account * refactor: reuse shared provider display names in ChatView ChatView no longer maintains its own provider label table and now reads provider labels from the shared desktop display-name utility. This keeps provider naming consistent across the desktop UI and avoids a second map drifting when providers are added or renamed. Constraint: Desktop provider labels already have a canonical shared utility and ChatView should stay aligned with it Rejected: Keep the duplicate ChatView label map and update both places manually | future provider additions would drift again Confidence: high Scope-risk: narrow Reversibility: clean Directive: When adding or renaming desktop provider labels, update the shared display-name utility instead of introducing local maps Tested: bun run typecheck; bun test apps/desktop/test/chat-view.stability.test.tsx apps/desktop/test/onboarding.test.ts apps/desktop/test/providers-page.test.ts Not-tested: Full repo suite rerun after this follow-up cleanup * fix: narrow bedrock discovery refreshes to bedrock flows Provider auth mutations now only request live Bedrock discovery when the mutated provider is Bedrock, so unrelated auth updates no longer wake Bedrock discovery through ambient AWS credentials. The desktop manual refresh helper also waits for the Bedrock status refresh to finish before reading the catalog when discovery is explicitly requested, so newly discovered models show up on the same refresh cycle. Constraint: Bedrock discovery can trigger live AWS API traffic and should not run on unrelated provider updates or race its own catalog snapshot refresh Rejected: Keep forcing discovery on every successful auth mutation | unrelated provider updates would still cause avoidable Bedrock traffic and latency Rejected: Leave catalog/status reads fully parallel during explicit discovery refreshes | the UI can observe a stale catalog immediately after a successful discovery refresh Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep Bedrock discovery opt-in and provider-specific; cache-mutating discovery paths must not be coupled to generic provider refresh flows Tested: bun run typecheck; bun test test/session.test.ts apps/desktop/test/provider-actions.test.ts; bun test Not-tested: Live AWS Bedrock discovery against a real account * fix: hide exa auth fallback from onboarding The shared fallback auth-method list now stays centralized, but onboarding and settings both apply the same visibility filter so Google only exposes its primary API key while Exa remains in the dedicated tool-provider flow. This keeps the Bedrock dedupe intact without surfacing an extra onboarding credential field to new users. Constraint: Onboarding should only present model-provider credentials, while Exa remains a separate tool-provider setup path Rejected: Revert the shared fallback consolidation | that would reintroduce the Bedrock auth-method drift we just removed Rejected: Patch onboarding alone with an inline Google special case | the visibility rule would still be duplicated between onboarding and settings Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep provider auth fallback visibility rules shared across onboarding and settings so Google/Codex filtering cannot drift Tested: bun run typecheck; bun test apps/desktop/test/onboarding.test.ts apps/desktop/test/onboarding-ui.test.tsx apps/desktop/test/providers-page.test.ts; bun test Not-tested: Manual desktop onboarding walkthrough outside automated UI tests * fix: keep lm studio refresh on the fast path LM Studio refresh buttons in onboarding and settings no longer request Bedrock discovery, so their status/catalog reads stay parallel and avoid unnecessary AWS discovery latency while Bedrock remains hidden from onboarding. The shared provider visibility helper stays intact for Google/Codex filtering, and the regression tests now assert that LM Studio refresh buttons call the plain refresh path. Constraint: LM Studio UI refreshes should not pay Bedrock discovery cost when Bedrock is hidden or unrelated to the active card Rejected: Leave all explicit UI refresh buttons on the Bedrock discovery path | LM Studio refreshes would wait on unrelated Bedrock status work with no user-visible benefit Rejected: Revert the shared visibility helper changes | onboarding/settings would be more likely to drift again on Google/Codex auth-method filtering Confidence: high Scope-risk: narrow Reversibility: clean Directive: Only Bedrock-specific or global provider refresh affordances should opt into ; provider-local refresh buttons should stay provider-specific Tested: bun run typecheck; bun test apps/desktop/test/onboarding-ui.test.tsx apps/desktop/test/providers-page.test.ts apps/desktop/test/onboarding.test.ts apps/desktop/test/provider-actions.test.ts; bun test Not-tested: Manual desktop onboarding/settings interaction outside automated UI tests * test: lock bedrock profile wiring in discovery config The latest review question about aws_profile discovery turned out to be a false alarm: the installed Bedrock SDK carries through client runtime config and resolves credentials from that profile. This regression test makes that contract explicit so future refactors do not accidentally strip the profile from discovery/status clients. Constraint: Bedrock discovery must keep honoring selected AWS profiles for multi-profile users, and review-thread rebuttals should be backed by repo evidence instead of prose alone Rejected: Change discovery to bypass the SDK profile path with a custom credential provider | the current Bedrock client already exposes and preserves , so extra wiring would add complexity without fixing a demonstrated bug Confidence: high Scope-risk: narrow Reversibility: clean Directive: Preserve the SDK-level field on Bedrock client config unless upstream Bedrock runtime behavior demonstrably changes Tested: bun run typecheck; bun test test/providers/bedrock-shared.test.ts; bun test Not-tested: Live AWS Bedrock discovery against a real multi-profile account --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Parent08c73e7
