fix(protocols): relay heartbeat keepalives from the direct encoders

[lidge-jun]

Summary

dev at 76db92a4cd fails tests/responses/protocol-direct-encoders-chat.test.ts > "stall watchdog fails the turn like the bridge". Each parent passed CI on its own head; the defect is their union:

• fix(chat): relay translated heartbeats #5806 made the Responses-to-Chat converter answer each typed heartbeat with the SSE comment : opencodex heartbeat (ADR-5805).
• feat(protocols): protocol paths as a first-class concern — PF-01..PF-12 #5820 added the direct Chat encoder (PF-09), whose writer mapped heartbeat to ensureRole only, so on wire silence it sent no byte after the role frame.

The parity test's frame normalizer then parsed the bridge's comment block as JSON and threw. Every PR based on current dev inherits the failure (#5835, #5837, #5838, #5826, #5836 fail only on this test).

This change:

• makes the direct Chat encoder emit the same comment through the sink's keepalive channel after the role frame, guarded by termination (src/protocols/encoders/chat.ts);
• stops counting keepalives as relayed events in the shared driver, matching the bridge, whose heartbeat bypasses the relay counter (src/protocols/encoders/adapter-events.ts); the Messages encoder's ping keepalive was over-counted the same way;
• teaches both direct-encoder parity tests to compare comment-only blocks and cover heartbeats on both wires;
• notes the keepalives in structure/data-planes/protocol-paths.md and structure/transports/responses.md.

The direct encoders are off by default (protocols.rollout.directEncoders), so only opted-in installs saw the missing keepalive; the red test affected everyone's CI.

Maintainer integration: this is a maintainer PR landed by the owner under the MAINTAINERS.md 2026-09-06 rule once its exact-head checks are green; the exact-head evidence is added as a comment before merge.

Verification

• Red/green: with only the test change, the stall parity test fails on a frame mismatch; with the encoder change, bun test ./tests/responses/protocol-direct-encoders-chat.test.ts ./tests/responses/protocol-direct-encoders-messages.test.ts passes (93 pass, 0 fail).
• bun run typecheck exit 0; bun run structure:check passed; ./tests/test-layout.test.ts and ./tests/ci-workflows/file-size-ratchet.test.ts pass.
• bun run test:changed (import graph against dev) on the same commit; result added below once complete. The full suite is left to CI because this host runs several concurrent worktrees.
• An independent reviewer audited the diff against the legacy converter (src/chat/outbound.ts:615-623): byte order matches, the only activity=false enqueue is emitKeepalive, real-frame accounting is unchanged.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

• Bug Fixes
  • Heartbeat keepalives are relayed without being counted as relayed events.
  • Chat heartbeat output now matches the established SSE comment format and ensures the role frame is sent first.
• Documentation
  • Clarified Chat’s limits for representing certain calls and server-side search activity, and documented keepalive accounting.
• Tests
  • Added lifecycle checks for heartbeat output, relay counts, and first-output notifications across Chat and Messages encoders.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-25T11:33:07.657152Z	ce90509	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 63b2c61f-fa41-4751-a866-4c3d2e13773e

📥 Commits

Reviewing files that changed from the base of the PR and between 76db92a and ce90509.

📒 Files selected for processing (6)

• src/protocols/encoders/adapter-events.ts
• src/protocols/encoders/chat.ts
• structure/data-planes/protocol-paths.md
• structure/transports/responses.md
• tests/responses/protocol-direct-encoders-chat.test.ts
• tests/responses/protocol-direct-encoders-messages.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The direct Chat encoder emits wire-silence heartbeats as SSE comments. Keepalive frames are delivered but excluded from relayed-event counts. Tests compare direct and legacy heartbeat behavior for Chat and Messages.

Changes

Heartbeat emission and accounting

Layer / File(s)	Summary
Emit and account for heartbeat frames src/protocols/encoders/adapter-events.ts (lines 98, 204–206), src/protocols/encoders/chat.ts (lines 8–9, 126–132)	The Chat encoder emits a heartbeat comment when the stream is active. The adapter calls relayed only when a frame’s activity flag is true.
Document and test heartbeat behavior structure/data-planes/protocol-paths.md (lines 229–232), structure/transports/responses.md (line 505), tests/responses/protocol-direct-encoders-chat.test.ts (lines 57–61, 390–436), tests/responses/protocol-direct-encoders-messages.test.ts (lines 76–81, 322–372)	The documentation describes Chat representation limits and keepalive accounting. The tests compare direct and legacy heartbeat output, check relay observations, and verify first-output hook calls.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to ce905

No actionable heartbeat issue remains; the PR is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to ce905

Opt-in direct streams will send keepalives consistently with the existing stream path, without counting those keepalives as relayed events. No security boundary bypass was identified, but the change affects client-visible streaming behavior.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed client-wire behavior is limited to eligible installations that opt into direct encoding; the examined route gates do not extend it to policy, combo, or Responses-wire routes.

Trust Boundaries and Controls

• inferred — Both protocol writers use the shared sink for keepalives. Skipping the relayed callback changes delivery telemetry, while the examined enqueue path continues to enforce its byte-budget reservation.

Resilience and Maintainability Implications

• observed — The heartbeat callback catches writer errors, and the stream's cancellation path stops timers, stops upstream work, and drains queued-byte charges.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 4 files. (2 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely describes the main change: direct encoders now relay heartbeat keepalives. This matches the implementation in the Chat encoder, shared relay accounting, and parity t…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 4 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[lidge-jun]

리뷰 · 우선순위 63 / 80

답이 잠깐 조용할 때, 채팅 줄은 살아 있다는 표시로 : opencodex heartbeat 주석을 보내야 해요. 다리를 거친 옛 변환기는 그 주석을 보냅니다. 직접 채팅 인코더는 역할 프레임만 만들고 주석을 안 보냈어요. 짝 맞추기 테스트가 그 주석을 JSON으로 읽다가 중간에 끊겨서, 지금 dev 위의 여러 PR이 이 테스트 하나에서 실패해요.

이 PR은 직접 채팅 인코더가 역할 프레임 뒤에 같은 주석을 넣게 해요. 그 주석과 메시지 쪽 침묵 ping은 전달된 이벤트 수에 안 넣어요. 테스트는 주석만 있는 블록을 남긴 채 양쪽 줄을 비교하고, 침묵 중 신호가 가는지 확인해요. 직접 인코더는 기본으로 꺼져 있어요. 켠 설치에만 빠졌던 바이트가 채워지고, 깨진 테스트는 모두의 CI에 있었어요.

tests/responses/protocol-direct-encoders-messages.test.ts:325 - 주석은 릴레이 횟수가 핑을 무시해야 한다고 적혀 있어요. 368행의 핑은 3개예요. 369행은 프레임 수에서 2만 빼요. 처음 message_start가 emit으로 보내는 핑은 횟수에 남아요. 366행 주석이 그 뜻을 적고, 325행은 더 넓게 읽혀요.

메인테이너의 판단이 필요한 지점

#5846도 같은 빨간 테스트를 고쳐요. 그 PR은 주석 블록을 비교에서 빼서 테스트를 초록으로 만들어요. 직접 인코더가 주석을 보내게 하는 코드는 #5846에 없어요. 두 PR이 tests/responses/protocol-direct-encoders-chat.test.ts의 normalizeFrames를 같이 고칩니다.

너의 추천

#5847을 머지하세요. #5846은 머지하지 말고 닫으세요. 325행 주석은 message_start의 첫 핑은 세고, 그 뒤 침묵 ping 두 개는 세지 않는다고 고치면 369행과 맞아요. 주석 한 줄 때문에 머지를 늦출 필요는 없어요.

이 댓글은 grok-bot이 작성했습니다

[lidge-jun]

Maintainer integration (MAINTAINERS.md, 2026-09-06 rule) by the owner.

Exact-head evidence for ce9050997d96f2148a3129c2dbc5ddd6e37f1ca8:

• Cross-platform CI run 36129779360 (pull_request): success, all test shards 1/4-4/4, gates, structure gate, keyring, npm-global, docker smoke, desktop shell passed; macOS/Windows/privacy/docs legs skipped by the workflow's path conditions for this diff.
• React Doctor 36129779230, PR hygiene, labeler, enforce-target: success.
• Local on the same commit: focused parity tests 104 pass / 0 fail, bun run typecheck 0, bun run structure:check passed, bun run test:changed 7016 pass / 0 fail.