Skip to content

fix(copilot): guard legacy event data shapes#236

Open
ozymandiashh wants to merge 1 commit into
getagentseal:mainfrom
ozymandiashh:fix/copilot-legacy-event-types
Open

fix(copilot): guard legacy event data shapes#236
ozymandiashh wants to merge 1 commit into
getagentseal:mainfrom
ozymandiashh:fix/copilot-legacy-event-types

Conversation

@ozymandiashh
Copy link
Copy Markdown
Contributor

Summary

origin/main currently fails npx tsc --noEmit in src/providers/copilot.ts after the legacy Copilot parser gained a catch-all event variant:

| { type: string; timestamp?: string; data: Record<string, unknown> }

That catch-all is useful for newer/unknown Copilot events, but it also widens the discriminated union. TypeScript can no longer prove that event.data has content, outputTokens, messageId, or toolRequests inside the user.message / assistant.message branches, so the parser trips strict type errors.

This PR keeps the catch-all behavior and adds small local field readers for legacy event data instead of destructuring from an unsafe union shape.

What changed

  • Add typed helpers for legacy event fields:
    • legacyStringField()
    • legacyNumberField()
    • legacyToolRequests()
  • Use those helpers in parseLegacyEvents() for model IDs, user content, assistant IDs, token counts, and tool requests.
  • Keep valid legacy event behavior the same: same model, dedup key, token counts, mapped tools, timestamps, user message, and session ID.
  • Make malformed/newer event payloads safer: non-string messages become '', non-number outputTokens become 0, and malformed tool request entries are ignored.
  • Add a regression test covering generic/malformed legacy event shapes while preserving a valid assistant message that carries data.model directly.

Why this shape

The parser already needs to tolerate unknown Copilot event types. Removing the catch-all would make the type error disappear, but it would also weaken the intent of the previous Copilot parsing change. Reading the few required fields from Record<string, unknown> keeps the parser tolerant while satisfying strict TypeScript.

Validation

  • npx tsc --noEmit
  • npx vitest run tests/providers/copilot.test.ts
  • npx vitest run
  • npm run build

@AgentSeal AgentSeal added needs-testing needs-validation PR requires validation against real-world usage before review and removed needs-testing labels May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-validation PR requires validation against real-world usage before review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants