feat(a2a): A2A facade session and plan lifecycle operations functional via CLI #1041

Merged
freemo merged 4 commits from feature/m6-a2a-facade-cli into master 2026-03-22 05:40:51 +00:00
Owner
No description provided.
freemo added this to the v3.5.0 milestone 2026-03-18 03:43:44 +00:00
freemo added the
Type
Feature
label 2026-03-18 03:44:06 +00:00
freemo force-pushed feature/m6-a2a-facade-cli from 99fd845426 to 36547fd3fa 2026-03-18 16:17:11 +00:00 Compare
freemo force-pushed feature/m6-a2a-facade-cli from 36547fd3fa to 816f725ed4 2026-03-18 18:23:13 +00:00 Compare
Author
Owner

Code Review — PR #1041 feat(a2a): A2A facade session and plan lifecycle operations

(Note: Cannot submit formal review approval since this PR is authored by the same account. Leaving review as a comment instead.)

The facade integration is a useful step toward A2A protocol compliance. The best-effort/fire-and-forget approach is reasonable for first-pass wiring. 7 Behave scenarios provide decent coverage.

Process Issues

  1. Empty PR body — Per CONTRIBUTING.md, every PR needs a description, issue reference, and dependency link. Please add a proper body.

  2. No linked issue — This should reference a tracking issue with a closing keyword.

Code Issues

  1. Inconsistent error handling patternsplan.py uses bare except Exception: pass (completely silent failure), while session.py uses contextlib.suppress(Exception) (same effect but more Pythonic). These should be unified — prefer contextlib.suppress.

  2. _build_facade() suppresses all exceptionscontextlib.suppress(Exception) around each service lookup means any DI wiring bug will be silently swallowed. Consider narrowing to specific expected exceptions or logging at debug level.

  3. Naming inconsistencysession.py has _facade_dispatch() (raises on error) while plan.py has _notify_facade() (swallows everything). Two different names and behaviors for the same concept. Consider unifying.

  4. Inline importsfrom cleveragents.a2a.cli_bootstrap import get_facade at call-time. If deliberate to avoid circular deps, add a comment explaining why.

