reconcile PR #387 remediation — AG-UI core (PR-A, best of both implementations)#19
Draft
plauzy wants to merge 21 commits into
Draft
reconcile PR #387 remediation — AG-UI core (PR-A, best of both implementations)#19plauzy wants to merge 21 commits into
plauzy wants to merge 21 commits into
Conversation
Review remediation for awslabs#387 (review 4632216702, fanhongy's decomposition request): the AG-UI core lands first; the A2A JSON-RPC + signed Agent Card listener moves to its own branch/PR where auth enforcement and store bounds are added before it ships. - remove a2a/, agent_card/, their test suites, and the e2e roundtrip - strip the CAO_AGENT_CARD_ENABLED lifespan wiring + constants from api/main.py - drop authlib/python-multipart runtime deps (authlib stays in the dev group for the JWT test fixtures); move the OTel deps to an [otel] optional extra with a no-op fallback in telemetry/__init__.py, covered by subprocess tests - retarget test_default_off_listeners.py at the stronger contract: the A2A modules must not exist here, no /a2a routes mount, and AG-UI stays 404 with no flags - point docs/auth.md PRM examples at the main API (:9889) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jh2kqS95Qg7nP69DfH9zKb
…om access logs Two accepted findings from the awslabs#387 review: - extract_scopes_from_token raises PyJWTError subclasses (or JWKS fetch errors) on bad tokens; the SSE path caught nothing and the WS path caught only HTTPException, so malformed/expired bearers surfaced as opaque 500s / handshake errors. Both paths now fail closed with a clean 401 (SSE) or 4401 (WS). Tests exercise the real parse path for malformed tokens and pin expired-token behavior. - /agui/v1/stream authenticates via ?access_token= because EventSource cannot set an Authorization header, and uvicorn's access log would have persisted the JWT (replayable until exp). RedactQueryTokenFilter now scrubs access_token/ticket values from uvicorn.access records (installed idempotently before uvicorn.run); docs/pwa.md documents short-TTL guidance and the planned ticket handshake. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jh2kqS95Qg7nP69DfH9zKb
… enablement paths - the endpoint test's fake event log ignored the since kwarg, so the replay wiring in the stream handler was never exercised; the fake now honors the strictly-greater-than contract and the test asserts the boundary include/exclude through the endpoint - new parametrized tests pin the deliberate enablement contract: CAO_AGUI_ENABLED or CAO_MCP_APPS_ENABLED each expose the surface, no flags means 404, and falsey CAO_AGUI_ENABLED values do not enable Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jh2kqS95Qg7nP69DfH9zKb
…avity_cli The profile declared provider: gemini_cli — a provider removed upstream in awslabs#353 — so it was dead-on-arrival configuration (a defect none of the awslabs#387 reviews caught; the review instead attributed a provider *deletion* to the PR). Retargeted to antigravity_cli, the documented Gemini successor (awslabs#323), and indexed in the cross-provider README. test/examples/test_example_profiles.py generalizes the fix: every frontmattered example profile's provider (and per-worker overrides) must resolve against the live ProviderType enum, so examples can't drift from the provider set again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jh2kqS95Qg7nP69DfH9zKb
…view - delete the stray tests/__init__.py second test root (real suite is test/) - mypy python_version 3.11 -> 3.10 to type-check at the requires-python floor (mypy stays green at 3.10) - plugins/events.py: traceparent is W3C Trace Context, not RFC 9114 (HTTP/3) - InstancePicker: the remove control was an interactive element nested inside the activate <button> — invalid HTML, unreachable for keyboard/AT. Now sibling buttons in a visually-fused group; component tests pin the structure (no interactive content inside any button) and the activate/remove behaviors - test_headless_ci.py docstring no longer claims a manual cao-server is needed (conftest's require_cao_server starts it) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jh2kqS95Qg7nP69DfH9zKb
Dated implementation logs drift; the content (safety model, wire path, visual-proof pointers, follow-ups) now lives undated in docs/pwa.md's Generative UI section, with the CHANGELOG link updated and _agui_enabled's dual-flag contract spelled out. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jh2kqS95Qg7nP69DfH9zKb
…strict mypy at 3.10 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jh2kqS95Qg7nP69DfH9zKb
Found by the new live-path e2e (a canned replay can't catch this): when the server dies, the native EventSource stays in CONNECTING and auto-retries its ORIGINAL URL — silently dropping the ?since= cursor, so the gap replay never happens and events emitted during the outage are lost. connectAGUI's custom reconnect only engaged on readyState CLOSED, which a dropped connection never reaches. onerror now always takes over: close the source and reopen through the backoff path with the up-to-date cursor. Unit tests pin the resumed-URL behavior and that close() still stops reconnection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jh2kqS95Qg7nP69DfH9zKb
…usal, ?since= recovery The prior recording harness drove a canned replay page; the project's own standard is that demos drive the live path. live-dashboard.spec.ts boots a real cao-server (CAO_AGUI_ENABLED=true) plus the built PWA, connects through the actual add-instance dialog, then proves end-to-end: - every allow-listed component emitted via POST /agui/v1/emit_ui renders live off the SSE stream - an off-list iframe intent is refused server-side (400) and nothing renders - a hard server kill + restart recovers via the ?since= cursor: an event emitted during the outage appears after reconnect (this is the scenario that exposed the cursor-loss bug fixed in the previous commit) Video records via the existing config (video: on); the committed capture is docs/media/agui-live-remediation-demo.webm. playwright.config.ts gains a PLAYWRIGHT_CHROMIUM_EXECUTABLE escape hatch for sandboxes with a preinstalled Chromium; the recording workflow gains the uv/Python toolchain so the spec runs in CI and now also triggers on src/ changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jh2kqS95Qg7nP69DfH9zKb
examples/agui-dashboard/ is a runnable, credentials-free tour: run.sh brings up a 2-worker mock_cli fleet against a running cao-server and drives showcase.sh, which emits all six allow-listed components plus the off-list refusal via emit_ui — exiting non-zero unless six accepts AND one refusal, so it doubles as a deployment smoke test. fleet_worker.md is the mock profile. skills/agui-author (both skill trees) teaches any CAO agent the emit_ui vocabulary: when to emit which component, the props each reference renderer actually displays (verified against GenerativeUI.tsx), the 8 KB bound, the metadata-only contract, and the refusal behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jh2kqS95Qg7nP69DfH9zKb
…lementation notes The unified audit of review 4632216702 + fanhongy + Copilot + the prior Kiro and Gemini agent audits, with per-finding verdicts backed by reproducible evidence; the paste-ready reply (v2 — withdraws the provider-deletion concessions the earlier draft made on a false premise); and the finding-to-commit map for comparing this implementation against Kiro's. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jh2kqS95Qg7nP69DfH9zKb
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jh2kqS95Qg7nP69DfH9zKb
…HIPPED_SKILLS Reconciliation of the two awslabs#387 remediations (per the approved plan, P2-A steps 2-3): - skills/agui-author: adopt the prop table verified against GenerativeUI.tsx — diff_summary titles via 'title' (the renderer never reads 'summary') and progress 'value' on the 0.0-1.0 scale (the renderer clamps to [0,1], so 0-100 values rendered as a full bar). Keeps the stronger disabled-shape wording, the metadata-only guidance, the approval-card caveat, and the local curl verification section. - scripts/sync_skills.py: agui-author joins SHIPPED_SKILLS, putting the skill under the sync/parity machinery (mirror regenerated; --check now reports 10 skills in sync; the packaging-parity suite covers it). - live-dashboard.spec.ts: emit renderer-true payloads and pin them — the diff card heading text and the progress aria-valuenow — so a vocabulary/renderer drift fails the live spec instead of hiding behind HTTP 200. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0123WW42FsCBRwQrK2uwF27s
…t the [otel] extra Ports the graceful-degrade behavior from the Kiro remediation onto the full-extra packaging (approved reconciliation plan, P2-A step 4): - the no-extra fallback's init_telemetry logs the install hint when OTEL_SDK_DISABLED=false instead of silently no-opping; - otel.py guards the SDK imports again, so opentelemetry-api arriving transitively (without the SDK) degrades to the same logged no-op rather than raising at enable-time; - the subprocess blocker uses the find_spec finder protocol — the legacy find_module/load_module pair is gone in Python 3.12, where the old blocker silently stopped blocking and both probes failed (executed failure confirmed by the parallel evaluation on a 3.12 host); - new subprocess probe pins the warning: requested-but-absent telemetry must emit the cli-agent-orchestrator[otel] hint on stderr. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0123WW42FsCBRwQrK2uwF27s
…ce step; vite orphan fix Reconciliation plan P2-A steps 5-6 — the config separation from the Kiro remediation composed with this branch's self-managed live spec: - default `npm run test:e2e` is hermetic again (testIgnore on the live spec); the live path runs via `npm run test:e2e:live` / playwright.live.config.ts (video on, PLAYWRIGHT_CHROMIUM_EXECUTABLE hook for CDN-blocked sandboxes); - the spec spawns node_modules/.bin/vite directly — killing the npm wrapper orphaned the real vite child, which then squatted :4173 for the next run (observed empirically during the evaluation); same rationale the spec already applied to .venv/bin/cao-server; - after the server-restart ?since= replay step, a reload-persistence step (from the Kiro spec): a full page reload tears the client down and the IndexedDB-persisted instance auto-activates back to the live stream — the across-session property, complementary to the in-session gap recovery. The offline/online variant stays out for cause: Chromium does not sever an established SSE on setOffline (Kiro CI, 660e3f4); - CI runs replay and live as separate steps and converts/uploads both recordings (the live one under the agui-live-remediation-demo name it regenerates). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0123WW42FsCBRwQrK2uwF27s
Reconciliation plan P2-A step 7 — one demo layer with the strengths of both remediations: - showcase.sh: SSE-tail structure with mktemp/trap hygiene (Kiro) + CAO_TOKEN bearer support and strict per-emit expected-code asserts (Claude) + a stronger PASS gate: the six GENERATIVE_UI frames must actually arrive on the live stream (previously display-only on one side and unchecked on the other). Payloads are renderer-true. - run.sh: self-contained server start with health gate, optional tmux-guarded mock_cli fleet with graceful degrade (Kiro) + venv-aware cao-server resolution, exported CAO_API_PORT so the spawned server and the printed URLs always agree, server-log cleanup, and an opt-in CAO_AGUI_RUN_SHOWCASE=1 auto-showcase (Claude's flow, now opt-in). - README: documents both scripts and their flags; the committed webm is referenced as a link (GitHub does not render .webm via image syntax) with a note that CI regenerates it. Verified live: showcase PASS — 6x200 + iframe 400 + 6 GENERATIVE_UI frames off the real SSE stream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0123WW42FsCBRwQrK2uwF27s
Reconciliation plan P2-A step 8:
- ?since= endpoint suite gains the forwarded-value capture from the Kiro
suite (main.py must pass the query param through verbatim) and the
equal-timestamp boundary case NEITHER side covered — an event exactly
AT the cursor is excluded (strictly-greater-than contract).
- enablement matrix gains the truthy spellings ('1' from the Kiro
matrix, plus 'yes'/'TRUE'), still asserted at the route level.
- log-redaction suite gains the token-in-msg-AND-args case neither side
covered (a one-branch regression would leak through the other path).
- e2e: malformed-bearer WS handshake now asserted to close 4401 through
a real server (both remediations stopped at the helper-returns-None
layer; only the no-token case had an e2e). 5 passed locally with
'-m e2e'.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0123WW42FsCBRwQrK2uwF27s
Reconciliation plan P2-A steps 9-10: - docs/reviews/reconciliation-notes.md replaces the single-sided implementation notes: the ported-change -> source -> commit map for this branch, the retraction of the false 'props verified against the renderers' claim (now made true and spec-enforced), and the known latent conftest trap repaired by the sibling A2A branch. - pr387-agui-response-draft-v2.md: fixes the one falsehood (no [a2a] extra exists or is planned — authlib -> dev group, python-multipart removed); adds the 429/RESOURCE_EXHAUSTED store-full semantics and the live-proof paragraph (server-restart ?since= replay, CI-regenerated recording, offer to drop the committed binary); adds the reply to review 4638092590 (task-id idempotent-create fix + from_dict KeyError fix land in PR-B; authlib->dev in PR-A). R1-R6 pushbacks unchanged. - docs/pwa.md names both scrubbed query params (access_token + ticket); CHANGELOG generative-UI bullet links the precise #generative-ui anchor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0123WW42FsCBRwQrK2uwF27s
…re live-run outputs The live-config split writes to test-results-live/ and playwright-report-live/ — ignored like their default-run counterparts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0123WW42FsCBRwQrK2uwF27s
plauzy
pushed a commit
that referenced
this pull request
Jul 7, 2026
…12,#17,#19,#20) Add PR #19/#20 (Claude's executed reconciliations) to the branch inventory, add a dedicated Section 2.1 mandatory-audit guarantee with a per-PR checklist (full-diff read, claims-vs-code verification, real gate run, wins/regressions/gaps, review 4638092590 + media verification), reframe the reconciliation approach so #19/#20 are the strongest existing baseline Kiro must audit rather than regenerate, extend the audit matrix and /cao-eval rubric (new claims-vs-code fidelity criterion) to score #19/#20, and add hard constraints to audit all five and not modify #19/#20.
plauzy
pushed a commit
that referenced
this pull request
Jul 7, 2026
…erify-and-author mission - Add docs/reviews/pr387-kiro-handoff.md: authoritative, self-contained kickoff prompt making Kiro the finishing author (verify #19/#20, fix on reconcile/*, author final upstream submission). - Realign Section 0 of the orchestration plan to the reframed mission: independently verify the already-existing reconcile branches, fix directly on them (no new variants / no fresh reconciliation), author the final PR bodies + upstream reply, then retarget upstream with maintainer approval. - Point Section 0 at the handoff as the operative document; note plan #18 is superseded by #19/#20 and retained as reference. Sections 1-11, media gate, and Definition of Done left intact.
PR #19 body covers: AG-UI typed-event stream, generative UI, standalone PWA, mock_cli provider, opt-in OTel, WS auth hardening, example fixes. Gate results: 3519 passed / mypy 132 files / PWA 24 tests / lint clean. PR #20 body covers: A2A JSON-RPC transport, signed Agent Card, per-method auth enforcement, bounded store with 429+RESOURCE_EXHAUSTED, idempotent- create, Task.from_dict fix, real-JWT test matrix. Gate results: 3590 passed / mypy 142 files / lint clean. Both drafts credit the two source implementations neutrally and mark unverified claims (Playwright/Chromium) explicitly.
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
3 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.
Why (answer before "what")
What outcome are we trying to change?
Two independent remediations of the upstream feat: AG-UI protocol adapter + agentic protocol surface & generative UI (#386) awslabs/cli-agent-orchestrator#387 review existed for the AG-UI core (PRs remediate PR #387 review — AG-UI core (PR-A shape, Claude implementation) #11/[DRAFT][Kiro] remediate PR #387 review — AG-UI core + live demo (PR-A shape) #14) with adjudicated strengths on each side. This branch is the reconciled PR-A: the approved plan (#17,
docs/reviews/pr387-reconciliation-plan.md) executed — one branch carrying the objectively better implementation per finding.What would have to be true for that outcome to look different than today?
Every adjudicated win ported without regressing the base's verified properties: default-off at its strongest (A2A modules absent, test-asserted), metadata-only redaction, the PWA
?since=cursor fix, and the live-path proof.What is the smallest change that makes those things true?
Start from
claude/pr387-agui-core(ahead on more axes: cursor fix + restart-replay proof + test breadth + retained suites) and port the Kiro implementation's five adjudicated wins as six focused commits — no rewrites of anything either side already got right.How will we know it worked?
All gates green (below), the live spec passing with stronger assertions than either original (renderer-true props now pinned), and the previously-false "props verified against the renderers" claim now both true and spec-enforced.
Summary of changes (on top of the
claude/pr387-agui-corehistory)agui-authorregistered inSHIPPED_SKILLS; the pre-existing packaging-parity suite now guards its two trees (sync_skills.py --check→ 10 in sync; the suite'sparity[agui-author]references-subdir skip is expected).diff_summarytitles viatitle(the renderer never readssummary) andprogress.valueon the 0.0–1.0 scale (the renderer clamps to [0,1]) across the skill, showcase, and live spec — with new live-spec assertions on the diff heading text andaria-valuenow, so drift fails the spec instead of hiding behind HTTP 200. The earlier "props verified against the actual renderers" claim is retracted indocs/reviews/reconciliation-notes.mdand made true here.[otel]; telemetry requested without the extra now logs the actionable install hint (never a silent no-op);otel.pyguards the SDK imports for transitive-api installs; the subprocess blocker usesfind_spec(the legacy finder was an executed 2-test failure on Python 3.12 — confirmed by the parallel evaluation in docs: PR #387 Kiro×Claude evaluation & reconciliation plan (review before code) #15) plus a new probe pinning the warning.testIgnore+test:e2e:live/playwright.live.config.ts, Kiro's separation with this branch's self-managed spec +PLAYWRIGHT_CHROMIUM_EXECUTABLEhook); the server-restart?since=replay step is followed by Kiro's reload-persistence step (IndexedDB auto-activation); vite is spawned wrapper-free (the npm wrapper orphaned the real child, which squatted :4173 — observed empirically); offline/online stays out for cause (Chromium doesn't sever an established SSE onsetOffline— Kiro CI,660e3f4).CAO_TOKENsupport and strict per-emit asserts, with a stronger PASS gate (the sixGENERATIVE_UIframes must arrive on the live stream); run.sh = Kiro's self-contained graceful version + exportedCAO_API_PORT, venv-aware binary, log cleanup, opt-in auto-showcase; README documents both (webm referenced as a link — GitHub won't render.webmimage syntax).?since=capture + truthy-1spelling (Kiro's unique cases) + the equal-timestamp boundary, msg+args redaction, and malformed-token WS4401e2e cases neither side had.pwa.mdnames both scrubbed params; CHANGELOG links the precise#generative-uianchor; the paste-ready upstream reply (docs/reviews/pr387-agui-response-draft-v2.md) fixes its one falsehood (no[a2a]extra — authlib→dev, multipart removed), adds the 429 semantics, the live-proof paragraph, and the reply to review 4638092590.Everything else — the A2A subtraction at its strongest test-asserted form, the
api.tscursor fix, token-parse 401/4401 on the real parse path, access-log redaction, example-profile validation,test_oauth_prm.pyretention, the Copilot five — is the base history, unchanged.Test plan
uv run pytest; the 22nd skip isparity[agui-author]references-subdir, expected)mypystrict passes (uv run mypy src/→ no issues, 132 files);black+isortcleancao_pwa:npx tsc --noEmitclean, 24 vitest green,vite buildcleannpm run test:e2e:live→ 1 passed (six components with renderer-true assertions, iframe refusal, server-restart?since=replay, reload persistence; video recorded);showcase.sh→ PASS with the 6-frame stream gate;pytest -m e2e test/e2e/test_websocket_auth.py→ 5 passed incl. the new malformed-token 4401uv buildwheel shipsskills/agui-author/;sync_skills.py --check→ 10 skills in sync/a2aroutes, AG-UI 404s with no flagsProvider tier
🤖 Generated with Claude Code
https://claude.ai/code/session_0123WW42FsCBRwQrK2uwF27s
Generated by Claude Code