feat: add canonical bot invocation source revisions - #1817
feat: add canonical bot invocation source revisions#1817kristofferremback wants to merge 6 commits into
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesThe PR adds message revision tracking and migration backfills. It introduces canonical bot-invocation route resolution, revision-aware reconciliation, transactional claim validation, stale-source cancellation, and public API guards. Bot-owned sessions are excluded from message mutation cascades. Invocation revision and storage
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/backend/src/db/migrations/20260808073545_bot_invocation_source_revisions.sql`:
- Around line 43-48: Retain the unconditional unique constraint in this
migration; remove the DROP CONSTRAINT statement while preserving the active-row
unique index creation. Defer removing
bot_invocations_workspace_id_source_message_id_actor_type_a_key until all
existing replicas no longer write using the unconditional ON CONFLICT target.
In `@apps/backend/src/features/bot-runtimes/invocation-outbox-handler.test.ts`:
- Around line 315-325: Remove the duplicate test around processMessageCreated,
or revise it to explicitly configure createdPayload and its reconciliation
inputs to represent canonical source deletion. Keep only a test whose setup
distinguishes the deletion scenario from the preceding live-source no-op test,
while preserving coverage through the existing deletion-route test if no
distinct setup is needed.
In `@apps/backend/src/features/bot-runtimes/invocation-outbox-handler.ts`:
- Around line 113-124: Rename processMessageCreated to a name that reflects
handling both created and edited message events, and update every call site or
dispatch reference to use the new method name.
In `@apps/backend/src/features/bot-runtimes/invocation-route-resolver.ts`:
- Around line 128-159: Refactor the active-scratchpad route construction in the
resolver so the shared fields are defined once and reused by both outcomes.
Colocate the variant values for targetInstanceId, targetRuntimeSessionId, and
missingLinkNotice, using nulls when missingLinkNotice is absent and the existing
link-derived values otherwise. Preserve the current return behavior for
missingLinkNotice and routes.push behavior for the normal path.
In `@apps/backend/src/features/bot-runtimes/repository.ts`:
- Around line 960-975: Update the cancellation predicate in
reconcileInvocationSource so rows at the current source message revision can be
cancelled when they are no longer in the desired routes. Change the strict
source_message_revision comparison in the UPDATE query to allow equality, while
preserving the existing workspace, message, status, and NOT EXISTS route
filters.
In `@apps/backend/src/features/public-api/handlers.ts`:
- Around line 1353-1361: Update the claimBotInvocation flow to track when
currentClaim is missing or has a different claimedSourceMessageRevision, then
return data: null after the transaction instead of continuing with claim
details. Preserve the existing audit behavior by setting res.locals.auditSkip
before the early response, allowing the runtime to poll again without starting a
session for a stale claim.
In `@apps/backend/tests/integration/bot-invocation-source-mutations.test.ts`:
- Around line 78-88: Replace the Date.now()-based sequence in the source helper
with a module-scoped deterministic counter, incrementing it for each source()
call so every inserted message receives a unique stream sequence across tests.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 013f1ab0-e1dd-4e5a-951f-7c685a8b67c2
📒 Files selected for processing (38)
apps/backend/evals/suites/boundary-extraction/suite.tsapps/backend/evals/suites/memorizer/suite.tsapps/backend/src/db/migrations/20260808073545_bot_invocation_source_revisions.sqlapps/backend/src/features/agents/conversation-summary-service.test.tsapps/backend/src/features/agents/episode-summary-service.test.tsapps/backend/src/features/agents/message-mutation-outbox-handler.test.tsapps/backend/src/features/agents/message-mutation-outbox-handler.tsapps/backend/src/features/agents/quote-resolver.test.tsapps/backend/src/features/agents/reflective-capture-service.test.tsapps/backend/src/features/agents/session-digest.test.tsapps/backend/src/features/agents/thread-anchor-context.tsapps/backend/src/features/bot-runtimes/invocation-outbox-handler.test.tsapps/backend/src/features/bot-runtimes/invocation-outbox-handler.tsapps/backend/src/features/bot-runtimes/invocation-route-resolver.tsapps/backend/src/features/bot-runtimes/repository.test.tsapps/backend/src/features/bot-runtimes/repository.tsapps/backend/src/features/bot-runtimes/service.test.tsapps/backend/src/features/bot-runtimes/service.tsapps/backend/src/features/conversations/boundary-extraction/llm-extractor.test.tsapps/backend/src/features/labels/label-message-service.test.tsapps/backend/src/features/messaging/index.tsapps/backend/src/features/messaging/repository.tsapps/backend/src/features/messaging/steered-message-service.tsapps/backend/src/features/messaging/version-repository.tsapps/backend/src/features/public-api/claim-context.test.tsapps/backend/src/features/public-api/complete-invocation-floor.test.tsapps/backend/src/features/public-api/e2e-stream-gate.test.tsapps/backend/src/features/public-api/handlers.tsapps/backend/src/features/public-api/sealed-claim.test.tsapps/backend/src/features/public-api/sealed-complete.test.tsapps/backend/src/features/saved-messages/service.test.tsapps/backend/src/lib/ai/message-formatter.test.tsapps/backend/src/lib/outbox/payload-parsers.test.tsapps/backend/tests/integration/bot-invocation-claim-scope.test.tsapps/backend/tests/integration/bot-invocation-source-mutations.test.tsapps/backend/tests/integration/bot-invocation-source-revisions-migration.test.tspackages/prosemirror/src/extractors.tspackages/prosemirror/src/index.ts
| ALTER TABLE bot_invocations | ||
| DROP CONSTRAINT bot_invocations_workspace_id_source_message_id_actor_type_a_key; | ||
|
|
||
| CREATE UNIQUE INDEX idx_bot_invocations_active_source_actor_trigger | ||
| ON bot_invocations (workspace_id, source_message_id, actor_type, actor_id, trigger) | ||
| WHERE status IN ('pending', 'claimed'); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the dropped constraint name and the ON CONFLICT inference target for bot_invocations.
set -euo pipefail
# 1. Find the migration that created the original unique constraint.
rg -n -C 4 'bot_invocations' --glob '*.sql' | rg -n -C 4 -i 'UNIQUE|CONSTRAINT' || true
# 2. Show the exact dropped name usage.
rg -n 'bot_invocations_workspace_id_source_message_id_actor_type_a_key' --glob '*.sql'
# 3. Inspect insertIdempotent and every ON CONFLICT target on bot_invocations.
fd -t f 'repository.ts' apps/backend/src/features/bot-runtimes --exec rg -n -C 12 'insertIdempotent|ON CONFLICT' {}Repository: threahq/threa
Length of output: 151
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- candidate migration files ---'
git ls-files 'apps/backend/src/db/migrations/*bot*' '*migration*' | head -200
printf '%s\n' '--- bot_invocations schema and constraint definitions ---'
rg -n -C 8 'CREATE TABLE.*bot_invocations|ALTER TABLE bot_invocations|UNIQUE.*bot_invocations|CONSTRAINT.*bot_invocations|source_message_id|insertIdempotent|ON CONFLICT' \
apps/backend --glob '*.sql' --glob '*.ts' --glob '*.tsx' | head -500
printf '%s\n' '--- repository files near bot runtime feature ---'
git ls-files apps/backend/src | rg 'bot|invocation|repository' | head -300Repository: threahq/threa
Length of output: 14901
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- migration directory inventory ---'
git ls-files apps/backend/src/db/migrations | tail -100
printf '%s\n' '--- all references to the original constraint and index ---'
rg -n -C 6 'bot_invocations_workspace_id_source_message_id_actor_type_a_key|idx_bot_invocations_active_source_actor_trigger' . \
--glob '!node_modules/**' --glob '!dist/**' --glob '!build/**'
printf '%s\n' '--- all bot invocation conflict clauses and insertIdempotent definitions ---'
rg -n -C 15 'insertIdempotent|ON CONFLICT|bot_invocations' apps/backend/src \
--glob '*.ts' --glob '*.sql' | head -800Repository: threahq/threa
Length of output: 50369
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- original bot invocation migration ---'
cat -n apps/backend/src/db/migrations/20260516190000_bot_runtime_invocations.sql
printf '%s\n' '--- revision migration ---'
cat -n apps/backend/src/db/migrations/20260808073545_bot_invocation_source_revisions.sql
printf '%s\n' '--- bot invocation source files ---'
git ls-files apps/backend/src | rg -i 'bot.*(invocation|runtime)|invocation.*(bot|repository)|repository.*bot'
printf '%s\n' '--- insertIdempotent and conflict clauses in bot-related TypeScript files ---'
while IFS= read -r file; do
rg -n -C 18 'insertIdempotent|ON CONFLICT|bot_invocations' "$file" || true
done < <(git ls-files apps/backend/src | rg -i 'bot.*(invocation|runtime)|invocation.*(bot|repository)|repository.*bot')Repository: threahq/threa
Length of output: 8155
🏁 Script executed:
set -euo pipefail
repo=apps/backend/src/features/bot-runtimes/repository.ts
printf '%s\n' '--- current repository implementation ---'
rg -n -C 25 'insertIdempotent|ON CONFLICT|bot_invocations' "$repo"
printf '%s\n' '--- repository outline ---'
ast-grep outline "$repo" | head -120
printf '%s\n' '--- recent file history ---'
git log -8 --oneline -- "$repo" apps/backend/src/db/migrations/20260808073545_bot_invocation_source_revisions.sql
printf '%s\n' '--- previous revision of repository implementation ---'
git show HEAD^:"$repo" 2>/dev/null | rg -n -C 25 'insertIdempotent|ON CONFLICT|bot_invocations' || true
printf '%s\n' '--- integration migration setup and assertions ---'
cat -n apps/backend/tests/integration/bot-invocation-source-revisions-migration.test.ts | sed -n '1,220p'Repository: threahq/threa
Length of output: 50369
🏁 Script executed:
set -euo pipefail
repo=apps/backend/src/features/bot-runtimes/repository.ts
old=589db96d
printf '%s\n' '--- current conflict clause only ---'
sed -n '1048,1070p' "$repo"
printf '%s\n' '--- previous conflict clause only ---'
git show "$old:$repo" | sed -n '930,1010p'
printf '%s\n' '--- exact generated-name length ---'
python3 - <<'PY'
name = "bot_invocations_workspace_id_source_message_id_actor_type_a_key"
print(len(name), name[:63], len(name[:63]))
PY
printf '%s\n' '--- migration integration test conflict behavior ---'
rg -n -C 12 'ON CONFLICT|constraint|index|pending|claimed|cancelled|completed' \
apps/backend/tests/integration/bot-invocation-source-revisions-migration.test.tsRepository: threahq/threa
Length of output: 10148
🏁 Script executed:
set -euo pipefail
repo=apps/backend/src/features/bot-runtimes/repository.ts
old=589db96d
printf '%s\n' '--- previous insertIdempotent location and conflict clause ---'
git show "$old:$repo" | rg -n -C 30 'insertIdempotent|ON CONFLICT \(workspace_id, source_message_id, actor_type, actor_id, trigger\)' | head -180
printf '%s\n' '--- commits containing the repository transition ---'
git log --format='%H %s' --all -- "$repo" | head -12
printf '%s\n' '--- current and previous occurrences of the exact conflict target ---'
printf '%s\n' 'current:'
rg -n 'ON CONFLICT \(workspace_id, source_message_id, actor_type, actor_id, trigger\)' "$repo"
printf '%s\n' 'previous:'
git show "$old:$repo" | rg -n 'ON CONFLICT \(workspace_id, source_message_id, actor_type, actor_id, trigger\)' || trueRepository: threahq/threa
Length of output: 6349
Defer dropping the unconditional unique constraint.
The constraint name is deterministic because the original migration defines it. However, previous replicas use the unconditional ON CONFLICT target. Drop the constraint only after those replicas stop writing.
🧰 Tools
🪛 Squawk (2.61.0)
[warning] 46-48: During normal index creation, table updates are blocked, but reads are still allowed. Use concurrently to avoid blocking writes.
(require-concurrent-index-creation)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/backend/src/db/migrations/20260808073545_bot_invocation_source_revisions.sql`
around lines 43 - 48, Retain the unconditional unique constraint in this
migration; remove the DROP CONSTRAINT statement while preserving the active-row
unique index creation. Defer removing
bot_invocations_workspace_id_source_message_id_actor_type_a_key until all
existing replicas no longer write using the unconditional ON CONFLICT target.
165bae3 to
23e6618
Compare
- version message mutations and reconcile bot routes from locked source state - fence claims, failures, parking, and completions against stale input - add migration, lifecycle isolation, and deterministic race coverage 🤖 Generated with [Codex](https://github.com/openai/codex) Co-authored-by: codex <codex@users.noreply.github.com>
- count historical deletion in canonical message revisions - close migration-cancelled running sessions through lifecycle outbox repair 🤖 Generated with [Codex](https://github.com/openai/codex) Co-authored-by: codex <codex@users.noreply.github.com>
- allow full migration setup to complete under concurrent integration files - keep teardown safe when setup fails before assigning the pool 🤖 Generated with [Codex](https://github.com/openai/codex) Co-authored-by: codex <codex@users.noreply.github.com>
- consume old-replica session-start events after the startup repair scan - target repair by workspace and invocation-backed session identity 🤖 Generated with [Codex](https://github.com/openai/codex) Co-authored-by: codex <codex@users.noreply.github.com>
- preserve embedded session controls while reconciling message edits - skip bot-backed sessions when selecting persona rerun owners - reject claims that become stale before session creation 🤖 Generated with [Codex](https://github.com/openai/codex) Co-authored-by: codex <codex@users.noreply.github.com>
- page past multiple newer bot sessions on trigger edits - select the latest persona session for referenced-context edits 🤖 Generated with [Codex](https://github.com/openai/codex) Co-authored-by: codex <codex@users.noreply.github.com>
923317a to
16c7c9d
Compare
Problem
Remote bot invocations cache
prompt_markdownwhen a source message is first routed. An edit or deletion can therefore race the outbox and claim paths, hand a runtime stale work, or let a stale completion persist a reply. Bot-backedagent_sessionsalso entered persona mutation handling, which could rerun or delete output under the wrong lifecycle owner.Solution
Make the current message row authoritative at every work boundary:
messages.revisionand source/claimed revision fields onbot_invocations; backfill existing rows and replace the global source/actor uniqueness constraint with active-route uniqueness.content_json, then reuse that resolver for ordered reconciliation and claim validation.This is the backend safety floor only. Durable
input_updated/cancelledruntime events and capability negotiation are the next stack layer.Files
apps/backend/src/db/migrations/20260808073545_bot_invocation_source_revisions.sqlapps/backend/src/features/bot-runtimes/invocation-route-resolver.tsapps/backend/src/features/bot-runtimes/repository.tsapps/backend/src/features/bot-runtimes/service.tsapps/backend/src/features/bot-runtimes/invocation-outbox-handler.tsapps/backend/src/features/messaging/{repository,version-repository}.tsapps/backend/src/features/public-api/handlers.tsapps/backend/src/features/agents/message-mutation-outbox-handler.tspackages/prosemirror/src/extractors.tsapps/backend/tests/integration/bot-invocation-*.test.tsTest plan
@threa/prosemirrortypecheck and tests — 154 passed, 211 assertions.git diff --check.📋 Full implementation plan
Goal
Support source-message edits and deletions across remote runtimes without trusting creation-time prompt snapshots. The stack establishes canonical backend state first, adds a generic durable control protocol second, then teaches the shared runtime client and adapters to consume it.
Plan reference: https://seer.build/ws_vbyzjvdg6g/b/invocation-source-mutations-plan-4a104e/
Stack
onInputUpdated/onCancelledcallbacks and bootstrap recovery.What Was Built
Canonical message revisions
messages.revisionstarts at 1, increments atomically on each content edit, and increments once on soft deletion.message_versions.bot_invocations.source_message_revisionrecords the latest reconciled source revision.claimed_source_message_revisionpins the exact input accepted by a runtime claim.Canonical route reconciliation
resolveCanonicalInvocationRoutesreads resolved mention IDs fromcontent_json; markdown slugs remain display metadata only (INV-35/58/64).Claim and terminal fences
Lifecycle ownership
agent_sessionsare recognized by workspace-scoped invocation identity, not ID-prefix inference.deletedand emits the existing lifecycle event/outbox atomically; startup repair covers migration-cancelled rows, and old-replicaagent_session:startedevents trigger targeted repair during rolling deploys.superseded; already-posted bot messages are retained.Plaintext and sealed safety
Verification
Design Decisions
Claim as the processing boundary
Chose: Pending work may be rewritten/cancelled; claimed work is pinned and every terminal write is revision-fenced.
Why: Outbox delivery is asynchronous, so event order alone cannot prove the runtime received current input.
Canonical row over mutation-event payload
Chose: Resolve from the current locked message row.
Why: A delayed revision-N event can arrive after revision N+1 is already canonical. Stamping N routing with N+1 prompt state is unsafe.
Source then actor advisory locks
Chose: One source lock followed by sorted actor locks before invocation rows.
Why: Actor-only locking allowed a route-changing reconciliation to hold an old invocation row while completion held the actor lock, creating a deadlock cycle.
No automatic terminal reruns
Chose: Completed/failed/parked/expired same-actor work suppresses edit redispatch; cancelled predecessors are replaceable and newly added actors remain eligible.
Why: Re-running a completed remote turn can repeat filesystem, network, or PR-creation side effects.
Design Evolution
Schema Changes
20260808073545_bot_invocation_source_revisions.sql:messages.revision INTEGER NOT NULL DEFAULT 1and backfills from version counts.source_message_revision,claimed_source_message_revision,claimed_input_update_mode,cancellation_reason, andavailable_attobot_invocations.(workspace, source, actor type, actor id, trigger)routes.What's NOT Included
bot_invocation:input_updatedorbot_invocation:cancelledwire events yet.Status
🤖 PR by Codex
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.