feat(knowledge): add Amazon Bedrock Knowledge Base retrieval sources

[jingchaodev]

Problem / Motivation

The Knowledge library can only search content KiroCrew ingests itself
(files, folders, vaults). Teams that already maintain a curated Amazon
Bedrock Knowledge Base — often gigabytes of code and docs indexed in their
own AWS account — cannot point KiroCrew at it: the only options are
re-ingesting the whole corpus locally or leaving the KB unreachable from
knowledge search. Requested in #7947.

Why it matters

• Reuses existing, curated KBs instead of duplicating ingestion (and paying
  local embedding cost) for corpora that are already indexed.
• Retrieval and data stay in the user's own AWS account (governance/data
  residency), addressing the security-orchestrator grounding use case Configurable knowledge backend: use an Amazon Bedrock Knowledge Base as a retrieval source #7947
  describes.

What changed (motivation → approach → change)

Goal: registered Bedrock KBs should answer through the SAME retrieval
surfaces as local knowledge — local_knowledge_search (MCP) and the
dashboard's search-for-context — without touching the local index.

Approach: a new bedrock_kb source type that holds no local items and is
queried live at search time, alongside (not replacing) the local store —
answering the merge/reach questions the issue triage left open. Alternatives
considered: mirroring the KB's S3 bucket into local ingestion (rejected:
duplicates storage + embeddings and drifts between syncs) and a network leg
inside HybridRetriever.search (rejected: that path is sync and the
scoped-search exhaustion loop would multiply remote round trips); and
save-first consent (persist the source in a pending state so its target
lives in config and the consent card reuses the config-read path every
other service uses — rejected: a pending-but-unusable source is visible in
every consumer of the sources table (search legs, the dashboard list, the
orphan sweep) and each would need a pending-state carve-out, trading the
request-target TOCTOU surface, which is closed by construction at two
choke points, for a lifecycle state smeared across every reader; the
grantable-card-in-form keeps the unsaved target where the user is typing
it and the store free of half-born rows).

The change:

• knowledge/connectors/bedrock_kb.py — BaseConnector subclass plus a
  search() retrieval path: bedrock-agent-runtime Retrieve per configured
  KB (kb_ids accepts ids or full ARNs for cross-account KBs), merged by
  relevance score. Retrieval tries vectorSearchConfiguration and falls
  back to managedSearchConfiguration when the ValidationException names it
  — MANAGED-type KBs (the current console/crawler default) reject the vector
  key, so without the fallback the first call fails. Result URLs come from
  the source_uri metadata attribute (falling back to the reserved
  x-amz-bedrock-kb-source-uri, then the storage location), so citations
  point at original documents rather than S3 objects.

• Bedrock KB retrieval is a consent-gated paid service (aws_consent.SERVICE_BEDROCK_KB): the add-time probe and every retrieval require an account-bound grant for (service, profile, region), refusing fail-closed, so a repointed profile cannot receive so much as an access check.

• Config carries no secrets: kb_ids, region, optional profile; credentials resolve through the standard AWS chain for the named
  profile at call time. validate_config runs a 1-result live probe per KB
  so a bad id/region/profile is refused when the source is added.

• Query path: search_remote_sources_bounded — a shared worker pool under a
  wall-clock budget (REMOTE_SEARCH_TIMEOUT_SECS) over bounded boto connect/read timeouts, failing OPEN
  to local-only results — called from local_knowledge_search and
  search_for_context, then interleaved by rank (merge_by_rank): local RRF
  scores and Bedrock relevance scores are not comparable, so a raw score sort
  would always favor one leg wholesale. Installs with no bedrock_kb
  sources pay one indexed SELECT and no thread.

• boto3 ships as a new optional [bedrock] extra (same pin as [voice-aws]),
  lazily imported; without it the source type reports the missing extra.

• Frontend: the Sources tab's Add Source dialog gains a Bedrock KB type with
  KB-IDs/region/profile fields (namespace picker hidden — namespaces scope
  ingested items). 9 new i18n keys across en.manual.json, all 11
  translations, and the regenerated pseudolocale.

• Spec: docs/system-specs/modules/knowledge.md documents the remote-source
  semantics in the same commit.

• Three changes reach beyond the new source type, all required by it: add_source now runs every connector's validate_config via asyncio.to_thread (a live network probe must not block the event loop; folder validation is filesystem-touching and benefits equally); bedrock-kb:// joins the non-filesystem URI prefixes exempt from the local-path sensitive check (it is a logical handle, not a path); and drift revocation in aws_consent (reconcile_drift + the new revoke_if_matches) now judges only the exact grant its probe evidence is about, which also applies to Polly/Transcribe/S3/Cost Explorer — derived from review-found races on the new targeted consent GET (both race shapes are test-pinned), and strictly narrowing: a grant is never revoked on evidence about a different target or a predecessor grant.

