Skip to content

fix(api): keep queued messages behind pending approvals - #6598

Merged
mmabrouk merged 1 commit into
release/v0.115.2from
fix/release-1152-queue-paused-approval
Sep 6, 2026
Merged

fix(api): keep queued messages behind pending approvals#6598
mmabrouk merged 1 commit into
release/v0.115.2from
fix/release-1152-queue-paused-approval

Conversation

@mmabrouk

@mmabrouk mmabrouk commented Sep 6, 2026

Copy link
Copy Markdown
Member

Context

Sending a queued message while an agent waited for approval started a new run and cancelled the unanswered approval. Admission treated the paused session as idle because its running heartbeat had stopped.

Changes

Queue now checks the current execution's durable pending interactions and saves the message as a pending input while the approval remains unanswered. It rechecks after acquiring the execution lock; if another client has approved in the meantime, admission resumes outside the transaction so the approved continuation retains ownership. Steer and ordinary idle Send retain their existing behavior.

Tests

  • 16 focused admission tests passed, including Queue behind a settled approval, Steer, ordinary idle input, and approval winning the Queue lock.
  • Real local native-approval journey passed: Queue returned 202, preserved the same approval, created one pending input, and started no replacement execution. Explicit approval completed the tool, then the queued follow-up completed and the session returned idle.
  • Durable records confirmed paused, resumed-completed, then queued-completed execution terminals. The original harness expected three legacy turn projection rows; that assertion was corrected against retained durable records without repeating the paid run.
  • Ruff formatting/lint and independent reviews passed.

What to QA

Open a session with a pending native approval in two clients. Queue a follow-up from the observer: the approval must remain answerable and the follow-up must appear in the queue. Approve the tool and verify it finishes before the queued follow-up runs. Repeat with approval and Queue submitted close together from separate clients.

@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 1:15am UTC

Request Review

@mmabrouk

mmabrouk commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

🤖 The AI agent says: @coderabbitai review

Please review exact commit 094ee60. Focus on Queue preserving an unanswered approval, the execution/interaction lock ordering, and re-admission outside the transaction when approval wins the race. Steer remains intentional. Root and independent review passed; 16 unit tests and one real local approval→Queue→approved tool→queued completion journey passed.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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: f10ec2ab-3d43-4820-9bf8-76409a40571c

📥 Commits

Reviewing files that changed from the base of the PR and between e3cecf0 and 094ee60.

📒 Files selected for processing (3)
  • api/entrypoints/routers.py
  • api/oss/src/core/sessions/inputs/service.py
  • api/oss/tests/pytest/unit/sessions/test_pending_inputs_service.py

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


📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Improved session handling when an approval or interaction is awaiting a response.
    • New inputs now wait appropriately behind unanswered approvals instead of being incorrectly queued or treated as executable.
    • Answered approvals no longer unnecessarily block idle queued inputs.
    • Steering can replace an unanswered approval as expected.
    • Approval interactions now correctly take precedence when competing for session processing.

Walkthrough

SessionInputsService now detects pending approval interactions during queue admission. It defers input creation until the approval continuation resumes, while preserving immediate execution for steer policies and answered approvals. Tests cover these paths and DAO wiring passes the interaction dependency.

Changes

Pending approval admission

Layer / File(s) Summary
Interaction DAO wiring
api/entrypoints/routers.py, api/oss/src/core/sessions/inputs/service.py
SessionInputsService accepts an optional interactions_dao. The router passes the shared SessionInteractionsDAO instance.
Pending interaction admission flow
api/oss/src/core/sessions/inputs/service.py, api/oss/tests/pytest/unit/sessions/test_pending_inputs_service.py
Queue admission checks pending interactions, uses transaction locking when required, and retries after the interaction instead of creating a queued input. Tests cover unanswered, answered, steered, and resumed approval cases.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 094ee

