fix(a2a): PR #43 blockers - unsafe router path, audit, SDK#205
fix(a2a): PR #43 blockers - unsafe router path, audit, SDK#205dgarson merged 1 commit intoa2a-protocolfrom
Conversation
…, SDK - Router: Add structural guard before unsafe dereference when validator not provided - Router: Add audit logging for validation_failed (was missing, creating blind spots) - SDK: Throw explicit error when sendFn is unset (was silently returning message) These are the 3 critical blockers from David's architecture review.
49f3d72 to
b606228
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49f3d7272b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const result: RouteResult = { | ||
| status: "validation_failed", | ||
| error: "Invalid input: must be a non-null object", | ||
| }; | ||
| return result; |
There was a problem hiding this comment.
Audit structural validation failures in no-validator mode
When validate is not configured, malformed inputs that fail the new structural checks return validation_failed immediately without calling auditResult, so these routing attempts are invisible to audit consumers even though AuditFn is documented as running after every attempt. This creates an observability gap specifically for deployments relying on no-validator mode, and it is inconsistent with the newly added audited validation-failure path above.
Useful? React with 👍 / 👎.
Summary
Addresses the 3 critical blockers from David's architecture review on PR #43.
Changes
Router unsafe no-validator path (router.ts):
Router inconsistent audit behavior (router.ts):
SDK silently drops messages (sdk.ts):
Testing
These are surgical fixes to existing tested code paths. The router and SDK already have test coverage.
Relation to PR #43
This is the "minimal split" - a mergeable PR with the first safe chunk. Once merged, the remaining reliability gaps (circuit breaker collision, validator ISO 8601, audit corruption tolerance) can be addressed in follow-ups or as part of the main PR.
Target: merge into a2a-protocol branch, then PR #43 can proceed.