feat: add table for session inbound events - #805
heerambavi1998 wants to merge 9 commits into
Conversation
🦋 Changeset detectedLatest commit: 27a53ad The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
bf84084 to
78506cf
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 4 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6e5ccea. Configure here.
| export async function down(db: Kysely<unknown>): Promise<void> { | ||
| await sql`DROP INDEX IF EXISTS session_inbound_events_unconsumed_idx`.execute(db); | ||
| await sql`DROP TABLE IF EXISTS session_inbound_events`.execute(db); | ||
| } |
There was a problem hiding this comment.
SQLite migration missing transaction wrap
Medium Severity
The new SQLite session_inbound_events up/down run DDL without db.transaction(). Kysely does not wrap SQLite migrations, so a failure between CREATE TABLE and CREATE INDEX (or the matching drops) leaves a half-applied schema that a retry cannot complete.
Triggered by project rule: @truefoundry/trueforge review rules
Reviewed by Cursor Bugbot for commit 6e5ccea. Configure here.
| } | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
Inbound insert races completed turns
Medium Severity
insertSessionInboundEvents checks that the tip is running with a plain SELECT, then inserts in a later statement with no lock or transaction. A concurrent freeze can complete the turn in between, so a terminal tip still receives inbox rows instead of TurnNotRunningError.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6e5ccea. Configure here.
| this.session_id = session_id; | ||
| this.event_id = event_id; | ||
| } | ||
| } |
There was a problem hiding this comment.
Error constructor takes two strings
Low Severity
SessionInboundEventAlreadyExistsError takes positional session_id and event_id strings. Callers can swap the two identifiers, and the thrown event_id used by contract tests would then be wrong.
Triggered by project rule: TrueForge review rules
Reviewed by Cursor Bugbot for commit 6e5ccea. Configure here.
1 participant
Summary
add table for session inbound events
Changes
durable session_inbound_events inbox + ISessionStore API (Postgres / SQLite / InMemory) for the upcoming session send-event path.
Table: session_inbound_events
PK: (session_id, event_id) — caller-minted monotonic ULID (same contract as session_event)
turn_id column is nullable (reserved for future session-scoped policies)
No idempotency_key, we will add this later if needed.
Event
SendTurnEventItem = user.tool_approval | user.tool_response. We will add approval policies later.
Store
insertSessionInboundEvents / listUnconsumedSessionInboundEvents / markSessionInboundEventsConsumed
Duplicate event_id → SessionInboundEventAlreadyExistsError
Cascades with deleteSession
How was this tested?
Store contract tests.
Checklist
pnpm build,pnpm test,pnpm typecheck,pnpm lint:ci, andpnpm format:checkpass locallypackages/trueforge-sdk,python/trueforge_sdk,.github/fern/openapi/openapi.json,docs/openapi.json) — fork PRs omit SDK regen; maintainers regenerate after merge.env.exampleupdated if configuration or behavior changedNote
Medium Risk
Adds a new persisted table and store contract on the session/turn path (migrations plus HITL inbox semantics); behavior is well tested but will underpin future send-event handling.
Overview
Adds a durable session inbound send-event inbox (separate from the turn stream log) to support upcoming client→harness tip HITL flows such as tool approvals and tool responses.
ISessionStoregainsinsertSessionInboundEvents,listUnconsumedSessionInboundEvents(ordered byevent_id, optionalturn_idfilter), andmarkSessionInboundEventsConsumed. Inserts require a running turn, reject duplicate caller-mintedevent_idvalues viaSessionInboundEventAlreadyExistsError, and v1 payloads are typed asSessionInboundEventItem(user.tool_approval|user.tool_response).Postgres and SQLite add a
session_inbound_eventstable (PK(session_id, event_id), nullableturn_id,consumedflag, partial index on unconsumed rows) with cascade delete on session removal. InMemory, Postgres, and SQLite stores implement the contract; shared helpers handle batch duplicate detection and PK collision mapping.Store contract tests cover insert/list/mark, filtering, terminal-turn rejection, failed-batch atomicity, and delete cascade. No HTTP send-event route in this PR—storage and API contract only.
Reviewed by Cursor Bugbot for commit 27a53ad. Bugbot is set up for automated code reviews on this repo. Configure here.