Handle reused Codex approval request IDs

Treat app-server request IDs as connection-local by reopening reused approval rows when the thread, turn, or item context changes.

Keep duplicate resolved approvals in the same context closed, and add focused approval-path diagnostics without changing the Telegram approval UI.
This commit is contained in:
Codex
2026-05-28 10:11:42 +00:00
parent c00ffb42f2
commit 34e909f9cf
3 changed files with 122 additions and 6 deletions

View File

@@ -496,9 +496,41 @@ INSERT INTO pending_approvals (
message_chat_id, message_id, status message_chat_id, message_id, status
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 'pending') ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 'pending')
ON CONFLICT(telegram_user_id, codex_request_id) DO UPDATE SET ON CONFLICT(telegram_user_id, codex_request_id) DO UPDATE SET
codex_thread_id = excluded.codex_thread_id,
turn_id = excluded.turn_id,
item_id = excluded.item_id,
kind = excluded.kind,
payload_json = excluded.payload_json, payload_json = excluded.payload_json,
message_chat_id = CASE WHEN pending_approvals.status = 'pending' THEN excluded.message_chat_id ELSE pending_approvals.message_chat_id END, message_chat_id = CASE
message_id = CASE WHEN pending_approvals.status = 'pending' THEN excluded.message_id ELSE pending_approvals.message_id END`, WHEN pending_approvals.codex_thread_id = excluded.codex_thread_id
AND pending_approvals.turn_id = excluded.turn_id
AND pending_approvals.item_id = excluded.item_id
THEN pending_approvals.message_chat_id
ELSE excluded.message_chat_id
END,
message_id = CASE
WHEN pending_approvals.codex_thread_id = excluded.codex_thread_id
AND pending_approvals.turn_id = excluded.turn_id
AND pending_approvals.item_id = excluded.item_id
THEN pending_approvals.message_id
ELSE excluded.message_id
END,
status = CASE
WHEN pending_approvals.status <> 'pending'
AND pending_approvals.codex_thread_id = excluded.codex_thread_id
AND pending_approvals.turn_id = excluded.turn_id
AND pending_approvals.item_id = excluded.item_id
THEN pending_approvals.status
ELSE 'pending'
END,
resolved_at = CASE
WHEN pending_approvals.status <> 'pending'
AND pending_approvals.codex_thread_id = excluded.codex_thread_id
AND pending_approvals.turn_id = excluded.turn_id
AND pending_approvals.item_id = excluded.item_id
THEN pending_approvals.resolved_at
ELSE ''
END`,
approval.TelegramUserID, approval.CodexRequestID, approval.CodexThreadID, approval.TurnID, approval.TelegramUserID, approval.CodexRequestID, approval.CodexThreadID, approval.TurnID,
approval.ItemID, approval.Kind, approval.PayloadJSON, approval.MessageChatID, approval.MessageID) approval.ItemID, approval.Kind, approval.PayloadJSON, approval.MessageChatID, approval.MessageID)
if err != nil { if err != nil {

View File

@@ -267,6 +267,62 @@ func TestUpsertPendingApprovalDoesNotReopenResolved(t *testing.T) {
} }
} }
func TestUpsertPendingApprovalReopensReusedRequestIDForNewContext(t *testing.T) {
ctx := context.Background()
st, err := Open(ctx, filepath.Join(t.TempDir(), "bot.db"))
if err != nil {
t.Fatal(err)
}
defer st.Close()
first, err := st.UpsertPendingApproval(ctx, PendingApproval{
TelegramUserID: 42,
CodexRequestID: "i:0",
CodexThreadID: "thread-1",
TurnID: "turn-1",
ItemID: "item-1",
Kind: "item/commandExecution/requestApproval",
PayloadJSON: `{"command":"go test ./..."}`,
})
if err != nil {
t.Fatal(err)
}
if err := st.UpdatePendingApprovalMessage(ctx, first.ID, 1001, 2002); err != nil {
t.Fatal(err)
}
if err := st.ResolvePendingApproval(ctx, 42, first.ID, "accept"); err != nil {
t.Fatal(err)
}
second, err := st.UpsertPendingApproval(ctx, PendingApproval{
TelegramUserID: 42,
CodexRequestID: "i:0",
CodexThreadID: "thread-2",
TurnID: "turn-2",
ItemID: "item-2",
Kind: "item/commandExecution/requestApproval",
PayloadJSON: `{"command":"git push"}`,
})
if err != nil {
t.Fatal(err)
}
if second.ID != first.ID {
t.Fatalf("reused request id row = %d, want %d", second.ID, first.ID)
}
if second.Status != "pending" {
t.Fatalf("reused request status = %q, want pending", second.Status)
}
if second.CodexThreadID != "thread-2" || second.TurnID != "turn-2" || second.ItemID != "item-2" {
t.Fatalf("reused request context = thread %q turn %q item %q", second.CodexThreadID, second.TurnID, second.ItemID)
}
if second.MessageChatID != 0 || second.MessageID != 0 {
t.Fatalf("reused request kept stale message: chat=%d message=%d", second.MessageChatID, second.MessageID)
}
if second.ResolvedAt != "" {
t.Fatalf("reused request resolved_at = %q, want empty", second.ResolvedAt)
}
}
func TestValidateWorkspacePath(t *testing.T) { func TestValidateWorkspacePath(t *testing.T) {
if _, err := ValidateWorkspacePath("relative/path"); err == nil { if _, err := ValidateWorkspacePath("relative/path"); err == nil {
t.Fatal("relative path should be rejected") t.Fatal("relative path should be rejected")

View File

@@ -1878,6 +1878,7 @@ func (b *Bot) handleCodexServerRequest(ctx context.Context, event codexapp.Event
if err != nil { if err != nil {
return err return err
} }
b.logger.Printf("codex approval thread mapped: request=%s telegram_user=%d", event.ID.Key(), thread.TelegramUserID)
pretty, _ := json.MarshalIndent(json.RawMessage(event.Params), "", " ") pretty, _ := json.MarshalIndent(json.RawMessage(event.Params), "", " ")
if len(pretty) == 0 { if len(pretty) == 0 {
pretty = event.Params pretty = event.Params
@@ -1895,15 +1896,18 @@ func (b *Bot) handleCodexServerRequest(ctx context.Context, event codexapp.Event
if err != nil { if err != nil {
return err return err
} }
b.logger.Printf("codex approval stored: request=%s approval_id=%d status=%s item=%s", event.ID.Key(), approval.ID, approval.Status, approval.ItemID)
if approval.Status != "pending" { if approval.Status != "pending" {
return nil return nil
} }
text := renderApprovalHTML(kind, event.Params, "") text := renderApprovalHTML(kind, event.Params, "")
markup := approvalMarkupForPayload(approval.ID, event.Params) markup := approvalMarkupForPayload(approval.ID, event.Params)
b.logger.Printf("codex approval render complete: request=%s approval_id=%d text_runes=%d", event.ID.Key(), approval.ID, len([]rune(text)))
msg, err := b.upsertApprovalMessage(ctx, thread.TelegramUserID, threadID, params.TurnID, itemID, text, markup) msg, err := b.upsertApprovalMessage(ctx, thread.TelegramUserID, threadID, params.TurnID, itemID, text, markup)
if err != nil { if err != nil {
return err return err
} }
b.logger.Printf("codex approval telegram sent: request=%s approval_id=%d chat=%d message=%d", event.ID.Key(), approval.ID, msg.Chat.ID, msg.MessageID)
return b.store.UpdatePendingApprovalMessage(ctx, approval.ID, msg.Chat.ID, msg.MessageID) return b.store.UpdatePendingApprovalMessage(ctx, approval.ID, msg.Chat.ID, msg.MessageID)
} }
@@ -2316,15 +2320,34 @@ func (b *Bot) upsertApprovalMessage(ctx context.Context, chatID int64, threadID,
if approvalHTML == "" { if approvalHTML == "" {
return Message{}, errors.New("approval message is empty") return Message{}, errors.New("approval message is empty")
} }
if threadID == "" || itemID == "" || !b.hasOutputTurn(threadID, turnID) { b.logger.Printf("codex approval ui upsert start: thread=%s turn=%s item=%s chat=%d text_runes=%d", threadID, turnID, itemID, chatID, len([]rune(approvalHTML)))
return b.tg.SendMessage(ctx, chatID, approvalHTML, SendMessageOptions{ParseMode: "HTML", ReplyMarkup: markup}) trackedTurn := threadID != "" && itemID != "" && b.hasOutputTurn(threadID, turnID)
if !trackedTurn {
b.logger.Printf("codex approval ui direct send start: thread=%s turn=%s item=%s chat=%d", threadID, turnID, itemID, chatID)
msg, err := b.tg.SendMessage(ctx, chatID, approvalHTML, SendMessageOptions{ParseMode: "HTML", ReplyMarkup: markup})
if err != nil {
b.logger.Printf("codex approval ui direct send failed: thread=%s turn=%s item=%s err=%v", threadID, turnID, itemID, err)
return Message{}, err
}
b.logger.Printf("codex approval ui direct send done: thread=%s turn=%s item=%s chat=%d message=%d", threadID, turnID, itemID, msg.Chat.ID, msg.MessageID)
return msg, nil
} }
b.logger.Printf("codex approval ui flush assistant start: thread=%s turn=%s item=%s", threadID, turnID, itemID)
if err := b.flushAssistantMessage(ctx, threadID); err != nil { if err := b.flushAssistantMessage(ctx, threadID); err != nil {
b.logger.Printf("codex approval ui flush assistant failed: thread=%s turn=%s item=%s err=%v", threadID, turnID, itemID, err)
return Message{}, err return Message{}, err
} }
b.logger.Printf("codex approval ui flush assistant done: thread=%s turn=%s item=%s", threadID, turnID, itemID)
trackedChatID, err := b.outputChatID(ctx, threadID) trackedChatID, err := b.outputChatID(ctx, threadID)
if err != nil { if err != nil {
return b.tg.SendMessage(ctx, chatID, approvalHTML, SendMessageOptions{ParseMode: "HTML", ReplyMarkup: markup}) b.logger.Printf("codex approval ui output state missing; direct send start: thread=%s turn=%s item=%s chat=%d err=%v", threadID, turnID, itemID, chatID, err)
msg, sendErr := b.tg.SendMessage(ctx, chatID, approvalHTML, SendMessageOptions{ParseMode: "HTML", ReplyMarkup: markup})
if sendErr != nil {
b.logger.Printf("codex approval ui output-state direct send failed: thread=%s turn=%s item=%s err=%v", threadID, turnID, itemID, sendErr)
return Message{}, sendErr
}
b.logger.Printf("codex approval ui output-state direct send done: thread=%s turn=%s item=%s chat=%d message=%d", threadID, turnID, itemID, msg.Chat.ID, msg.MessageID)
return msg, nil
} }
chatID = trackedChatID chatID = trackedChatID
@@ -2343,20 +2366,24 @@ func (b *Bot) upsertApprovalMessage(ctx context.Context, chatID int64, threadID,
combined := tool.html() combined := tool.html()
msg := Message{MessageID: tool.messageID, Chat: Chat{ID: tool.chatID}} msg := Message{MessageID: tool.messageID, Chat: Chat{ID: tool.chatID}}
b.mu.Unlock() b.mu.Unlock()
b.logger.Printf("codex approval ui edit start: thread=%s turn=%s item=%s chat=%d message=%d text_runes=%d", threadID, turnID, itemID, msg.Chat.ID, msg.MessageID, len([]rune(combined)))
_, err := b.tg.EditMessageText(ctx, msg.Chat.ID, msg.MessageID, combined, EditMessageTextOptions{ParseMode: "HTML", ReplyMarkup: editReplyMarkup(markup)}) _, err := b.tg.EditMessageText(ctx, msg.Chat.ID, msg.MessageID, combined, EditMessageTextOptions{ParseMode: "HTML", ReplyMarkup: editReplyMarkup(markup)})
if err := ignoreTelegramMessageNotModified(err); err != nil { if err := ignoreTelegramMessageNotModified(err); err != nil {
b.clearToolApproval(threadID, itemID) b.clearToolApproval(threadID, itemID)
b.logger.Printf("edit tool approval message %s/%s: %v", threadID, itemID, err) b.logger.Printf("codex approval ui edit failed: thread=%s turn=%s item=%s err=%v", threadID, turnID, itemID, err)
return Message{}, err return Message{}, err
} }
b.logger.Printf("codex approval ui edit done: thread=%s turn=%s item=%s chat=%d message=%d", threadID, turnID, itemID, msg.Chat.ID, msg.MessageID)
b.markOutputSent(threadID) b.markOutputSent(threadID)
return msg, nil return msg, nil
} }
} }
b.mu.Unlock() b.mu.Unlock()
b.logger.Printf("codex approval ui new combined message start: thread=%s turn=%s item=%s chat=%d", threadID, turnID, itemID, chatID)
msg, err := b.sendHTMLMessage(ctx, chatID, approvalHTML, markup) msg, err := b.sendHTMLMessage(ctx, chatID, approvalHTML, markup)
if err != nil { if err != nil {
b.logger.Printf("codex approval ui new combined message failed: thread=%s turn=%s item=%s err=%v", threadID, turnID, itemID, err)
return Message{}, err return Message{}, err
} }
b.mu.Lock() b.mu.Lock()
@@ -2368,6 +2395,7 @@ func (b *Bot) upsertApprovalMessage(ctx context.Context, chatID int64, threadID,
state.tools[itemID] = toolMessageState{chatID: msg.Chat.ID, messageID: msg.MessageID, approvalHTML: approvalHTML, approvalMarkup: markup} state.tools[itemID] = toolMessageState{chatID: msg.Chat.ID, messageID: msg.MessageID, approvalHTML: approvalHTML, approvalMarkup: markup}
} }
b.mu.Unlock() b.mu.Unlock()
b.logger.Printf("codex approval ui new combined message done: thread=%s turn=%s item=%s chat=%d message=%d", threadID, turnID, itemID, msg.Chat.ID, msg.MessageID)
b.markOutputSent(threadID) b.markOutputSent(threadID)
return msg, nil return msg, nil
} }