Queue inputs now remain behind unanswered approvals and resume in the approved continuation’s ordering, while steer and answered-approval behavior remain immediate. Current coverage supports merge readiness with no active merge-blocking risk.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant SessionInputsService
  participant SessionInteractionsDAO
  participant ContinuationResumer
  Client->>SessionInputsService: submit input
  SessionInputsService->>SessionInteractionsDAO: fetch turn interactions
  SessionInteractionsDAO-->>SessionInputsService: return interaction status
  alt interaction is pending
    SessionInputsService->>SessionInputsService: retry admission after interaction
    SessionInputsService->>ContinuationResumer: defer input to approval continuation
  else interaction is answered or absent
    SessionInputsService-->>Client: admit input
  end
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 10 functions across 3 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 and concisely describes the main change: keeping queued messages behind pending approvals.
Description check ✅ Passed The description directly explains the approval queue behavior, race handling, tests, and QA scope covered by the changeset.
  • 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-queue-paused-approval

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 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

commit='094ee6004cd36459ecefacc85bd538528a74db10'

git show --no-ext-diff --format=fuller --stat "$commit"
printf '\n===== NAME STATUS =====\n'
git diff-tree --no-commit-id --name-status -r "$commit"
printf '\n===== DIFF =====\n'
git diff --no-ext-diff --find-renames "${commit}^" "$commit" -- .
printf '\n===== OUTLINES =====\n'
git diff-tree --no-commit-id --name-only -r "$commit" |
  while IFS= read -r file; do
    case "$file" in
      *.py)
        printf '\n--- %s ---\n' "$file"
        ast-grep outline "$file" --items all || true
        ;;
    esac
  done

Length of output: 36150


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '===== ADMISSION SERVICE ====='
nl -ba api/oss/src/core/sessions/inputs/service.py | sed -n '1,250p'

printf '%s\n' '===== INPUT DAO / INTERFACE ====='
fd -a -t f '^(dao|interfaces)\.py$' api/oss/src | \
  while IFS= read -r f; do
    if rg -q 'class SessionInputsDAO|SessionInputsDAOInterface|def transaction|async def transaction' "$f"; then
      printf '\n--- %s ---\n' "$f"
      rg -n -C 8 'class SessionInputsDAO|SessionInputsDAOInterface|def transaction|async def transaction|create_input|fetch_active_successor' "$f"
    fi
  done

printf '%s\n' '===== EXECUTION / INTERACTION LOCK IMPLEMENTATIONS ====='
rg -n -C 10 --glob '*.py' \
  'async def lock_for_control|def lock_for_control|async def fetch_turn_interactions|def fetch_turn_interactions|for_update' \
  api/oss/src

printf '%s\n' '===== APPROVAL / INTERACTION RESOLUTION AND CONTINUATION RESUME ====='
rg -n -C 12 --glob '*.py' \
  'resolve.*interaction|respond.*interaction|SessionInteractionStatus\.(responded|resolved)|resume_recoverable_continuation|fetch_turn_interactions' \
  api/oss/src/core/sessions api/oss/src/apis api/oss/src/dbs

printf '%s\n' '===== SERVICE CONSTRUCTION / DIRECT INSTANTIATIONS ====='
rg -n -C 4 --glob '*.py' \
  'SessionInputsService\s*\(' \
  .

Length of output: 1667


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '===== ADMISSION SERVICE ====='
awk '{printf "%6d  %s\n", NR, $0}' api/oss/src/core/sessions/inputs/service.py | sed -n '1,250p'

printf '%s\n' '===== INPUT DAO / INTERFACE ====='
fd -a -t f '^(dao|interfaces)\.py$' api/oss/src |
  while IFS= read -r f; do
    if rg -q 'class SessionInputsDAO|SessionInputsDAOInterface|def transaction|async def transaction' "$f"; then
      printf '\n--- %s ---\n' "$f"
      rg -n -C 8 'class SessionInputsDAO|SessionInputsDAOInterface|def transaction|async def transaction|create_input|fetch_active_successor' "$f" || true
    fi
  done

printf '%s\n' '===== EXECUTION / INTERACTION LOCK IMPLEMENTATIONS ====='
rg -n -C 10 --glob '*.py' \
  'async def lock_for_control|def lock_for_control|async def fetch_turn_interactions|def fetch_turn_interactions|for_update' \
  api/oss/src || true

