Skip to content

[fix] Restore queued message editing on web and mobile - #6607

Merged
mmabrouk merged 5 commits into
release/v0.115.2from
fix/release-1152-queued-input-edit
Sep 6, 2026
Merged

[fix] Restore queued message editing on web and mobile#6607
mmabrouk merged 5 commits into
release/v0.115.2from
fix/release-1152-queued-input-edit

Conversation

@mmabrouk

@mmabrouk mmabrouk commented Sep 6, 2026

Copy link
Copy Markdown
Member

Context

Queued messages lost their Edit action when the queue moved to the server. Users could remove a message or send it immediately, but could not correct its text while preserving its place.

Changes

Restore editing on desktop and mobile. Saving updates the existing queued message, preserving its ID, position and existing attachments. New attachments are appended without duplicating them on retry. A message that has already started or been selected for Send Now cannot be edited. Failed saves retain the draft, and Cancel editing remains available if the row leaves the queue.

Remove the Steer and attachment-count text badges. Keep attachment tiles and the Send Now action; after clicking it, the button reads Sending. Save the component examples in the existing tracked Storybook story.

How to review

Start with the pending-input update service and transaction, then the shared queue hook and both composer integrations. The existing admission fingerprint stays unchanged so a retry of the original message cannot recreate or overwrite the edited row.

Tests

The original database suite passed 36 checks. The final attachment correction passes four focused database-backed checks and 74 frontend tests; Ruff, 25 frontend lint tasks, chat/entity type checks and the generated client build pass. The Storybook build includes all 10 queue stories. Live desktop and mobile checks passed text editing, draft restoration, failed-save retry, row identity/order, Send Now and final idle. The final live attachment check passed all 12 assertions: a lost save response and normal retry preserved the row and both files without duplicates; both files were delivered and read by the runner, and desktop/mobile finished idle. CodeRabbit completed review of the final commit, its blank-ID finding is fixed, and it withdrew the earlier migration finding after verifying the committed snapshot boundary. Staging verification remains after merge. This PR adds no migration.

What to QA

  • While a run is active, queue two messages. Edit the first: the text changes in both clients, its place is retained, and its attachments remain.
  • Fail a save, then retry. The edit stays available and creates no duplicate message or attachment.
  • Let the edited message start in another client. Saving is refused and Cancel editing remains reachable.
  • Check desktop and mobile: Edit and Send Now are available, with no Steer or attachment-count text badges.

@vercel

vercel Bot commented Sep 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
agenta-documentation Ready Ready Preview Sep 6, 2026 7:06pm UTC

Request Review

@mmabrouk

mmabrouk commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

🤖 The AI agent says:

This implements the requested Edit restoration and badge removal on both desktop and mobile. Independent API and frontend reviews found no blocking issues. Validation:36 real-database checks,60 focused frontend checks, both host types,25 lint tasks, and10 focused Storybook stories. The generated client diff preserves canonical Fern formatting and adds only the new method/types.

The tracked Storybook source is web/storybook/stories/presentational/QueuedMessagesDock.stories.tsx. Live application QA remains outstanding and is required before merge. This branch will also be synchronized with the reviewed questionnaire follow-up before final release verification.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Team

Run ID: 381c4031-6fcc-4edc-8588-6ff9d02fec77

📥 Commits

Reviewing files that changed from the base of the PR and between 1445ef5 and d68162e.

📒 Files selected for processing (2)
  • api/oss/src/core/sessions/inputs/dtos.py
  • api/oss/tests/pytest/unit/sessions/test_session_inputs_dao.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • api/oss/src/core/sessions/inputs/dtos.py
  • api/oss/tests/pytest/unit/sessions/test_session_inputs_dao.py

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.


📝 Summary