## Code Review — PR #1041 `feat(a2a): A2A facade session and plan lifecycle operations` *(Note: Cannot submit formal review approval since this PR is authored by the same account. Leaving review as a comment instead.)* The facade integration is a useful step toward A2A protocol compliance. The best-effort/fire-and-forget approach is reasonable for first-pass wiring. 7 Behave scenarios provide decent coverage. ### Process Issues 1. **Empty PR body** — Per CONTRIBUTING.md, every PR needs a description, issue reference, and dependency link. Please add a proper body. 2. **No linked issue** — This should reference a tracking issue with a closing keyword. ### Code Issues 3. **Inconsistent error handling patterns** — `plan.py` uses bare `except Exception: pass` (completely silent failure), while `session.py` uses `contextlib.suppress(Exception)` (same effect but more Pythonic). These should be unified — prefer `contextlib.suppress`. 4. **`_build_facade()` suppresses all exceptions** — `contextlib.suppress(Exception)` around each service lookup means any DI wiring bug will be silently swallowed. Consider narrowing to specific expected exceptions or logging at debug level. 5. **Naming inconsistency** — `session.py` has `_facade_dispatch()` (raises on error) while `plan.py` has `_notify_facade()` (swallows everything). Two different names and behaviors for the same concept. Consider unifying. 6. **Inline imports** — `from cleveragents.a2a.cli_bootstrap import get_facade` at call-time. If deliberate to avoid circular deps, add a comment explaining why.
freemo requested review from CoreRasurae 2026-03-19 05:16:48 +00:00
freemo requested review from brent.edwards 2026-03-19 05:16:48 +00:00
freemo force-pushed feature/m6-a2a-facade-cli from 816f725ed4 to 24aad463a1 2026-03-22 01:54:35 +00:00 Compare
freemo scheduled this pull request to auto merge when all checks succeed 2026-03-22 03:10:30 +00:00
freemo added 1 commit 2026-03-22 03:42:28 +00:00
fix(test): update facade operation count from 11 to 42
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 44s
CI / lint (pull_request) Successful in 3m19s
CI / unit_tests (pull_request) Failing after 3m42s
CI / quality (pull_request) Successful in 3m48s
CI / typecheck (pull_request) Successful in 4m55s
CI / security (pull_request) Successful in 5m6s
CI / docker (pull_request) Has been skipped
CI / e2e_tests (pull_request) Successful in 5m9s
CI / integration_tests (pull_request) Successful in 6m52s
CI / coverage (pull_request) Failing after 19m27s
CI / benchmark-regression (pull_request) Successful in 48m13s
CI / status-check (pull_request) Failing after 1s
0c301ac581
The A2A facade now exposes 42 operations (31 extension + 11 legacy)
after the spec-aligned _cleveragents/ extension methods were added.
Update the BDD assertion and docs to match the actual count.
freemo added 1 commit 2026-03-22 05:01:14 +00:00
fix(test): guard result.stderr access against ValueError
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 3m20s
CI / typecheck (pull_request) Successful in 3m56s
CI / build (pull_request) Successful in 37s
CI / security (pull_request) Successful in 4m2s
CI / quality (pull_request) Successful in 4m8s
CI / unit_tests (pull_request) Failing after 4m13s
CI / docker (pull_request) Has been skipped
CI / integration_tests (pull_request) Successful in 7m10s
CI / e2e_tests (pull_request) Successful in 9m34s
CI / coverage (pull_request) Successful in 11m1s
CI / status-check (pull_request) Failing after 1s
CI / benchmark-regression (pull_request) Has been cancelled
4f2aa4189c
Click/Typer CliRunner.Result.stderr is a property that raises
ValueError when stderr was not separately captured (mix_stderr=True
is the default).  Wrap all result.stderr accesses in try/except
to handle this gracefully.
freemo added 1 commit 2026-03-22 05:25:52 +00:00
fix(a2a): suppress stdout/stderr in facade bootstrap to prevent test pollution
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 41s
CI / lint (pull_request) Successful in 3m19s
CI / quality (pull_request) Successful in 3m44s
CI / typecheck (pull_request) Successful in 3m56s
CI / security (pull_request) Successful in 4m5s
CI / e2e_tests (pull_request) Successful in 8m33s
CI / integration_tests (pull_request) Successful in 8m37s
CI / unit_tests (pull_request) Successful in 8m42s
CI / docker (pull_request) Successful in 1m12s
CI / coverage (pull_request) Successful in 10m58s
CI / status-check (pull_request) Successful in 1s
CI / lint (push) Successful in 3m28s
CI / build (push) Successful in 14s
CI / typecheck (push) Successful in 3m52s
CI / benchmark-regression (push) Has been skipped
CI / security (push) Successful in 4m0s
CI / quality (push) Successful in 4m5s
CI / integration_tests (push) Successful in 6m56s
CI / unit_tests (push) Successful in 7m15s
CI / docker (push) Successful in 1m12s
CI / e2e_tests (push) Successful in 8m36s
CI / coverage (push) Successful in 11m35s
CI / status-check (push) Successful in 2s
CI / benchmark-publish (push) Successful in 28m44s
CI / benchmark-regression (pull_request) Successful in 51m20s
a2113deace
The _notify_facade and _facade_dispatch functions call get_container()
during lazy facade construction. This can trigger structlog output that
corrupts CLI stdout captured by CliRunner in tests.

Wrap facade construction in redirect_stdout/redirect_stderr to
suppress any side-effect output. Also reset the facade singleton
in after_scenario for test isolation.
freemo merged commit a2113deace into master 2026-03-22 05:40:51 +00:00
freemo deleted branch feature/m6-a2a-facade-cli 2026-03-22 05:40:51 +00:00
Sign in to join this conversation.
No Label
Type
Feature
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: cleveragents/cleveragents-core#1041