fix: correct bash array syntax, exception handling, and spec JSON examples#1657
fix: correct bash array syntax, exception handling, and spec JSON examples#1657wishhyt wants to merge 3 commits intoa2aproject:mainfrom
Conversation
The comma after "llms.txt" in the FILES_TO_DEPLOY array caused bash to interpret the element as the literal string "llms.txt," (with trailing comma), so the real llms.txt was never found and silently skipped during gh-pages deployment. Made-with: Cursor
proto_to_table only caught FileNotFoundError, while the sibling macros proto_enum_to_table and proto_service_to_table both catch Exception. A proto parse error would propagate uncaught and crash the entire MkDocs build instead of rendering a graceful in-page error message. Made-with: Cursor
…cation examples - §4.6.1: remove trailing comma in AgentCard JSON example (invalid JSON) - §6.6: rename `pushNotificationConfig` to `taskPushNotificationConfig` to match proto field `SendMessageConfiguration.task_push_notification_config` - §6.6: change `"schemes": ["Bearer"]` to `"scheme": "Bearer"` to match proto `AuthenticationInfo.scheme` (singular string, not array) - §6.6: change `"state": "submitted"` to `"TASK_STATE_SUBMITTED"` to match the SCREAMING_SNAKE_CASE enum convention required by §5.5 and used consistently in all other examples - §6.7: add missing comma after `"raw"` value and remove trailing comma after `"mediaType"` (invalid JSON) Made-with: Cursor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the reliability of documentation deployment, improves the robustness of the MkDocs build process, and ensures the accuracy of JSON examples within the project's specification. These changes collectively contribute to a more stable and correct development and documentation environment. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several important fixes that significantly enhance the robustness and accuracy of the project. The corrections to the bash array syntax in deploy_root_files.sh resolve a silent deployment failure, ensuring all intended files are correctly handled. The broadened exception handling in .mkdocs/macros.py improves the stability of the documentation build process by preventing crashes from proto parse errors. Furthermore, the numerous adjustments to JSON examples in docs/specification.md address critical syntax errors and field mismatches, ensuring the documentation accurately reflects the A2A protocol definitions and adheres to ProtoJSON conventions. These changes collectively contribute to a more reliable system and clearer, more accurate documentation.
|
|
||
| # --- Configuration --- | ||
| # List of files to copy from the source directory to the root of the gh-pages branch. | ||
| FILES_TO_DEPLOY=("404.html" "robots.txt" "llms.txt", "llms-full.txt") |
There was a problem hiding this comment.
The trailing comma after "llms.txt" in the FILES_TO_DEPLOY bash array caused llms.txt to be interpreted as llms.txt, (with the trailing comma), leading to silent failures where the file was not correctly deployed. Bash array elements should be space-separated, not comma-separated.
| { | ||
| "url": "https://research-agent.example.com/a2a/v1", | ||
| "protocolBinding": "HTTP+JSON", | ||
| "protocolVersion": "0.3", |
There was a problem hiding this comment.
This line contained a trailing comma, which is invalid JSON syntax. It's important for documentation examples to be syntactically correct to avoid misleading users.
| "messageId": "6dbc13b5-bd57-4c2b-b503-24e381b6c8d6" | ||
| }, | ||
| "configuration": { | ||
| "pushNotificationConfig": { |
There was a problem hiding this comment.
The field name pushNotificationConfig here did not match the proto definition SendMessageConfiguration.task_push_notification_config. Consistency with proto definitions is crucial for accurate specification.
| "url": "https://client.example.com/webhook/a2a-notifications", | ||
| "token": "secure-client-token-for-task-aaa", | ||
| "authentication": { | ||
| "schemes": ["Bearer"] |
There was a problem hiding this comment.
The authentication object incorrectly used an array for schemes (["Bearer"]) instead of a singular string scheme ("Bearer"), which is required by the AuthenticationInfo.scheme proto definition. This was a type mismatch in the example.
| "id": "43667960-d455-4453-b0cf-1bae4955270d", | ||
| "contextId": "c295ea44-7543-4f78-b524-7a38915ad6e4", | ||
| "status": { | ||
| "state": "submitted", |
There was a problem hiding this comment.
The enum value "submitted" did not adhere to the SCREAMING_SNAKE_CASE convention required by the ProtoJSON specification (Section 5.5) and general rules for enum values in the A2A protocol. This could lead to parsing issues or inconsistencies.
References
- Enum values in the A2A protocol should be in
SCREAMING_SNAKE_CASEto comply with the ProtoJSON specification.
| "text": "Analyze this image and highlight any faces." | ||
| }, | ||
| { | ||
| "raw": "iVBORw0KGgoAAAANSUhEUgAAAAUA..." |
There was a problem hiding this comment.
A comma was missing after the "raw" field, which resulted in invalid JSON syntax. This is a common JSON parsing error.
| "raw": "iVBORw0KGgoAAAANSUhEUgAAAAUA..." | ||
| "raw": "iVBORw0KGgoAAAANSUhEUgAAAAUA...", | ||
| "filename": "input_image.png", | ||
| "mediaType": "image/png", |
There was a problem hiding this comment.
This line contained a trailing comma, which is invalid JSON syntax. It's important for documentation examples to be syntactically correct to avoid misleading users.
| try: | ||
| elements = _parse_proto(proto_file) | ||
| except FileNotFoundError as e: | ||
| except Exception as e: |
There was a problem hiding this comment.
Catching a generic Exception can sometimes mask other, more specific issues that might warrant different handling. While this change aligns with sibling macros as noted in the PR description, consider if a more targeted exception type (or a union of specific exceptions) could be used here to provide more precise error handling and debugging capabilities.
1 participant
Summary
deploy_root_files.shthat silently preventedllms.txtfrom being deployed to gh-pagesproto_to_tableMkDocs macro to match sibling macros and prevent build crashes on proto parse errorsDetails
scripts/deploy_root_files.shThe comma after
"llms.txt",in theFILES_TO_DEPLOYbash array caused the element to be interpreted as the literal stringllms.txt,(with trailing comma), so the realllms.txtwas never found and silently skipped during gh-pages deployment..mkdocs/macros.pyproto_to_tableonly caughtFileNotFoundError, while the sibling macrosproto_enum_to_tableandproto_service_to_tableboth catchException. A proto parse error would propagate uncaught and crash the entire MkDocs build instead of rendering a graceful in-page error message.docs/specification.mdpushNotificationConfigtotaskPushNotificationConfigto match proto fieldSendMessageConfiguration.task_push_notification_config"schemes": ["Bearer"]to"scheme": "Bearer"to match protoAuthenticationInfo.scheme(singular string, not array)"state": "submitted"to"TASK_STATE_SUBMITTED"to match the SCREAMING_SNAKE_CASE enum convention required by §5.5"raw"value and remove trailing comma after"mediaType"(invalid JSON)Test plan
scripts/deploy_root_files.shcorrectly lists all 4 files without trailing commaspecification.mdare valid JSON and match proto definitionsCloses #1656