• Deliberate v1 trade-off flagged for maintainer sign-off: every remote search pays the uncached STS consent probe (the drift window is exactly what the uncached probe closes, per authorize's documented reasoning). A short-TTL verified-account cache would bound rather than eliminate that window and is listed as a follow-up decision.

• Deliberate v1 trade-off flagged for maintainer sign-off: the target-conflict subsystem (two gates, the shared target_change_lock, three refusal codes) is permanent-looking complexity defending the TEMPORARY one-account limit of the single-grant-per-service consent store. Keying grants by (service, profile, region) — the declared follow-up — makes Bedrock natively multi-account and deletes that subsystem outright.

• Remote hits carry a relevance floor (MIN_REMOTE_SCORE = 0.25): Bedrock Retrieve returns nearest neighbors unconditionally, so an off-topic KB would otherwise occupy merged result slots on every search.

Tests

• test/test_bedrock_kb_connector.py (20 tests, mocked client, no network):
  the managed fallback fires only on the error naming it; non-managed errors
  propagate; multi-KB fan-out merges by score and survives partial failure;
  total failure raises; citation precedence (source_uri → reserved key →
  location); validate_config field checks, per-KB probing, auth-failure
  refusal, throttle-as-accessible, missing-extra message; store enumeration +
  source_id scoping + per-source fail-open; the bounded entry's no-sources
  fast path, timeout fail-open, and error swallowing; rank interleave; the
  connector never syncs.
• website/src/test/SourcesList.bedrock.test.tsx (3 tests): the type button
  renders, submit gates on KB ids + region, the POST body carries the derived
  bedrock-kb:// uri and kb properties with nothing credential-shaped, and
  fields reset after a successful add.

Manual verification

Verified end-to-end against a real Bedrock Managed Knowledge Base
(~45K documents) with profile-based credentials: the raw vector-config call
is rejected (confirming the fallback is load-bearing), the connector returns
scored results whose URLs are the original document links (not s3://), and
the add-source live probe passes. The screenshot harness
(website/scripts/capture-bedrock-kb-source.mjs) also asserts the form's
presence, submit gating, and hint text against the built SPA.

Screenshots / video

Add Source dialog, Bedrock KB type (dark, empty — submit disabled):

Filled (dark) — submit enabled:

Filled (light)

Connected source row (Live badge, no sync/staleness controls):

Consent granted — the receipt inside the form:

Related Issues

Closes #7947

Pattern harvest

N/A — feature PR, not a fix/revert.

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 1f77fa86e032ac6b14a64c36515a168553cbc849 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All design premises the PR rests on verify against the base tree: the sandbox bind-mount does pin the consent file's inode (sandbox.py:554-558), so routing the sandboxed MCP server's remote leg through the gateway is derived, not decorative; temp-screenshots/ is the repo's committed-deliverable convention; the consent store is genuinely one-grant-per-service; and the spec is updated in the same commit. The remaining signals are the two trade-offs the author flagged for sign-off, which are real design risks a human should weigh.

Design-Verdict: CONCERNS

Sound feature, but a temporary one-grant limit bought permanent cross-subsystem coupling, and per-search STS probes can silently starve the remote leg.

Watch

• The target-conflict subsystem (shared target_change_lock owned by the connector, the consent handler importing knowledge.connectors.bedrock_kb and querying the knowledge store, three refusal codes, the bedrock carve-out in _effective_target) is permanent-looking machinery whose sole cause is the temporary single-grant-per-service store — the PR itself names the fix (key grants by service+profile+region) that deletes it all. Merging the v1 shape makes the consent surface knowledge-aware in ways the follow-up must unwind.
  Clears when: a maintainer explicitly accepts the v1 coupling with the multi-key follow-up tracked, or grants are keyed by target in this PR.
• Every remote search pays an uncached STS CLI probe (~0.5–1.5s) plus a boto STS freeze-verify inside the 5s fail-open budget, per source; under ordinary AWS latency remote hits silently drop with only a log line, which users will read as "the KB has nothing relevant" rather than "the budget expired."
  Clears when: a maintainer signs off on the flagged trade-off, or a short-TTL verified-account cache bounds the per-search probe cost.

[DESIGN-REVIEWED] 1f77fa8

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 1f77fa86e032ac6b14a64c36515a168553cbc849 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks done — the sandbox inode-pin claim, the revoke call-site sibling count (2 evidence-based sites, both converted; the 2 operator-initiated ones correctly untouched), the screenshot-harness convention (435 sibling scripts, 1288 committed PNGs), and the race pins added in test/test_aws_consent.py are all verified against the base tree. Final review follows.

First-Principles-Verdict: CONCERNS

The one-account target-conflict subsystem guards a cause the author names and defers — the consent store's single-grant-per-service keying — and asks you to sign off on it.

Not justified as shipped

• Item 8 — symptom-level: two 409/400 gates, target_change_lock, three refusal codes defend the store's one-grant-per-service keying; the author's own description says re-keying grants by (service, profile, region) "deletes that subsystem outright". The general fix is genuinely larger (it touches Polly/Transcribe/S3/CE handlers), so this is accepted-and-deferred — but it needs the maintainer decision the description requests, not silence.

What this change ships

Intent: point knowledge search at an existing Bedrock Knowledge Base in the user's own AWS account (linked issue #7947) — ADDITION.

1. Add Source dialog gains a Bedrock Knowledge Base type (KB ids, region, optional profile) — justified
2. Knowledge search (MCP tool + dashboard) merges live KB hits, rank-interleaved, fail-open under a 5s budget — justified
3. Adding/using a KB requires an in-form AWS account confirmation; submit stays disabled until granted — justified
4. New internal-only POST /api/knowledge/remote-search for the sandboxed MCP leg — justified
5. New [bedrock] pip extra, boto3 lazily imported — justified
6. Consent GET/POST accepts a request-supplied (profile, region) for bedrock-kb only — justified
7. Drift revocation for all AWS consents now deletes only the exact grant probed (grant_id, revoke_if_matches) — justified
8. A second Bedrock source or consent for a different (profile, region) is refused — symptom-level, cause named (single-grant store keying), author-deferred
9. Every connector's add-time validation now runs off the event loop — justified
10. Bedrock rows show "Live" (no sync/items/staleness) and survive the orphan sweep — justified

Item 4's zero option is real: base sandbox.py:554-563 documents the inode-pinning seal on aws_service_consent.json, so an in-sandbox consent read goes stale — the endpoint is derived, its 1 consumer (mcp_tools/knowledge.py) is the constraint, not premature generalization. Item 7's provenance is the race tests this PR adds; the two evidence-based revoke() sites on base (aws_consent.py:498, :716) are both converted, zero siblings left.

Watch

• Item 8, above: the subsystem is permanent-looking surface for a temporary limit. Clears when: grants are keyed by (service, profile, region) — the declared follow-up — or the maintainer explicitly accepts the v1 gates.
• Every remote search pays TWO STS round trips: the uncached consent probe in _consent_allows plus the freeze-verify get_caller_identity in _get_client (count: 2, grepped the retrieval path in bedrock_kb.py) — inside a 5s budget the description sizes for one. Clears when: the maintainer accepts the flagged trade-off or the short-TTL verified-account-cache follow-up is decided.

[FIRST-PRINCIPLES-REVIEWED] 1f77fa8

[github-actions]

UX Review (Fable 5, fork) — 🟡 CONCERNS

UX-level review of 1f77fa86e032ac6b14a64c36515a168553cbc849 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

I have everything I need: the full frontend diff, the new strings across locales, the backend error strings that surface in the form, the base AwsConsentGate, and confirmation that all five screenshots exist only as binary markers (fork PR — not materialized) with no blind read in this lane. Emitting the review.

UX-Verdict: CONCERNS

Well-shaped consent-in-form flow, but no cold reader has seen any of it (fork lane), and first-failure error copy is raw AWS exception codes.

Watch

• First-run failure copy is raw backend text: a mistyped KB id/region/profile returns "Cannot access knowledge base {kb_id}: {code}" (e.g. ResourceNotFoundException) into the inline ErrorNotice, and the consent 503 says "could not verify the one-account rule, nothing was confirmed: {e}" — internal rule name plus exception text. High frequency (every typo on the main add path) × friction × every retry. Map _FATAL_PROBE_CODES to plain cause + next step ("Check the KB ID and region, then try again").
• The consent target commits on profile blur, and AwsConsentGate returns null while its query is unresolved — so typing a profile and clicking Confirm in one motion blurs → query key changes → the card unmounts mid-click; the confirmation is lost or recorded for the previous target and the card silently re-asks. Moderate frequency (multi-account users are exactly the profile-typers) × confusion × every time. Commit the profile on change (debounced) or keep the card mounted while refetching.
• After add, a revoked grant or expired credentials fail open with only a log line while the row keeps its "● Live" chip — searches silently go local-only under a label asserting live retrieval. Author defers a health surface (comment at the removed status badge); a human should sign off on "Live" overstating.

Evidence gaps

• No blind read ran (fork head never checked out): first-time comprehension of every control below is unestablished — push the branch to this repo to run it.
• All five screenshots are binary markers only, so none open here: the "Bedrock Knowledge Base" type button (bedrock-form-empty-dark.png); the KB IDs / AWS region / AWS profile fields with the "Queried live…" hint (bedrock-form-filled-dark/light.png); the consent ask card and "Confirm the AWS account above to enable" beside the disabled "Add Knowledge Base" submit (bedrock-form-filled-dark.png); the granted receipt with enabled submit (bedrock-form-consent-granted-dark.png); the "● Live" source row with sync/items/staleness hidden (bedrock-source-row-live-dark.png).
• Two introduced states appear in no screenshot at all: the "Region must look like us-east-1 (lowercase)." warning and the add-failure ErrorNotice (bedrock-kb-add-error).

Suggestions

• The reused remove confirm "Remove this source and all its ingested items?" is false for a live source (it ingests nothing) — add a bedrock variant: "Remove this Knowledge Base source? Searches will stop querying it."
• ru live_query: "Живой" reads as "alive"; use "В реальном времени" (or "Онлайн") for the chip.

[UX-REVIEWED] 1f77fa8

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 1f77fa86e032ac6b14a64c36515a168553cbc849 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 1f77fa8

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 1f77fa86e032ac6b14a64c36515a168553cbc849 via the fork AI-review pipeline; updated in place on each push.

Review details

I verified the consent gate (refuse_and_log returns granted = True-to-proceed, so _consent_allows is not inverted), the slot-release paths in search_remote_sources_bounded (no double-release: worker's finally vs. cancel()-confirmed-never-ran are mutually exclusive), and the fail-closed lookup/lock paths. The candidate list was empty, and nothing in the changed lines rises to a grounded defect at the required bar.

No findings.

[OPUS-REVIEWED] 1f77fa8

[jingchaodev]

First Principles round 1 — accepted and applied (head 5eab1e8): top_k was indeed validated and documented with zero consumers — search() derives its per-KB count from the caller's limit alone. Applied the suggested subtraction rather than inventing a consumer: dropped top_k from validate_config, removed DEFAULT_TOP_K, and removed both spec/docstring mentions (MAX_TOP_K stays — it is the fan-out clamp search() reads). PR description updated to match. Connector suite 19/19 after removing the now-moot validation assertion.

[jingchaodev]

Round 3 — all findings accepted and fixed (head 207fb57):

• GPT F1 + Opus (blocking, same finding): add_source ran connector.validate_config inline on the event loop while the bedrock_kb path issues live Retrieve probes. Fixed at the call site with await asyncio.to_thread(...) — same discipline as the sibling discovery walk.
• GPT F2 (blocking): the bedrock add-form error now renders through inline ErrorNotice (with a comment on why there is no askAgent hand-off: the agent cannot see the unsaved form fields). The pre-existing folder-branch div is untouched — separate surface, grandfathered.
• GPT F3 (non-blocking, real): ARN inputs are now normalized to their trailing KB ID in _parse_kb_ids — the Retrieve API's knowledgeBaseId field is ID-only, so a forwarded ARN would save a source that never retrieves. New unit test pins the normalization; spec + docstring updated to the honest semantics.
• Backend Lint (3.12): the two new Python files were not black-formatted (my pre-push floor missed black); formatted with the pinned 26.3.1.

Connector suite 21/21, black/isort/flake8/mypy/tsc green, SourcesList suites 38/38.

[jingchaodev]

Round 4 — all four items fixed (head 3798cb3):

• GPT F1 (blocking): Knowledge feature-map row updated to name the Bedrock KB live-retrieval source and the search-for-context endpoint.
• GPT F2: ValidationException/ParamValidationError are now fatal probe codes — by the time one escapes _retrieve_one the managed-config fallback is exhausted, so what remains is a malformed id/request and the source must be refused at add time, not saved to fail every search. New test pins the rejection.
• GPT F3: ThreadPoolExecutor import hoisted to module level per top-level-imports.
• Backend Lint (3.12): the black baseline ratchet — round 3's formatting made 2 baselined files clean; baseline pruned via check_black_formatting.py --update-baseline (1165 remain).

Connector suite 22/22; black gate + baseline check, flake8, mypy green.

[jingchaodev]

Round 5 — both blockers and both findings fixed (head f519b8d):

• Boot-path eager load (blocking): the handler no longer imports the bedrock_kb module at import time. Registration is a zero-work lazy proxy whose validate_config/fetch load the module on first Bedrock use; detect_changes answers False locally so SyncScheduler sweeps never load it either. Both search legs defer the import into the request path with a comment citing the rule.
• Unbounded executor queue (blocking): admission is now bounded by a 4-slot semaphore — when every slot is held, new searches fail open immediately instead of queueing. Timed-out futures are cancelled; the slot releases exactly once (worker finally when it ran, cancel-confirmed branch when it never started). Tests pin slot exhaustion (pool must not even be consulted), timeout fail-open, and full slot recovery after the worker drains.
• Optional-import contract: one module-level try/except ImportError with sentinels; _get_client reports the missing [bedrock] extra. Test pins the sentinel path.
• Stale client cache: the (region, profile) client cache is gone — a cached client pinned rotated credentials until restart. A client is built per call; milliseconds against a multi-second network budget.
• Backend Lint (3.12): root-caused to round 3's blanket black run rewrapping untouched lines in two BASELINED files, which the diff-scoped sync-io gate then read as added blocking calls. Reverted the rewraps (files stay baselined), re-applied only the functional edits; the sync-io gate now passes with zero additions.

Connector suite 26/26 (74 with neighbors), black gate + new-files check, flake8, isort, mypy (1302 files) all green; single commit.

[jingchaodev]

Advisory rounds (Design / UX / First Principles) — addressed (head f66dc61):

• Design: remote relevance floor — added MIN_REMOTE_SCORE = 0.25 (live probes against a real ~45K-doc KB scored relevant hits 0.43-0.60); sub-floor hits are dropped before the merge, so an off-topic KB can no longer occupy interleaved slots. Test pins it.
• Design: duplicated merge snippet — both retrieval surfaces now call one augment_with_remote() helper.
• Design: serial remote leg — deferred as a follow-up: overlapping local+remote saves at most the local-search latency in the worst case, and the concurrency plumbing (kick off before the embed-pool call, join after) is not worth the complexity until someone measures the stack-up. Declared here rather than silently skipped.
• UX: dead row — a connected KB row now shows a Live badge and a '● live' meta chip (i18n'd in all locales) instead of 'pending / 0 items / Never synced', and the Sync button is gone for live sources. Test pins the presentation.
• First Principles: undeclared riders — both now declared in the PR description (the all-connector to_thread validate and the bedrock-kb:// prefix exemption).
• First Principles: drop the lazy proxy — declined with reasons: the proxy keeps the whole bedrock_kb MODULE import off the boot path (not just boto3), and the module-level sentinel import shape was itself requested by the GPT round-5 optional-import finding; both blocking lanes are green on exactly this shape, so re-cutting it would churn settled findings. Happy to revisit if a maintainer prefers the cloudwatch.py convention.

Backend 26/26, frontend SourcesList suites 52/52, i18n 19/19, tsc, black/sync-io/brand ratchets green; single commit.

[jingchaodev]

Round 7 — security finding fixed (head cc3794f): the remote query is now passed through redact_credentials() + redact_exfiltration_urls() at the connector's search entry — the ONE leg of knowledge search that leaves the host. Redaction is byte-identical for ordinary queries, so local search semantics are untouched and the subsystem's raw-query contract for local legs stands (the adjudication annotation is right that the destination is the user's own account; defense-in-depth here costs nothing, so complying rather than contesting). New test pins that a credential-shaped token never reaches the Retrieve call while ordinary text survives. Connector suite 26/26; black/isort/flake8/mypy/sync-io green; single commit.

[jingchaodev]

Round 8 — security finding fixed as prescribed (head 1459ff6): Bedrock KB retrieval is now registered with aws_consent (SERVICE_BEDROCK_KB, labeled for the consent cards) and BOTH egress paths gate on the account-bound authorization — validate_config refuses before constructing a client (a repointed profile cannot receive even the access probe; the error names the consent page), and search() contributes nothing for an unconsented/revoked source (denial logged + audited by the consent layer). The consent bridge runs the async check via asyncio.run from the always-off-loop call sites and REFUSES on any failure mode, including the on-a-running-loop edge — fail-closed everywhere, matching the module's contract. Also fixed the stale _parse_kb_ids docstring (non-blocking finding). Tests: consent-refused paths pinned for both validate and search (the client factory asserts if ever consulted without consent); consent module suite + connector suite 185/185 green; live retrieval re-verified against a real Managed KB on this exact head. Spec documents the gate. Single commit.

[jingchaodev]

Round 27 (head 2d87683): GPT's crash finding on the new internal route, legitimate and exactly right — the handler read a bare request.app["knowledge_store"] key where every sibling in the module resolves through _store(request); the key is absent, so the route KeyError'd on every call and the MCP leg's results were always lost (fail-open masked it). Fixed to _store(request); new handler test pins store resolution through the accessor AND the fail-open contract (connector leg raising → 200 with empty results). Suites 138/138, gates green, single commit.

[jingchaodev]

Round 28 (head d9b4086): GPT's non-dict JSON body finding on the internal route — legitimate, fixed with an explicit isinstance(body, dict) → 400 guard (the consent handler's own pattern; [] or a bare scalar is valid JSON but reached .get). Route test extended with the non-dict 400 case. Suites 138/138, gates green, single commit.

[jingchaodev]

Round 29 (head 4d86d7d): both GPT findings on the internal route, legitimate and fixed.

• F1 (browser-reachable raw results): the mixed-internal registry admits cookie-authenticated browsers too, and this route returns RAW connector output (un-redacted content) — so it now additionally requires the internal_auth request marker that token_auth sets only for X-Internal-Secret callers; a browser session gets 403. The dashboard's own surfaces keep getting remote results through search-for-context, which redacts.
• F2 (bounded admission bypass): the route called the raw search_remote_sources, bypassing the connector's admission semaphore and wall-clock budget — repeated searches could queue unbounded default-executor threads that keep issuing paid calls after their caller timed out. Now routed through search_remote_sources_bounded (the same bounded entry every other surface uses); the handler-side wait_for is gone since the budget lives inside.

Route test extended: no-marker → 403; bounded-variant stub raising → fail-open 200/[]. Suites 138/138, gates green, single commit.

[jingchaodev]

Round 30 (head 8b21b0c): the two backend shards' test_error_code_contract failure — my new internal route's three error responses carried prose without a machine-readable code (the ratchet's baseline is 61 for this module; my route pushed it to 64). Added code fields (internal_only, invalid_json) per the RFC 9457-shaped contract. Contract + connector + consent suites 144/144, gates green, single commit.

[jingchaodev]

Round 31 — GPT ✅ and Opus ✅ clean again; the three advisory lanes addressed (head d57a609):

• UX (consent card churn / wrong-target binding): the card's target now binds to the BLUR-committed profile — per-keystroke targets churned the probe and could record a grant for a half-typed profile. Region input lowercase-normalizes on change, so US-EAST-1 no longer sits dead against the shape check.
• Design (stale 3s in the description): correct — one more "3s wall-clock budget" lingered in a body bullet my earlier de-numbering missed; now names REMOTE_SEARCH_TIMEOUT_SECS.
• Design suggestion (MCP hop on zero-bedrock installs): taken — the MCP leg runs the same SELECT 1 … LIMIT 1 fast path the connector uses (the sources TABLE is not consent data) and skips the loopback POST entirely when no bedrock_kb source exists.
• FP (docstring overstatement): augment_with_remote's "single entry every surface uses" reworded to name the true split (gateway-process surfaces use it; the sandboxed MCP server necessarily goes through the internal route).
• FP subtraction (drop grant_id) — DECLINED, cross-lane conflict: GPT's security lane demanded exactly this field (round 25, upheld-fenced: byte-identical same-second re-confirmations are otherwise indistinguishable), and FP itself notes the residual is fail-closed. Removing it re-opens a finding another blocking lane upheld; the field stays.
• Silent-dead Live row / one-grant-per-service: unchanged standing disposition — declared v1 limit + health-surface follow-up, flagged for maintainer sign-off in the description (both lanes acknowledge the declaration).

Suites 138/138, tsc + form tests green, screenshots re-captured, single commit.

[jingchaodev]

Round 32 (head 4f1b5a1): GPT's last crack in the stale-revocation class, fixed as prescribed — the targeted consent GET now captures the grant BEFORE its probe and reconcile_drift is pinned to that grant_id, so a SAME-target re-confirmation landing mid-probe (which passes the target match) is never judged by its predecessor's evidence. This is exactly what the grant_id field (round 25) exists to enable, now applied at the last call site that predated it. New GET-path test constructs the mid-probe same-target re-confirmation and asserts the fresh grant survives. Suites 139/139, gates green, single commit.

[jingchaodev]

Round 33 (head a78dbdb): GPT's validate mis-classification, legitimate and fixed — the probe loop treated the connector's own consent refusals (a RuntimeError from withdrawal mid-probe or a freeze-verify mismatch) as "non-authorization" and saved the source. RuntimeError is now caught FIRST and refuses validate outright: local authorization failures are not transient AWS weather. Only listed transient AWS codes remain pass-through. (The adjudicator's own note stands: such a source was inert anyway — every retrieval re-verifies — but recording a source without current authorization was wrong regardless.) New test pins the mid-probe withdrawal refusing validate. Suites 140/140, gates green, single commit.

[jingchaodev]

Round 34 (head 1f4c054): the Windows shard's test_security.py::test_chained_cd_expansions_do_not_blow_up_the_gate failure is NOT attributable to this diff — it fails identically on a pristine upstream/main checkout (aba8d79c4) with none of this PR's changes (verified in a clean worktree: the test monkeypatches security._dir_holds_sensitive_leaf, which no longer exists at that name on main). Rebased onto current main anyway so the branch tracks tip (content unchanged, still a single commit); connector+consent suites 140/140 on the rebased tree. The upstream breakage will presumably be fixed on main independently.

[jingchaodev]

Round 35 (head 2b31692): GPT's base/secret pairing finding on the MCP fetch — fixed with the atomic-pair shape it asked for. _api_base() is resolved FIRST (forcing the one cached _api_port() resolution both components derive from — _internal_secret is per-port by design), both are captured before the request is built, and an EMPTY secret now refuses the dial outright instead of offering a credential-less request to whatever answers on the port. (The adjudicator's own analysis stands: the launcher's bound port source makes the desync effectively unreachable for the real caller — but the pairing is now explicit rather than incidental.) Side effect: the black ratchet graduated mcp_tools/knowledge.py (the file became fully clean), so its baseline entry is pruned per the gate's own instruction. Suites 140/140, gates green, single commit.

[jingchaodev]

Round 36 (head e10af5d): the E2E i18n-render failure was my commit carrying a pre-rebase en-XA.json — upstream added app-detail strings since, and the stale pseudolocale clobbered their entries in the rebase, leaking Latin text on en-XA surfaces. Regenerated on the rebased tree (12918 keys, checker green). Single commit, ratchets green.

[jingchaodev]

Round 37 (head a1c8ff6): GPT's missing-audit finding on the internal route's 403, legitimate — every peer internal-auth handler SEL-logs its denial and this refusal is reachable by an ordinary cookie session, so the silent 403 was invisible to kirocrew security events. Now emits the best-effort SEL denial (knowledge_remote_search/denied) with the package's standing late-binding sel() pattern before returning 403. Suites 140/140, gates green, single commit.

[jingchaodev]

Round 38 (head d4b748f): GPT's refined pairing finding — fixed with exactly the mcp_core surface it named, which turns out to be purpose-built for this: _resolve_api_target() returns (base, socket_path) from ONE port resolution (its docstring documents precisely the replaced-mid-attempt gateway desync), _secret_for_base(base) reads the credential belonging to that generation, and the socket path is passed to _api_urlopen so the component actually carrying the request is the one the resolution named. Empty-secret refusal retained. Suites 140/140, gates green, single commit.

[jingchaodev]

Round 39 — GPT ✅ / Opus ✅ third clean set; advisory items taken (head 1abd52a):

• FP subtraction (hand-rolled dial), taken: _fetch_remote_results now goes through mcp_core._post — the shared gateway chokepoint every other MCP tool uses — which owns target resolution, per-generation secret handling, and the moved-gateway replay my urllib dial silently lacked. Error payloads map to [] (fail-open preserved). This CLOSES the base/secret pairing thread for good: the pairing now lives where it always belonged.
• FP subtraction 2, taken: augment_with_remote's never-passed source_id parameter dropped (its one consumer never used it).
• UX (hidden region gate): an inline warn hint now renders when the region is non-empty and unmatched — the dead-form-with-no-reason path is gone. New i18n key across all 12 catalogs + pseudolocale.
• UX (failure-state screenshot): attempted; the click-driven capture flaked in the static-served harness (React handler binding, harness-only) and was timeboxed out. The refusal copy itself is unit-tested and points at the inline card; the POST path is covered by the form tests.
• Standing dispositions unchanged: one-grant-per-service v1 + health-surface follow-up + STS-per-search trade-off remain declared in the description for maintainer sign-off (both lanes acknowledge them); submit-until-granted was considered and left — the server refusal names the inline card and gating submit on grant state would dead-end the flow when consent probing is slow.

Suites 140/140, form tests 4/4, tsc green, 5 screenshots re-captured, single commit.

[jingchaodev]

Round 40 — FP upgraded to ✅ PASS; Design's enforcement suggestion taken (head 0cb409c):

• Add-time conflict guard (Design + UX shared Watch): a second bedrock_kb source whose (profile, region) differs from a registered one is now REFUSED at add (bedrock_kb_target_conflict) with a message naming the conflicting source — the v1 one-account limit is enforced and visible instead of letting a re-confirmation silently disable a working source's retrievals. Spec updated; store-level test pins the conflict detection.
• UX ("Live" reads as health): the chip now says "Live query" — visible retrieval-mode disambiguation instead of tooltip-only (new key, all 12 catalogs + pseudolocale).
• Screenshots in temp-screenshots/: kept deliberately — fork PRs get no artifact upload; committed images are the only way this repo's lanes and reviewers can see the UI, and the directory is the established pattern from prior fork PRs. Happy to drop them at merge time if the maintainer prefers.
• Submit-until-granted: standing decline (round 39) — gating submit on grant state dead-ends the flow when the consent probe is slow; the refusal names the inline card.

Suites 141/141, form tests 4/4, gates green, single commit.

[jingchaodev]

Round 41 (head 70b7622): the changed-passthrough i18n gate caught my hi catalog's live_query value still reading as English — now properly Devanagari ("लाइव क्वेरी"), pseudolocale regenerated. Single commit, gates green.

[jingchaodev]

Round 42 (head babd9aa): the dead-keys ratchet caught the orphaned sourcesList.live key (the chip moved to live_query in round 40) — pruned from all 12 catalogs + pseudolocale; dead-keys test back at baseline. Gates green, single commit.

[jingchaodev]

Round 43 — GPT ✅ / Opus ✅ hold; the three advisory lanes' takeable items taken (head 65e44b1):

• FP subtraction (unrelated reformat), taken: mcp_tools/knowledge.py restored to upstream's formatting with ONLY the two load-bearing hunks re-applied (_fetch_remote_results + the remote leg with its fast path); its black-baseline entry is restored, so the whitespace-only knowledge_add_document reflows and the baseline churn are out of the diff.
• UX (green chip = health), smallest fix taken: the "● Live query" chip renders in the muted token — mode label, not health assertion; color is reserved for the future last-retrieval health surface.
• Design suggestion (revocation contract), taken: revoke_if_matches now carries a CONTRACT comment stating every evidence-based revocation for ALL gated services goes through the full captured grant identity, never key-wise revoke() — so the cross-service semantics FP asks a human to confirm are documented as deliberate and regression-guarded in prose.
• Cross-service revoke breadth (FP Watch): remains declared in the description's riders for maintainer confirmation — that call is genuinely the maintainer's.
• Row health surface + fork blind-read: standing declared follow-ups.

Suites 141/141, form tests 4/4, tsc green, screenshots re-captured, single commit.

[jingchaodev]

Round 44 (head a16b799): the Fork workflow-change guard was right — the diff touched .github/black-baseline.txt (a leftover of the round-38/43 baseline churn). Restored upstream's exact file; .github/** is now untouched by this PR and both format gates stay green (both knowledge files are baselined in upstream's copy). Also on 65e44b1, two non-attributable reds for the record: Backend (Windows) shard 3 fails in test_playwright_cli_installer on upstream's own os.killpg (does not exist on Windows; file untouched by this diff), and Backend (3.12) shard 2 was 1/22505 — test_job_sdk.py::TestLiveness, an async-teardown flake in an untouched file. Coverage Gate failed closed purely downstream of those (backend-test=failure).

[jingchaodev]

Round 45 — best round yet, one takeable UX item taken (head 8784607). On a16b799 the round settled fully clean: zero CI failures, GPT ✅, Opus ✅, First-Principles ✅ PASS, Design/UX advisory-CONCERNS.

• UX (submit enabled before consent), smallest fix taken: the Add Knowledge Base submit now subscribes to the SAME consent query the embedded card polls (identical react-query key — one fetch serves both) and stays disabled until the grant exists, so validate_config's server-side refusal is never a first-time user's first signal; the card sitting above the button is the visible reason, and the grant mutation's invalidation frees the button the moment consent lands. Pinned in SourcesList.bedrock.test.tsx (granted-stubbed, async enable) and in the capture harness both ways (ask-state: disabled; granted leg: enabled). Spec updated.
• Design (bless the two consent-path deviations): both deviations are declared in the description riders as deliberate, with rationale — request-scoped target (safe: fresh probe + 409 echo mismatch + exact-equality is_granted) and grant-evidence revocation via revoke_if_matches (now a documented CONTRACT in aws_consent.py). Explicit maintainer blessing is exactly what the riders request.
• UX evidence gaps (cold read): fork-structural — the five committed screenshots are in the PR body for a blind read; a maintainer-side branch push would let the harness lanes materialize them.

[jingchaodev]

Round 46 — GPT's blocking finding fixed; the advisory taken too (head 4c891b8):

• BLOCKING (retarget-consent wedge), fixed at the write: the consent POST now refuses (409, bedrock_kb_target_conflict) a bedrock-kb confirmation whose (profile, region) differs from a registered bedrock_kb source's — recording it would overwrite the single per-service grant in place, silently disabling that source while the new target still could not be added past the add-time guard (the wedge GPT traced). The guard runs BEFORE the identity probe and any write; same-target re-confirmation stays allowed (the mid-probe re-confirmation path depends on it); with no registered sources the first grant flows untouched. Pinned three ways in test_aws_consent.py: different-target → 409 + probe never awaited + grant survives; same-target → 200; no-sources → 200.
• Advisory (worker outlives the budget), taken: search_remote_sources_bounded now derives a monotonic deadline from its timeout and threads it through search_remote_sources → connector.search, which checks it before each per-source and per-KB request — future.result(timeout) alone only abandons the waiter while the worker keeps issuing paid Retrieve calls. Budget skips are fail-open silence (logged), never validate-visible KB failures. Pinned both ways: expired deadline → zero Retrieve calls; live deadline → all KBs queried.
• Spec updated for both. Suites 146/146, flake8/mypy clean, single commit. (The adjudication crash in the lane output is infra — Claude Code version gate — and changed nothing: security-class findings are withheld from adjudication anyway.)

[jingchaodev]

Round 47 (head 131202b): Semgrep flagged the round-46 guard helper's returned f-string as flask...directly-returned-format-string (XSS-if-rendered). False positive by rule scope — this is not a Flask route: the helper's return value lands in an aiohttp json_response error payload, never an HTML body, so there is no markup sink. Annotated with the repo's standard # nosemgrep + inline justification (pattern as in token_secret.py/credwatch.py). No behavior change; suites 106/106 on the touched file's coverage.

[jingchaodev]

Round 48 — Design ✅ PASS (first); FP subtraction + both UX takeables taken (head eef6d14):

• FP subtraction (doubled guard), taken: the one-account compare now lives ONCE as registered_target_mismatch in the connector module; add_source's _conflicting_target and the consent POST's helper both delegate. Same SELECT + compare, one owner. 146/146.
• UX (dead button with no adjacent reason), taken: when KB ids + region are valid but the grant is missing, a one-line helper renders above the submit — "Confirm the AWS account above to enable" — properly translated ×12 catalogs; harness asserts it in the ask-state shot.
• UX (chip verb+noun reads as an action), taken: the chip now reads "Live source" ×12 — a state label matching its siblings ("● Auto", "manual") while keeping the mode disambiguation Design asked for in round 40 (key unchanged, values only — no dead-keys churn).
• UX suggestion (ARN gloss), taken: the KB-IDs placeholder now shows the concrete pair ("e.g. KXABC12345 or arn:aws:bedrock:…:knowledge-base/KXABC12345") per the region-hint pattern, ×12.
• Cross-service revoke rider + fork blind-read: standing declared items (maintainer confirm / maintainer-side branch push).

Pseudolocale regenerated (12920 keys); dead-keys baseline preserved; suites 7/7 + 146/146; single commit.

[jingchaodev]

Round 49 — GPT's two blocking findings fixed (#19, #20) (head d1370f5). Design ✅ PASS and FP ✅ PASS both held on eef6d14.

• BLOCKING (check/write race), fixed with one lock: the connector module now owns target_change_lock; the consent POST runs its one-account compare AND the grant write as a single critical section under it (probe stays outside), and add_source runs a recheck AND the sqlite insert under the same lock (validation stays outside). A concurrent add can no longer land between the consent check and the grant write — the exact interleave GPT traced. Pinned structurally (both gates reference the shared lock) plus behaviorally.
• BLOCKING (fail-open on store errors), fixed fail-closed: registered_target_mismatch now raises TargetLookupError instead of swallowing lookup failures; both gates refuse with bedrock_kb_conflict_check_failed (503) when the compare cannot run. An ABSENT knowledge store (subsystem off) still means "no sources can exist" — only failing lookups refuse. Pinned: broken store → 503 + no grant recorded.
• Conflict-refusal ordering changed as a consequence: the compare now runs inside the locked write (after the probe), so the 409 comes post-probe — the round-46 test updated accordingly; same-target and first-grant paths unchanged. 148/148, mypy clean, spec updated.
• E2E i18n-render red: the added finding is layout/ellipsis-with-flex-parent in file-explorer/TabStrip.tsx (en-XA) — this PR does not touch file-explorer, and its en-XA delta is exclusively sourcesList.* keys, which do not render on that surface. Watching for recurrence on this push; if it reproduces deterministically I'll dig further.

[jingchaodev]

Round 50 — GPT's blocking finding fixed (#21) + both advisories + FP's subtraction (head fc44ff9):

• BLOCKING (source persisted against the wrong grant), fixed inside the lock: the locked insert now re-checks the LIVE grant for the source's exact (profile, region) via is_granted — under the same target_change_lock the consent write holds — and refuses with bedrock_kb_grant_changed (409) when a different-target confirmation landed during validation. A source can no longer be registered grantless-but-Live. Same-target re-confirmation (fresh grant_id, same target) passes, since retrieval demands target equality, not grant identity. Structurally pinned (locked section must consult is_granted + carry the code).
• FP subtraction (two spellings, one authority), taken: the early _conflicting_target pre-check is deleted; the locked insert is now the single conflict authority (cost: a conflicting add spends validation before refusal — the corner FP called acceptable).
• Advisory (function-local uuid import): hoisted to module scope, isort-clean.
• Advisory (stale uniqueness attribution): both comments now attribute re-confirmation identity to grant_id (granted_at can collide within a second).
• FP's standing Watch items (cross-service revoke rider; grant-store key granularity as the guard's root cause) remain declared — the second is named in the description as the v1 one-account limit with the per-(service,target) store as the follow-up shape. 148/148, mypy/isort clean, spec updated.

[jingchaodev]

Round 51 — GPT ✅ clean; UX takeable taken; one Design suggestion DECLINED with citation (head cc81907):

• UX (chip collides with sync vocabulary), taken: the row chip now reads "Remote" ×12 catalogs — reusing the product's existing match_type: "remote" vocabulary as UX suggested, so it can't be parsed as "actively syncing" or as an action. Test + harness locators updated; pseudolocale regenerated.
• Design suggestion (drop temp-screenshots/), DECLINED — it is the sanctioned location: upstream commit ea2355a03 ("chore: relocate PR screenshots to temp-screenshots + weekly cleanup") established temp-screenshots/ as exactly where PR evidence screenshots live, with automated cleanup. The five shots and the capture script are the PR's UX evidence per that convention.
• Design/FP shared Watch (the conflict machinery props up a single-grant store): agreed and already named in the description — the store keyed by (service, profile, region) is the declared follow-up shape, at which point the target-conflict subsystem deletes. The machinery is the price of shipping v1 against the existing store without silently-dark sources; both lanes call it acceptable-and-reversible.
• Design suggestion (separable commit for the grant-id fix): the repo's fork-PR flow squashes to one commit (two max) — the change is prominently declared in the description riders instead, so it cannot be lost as "Bedrock plumbing".
• UX evidence gaps: fork-structural (blind read + materialized shots need a maintainer-side branch push) — standing.

[jingchaodev]

Round 52 (head 1af66fc):

• Chip label, final (and closing the ping-pong): the row chip has now been asked to change three times across lanes — round 40 Design: tooltip-only → visible "Live query"; round 48 UX: verb+noun reads as an action → "Live source"; round 51 UX: "Live source" collides with sync vocabulary → "Remote"; round 52 UX: "Remote" diverges from the feature's own name → "Live". This round takes UX's "Live" (matches the feature's own vocabulary; sibling-state-shaped; the tooltip still carries the full "queried live at search time" meaning) ×12 catalogs. This is the last chip rename this PR will take from lane feedback alone — the lanes have each preferred a different one-word answer, which is precisely the question the standing blind-read gap exists to settle; further renames follow from a blind read or maintainer preference, not another lane pass.
• FP (save-first alternative unweighed), taken as a description fix: "Alternatives considered" now weighs save-first consent (pending-state source so the target lives in config) and states why the grantable-card-in-form won: a pending-but-unusable source leaks a lifecycle state into every reader of the sources table, versus a TOCTOU surface closed by construction at two choke points.
• Design (consent-layer coupling + store-shape root cause): agreed on the diagnosis, standing as the declared per-(service, profile, region) store follow-up — at which point the branch in the consent POST, the shared lock, and both conflict gates delete. The PR's permanent-complexity-for-temporary-limit tradeoff is now stated verbatim in the description riders for the maintainer to weigh.
• Design (temp-screenshots confirm): confirmed intentional per upstream ea2355a03 (see round 51).
• Evidence gaps: standing fork-structural.

[bolichen97]

Rebased onto main 6fbb06bc by a maintainer as part of the 2026-09-08 open-PR audit. The branch was 86 commits behind.

Conflicts: none, clean rebase. I did check the two spots main moved under you: merged #9032 rewrote local_knowledge_search (it now takes namespace), and your remote leg still lands cleanly right after the min_score filter; #9183 split security.py into a package and your imports still resolve.

Gates run locally: black --check (3 touched files are pre-existing baseline entries, untouched), isort clean, flake8 clean, pytest test/test_aws_consent.py test/test_bedrock_kb_connector.py test/test_spawn_audit.py = 159 passed / 1 failed, and that one failure is only boto3 missing in my venv (your own [bedrock] extra), npx tsc --noEmit clean.

Please review the result. A maintainer push makes the maintainer the last pusher, so under the repo's last-push rule this now needs a second approver. Reply if anything looks wrong.

[jingchaodev]

@bolichen97 Thank you for the audit rebase — reviewed it, and both overlap calls are right: the remote leg still belongs immediately after the min_score filter with #9032's namespace parameter untouched around it, and the #9183 package-split imports resolve (from kiro_crew.security paths were already package-form in this branch). Your local sweep was accurate too; the one red CI added on your head was a gate outside it: comment-history (added upstream after this PR's last push) flags review-round references in comments, and this branch carried nine ("(GPT round-49)" style) plus two keyword phrases. Head 5ec1e3116 rewrites them as present-tense invariants — no behavior change, comments only, plus the gate's own baseline ratchet-down it demands (--write-baseline, 1 entry lowered). Local gates on the new head: comment-history green, 148/148 on the consent + connector suites, black/isort/flake8/mypy-linux clean. The boto3-missing failure you saw is expected without the [bedrock] extra — test_bedrock_kb_connector.py skips its live-shape cases under a bare venv and CI installs the extra. I am the last pusher again, so the last-push rule is back to one approver.