Summary by CodeRabbit

  • New Features

    • Edit pending queued messages, including text and attachments.
    • Edits are saved reliably during retries, conflicts, and queue transitions.
    • Pending messages clearly indicate when they are editable.
    • Attachment references are preserved and resolved for edited messages.
    • Added support for updating pending messages through the sessions API.
  • Bug Fixes

    • Editing remains accessible if a message leaves the queue.
    • Improved cancellation handling for unavailable or no-longer-editable messages.
    • Prevented stale queue data when switching sessions or projects.
    • Improved queue synchronization during editing and capability checks.

Walkthrough

The change adds a PATCH endpoint for editing pending session inputs. It adds transactional content updates, attachment merging, locking, client persistence, durable queue editing, capability resolution, and UI handling for edited rows that leave the queue.

Changes

Pending input editing

Layer / File(s) Summary
Backend update contracts and route
api/oss/src/apis/fastapi/sessions/..., api/oss/src/core/sessions/inputs/...
Adds update payloads, edit-related exceptions, DAO contracts, and the PATCH route with permission checks and HTTP error mapping.
Transactional content editing
api/oss/src/core/sessions/inputs/service.py, api/oss/src/dbs/postgres/sessions/inputs/dao.py
Rewrites supported message formats, merges attachments, locks pending inputs, persists content, and serializes promotion with session locking.
Backend edit validation
api/oss/tests/pytest/unit/sessions/test_session_inputs_dao.py
Tests content preservation, attachment merging, rejected states, locking, scope errors, invalid content, and idempotent edits.
Client update API and server queue persistence
web/packages/agenta-entities/src/session/..., web/packages/agenta-chat/src/hooks/...
Exposes the update API and atom, validates responses, converts attachments, and sends server-backed edits through the queue adapter.
Capability admission and editing states
web/packages/agenta-chat/..., web/oss/..., web/mobile/..., web/storybook/...
Coordinates capability resolution, migration, retries, attachment rendering, editable pending rows, and cancellation when an edited row leaves the queue.

Estimated code review effort: 4 (Complex) | ~60 minutes

Merge Risk: 🟡 Moderate · up to d6816

Queued-message editing may duplicate a message during local-to-server transfer and may report success for an invalid update response, risking unexpected queued content and unreliable retry behavior. These issues should be resolved before merge.

Sequence Diagram(s)

sequenceDiagram
  participant ChatUI
  participant QueueHook
  participant SessionsAPI
  participant SessionInputsService
  participant PostgreSQL
  ChatUI->>QueueHook: Commit server-backed edit
  QueueHook->>SessionsAPI: PATCH pending input
  SessionsAPI->>SessionInputsService: Update input
  SessionInputsService->>PostgreSQL: Lock and persist content
  PostgreSQL-->>SessionInputsService: Updated pending input
  SessionInputsService-->>SessionsAPI: PendingInputResponse
  SessionsAPI-->>QueueHook: Update result
  QueueHook-->>ChatUI: Refresh edited queue state
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 60.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 26 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the primary change: restoring queued message editing on web and mobile.
Description check ✅ Passed The description directly explains queued message editing, attachment preservation, retry behavior, API changes, tests, and QA scope.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/release-1152-queued-input-edit

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Team

Run ID: a4b90ea8-b2a1-4942-a790-2f3646272511

📥 Commits

Reviewing files that changed from the base of the PR and between a12ce06 and bc1c1cd.

