Skip to content

fix(approval): stop a pending approval dying on the session idle TTL (#13478) - #13494

Merged
mrveiss merged 4 commits into
Dev_new_guifrom
issue-13478
Aug 4, 2026
Merged

mrveiss merged 4 commits into
Dev_new_guifrom
issue-13478

Conversation

@mrveiss

@mrveiss mrveiss commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Thinking Path

#13216 reported that approvals "expire after 30 minutes and cannot be honoured late". Reading the code showed that is not what expires: the 30-minute poll budget only bounds the turn, it never clears the pending approval, and _approve_command_internal executes on approve with no coroutine waiting. #13486 fixed the message that made a live approval read as a failed command.

The real expiry is one layer down and an hour later.

# services/agent_terminal/session_manager.py:388
await self.redis_client.setex(key, TTL_1_HOUR, json_data)

set_pending_approval re-persists the session immediately (service.py:213), so the clock starts the moment the prompt is raised and nothing refreshes it while the human thinks. An hour on, the key is gone, get_session misses, and _approve_command_internal takes if not session: return {"error": "Session not found"}. The approval is unrecoverable — and a person taking more than an hour to answer a prompt is the ordinary case this feature exists for.

The second half is subtler and I nearly missed it. _restore_pending_approval rebuilds the state from chat history and is explicitly documented as surviving restarts — it works. But it runs only inside create_session, which mints a new session_id, while the approve endpoint is addressed by the old one (what the persisted approval message and the GUI button carry). A working restore that the only path needing it cannot reach.

What Changed

services/agent_terminal/session_manager.py

The TTL now depends on what the session is doing. TTL_1_HOUR still collects genuinely idle sessions — that GC is right and unchanged. A session holding a pending approval gets APPROVAL_PENDING_SESSION_TTL (default 7 days, AUTOBOT_APPROVAL_PENDING_SESSION_TTL_SECONDS). It is deliberately long: the thing being waited on is a person, and #13481 established that an approval does not expire on a timer. This bounds the stored session, not the approval's validity.

A pointer from session to conversation. agent_terminal:approval_conversation:{session_id} → conversation_id, written only while an approval is pending and with the same long TTL. get_session takes a session_id and _restore_pending_approval needs a conversation_id; without this the chat-history restore cannot be reached from the approve path.

get_session rebuilds a vanished session that still owes an answer. On a miss it follows the pointer, runs the existing restore, and returns a session under the original id — so the approve request that arrives with the old id resolves. If the approval turns out to have been answered already, nothing is resurrected. An unknown session id still returns None cheaply and quietly, which is the common case.

The rebuilt session takes AgentRole.CHAT_AGENT, the lowest privilege. The original role is not recorded in the restored approval, and a resurrected session exists only to let a human resolve a decision that was already gated — it must never be the thing that widens what the command may do.

Verification

$ python3 -m pytest autobot-backend/services/agent_terminal/ autobot-backend/tests/test_startup_imports.py -q
360 passed

Both halves invert-tested, each failing on exactly its own regression:

$ # TTL reverted to a flat TTL_1_HOUR (the shipped bug)
1 failed, 7 passed
$ # get_session rebuild removed (restore unreachable again)
1 failed, 7 passed
$ # restored
8 passed

test_an_idle_session_still_expires_in_an_hour passes in both directions on purpose — it guards the GC this change could plausibly have broken while fixing the other case.

Known limit

Both the session record and the pointer live in Redis. A full flush still loses a pending approval — but that is data loss, a different failure from a TTL quietly expiring something that was never meant to be time-limited. Worth naming rather than implying this makes approvals durable against anything.

Scope

Closes #13478. The remaining #13216 children are #13479 (folding a late execution result back into the chat turn that gave up on it) and #13480 (exactly-once execution when an approve races the poll).

Closes #13478
Refs #13216

Model Used

Opus 5 (1M context)

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!

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.

1 participant