printf '%s\n' '===== APPROVAL / INTERACTION RESOLUTION AND CONTINUATION RESUME ====='
rg -n -C 12 --glob '*.py' \
  'resolve.*interaction|respond.*interaction|SessionInteractionStatus\.(responded|resolved)|resume_recoverable_continuation|fetch_turn_interactions' \
  api/oss/src/core/sessions api/oss/src/apis api/oss/src/dbs || true

printf '%s\n' '===== SERVICE CONSTRUCTION / DIRECT INSTANTIATIONS ====='
rg -n -C 4 --glob '*.py' \
  'SessionInputsService\s*\(' \
  . || true

Length of output: 153436


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '===== INPUT DAO: CREATE AND PROMOTION ====='
sed -n '20,115p' api/oss/src/dbs/postgres/sessions/inputs/dao.py
sed -n '225,285p' api/oss/src/dbs/postgres/sessions/inputs/dao.py

printf '%s\n' '===== COMMAND RESPONSE TRANSACTION ====='
sed -n '466,710p' api/oss/src/core/sessions/commands/service.py

printf '%s\n' '===== CONTINUATION RESUME ====='
sed -n '790,930p' api/oss/src/core/sessions/commands/service.py

printf '%s\n' '===== PENDING-INPUT PROMOTION CALL SITES ====='
rg -n -C 10 --glob '*.py' \
  '\b(promote_next|claim_next|promote_input|promote_pending|fetch_active_successor)\s*\(' \
  api/oss/src

printf '%s\n' '===== TESTS: EXISTING POSTGRES SETTLEMENT RACES ====='
sed -n '300,435p' api/oss/tests/pytest/unit/sessions/test_session_inputs_dao.py

printf '%s\n' '===== CHANGED TEST SUPPORT ====='
sed -n '1,120p' api/oss/tests/pytest/unit/sessions/test_pending_inputs_service.py

Length of output: 44542


@mmabrouk Reviewed commit 094ee6004cd36459ecefacc85bd538528a74db10.

I found no issues in the requested paths.

  • Queue treats a durable pending interaction as busy when the stream heartbeat is stopped.
  • Queue admission and approval response both lock the execution before they lock interaction rows.
  • If approval settles the source execution while Queue waits, Queue exits its transaction and re-enters admission. This lets the approved continuation retain ownership.
  • Steer does not use the Queue pending-interaction guard. Its behavior remains intentional.
✅ Action performed

Review finished.

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-06T02:51:37.280Z

@mmabrouk

mmabrouk commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

Release QA review of the failed web acceptance job in run 34003401964, on 094ee6004c:

Both attempts failed during chat-app setup with response.text: Protocol error (Network.getResponseBody): No resource with given identifier found. The retry trace locates the failure at apiHelpers/index.ts:162 inside app lookup. Its workflows query began before navigation to /prompts, and the old document’s response body became unavailable afterward. The recorded retry contains zero /invoke requests, so this failure did not exercise the chat run or this PR’s queue behavior.

This is the stale-document response race fixed in #6595: app lookup now uses the authenticated request context after navigation, scoped to the active project. The final fixture head 9623488b6c also includes a separately verified provider-propagation retry; that second condition was not observed in this #6598 failure.

Validation of the final fixture correction: 7 focused helper tests passed, the exact previously failing chat acceptance passed against Railway with the mock provider in 38.8 seconds, and completed CodeRabbit review covered 9623488b6c without actionable findings. This classifies the historical failure; it does not claim that the old 094ee6004c workflow was rerun or passed. No blind rerun was performed.

— Codex (release QA)

@mmabrouk
mmabrouk changed the base branch from fix/release-1152-heartbeat-mirror to release/v0.115.2 September 6, 2026 02:51
@mmabrouk
mmabrouk merged commit a7f7252 into release/v0.115.2 Sep 6, 2026
66 of 68 checks passed
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