Skip to content

ADE-56: Guard agentChatGetEventHistory IPC handler against null agentChatService in runtime-backed mode#431

Merged
arul28 merged 1 commit into
mainfrom
ade-56-guard-agentchatgeteventhistory-ipc-handler-against-null-agentchatservice-in-runtime-backed-mode
May 30, 2026
Merged

ADE-56: Guard agentChatGetEventHistory IPC handler against null agentChatService in runtime-backed mode#431
arul28 merged 1 commit into
mainfrom
ade-56-guard-agentchatgeteventhistory-ipc-handler-against-null-agentchatservice-in-runtime-backed-mode

Conversation

@arul28
Copy link
Copy Markdown
Owner

@arul28 arul28 commented May 30, 2026

Fixes ADE-56

Summary

Describe the change.

What Changed

Key files and behaviors.

Validation

How you tested.

Risks

Anything to watch.

Linked Linear issues

ADE   Open in ADE  ·  ade-56-guard-agentchatgeteventhistory-ipc-handler-against-null-agentchatservice-in-runtime-backed-mode branch  ·  PR #431

Summary by CodeRabbit

  • Bug Fixes

    • More robust chat event history handling: when the chat service or its history method is unavailable, the app now returns a safe empty history snapshot instead of failing; requests also normalize session IDs and only forward valid maxEvents.
  • Tests

    • Added unit tests covering missing service, missing history support, and successful history retrieval scenarios.

Review Change Stack

Greptile Summary

Guards the agentChatGetEventHistory IPC handler against a null agentChatService, which occurs in runtime-backed mode where the service isn't instantiated. Instead of calling ensureAgentChatContext() (which throws), the handler now calls getCtx() directly and returns an empty history snapshot when the service is absent or lacks the method.

  • registerIpc.ts: Replaces ensureAgentChatContext() with getCtx() + a two-condition guard (!service || typeof service.getChatEventHistory !== "function"), consistent with the existing pattern in agentChatReadTranscript.
  • runtimeBridge.test.ts: Adds tests for null service, service missing the method (duck-type branch), and the successful forwarding path — full branch coverage for the new guard.

Confidence Score: 5/5

Safe to merge — the change adds a well-tested null guard that prevents a crash in runtime-backed mode and is consistent with the existing pattern used by the adjacent agentChatReadTranscript handler.

The guard logic is straightforward, the change is isolated to a single handler, all three new code paths have direct unit-test coverage, and the implementation mirrors an existing pattern in the same file.

No files require special attention.

Important Files Changed

Filename Overview
apps/desktop/src/main/services/ipc/registerIpc.ts Switches agentChatGetEventHistory from ensureAgentChatContext() (throws on null) to getCtx() + explicit null/duck-type guard, returning an empty snapshot instead of throwing when agentChatService is unavailable.
apps/desktop/src/main/services/ipc/runtimeBridge.test.ts Adds three unit tests covering: null service, service without getChatEventHistory (duck-type branch), and happy path — fully covering all new guard branches.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[IPC: agentChatGetEventHistory] --> B[getCtx]
    B --> C{sessionId valid?}
    C -- No --> D[return empty snapshot\nsessionId: '']
    C -- Yes --> E{service present AND\ngetChatEventHistory is function?}
    E -- No --> F[return empty snapshot\nsessionId: trimmed]
    E -- Yes --> G{maxEvents valid\nfinite positive number?}
    G -- No --> H[maxEvents = undefined]
    G -- Yes --> I[maxEvents = rawMaxEvents]
    H --> J[service.getChatEventHistory\nsessionId, undefined]
    I --> K[service.getChatEventHistory\nsessionId, maxEvents]
Loading

