Skip to content

fix: generate UUID for request confirmation events#559

Open
code2gold wants to merge 1 commit intogoogle:mainfrom
code2gold:fix/request-confirmation-event-id
Open

fix: generate UUID for request confirmation events#559
code2gold wants to merge 1 commit intogoogle:mainfrom
code2gold:fix/request-confirmation-event-id

Conversation

@code2gold
Copy link
Contributor

Fixes #558

Summary

generateRequestConfirmationEvent() creates session.Event structs without setting the ID field, resulting in empty string IDs. When persisted to a database backend, the composite primary key (ID, AppName, UserID, SessionID) causes a duplicate key violation on the second HITL confirmation event within the same session.

Fix: Add ID: uuid.NewString() to the Event struct initialization, consistent with session.NewEvent().

Changes

  • internal/llminternal/functions.go: Add ID: uuid.NewString() and uuid import
  • internal/llminternal/base_flow_test.go: Add test verifying generated events have non-empty, unique IDs

Testing Plan

Unit test added:

  • TestGenerateRequestConfirmationEvent_SetsEventID — verifies the generated event has a non-empty ID, and calling twice produces unique IDs

Manual E2E verification:

  • Configured ADK with database.NewSessionService (PostgreSQL via GORM)
  • Triggered multiple HITL confirmation flows within the same session
  • Before fix: second confirmation fails with duplicate key value violates unique constraint "events_pkey"
  • After fix: all confirmation events persist successfully with unique UUIDs
$ go test ./internal/llminternal/ -run TestGenerateRequestConfirmationEvent -v
=== RUN   TestGenerateRequestConfirmationEvent_SetsEventID
--- PASS: TestGenerateRequestConfirmationEvent_SetsEventID (0.00s)
PASS

generateRequestConfirmationEvent() creates Event structs without
setting the ID field, resulting in empty string IDs. When persisted
to a database with a composite primary key that includes ID, the
second confirmation event causes a duplicate key violation.

Set ID via uuid.NewString() to match session.NewEvent() behavior.
@gemini-code-assist
Copy link
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly addresses a bug where request confirmation events were generated without a UUID, leading to database primary key violations. The fix is simple and effective, adding uuid.NewString() to the event creation. The inclusion of a new unit test, TestGenerateRequestConfirmationEvent_SetsEventID, is excellent; it thoroughly verifies that the event ID is set and that subsequent calls produce unique IDs, preventing future regressions. The changes are well-contained and the pull request description is clear and comprehensive. Overall, this is a high-quality contribution.

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.

generateRequestConfirmationEvent creates events with empty ID, causing DB primary key violations

1 participant