feat(acp): kiro-cli v3 token refresh + auth callback wiring

[FakeRocket543]

kiro-cli v3 token refresh + auth callback for ACP

The kiro-cli v3 engine requires a _kiro/auth/getAccessToken JSON-RPC callback from the host, stricter prompt content (single text block per message), and notifications/initialized before it accepts session requests. Without this wiring, kiro v3 sessions fail auth against Bedrock and cannot run.

What this does

• New kiro_auth module: reads the cached access token from kiro-cli's sqlite store, refreshes it via AWS SSO OIDC (refresh_token + device registration), persists it back; falls back to spawning kiro-cli whoami to trigger its internal refresh.
• ACP reader loop auto-replies _kiro/auth/getAccessToken with the refreshed token + profile ARN from the local store (replaces the previous hardcoded ARN).
• Merges consecutive text content blocks before sending prompts (Bedrock rejects multiple text blocks) and sends notifications/initialized after initialize.
• Auth-failure reconnect trigger narrowed to unambiguous token-invalidity errors only (is_auth_token_failure); generic "Access denied" surfaces normally instead of silently churning reconnects.
• Missing-token callback replies with a JSON-RPC error (-32001), never an empty success — prevents the v3 engine from retrying against a blank token → Bedrock 403 → reconnect storm.
• 30s refresh-failure cooldown so a genuinely dead token cannot hammer AWS SSO OIDC or spawn kiro-cli whoami in a loop.
• Regression test pinning the narrowed matcher; plus rust-1.96 clippy lint fixes in existing test code (cron.rs, format.rs, discord.rs) that were blocking cargo clippy -- -D warnings.

Review Contract

Goal

Make kiro-cli v3 agents usable through OpenAB: the ACP connection must supply a valid, auto-refreshed Bedrock access token on demand and stay within the v3 engine's stricter protocol expectations (single text block per prompt, notifications/initialized).

Non-goals

• Replacing kiro-cli's own credential management; OpenAB only reads/refreshes what kiro-cli already cached and defers to kiro-cli login for initial auth.
• Supporting non-sqlite kiro-cli configurations or non-default KIRO_HOME paths.
• Fixing the pre-existing secrets::tests::resolve_exec_nonzero_exit test failure (environment-dependent, unrelated).
• The repo-wide cargo fmt drift (~1500 lines across 17 files, pre-existing).

Accepted Residual Risks

• Token refresh uses reqwest::blocking inside tokio::task::spawn_blocking — correct today; a future refactor that calls it from an async context would panic. Mitigation: doc comment on get_access_token states the contract.
• parse_rfc3339 fallback assumes UTC when sub-second parsing fails; only reachable for externally-written non-RFC3339 timestamps (the save path round-trips to_rfc3339 cleanly).
• save_token_data is UPDATE-only (no INSERT fallback) — fine because the row always pre-exists (we just read it).
• Concurrent _kiro/auth/getAccessToken callbacks could race a refresh; window is tiny (callbacks are ~1/hour) and the worst case is a duplicate refresh, not corruption.
• Recovery: if auth is genuinely broken, the callback returns -32001 with "run kiro-cli login" guidance; the 30s cooldown bounds retry cost.

Acceptance Criteria

• cargo test -p openab-core --lib acp::connection::reader_loop_tests passes, including auth_token_failure_matcher_is_narrow (generic "Access denied" must NOT reconnect; token-invalidity errors MUST).
• cargo clippy -p openab-core --lib --tests -- -D warnings is clean.
• Chosen-picked on latest origin/main (280db4d) with no conflicts; reader-loop behavior verified there.
• Manually verified (2026-08-11 review): missing-token path returns a JSON-RPC error object, not an empty success.

Follow-ups

• Unit test for the _kiro/auth/getAccessToken reply shapes (success / -32001 / -32603) via a run_reader_loop duplex test — currently covered by inspection only.
• Repo-wide cargo fmt normalization PR.
• Consider a stable error-code boundary (mirrors what breadiary did) if more string-matching on agent errors accumulates.

[openab-app]

Caution

This PR is missing a Discord Discussion URL in the body.
This PR will be automatically closed in 24 hours if the link is not added.

All PRs must reference a prior Discord discussion to ensure community alignment before implementation.

Please edit the PR description to include a link like:

Discord Discussion URL: https://discord.com/channels/...

[chaodu-obk]

Important

CHANGES REQUESTED ⚠️ - The AWS SSO OIDC refresh path uses the wrong JSON field casing and cannot succeed as written, and several kiro-specific behaviors are wired unconditionally into the generic ACP path shared by every agent.

What This PR Does

Makes kiro-cli v3 agents usable through OpenAB by answering the engine's _kiro/auth/getAccessToken JSON-RPC callback with an auto-refreshed Bedrock access token, and by conforming to the v3 engine's stricter protocol expectations (single text block per prompt, notifications/initialized after initialize).

How It Works

A new kiro_auth module reads the cached token from kiro-cli's sqlite store, refreshes it via AWS SSO OIDC when expired (falling back to spawning kiro-cli whoami), and persists it back. The ACP reader loop auto-replies the auth callback with the token plus profile ARN, replies with a JSON-RPC error (-32001) when no token is available, applies a 30s cooldown after failed refreshes, and narrows the auth-failure reconnect trigger to unambiguous token-invalidity errors (with a regression test). Consecutive text content blocks are merged before prompts are sent. Unrelated rust-1.96 clippy fixes in test code are bundled to keep cargo clippy -- -D warnings green.

Findings

#	Severity	Finding	Location
1	🔴	SSO OIDC refresh sends/parses snake_case JSON; AWS CreateToken requires camelCase - the primary refresh path can never succeed	kiro_auth.rs:50-56,127-132
2	🟡	Kiro auth callback is answered for ALL ACP agents with no agent-type gating - any agent subprocess can request the Kiro bearer token	connection.rs:314
3	🟡	notifications/initialized plus a 100ms sleep are sent unconditionally to every agent; the notification is an MCP-ism not defined by ACP, and sleep-based sync is race-prone	connection.rs:677-686
4	🟡	Text-block merging is applied globally, changing valid ACP prompt semantics for all agents to satisfy only kiro v3	connection.rs:840-843
5	🟡	Auth-failure break runs before pending-response resolution, so the awaiting caller gets a generic "connection closed" instead of the real error	connection.rs:354-359
6	🟡	OpenAB becomes an unsynchronized second writer to kiro-cli's private sqlite: refresh races can stale-overwrite a rotated refresh token, the TokenData round-trip drops unknown future fields, and save_token_data ignores the UPDATE result	kiro_auth.rs:159-165
7	🟡	kiro-cli whoami fallback builds a PATH that prefers user-writable ~/.local/bin, inherits the full parent environment, and has no timeout	kiro_auth.rs:258-270
8	🟡	Unvalidated region from the DB is interpolated into the secret-bearing OIDC URL; non-standard partitions (GovCloud, aws-cn) use different endpoint suffixes	kiro_auth.rs:118-122
9	🟡	Timestamp edges: fallback parse discards non-UTC offsets, timestamp_millis() as u64 turns pre-epoch values into far-future expiries, and duration_since(...).unwrap() is an avoidable panic path	kiro_auth.rs:68-91,181-186
10	🟡	The 300-line credential module ships with zero tests (callback reply shapes, refresh serialization, persistence, merge_text_blocks all untested); the discord.rs test edits also reduce the fallback test to asserting a literal	kiro_auth.rs, discord.rs:3611
11	🟢	Narrowed is_auth_token_failure matcher with positive and negative regression cases is a real improvement over broad reconnect churn	connection.rs:1235
12	🟢	Missing-token callback returns a JSON-RPC error instead of an empty success, plus the 30s refresh cooldown - good storm-prevention design	connection.rs:322-337
13	🟢	PR description includes an explicit review contract with non-goals, residual risks, and acceptance criteria	-

Finding Details

🔴 F1: OIDC wire format is camelCase; this code uses snake_case

refresh_token() posts {"grant_type", "client_id", "client_secret", "refresh_token"} and RefreshResponse derives Deserialize over access_token / refresh_token / expires_in. The AWS IAM Identity Center OIDC CreateToken API requires grantType, clientId, clientSecret, refreshToken in the request and returns accessToken, refreshToken, expiresIn (see the official CreateToken API reference). As written, every direct refresh either gets a 400 (invalid_request, missing required clientId/clientSecret) or fails deserialization, so the code always drops into the kiro-cli whoami fallback. The headline feature of the PR is dead code in practice.

Fix: use camelCase keys in the request body and #[serde(rename_all = "camelCase")] on RefreshResponse, and add a serialization unit test pinned to the documented wire format.

🟡 F2: Auth callback answered for every agent type

The _kiro/auth/getAccessToken handler lives in the generic run_reader_loop used by all ACP agents (codex, claude, gemini, etc.), not just kiro-cli. Any agent subprocess (or a compromised/buggy one) can emit this method name and receive the user's Bedrock bearer token over stdio. Gate the handler on the connection's agent type or an explicit capability flag.

🟡 F3: notifications/initialized sent to all agents, plus sleep-based sync

ACP v1 does not define notifications/initialized (it is an MCP concept). Sending it to every agent relies on all other agents silently ignoring unknown notifications, and the hardcoded 100ms sleep taxes every connection while providing no ordering guarantee. Gate it to kiro v3 and replace the sleep with protocol ordering or a ready/retry contract.

🟡 F4: Global text-block merging

merge_text_blocks is invoked for all agents in prompt(). Distinct text blocks are valid ACP; collapsing them (joined with \n\n) is a kiro/Bedrock-specific workaround that silently changes prompt semantics for every other agent. Apply per-agent.

🟡 F5: Break ordering loses the real error