Comments Outside Diff (1)

  1. apps/desktop/src/main/services/ipc/registerIpc.ts, line 671 (link)

    P2 AppContext type doesn't reflect nullable agentChatService

    main.ts explicitly sets agentChatService: null for the runtime-backed context (line 4304), so the field is null at runtime in that mode. However, AppContext still declares it as ReturnType<typeof createAgentChatService> — non-optional, non-nullable. As a result, TypeScript won't flag any future call sites that access the service without a null guard, meaning the pattern this PR protects against can silently recur. Updating the type to ReturnType<typeof createAgentChatService> | null (matching the treatment of other nullable services like devToolsService) would make the type system enforce the guard everywhere.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: apps/desktop/src/main/services/ipc/registerIpc.ts
    Line: 671
    
    Comment:
    **`AppContext` type doesn't reflect nullable `agentChatService`**
    
    `main.ts` explicitly sets `agentChatService: null` for the runtime-backed context (line 4304), so the field is null at runtime in that mode. However, `AppContext` still declares it as `ReturnType<typeof createAgentChatService>` — non-optional, non-nullable. As a result, TypeScript won't flag any future call sites that access the service without a null guard, meaning the pattern this PR protects against can silently recur. Updating the type to `ReturnType<typeof createAgentChatService> | null` (matching the treatment of other nullable services like `devToolsService`) would make the type system enforce the guard everywhere.
    
    How can I resolve this? If you propose a fix, please make it concise.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

    Fix in Claude Code

Reviews (4): Last reviewed commit: "fix: guard chat event history IPC servic..." | Re-trigger Greptile

@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 30, 2026

ADE-56

@vercel
Copy link
Copy Markdown

vercel Bot commented May 30, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
ade Ignored Ignored Preview May 30, 2026 7:56am

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 30, 2026

📝 Walkthrough

Walkthrough

The IPC handler for agentChatEventHistory now verifies agentChatService and its getChatEventHistory method before calling them, returning an empty event snapshot when missing. Tests added cover missing-service, missing-method, and successful-forwarding scenarios.

Changes

Defensive IPC Handler for Agent Chat Event History

Layer / File(s) Summary
Handler: service presence and method guard
apps/desktop/src/main/services/ipc/registerIpc.ts
The IPC handler assigns ctx.agentChatService to a local service variable and early-returns an empty event history (events: [], truncated: false, sessionFound: false) when the service or its getChatEventHistory method is missing. When available, it invokes service.getChatEventHistory with sanitized sessionId and maxEvents only if valid.
Tests: null/missing method and forwarding
apps/desktop/src/main/services/ipc/runtimeBridge.test.ts
Three Vitest cases: (1) agentChatService is null -> empty normalized history, (2) service present but no getChatEventHistory -> empty normalized history, (3) service with getChatEventHistory -> forwards trimmed sessionId and maxEvents, returns snapshot and asserts call args.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Suggested labels

desktop

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title clearly and specifically describes the main change: guarding the agentChatGetEventHistory IPC handler against null agentChatService in runtime-backed mode, which is the core focus of the changeset.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ade-56-guard-agentchatgeteventhistory-ipc-handler-against-null-agentchatservice-in-runtime-backed-mode

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 30, 2026

@copilot review but do not make fixes

@capy-ai
Copy link
Copy Markdown

capy-ai Bot commented May 30, 2026

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

Comment thread apps/desktop/src/main/services/ipc/runtimeBridge.test.ts
@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 30, 2026

Rebased on latest main and addressed Greptile review by adding coverage for a truthy agentChatService without getChatEventHistory. Re-ran the focused IPC test and desktop typecheck; both pass.

@arul28 arul28 force-pushed the ade-56-guard-agentchatgeteventhistory-ipc-handler-against-null-agentchatservice-in-runtime-backed-mode branch from 1c62034 to 5cba84c Compare May 30, 2026 07:12
@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 30, 2026

Rebased again on latest main after ADE-51 and #442 landed, resolving the test conflict by preserving both agentChatList and agentChatGetEventHistory IPC regression coverage. Re-ran focused IPC test and desktop typecheck successfully before force-pushing 5cba84c.

@arul28 arul28 force-pushed the ade-56-guard-agentchatgeteventhistory-ipc-handler-against-null-agentchatservice-in-runtime-backed-mode branch from 5cba84c to 9725264 Compare May 30, 2026 07:51
@arul28 arul28 merged commit c2b8b34 into main May 30, 2026
28 checks passed
@arul28 arul28 deleted the ade-56-guard-agentchatgeteventhistory-ipc-handler-against-null-agentchatservice-in-runtime-backed-mode branch May 30, 2026 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant