fix: close remaining protocol gaps (metadata, status, lifecycle)#108
fix: close remaining protocol gaps (metadata, status, lifecycle)#108
Conversation
Summary of ChangesHello @teng-lin, 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 significantly enhances the unified message protocol by closing several identified gaps related to metadata consistency, status reporting, authentication flow, and session lifecycle events. It standardizes how various backend adapters communicate these critical states, improving the reliability and predictability of the system. The changes ensure that the frontend receives more consistent and actionable information, while also clarifying which previously 'dropped' events are now intentionally handled or categorized for future consideration. Highlights
Changelog
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. 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 is an excellent pull request that successfully closes several protocol gaps across metadata normalization, status inference, authentication flow, and session lifecycle events. The changes are well-implemented and consistently applied across the OpenCode, Codex, and ACP adapters. I particularly appreciate the new tests that cover the new logic, such as the turnRunningEmitted flag in the ACP session and the new lifecycle events. The documentation updates are also clear and comprehensive. The code quality is high, and I have no concerns with merging these changes.
- Add canonical `error` key to OpenCode error metadata for consumer mapper - Emit `auth_status` before `provider_auth` errors in OpenCode (mirrors ACP) - Add `status: "running"` to OpenCode busy status for router compatibility - Remove unused `done: true` metadata from Codex adapter (3 locations) - Add `turnRunningEmitted` flag to ACP session for status_change(running) - Define `SessionLifecycleSubtype` type for canonical lifecycle events - Map OpenCode session.created/deleted/message.part.removed to session_lifecycle - Emit session_lifecycle(session_created) on Codex thread/started - Document all 12 remaining drops with categories and future work items
teng-lin
force-pushed
the
fix/protocol-gaps-v3
branch
from
3972832 to
27f5f12
Compare
…le changes E2e tests now expect: - ACP/Gemini: status_change(running) before first agent_message_chunk - OpenCode: session.created/session.deleted emit session_lifecycle - OpenCode: message.part.removed emits session_lifecycle
1 participant
Summary
Closes the remaining protocol gaps identified in the unified message protocol audit. Builds on #105 (close-protocol-gaps-v2).
Metadata normalization (ISSUE 2):
errorkey to OpenCode error metadata so the consumer mapper'smapResultMessagefallback worksdone: truemetadata from Codex adapter (3 locations) — no other adapter or consumer uses this fieldStatus inference (ISSUE 3):
status: "running"to OpenCodebusy→status_changemapping so the router recognizes itturnRunningEmittedflag to ACP session — emitsstatus_change(running)on firstagent_message_chunkper turn, reset on completion/error/new promptAuth flow:
auth_statusbeforeprovider_autherrors in OpenCode session (mirrors ACP pattern atacp-session.ts:376)Session lifecycle model:
SessionLifecycleSubtypetype (session_created | session_deleted | session_compacted | message_removed | message_part_removed)session.created,session.deleted,message.part.removed→session_lifecyclemessagessession_lifecycle(session_created)on Codexthread/startedDocumentation:
Test plan
npx tsc --noEmit— zero type errorsnpx vitest run— 2454 tests passing across 158 test filesturnRunningEmitted(3 tests), OpenCodeauth_statusemission, OpenCode lifecycle events (3 tests + info shape), Codexthread/startedlifecyclestatus_change(running), Codex e2e removesdoneassertionsGenerated with Claude Code