When is_auth_token_failure matches, the loop breaks before the msg.id pending-resolution branch. The drain-on-close path then resolves the in-flight request with a synthetic connection closed error, so the caller never sees the actual token-failure message. Resolve the pending response first, then break.

🟡 F6: Second-writer hazards on kiro-cli's sqlite store

kiro-cli owns this database; OpenAB writing to it races kiro-cli's own refreshes and any concurrent OpenAB connections (the atomic cooldown timestamp is a hint, not a lock). TokenData re-serialization also drops any fields a future kiro-cli version adds to the JSON, and save_token_data ignores the UPDATE result, so a lost write (including a rotated refresh token) is silent. At minimum: check the UPDATE outcome, log failures, and preserve unknown JSON fields (e.g. patch the parsed serde_json::Value instead of round-tripping a struct).

🟡 F7: Subprocess hygiene on the whoami fallback

The constructed PATH puts $HOME/.local/bin first (user-writable, classic hijack location), the child inherits the full parent environment (which may include OpenAB secrets), and there is no timeout so a hung kiro-cli whoami blocks the spawn_blocking task indefinitely. Use an absolute binary path or the inherited PATH, env_clear() plus an allowlist, and a timeout.

🟡 F8: Unvalidated region in endpoint construction

region comes from a JSON blob in the DB and is interpolated into https://oidc.{region}.amazonaws.com/token, which carries client_secret and refresh_token. Validate the region against ^[a-z0-9-]+$ (and note GovCloud/aws-cn partitions use different suffixes) before building the URL.

🟡 F9: Timestamp edge cases

parse_rfc3339's fallback truncates at the first . and appends Z, silently discarding a non-UTC offset and shifting expiry by hours. timestamp_millis() as u64 wraps negative values into huge u64s (pre-epoch becomes far-future). build_auth_response's duration_since(UNIX_EPOCH).unwrap() can panic on clock skew while the sibling now_ms() correctly uses unwrap_or(0).

🟡 F10: Test coverage gap

The PR's own follow-ups section admits the callback reply shapes are covered "by inspection only". A 300-line credential module handling live tokens deserves at least: wire-format serialization tests (which would have caught F1), callback success/-32001/-32603 shape tests, and merge_text_blocks unit tests. Separately, image_attachment_block_missing_content_type_falls_back in discord.rs now formats the literal "unknown", so it no longer exercises any fallback logic - the lint fix weakened the test instead of restructuring it.

Addressing External Reviewer Feedback

openab-app[bot]

This PR is missing a Discord Discussion URL in the body. This PR will be automatically closed in 24 hours if the link is not added.

⚠️ Unresolved - the PR body still contains no Discord Discussion URL and the PR carries the closing-soon label. The contributor must add the discussion link or the PR will be auto-closed regardless of review outcome.

Baseline Check

• PR opened: 2026-08-18
• Base branch: main (declared); merge-base 280db4db equals the base head - clean stack on latest main, no drift
• Head SHA reviewed: d27a598eb9ac287491cbaeaec1d7a5c3c1e7ee2d (8 files, +514/-9)
• Main already has: the ACP reader loop, permission auto-reply, and connection pool; no kiro token handling of any kind (previous ARN was hardcoded)
• Net-new value: kiro v3 auth callback wiring, token refresh module, narrowed reconnect matcher, v3 protocol accommodations
• CI: all 42 checks green (check, validate, full smoke-test matrix including kiro-cli)

5. Three Reasons We Might Not Need This PR

1. Credential management duplication - kiro-cli already owns token refresh. Given F1, the only path that actually works today is the kiro-cli whoami fallback; a much smaller PR that only reads the store and shells out to kiro-cli for refresh would deliver the same working behavior without a hand-rolled OIDC client.
2. Coupling to private internals - the sqlite schema, key names (kirocli:odic:token), and JSON shapes are undocumented kiro-cli internals that can change in any release, silently breaking auth. There is no version check or graceful degradation.
3. Vendor-quirk leakage into shared protocol code - the initialized notification, text-block merging, and auth callback all live in generic ACP code. If kiro v3 relaxes these requirements upstream, the workarounds remain as permanent complexity for every agent.

[chaodu-obk]

Thanks for the contribution - kiro-cli v3 support is genuinely needed, and parts of this design (the narrowed reconnect matcher, the -32001 error reply, the refresh cooldown) are solid.

Before we continue with review iterations, the maintainer asks that you first bring this proposal to our Discord server for community discussion. This PR introduces host-side credential handling and changes to the shared ACP connection path that affect every agent backend, so we want alignment on the approach (see the findings in the review comment above, especially F1-F4) before further implementation work.

Action items:

1. Start a discussion thread in our Discord server describing the goal and the intended design.
2. Add the Discord Discussion URL: https://discord.com/channels/... line to the PR body (this also resolves the auto-close warning from the bot).
3. Once the discussion settles on a direction, we will continue the review from there.

I am converting this PR to draft in the meantime so automated review cycles pause while the discussion happens. Feel free to mark it ready for review again once the discussion link is in place.