Prevent stale approval buttons

This commit is contained in:
Codex
2026-05-24 03:23:58 +00:00
parent fd80780581
commit e85d0eb928
4 changed files with 91 additions and 9 deletions

View File

@@ -460,10 +460,8 @@ INSERT INTO pending_approvals (
) 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
payload_json = excluded.payload_json, payload_json = excluded.payload_json,
message_chat_id = excluded.message_chat_id, message_chat_id = CASE WHEN pending_approvals.status = 'pending' THEN excluded.message_chat_id ELSE pending_approvals.message_chat_id END,
message_id = excluded.message_id, message_id = CASE WHEN pending_approvals.status = 'pending' THEN excluded.message_id ELSE pending_approvals.message_id END`,
status = 'pending',
resolved_at = ''`,
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

@@ -192,6 +192,57 @@ func TestRenameThread(t *testing.T) {
} }
} }
func TestUpsertPendingApprovalDoesNotReopenResolved(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: "request-1",
CodexThreadID: "thread-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: "request-1",
CodexThreadID: "thread-1",
Kind: "item/commandExecution/requestApproval",
PayloadJSON: `{"command":"go test ./...","duplicate":true}`,
MessageChatID: 3003,
MessageID: 4004,
})
if err != nil {
t.Fatal(err)
}
if second.ID != first.ID {
t.Fatalf("duplicate approval id = %d, want %d", second.ID, first.ID)
}
if second.Status != "accept" {
t.Fatalf("duplicate approval status = %q, want accept", second.Status)
}
if second.MessageChatID != 1001 || second.MessageID != 2002 {
t.Fatalf("resolved approval message changed: chat=%d message=%d", second.MessageChatID, second.MessageID)
}
if second.ResolvedAt == "" {
t.Fatal("resolved approval lost resolved_at")
}
}
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