⛔ Files ignored due to path filters (5)
  • web/packages/agenta-api-client/src/generated/api/resources/sessions/client/Client.ts is excluded by !**/generated/**
  • web/packages/agenta-api-client/src/generated/api/resources/sessions/client/requests/PendingInputUpdateRequest.ts is excluded by !**/generated/**
  • web/packages/agenta-api-client/src/generated/api/resources/sessions/client/requests/index.ts is excluded by !**/generated/**
  • web/packages/agenta-api-client/src/generated/api/types/PendingInputAttachment.ts is excluded by !**/generated/**
  • web/packages/agenta-api-client/src/generated/api/types/index.ts is excluded by !**/generated/**
📒 Files selected for processing (24)
  • api/oss/src/apis/fastapi/sessions/models.py
  • api/oss/src/apis/fastapi/sessions/router.py
  • api/oss/src/core/sessions/inputs/dtos.py
  • api/oss/src/core/sessions/inputs/interfaces.py
  • api/oss/src/core/sessions/inputs/service.py
  • api/oss/src/core/sessions/inputs/types.py
  • api/oss/src/dbs/postgres/sessions/inputs/dao.py
  • api/oss/tests/pytest/unit/sessions/test_session_inputs_dao.py
  • web/mobile/src/features/chat/LiveConversation.tsx
  • web/oss/src/components/AgentChatSlice/AgentConversation.tsx
  • web/oss/src/components/AgentChatSlice/components/QueuedMessagesDock.tsx
  • web/packages/agenta-chat/src/assets/pendingInputs.ts
  • web/packages/agenta-chat/src/components/QueuedMessagesDock.tsx
  • web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts
  • web/packages/agenta-chat/src/hooks/useAgentConversation.ts
  • web/packages/agenta-chat/src/hooks/useServerSessionInputs.ts
  • web/packages/agenta-chat/tests/unit/QueuedMessagesDock.test.tsx
  • web/packages/agenta-chat/tests/unit/assets/pendingInputs.test.ts
  • web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts
  • web/packages/agenta-chat/tests/unit/hooks/useServerSessionInputs.test.ts
  • web/packages/agenta-entities/src/session/api/api.ts
  • web/packages/agenta-entities/src/session/index.ts
  • web/packages/agenta-entities/src/session/state/pendingInputs.ts
  • web/storybook/stories/presentational/QueuedMessagesDock.stories.tsx

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.

Comment thread web/packages/agenta-chat/src/components/QueuedMessagesDock.tsx Outdated
@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

@mmabrouk I will review the changes. The review will also check the outstanding live application QA and merged-tree verification risks.

⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Railway Preview Environment

Status Destroyed (PR closed)

Updated at 2026-09-06T19:16:06.979Z

…se-1152-queued-input-edit

# Conflicts:
#	web/packages/agenta-chat/src/components/QueuedMessagesDock.tsx
#	web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts
#	web/packages/agenta-chat/src/hooks/useServerSessionInputs.ts
#	web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts
@mmabrouk

mmabrouk commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

🤖 The AI agent says:

Synchronized with the merged questionnaire follow-up in release/v0.115.2. Conflict resolution preserves capability readiness, failed mobile task retention, visible Send Now and the durable Edit path. Both sets of regression tests remain.

Independent merge-resolution review passed. All68 affected shared tests, chat/desktop/mobile types,25 lint tasks and direct test formatting pass. The API implementation is unchanged from the36-check real-database verification.

Review3944748578 is addressed by retaining the one-line visibility comment from the release. The earlier failed web unit job used the old lazy-editor fixture; the merge includes the fixture correction already passing CI in6606.

@coderabbitai review

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
web/packages/agenta-entities/src/session/api/api.ts (1)

221-221: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Validate the update response. !!data accepts truthy invalid payloads as success; use safeParseWithLogging.

Source: Coding guidelines

web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts (1)

354-354: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Preserve identity after migration. Recheck server.queued at commit because a row migrated mid-edit retains server:false and submit() creates a duplicate ID.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Team

Run ID: 6befb45f-6222-4bd6-81fa-da552f172413

📥 Commits

Reviewing files that changed from the base of the PR and between bc1c1cd and ada8a1c.

📒 Files selected for processing (7)
  • web/mobile/src/features/chat/LiveConversation.tsx
  • web/packages/agenta-chat/src/components/QueuedMessagesDock.tsx
  • web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts
  • web/packages/agenta-chat/src/hooks/useServerSessionInputs.ts
  • web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts
  • web/packages/agenta-chat/tests/unit/hooks/useServerSessionInputs.test.ts
  • web/packages/agenta-entities/src/session/api/api.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • web/packages/agenta-chat/src/components/QueuedMessagesDock.tsx

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

@mmabrouk I will review the synchronized changes and verify the merge-resolution paths.

⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@mmabrouk

mmabrouk commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

🤖 The AI agent says:

Addressed both outside-diff findings in review5126136093. Pending-input updates now validate the returned row before accepting a save. An edit keeps ownership of its original message when a held local queue is transferred to the server. It waits for an in-flight transfer, replays the original admission payload after an ambiguous failure, and updates the same ID. Promotion or a delayed snapshot cannot turn the edit into a second message.

Independent review passed. Fifty queue tests and24 entity tests pass, including regressions that failed before these changes. Chat, entities, desktop and mobile type checks,25 lint tasks and direct test formatting pass. No API/database implementation changed, so its existing36-check database verification remains applicable.

Only the changed checks were rerun. Live Edit verification remains outstanding before merge.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

@mmabrouk I will review the updated changes. I will check the queued-edit transfer, retry, and duplicate-prevention paths.

⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@mmabrouk

mmabrouk commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

🤖 The AI agent says:

Updated queued-input editing in 1445ef5ce0 after live QA exposed two ownership and attachment-delivery gaps.

  • An observed server-owned row now drops its stale local copy before an edit, while a failed edit keeps the draft and editing identity available for retry.
  • Queued file edits preserve providerMetadata.agenta.attachmentId through the frontend, Fern request, API DTO, and stored message.
  • The API emits canonical attachmentId and mimeType blocks for durable references. URL-only inputs use image or resource blocks, and retries deduplicate by attachment ID as well as URL.
  • Pending attachment reads restore durable metadata, and the active send action now reads Sending for both queued and steer inputs.

Validation:

  • 74 focused frontend tests passed: pending input reads (6), queue ownership (51), and server input edits (17).
  • 4 focused API attachment-edit tests passed against the local Postgres instance.
  • Ruff format and check passed.
  • Full frontend lint fix passed. It reported only three existing mobile hook warnings.
  • @agenta/chat and @agenta/entities type checks passed.
  • The generated Fern API client build passed.

@coderabbitai review

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Team

Run ID: 1d4e8099-2d81-48b4-bb87-d3cef3dc21b8

📥 Commits

Reviewing files that changed from the base of the PR and between 1d96e28 and 1445ef5.

⛔ Files ignored due to path filters (1)
  • web/packages/agenta-api-client/src/generated/api/types/PendingInputAttachment.ts is excluded by !**/generated/**
📒 Files selected for processing (12)
  • api/oss/src/core/sessions/inputs/dtos.py
  • api/oss/src/core/sessions/inputs/service.py
  • api/oss/tests/pytest/unit/sessions/test_session_inputs_dao.py
  • web/packages/agenta-chat/src/assets/pendingInputs.ts
  • web/packages/agenta-chat/src/components/QueuedMessagesDock.tsx
  • web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts
  • web/packages/agenta-chat/src/hooks/useServerSessionInputs.ts
  • web/packages/agenta-chat/tests/unit/assets/pendingInputs.test.ts
  • web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts
  • web/packages/agenta-chat/tests/unit/hooks/useServerSessionInputs.test.ts
  • web/packages/agenta-entities/src/session/api/api.ts
  • web/packages/agenta-entities/src/session/state/pendingInputs.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.

Comment thread api/oss/src/core/sessions/inputs/dtos.py Outdated
@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Current PR diff summary ==='
git diff --stat HEAD^ HEAD
echo
git diff --name-only HEAD^ HEAD

echo '=== Relevant outlines ==='
for f in \
  web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts \
  web/packages/agenta-chat/src/hooks/useServerSessionInputs.ts \
  web/packages/agenta-chat/src/assets/pendingInputs.ts \
  api/oss/src/core/sessions/inputs/service.py \
  api/oss/src/dbs/postgres/sessions/inputs/dao.py \
  web/packages/agenta-entities/src/session/api/api.ts
do
  if [ -f "$f" ]; then
    echo "--- $f ---"
    ast-grep outline "$f" --items all || true
  fi
done

echo '=== Latest diff for relevant implementation files ==='
git diff HEAD^ HEAD -- \
  web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts \
  web/packages/agenta-chat/src/hooks/useServerSessionInputs.ts \
  web/packages/agenta-chat/src/assets/pendingInputs.ts \
  api/oss/src/core/sessions/inputs/service.py \
  api/oss/src/dbs/postgres/sessions/inputs/dao.py \
  web/packages/agenta-entities/src/session/api/api.ts \
  api/oss/src/apis/fastapi/sessions/router.py \
  api/oss/src/core/sessions/inputs/dtos.py \
  web/packages/agenta-entities/src/session/state/pendingInputs.ts

Length of output: 24228


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Queue edit implementation ==='
sed -n '360,485p' web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts

echo '=== Server-session edit implementation ==='
sed -n '175,225p' web/packages/agenta-chat/src/hooks/useServerSessionInputs.ts

echo '=== Attachment ID helper and URL builder ==='
fd -a '^files\.ts$|^transcriptToMessages\.ts$' web/packages/agenta-chat/src -x sh -c '
  echo "--- $1 ---"
  rg -n -A18 -B4 "attachmentIdForPart|attachmentContentUrl" "$1" || true
' sh {}

echo '=== Pending-input edit service ==='
sed -n '35,155p' api/oss/src/core/sessions/inputs/service.py

echo '=== Generated attachment request type ==='
cat web/packages/agenta-api-client/src/generated/api/types/PendingInputAttachment.ts

echo '=== Focused latest regression tests ==='
rg -n -A65 -B8 \
  'attachment_id|attachmentId|server-owned|failed edit|failed.*edit|editingId|duplicate|ambiguous' \
  web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts \
  web/packages/agenta-chat/tests/unit/assets/pendingInputs.test.ts \
  web/packages/agenta-chat/tests/unit/hooks/useServerSessionInputs.test.ts \
  api/oss/tests/pytest/unit/sessions/test_session_inputs_dao.py

Length of output: 104292


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Migration lifecycle in useAgentChatQueue.ts ==='
rg -n -A60 -B16 'migrationPromiseRef|migrationRef|migrate|server\.submit|server\.queued' \
  web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts

echo '=== Numbered commitEdit region ==='
nl -ba web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts | sed -n '400,450p'

echo '=== Migration regression test setup with line numbers ==='
nl -ba web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts | sed -n '1060,1250p'

Length of output: 15817


@mmabrouk I found one blocking issue.

  • web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts, Line 417 clears migrationPromiseRef.current before Line 419 captures migration.
  • A server snapshot can show the migrated row before server.submit resolves.
  • In this case, serverOwnsInput is true. The code clears the in-flight migration promise. It then calls server.edit without waiting for admission to finish.
  • The PATCH can race the admission request and fail with a not-found or conflict response. The local row has already been removed, so the user must retry after the transfer completes.

Capture the migration object before clearing its refs. Wait for that captured promise before calling server.edit. Clear the refs only after the migration settles.

You are interacting with an AI system.

@mmabrouk

mmabrouk commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

🤖 The AI agent says:

Addressed CodeRabbit's blank attachment ID finding in d68162e06c.

  • attachment_id remains optional, but Pydantic now rejects an explicitly blank value before it can select the canonical durable-reference path.
  • The focused API test asserts blank IDs fail validation. The same test keeps exercising an omitted ID through the URL-only resource fallback.

Validation: Ruff format and check passed. All 4 focused queued-attachment edit tests passed against the local Postgres instance.

@coderabbitai review

@mmabrouk

mmabrouk commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

🤖 The AI agent says:

@coderabbitai The completed review covers d68162e06c, but its risk summary repeats the two findings from review5126136093 that were addressed in 1d96e28f and refined in 1445ef5c:

  • updatePendingSessionInput validates the response with pendingInputResponseSchema and safeParseWithLogging; it no longer accepts !!data.
  • commitEdit retains the original input ID across pending, accepted and ambiguous local-to-server admission. Observed server ownership takes precedence over admission retry. Regression tests cover migration during editing, a delayed snapshot, failed save followed by promotion, and an ambiguous admission with and without an observed server row. All 51 queue tests pass.

The only new actionable finding in review5126306272, blank attachment IDs, is fixed and covered by the four passing API checks in d68162e06c.

Please reconcile the stale risk summary with the current code, or point to a remaining concrete path these fixes do not cover. Final live attachment-delivery QA is still running before merge.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Commit summary and changed files ==='
git show --stat --oneline --decorate --no-renames d68162e06c
git diff d68162e06c^ d68162e06c --name-only

echo '=== Exact commit diff ==='
git diff --no-ext-diff --unified=80 d68162e06c^ d68162e06c -- \
  api/oss/src/core/sessions/inputs/dtos.py \
  api/oss/src/core/sessions/inputs/service.py \
  api/oss/tests/pytest/unit/sessions/test_session_inputs_dao.py

echo '=== DTO and service outlines ==='
ast-grep outline api/oss/src/core/sessions/inputs/dtos.py --items all
ast-grep outline api/oss/src/core/sessions/inputs/service.py --items all

echo '=== DTO, attachment transformation, and relevant tests ==='
nl -ba api/oss/src/core/sessions/inputs/dtos.py | sed -n '1,180p'
rg -n -A100 -B20 'attachment_id|attachmentId|PendingInputAttachment|edit_pending_input_content' \
  api/oss/src/core/sessions/inputs/service.py \
  api/oss/tests/pytest/unit/sessions/test_session_inputs_dao.py

Length of output: 11843


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Pending-input content transformation ==='
sed -n '35,165p' api/oss/src/core/sessions/inputs/service.py

echo '=== API model and endpoint ==='
sed -n '1,100p' api/oss/src/apis/fastapi/sessions/models.py
rg -n -A45 -B15 'update_pending_input|PendingInputUpdateRequest' \
  api/oss/src/apis/fastapi/sessions/router.py

echo '=== All attachment ID conditions and validation ==='
rg -n -i -A8 -B8 'attachment_id|attachmentId' \
  api/oss/src/core/sessions \
  api/oss/src/apis/fastapi \
  api/oss/tests/pytest/unit/sessions/test_session_inputs_dao.py

echo '=== Pydantic version and existing non-whitespace validation patterns ==='
rg -n -A3 -B3 'pydantic' api/oss/pyproject.toml api/oss/requirements*.txt 2>/dev/null || true
rg -n -P -i 'strip.*attachment|attachment.*strip|min_length=1|StringConstraints|constr\(' \
  api/oss/src api/oss/tests/pytest | head -n 240

Length of output: 66140


@mmabrouk I found one remaining blocking validation gap.

  • api/oss/src/core/sessions/inputs/dtos.py, Line 55 rejects "" but accepts whitespace-only values such as " ".
  • api/oss/src/core/sessions/inputs/service.py, Line 130 treats every non-None value as a durable attachment ID.
  • A whitespace-only ID therefore creates an attachment block with attachmentId: " " instead of the URL-only fallback.
  • Reject whitespace-only attachment_id values. Add a regression test for " " and, preferably, other whitespace characters.

You are interacting with an AI system.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Current PR head and relevant changed files ==='
git log -1 --oneline
git diff --name-only HEAD^ HEAD -- \
  web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts \
  web/packages/agenta-entities/src/session/api/api.ts \
  web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts \
  web/packages/agenta-entities/tests/unit/session-continuation-resume-api.test.ts

echo
echo '=== Relevant file outlines ==='
ast-grep outline web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts --items all --match 'commitEdit|migration' || true
ast-grep outline web/packages/agenta-entities/src/session/api/api.ts --items all --match 'updatePendingSessionInput' || true

echo
echo '=== commitEdit and migration lifecycle ==='
rg -n -C 18 \
  'commitEdit|migrationPromiseRef|migrationRef|serverOwnsInput|server\.edit|server\.submit' \
  web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts

echo
echo '=== updatePendingSessionInput implementation and validation imports ==='
rg -n -C 12 \
  'updatePendingSessionInput|pendingInputResponseSchema|safeParseWithLogging' \
  web/packages/agenta-entities/src/session/api/api.ts

echo
echo '=== Focused regression tests ==='
rg -n -C 12 \
  'ambiguous|migration|delayed snapshot|failed edit|server-owned|pendingInputResponseSchema|invalid response|safeParse' \
  web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts \
  web/packages/agenta-entities/tests/unit/session-continuation-resume-api.test.ts

echo
echo '=== Latest diff for the relevant implementations ==='
git diff HEAD^ HEAD -- \
  web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts \
  web/packages/agenta-entities/src/session/api/api.ts

Length of output: 48282


@mmabrouk The response-validation finding is resolved.

updatePendingSessionInput now validates its result with safeParseWithLogging(pendingInputResponseSchema, data, "[updatePendingSessionInput]"). It does not use !!data.

The migration finding remains in the current code.

In web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts:

  1. A local-to-server server.submit migration starts.
  2. The server snapshot exposes the row before server.submit settles.
  3. commitEdit detects serverOwnsInput.
  4. Line 417 clears migrationPromiseRef.current.
  5. Line 419 then reads migrationPromiseRef.current, which is now null.
  6. commitEdit calls server.edit immediately instead of waiting for the admission promise.

The PATCH can therefore reach the server before the admission request completes. The PATCH can fail with a not-found or conflict response.

The required ordering is still to capture the matching migration object first, await or retry that object, and clear its references only after it settles. The delayed-snapshot and ambiguous-failure tests are useful coverage, but the current implementation still permits this specific ordering.

You are interacting with an AI system.

@mmabrouk

mmabrouk commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

🤖 The AI agent says:

@coderabbitai The proposed ordering overlooks what establishes serverOwnsInput: a real server snapshot has already returned this input ID, or a previous admission promise has succeeded. It is not optimistic local state.

get_session_snapshot calls inputs_service.list_pending (api/oss/src/apis/fastapi/sessions/router.py:2234). SessionsInputsDAO.list_pending reads the row in its own database session (api/oss/src/dbs/postgres/sessions/inputs/dao.py:122 and :163). Admission writes under a separate transaction (service.py:242); the engine commits on session exit (api/oss/src/dbs/postgres/shared/engine.py:61). The snapshot cannot expose an uncommitted admission from another transaction. Therefore, a returned row already exists durably even if the browser's POST promise is still pending or its response was lost.

Waiting for or replaying that HTTP promise after committed ownership is observed is the bug this change fixes. PATCH may legitimately refuse an input that another client has since promoted; that refusal keeps the draft and Edit identity rather than admitting a duplicate. When ownership has not been observed, the code still waits for or retries the original migration.

I do not find a remaining admission-before-PATCH race in the described sequence. Please account for the committed snapshot boundary when updating the risk assessment.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Frontend queue ownership and edit flow ==='
ast-grep outline web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts --items all --match 'commitEdit|migration|queued' || true
rg -n -C 18 'serverOwnsInput|migrationPromiseRef|migrationRef|commitEdit|server\.edit|server\.submit' \
  web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts

echo '=== Snapshot route and pending-input list path ==='
rg -n -C 12 'get_session_snapshot|list_pending' \
  api/oss/src/apis/fastapi/sessions/router.py \
  api/oss/src/core/sessions/inputs/service.py \
  api/oss/src/dbs/postgres/sessions/inputs/dao.py

echo '=== Admission write and transaction lifecycle ==='
rg -n -C 18 'async def (create|submit|enqueue)|transaction|begin|commit|session' \
  api/oss/src/core/sessions/inputs/service.py \
  api/oss/src/dbs/postgres/shared/engine.py \
  api/oss/src/dbs/postgres/sessions/inputs/dao.py

echo '=== Edit lock and update transaction path ==='
rg -n -C 12 'async def update|lock_pending_for_edit|update_content' \
  api/oss/src/core/sessions/inputs/service.py \
  api/oss/src/dbs/postgres/sessions/inputs/dao.py

Length of output: 106428


@mmabrouk You are correct. I withdraw the migration finding.

serverOwnsInput is set only after one of these conditions:

  • server.submit succeeds and sets editSessionRef.current.server.
  • A get_session_snapshot response includes the input in server.queued.

SessionInputsService.admit creates the row in its transaction. TransactionsEngine.session() commits that transaction when the context exits. SessionInputsDAO.list_pending uses a separate session for the snapshot read. The snapshot cannot return the uncommitted row from admission.

Therefore, when commitEdit reaches Line 413 with serverOwnsInput === true, the target row already exists durably. server.edit can safely run without awaiting migrationPromiseRef.current. Clearing the stale migration state prevents an unnecessary admission replay and avoids a duplicate row.

A later SessionInputNotEditable response remains a valid concurrent promotion outcome. The current failure handling retains the edit session and draft for retry or cancellation.


✏️ Learnings added
Learnt from: mmabrouk
URL: https://github.com/Agenta-AI/agenta/pull/6607

Timestamp: 2026-09-06T19:12:03.760Z
Learning: In `web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts`, `serverOwnsInput` is authoritative rather than optimistic. It is true only after `server.submit` succeeds or after `get_session_snapshot` returns the input in `server.queued`. The session snapshot reads pending inputs in a separate PostgreSQL session, while `SessionInputsService.admit` commits its transaction before the row becomes visible. When `serverOwnsInput` is true, `commitEdit` must patch the existing server row without waiting for or replaying a stale local-to-server migration promise.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

@mmabrouk
mmabrouk merged commit 95a9e22 into release/v0.115.2 Sep 6, 2026
70 of 71 checks passed
@mmabrouk

mmabrouk commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

🤖 The AI agent says:

Merged the reviewed head d68162e06c into release/v0.115.2 at 95a9e227e2. The merge tree exactly matches the tested head (0cc543e9d2a4735fadd20181d714c6f2be5c6113).

Independent review passed. CodeRabbit completed coverage of the final head; its blank-ID finding is fixed, response validation is confirmed, and it withdrew the migration finding after verifying the committed snapshot boundary. Required short CI checks passed with no failures at merge.

Focused verification passed: 74 frontend checks, four database-backed attachment checks, formatting/lint/types/client build, and 12 final live assertions. A lost successful Edit response retained the draft; retry preserved the same queued row and two distinct files. Mobile Send Now delivered both original and added files, real tool reads verified their contents, and both clients finished idle.

Staging deployment and a focused check of the deployed release are next. This fix introduces no migration. Production remains untouched.

@mmabrouk

mmabrouk commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

🤖 The AI agent says:

Final staging verification passed on release 95a9e227e2, whose tree matches the reviewed head. All eight containers are healthy, the six session flags are enabled, and the old tasks have drained.

The final browser journey passed 14 assertions on desktop and the actual /m host: questionnaire submission from the following client, same-row Edit retry after a lost successful response, no duplicate attachments, mobile Send Now, both file contents read by tools, and reload to matching idle state. The focused API sanity check passed 11/11. The Send Now button now reads Sending after selection, with the separate Steer tag removed and Edit retained.

The broad Railway acceptance run was interrupted by automatic preview cleanup after merge, so it is not claimed fully green. Its relevant attachment send/render/reload test passed. An earlier generic-playground Error had no retained network cause; it is outside this change. Final staging also exposed an existing find/fd flag incompatibility; the agent recovered using ls/read and both clients displayed the failed tool. That unrelated runner observation is recorded for follow-up.

The requested focused release checks are complete. Production remains untouched and the release PR remains open for the user's later deployment. These follow-up fixes add no migration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant