Skip to content

fix: Make WebSocket event ingest idempotent#28

Merged
shahtajkhalid merged 1 commit into
secureagentics:mainfrom
Adarsh9977:fix/idempotent-ws-event-ingest
Jun 1, 2026
Merged

fix: Make WebSocket event ingest idempotent#28
shahtajkhalid merged 1 commit into
secureagentics:mainfrom
Adarsh9977:fix/idempotent-ws-event-ingest

Conversation

@Adarsh9977
Copy link
Copy Markdown
Contributor

Summary

  • Ignore duplicate event_id inserts during event ingest so SDK retry frames do not close the WebSocket.
  • Add a WebSocket retry regression test that sends the same paired event twice and verifies the connection still receives verdicts.

Test plan

  • go test ./internal/ws

Checklist

  • CLA signed (see CLA.md)
  • Tests pass locally
  • Docs updated where needed
  • British English; no em-dashes; no marketing fluff

@shahtajkhalid
Copy link
Copy Markdown
Contributor

Thanks @Adarsh9977 for the clean fix, merging it.

One follow-up: the path is idempotent for the events row but not end to end. On a retry the classifier re-runs and writes a second row for the same event_id, so replays quietly duplicate verdicts and repeat inference. If you want to take it, have InsertEvent report whether it actually inserted (RowsAffected() == 0 means duplicate), and on a duplicate skip classify plus the verdict insert and re-publish the existing verdict. Same tests should pass.

Also, while you're at it: ON CONFLICT (id) DO NOTHING is more precise than INSERT OR IGNORE, which also swallows NOT NULL and FK violations rather than just the duplicate key.

Merging now, follow-up is welcome but not required.

@shahtajkhalid shahtajkhalid merged commit 9bf76a8 into secureagentics:main Jun 1, 2026
@Adarsh9977
Copy link
Copy Markdown
Contributor Author

Thanks @Adarsh9977 for the clean fix, merging it.

One follow-up: the path is idempotent for the events row but not end to end. On a retry the classifier re-runs and writes a second row for the same event_id, so replays quietly duplicate verdicts and repeat inference. If you want to take it, have InsertEvent report whether it actually inserted (RowsAffected() == 0 means duplicate), and on a duplicate skip classify plus the verdict insert and re-publish the existing verdict. Same tests should pass.

Also, while you're at it: ON CONFLICT (id) DO NOTHING is more precise than INSERT OR IGNORE, which also swallows NOT NULL and FK violations rather than just the duplicate key.

Merging now, follow-up is welcome but not required.

Thanks @shahtajkhalid, I'll update it and open a PR shortly

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.

2 participants