@@ -329,7 +329,7 @@ func (b *Bot) sendResumeChoices(ctx context.Context, userID, chatID int64, page
text := resumeThreadListText(threads, page) text := resumeThreadListText(threads, page)
markup := resumeThreadMarkup(threads, page, hasNext) markup := resumeThreadMarkup(threads, page, hasNext)
if messageID != 0 { if messageID != 0 {
_, err := b.tg.EditMessageText(ctx, chatID, messageID, EscapeHTML(text), EditMessageTextOptions{ParseMode: "HTML", ReplyMarkup: markup}) _, err := b.tg.EditMessageText(ctx, chatID, messageID, EscapeHTML(text), EditMessageTextOptions{ParseMode: "HTML", ReplyMarkup: editReplyMarkup(markup)})
return err return err
} }
_, err = b.tg.SendMessage(ctx, chatID, EscapeHTML(text), SendMessageOptions{ParseMode: "HTML", ReplyMarkup: markup}) _, err = b.tg.SendMessage(ctx, chatID, EscapeHTML(text), SendMessageOptions{ParseMode: "HTML", ReplyMarkup: markup})
@@ -366,7 +366,7 @@ func (b *Bot) resumeThreadByID(ctx context.Context, userID, chatID int64, id int
} }
text := fmt.Sprintf("Active thread ID %d: %s", thread.ID, threadDisplayTitle(thread)) text := fmt.Sprintf("Active thread ID %d: %s", thread.ID, threadDisplayTitle(thread))
if messageID != 0 { if messageID != 0 {
_, err = b.tg.EditMessageText(ctx, chatID, messageID, EscapeHTML(text), EditMessageTextOptions{ParseMode: "HTML"}) _, err = b.tg.EditMessageText(ctx, chatID, messageID, EscapeHTML(text), EditMessageTextOptions{ParseMode: "HTML", ReplyMarkup: clearInlineKeyboardMarkup()})
return err return err
} }
_, err = b.tg.SendMessage(ctx, chatID, EscapeHTML(text), SendMessageOptions{ParseMode: "HTML"}) _, err = b.tg.SendMessage(ctx, chatID, EscapeHTML(text), SendMessageOptions{ParseMode: "HTML"})
@@ -1149,7 +1149,7 @@ func (b *Bot) handleApprovalCallback(ctx context.Context, callback *CallbackQuer
return err return err
} }
updated := b.resolveApprovalMessageHTML(approval, decision) updated := b.resolveApprovalMessageHTML(approval, decision)
_, err = b.tg.EditMessageText(ctx, callback.Message.Chat.ID, callback.Message.MessageID, updated, EditMessageTextOptions{ParseMode: "HTML"}) _, err = b.tg.EditMessageText(ctx, callback.Message.Chat.ID, callback.Message.MessageID, updated, EditMessageTextOptions{ParseMode: "HTML", ReplyMarkup: clearInlineKeyboardMarkup()})
return ignoreTelegramMessageNotModified(err) return ignoreTelegramMessageNotModified(err)
} }
@@ -1710,6 +1710,9 @@ func (b *Bot) handleCodexServerRequest(ctx context.Context, event codexapp.Event
if err != nil { if err != nil {
return err return err
} }
if approval.Status != "pending" {
return nil
}
text := renderApprovalHTML(kind, event.Params, "") text := renderApprovalHTML(kind, event.Params, "")
markup := approvalMarkup(approval.ID) markup := approvalMarkup(approval.ID)
if msg, ok, err := b.attachApprovalToToolMessage(ctx, params.ThreadID, params.ItemID, text, markup); err != nil { if msg, ok, err := b.attachApprovalToToolMessage(ctx, params.ThreadID, params.ItemID, text, markup); err != nil {
@@ -2023,7 +2026,7 @@ func (b *Bot) upsertToolMessage(ctx context.Context, threadID, itemID, htmlText
msgID := tool.messageID msgID := tool.messageID
markup := tool.approvalMarkup markup := tool.approvalMarkup
b.mu.Unlock() b.mu.Unlock()
_, err := b.tg.EditMessageText(ctx, msgChatID, msgID, combined, EditMessageTextOptions{ParseMode: "HTML", ReplyMarkup: markup}) _, err := b.tg.EditMessageText(ctx, msgChatID, msgID, combined, EditMessageTextOptions{ParseMode: "HTML", ReplyMarkup: editReplyMarkup(markup)})
if err := ignoreTelegramMessageNotModified(err); err != nil { if err := ignoreTelegramMessageNotModified(err); err != nil {
return err return err
} }
@@ -2078,7 +2081,7 @@ func (b *Bot) attachApprovalToToolMessage(ctx context.Context, threadID, itemID,
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()
_, err := b.tg.EditMessageText(ctx, msg.Chat.ID, msg.MessageID, combined, EditMessageTextOptions{ParseMode: "HTML", ReplyMarkup: 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("edit tool approval message %s/%s: %v", threadID, itemID, err)
@@ -2653,6 +2656,17 @@ func (b *Bot) rememberSettingsMessage(ctx context.Context, userID int64, chatID
} }
} }
func clearInlineKeyboardMarkup() *InlineKeyboardMarkup {
return &InlineKeyboardMarkup{InlineKeyboard: [][]InlineKeyboardButton{}}
}
func editReplyMarkup(markup *InlineKeyboardMarkup) *InlineKeyboardMarkup {
if markup != nil {
return markup
}
return clearInlineKeyboardMarkup()
}
func approvalMarkup(id int64) *InlineKeyboardMarkup { func approvalMarkup(id int64) *InlineKeyboardMarkup {
return &InlineKeyboardMarkup{InlineKeyboard: [][]InlineKeyboardButton{ return &InlineKeyboardMarkup{InlineKeyboard: [][]InlineKeyboardButton{
{ {

View File

@@ -84,6 +84,25 @@ func TestApprovalResponseForPermissions(t *testing.T) {
} }
} }
func TestEditReplyMarkupClearsInlineKeyboard(t *testing.T) {
markup := editReplyMarkup(nil)
if markup == nil {
t.Fatal("nil edit markup would leave Telegram buttons unchanged")
}
raw, err := json.Marshal(markup)
if err != nil {
t.Fatal(err)
}
if string(raw) != `{"inline_keyboard":[]}` {
t.Fatalf("clear markup JSON = %s", raw)
}
existing := approvalMarkup(7)
if editReplyMarkup(existing) != existing {
t.Fatal("non-nil markup should be preserved")
}
}
func TestParseCommand(t *testing.T) { func TestParseCommand(t *testing.T) {
name, args, ok := parseCommand("/resume@my_bot 123") name, args, ok := parseCommand("/resume@my_bot 123")
if !ok || name != "resume" || len(args) != 1 || args[0] != "123" { if !ok || name != "resume" || len(args) != 1 || args[0] != "123" {