Handle legacy Codex approval requests
This commit is contained in:
+55
-13
@@ -1714,23 +1714,29 @@ func (b *Bot) handleCodexServerRequest(ctx context.Context, event codexapp.Event
|
|||||||
}
|
}
|
||||||
switch event.Method {
|
switch event.Method {
|
||||||
case "item/commandExecution/requestApproval", "item/fileChange/requestApproval", "item/permissions/requestApproval":
|
case "item/commandExecution/requestApproval", "item/fileChange/requestApproval", "item/permissions/requestApproval":
|
||||||
|
case "execCommandApproval", "applyPatchApproval":
|
||||||
default:
|
default:
|
||||||
b.logger.Printf("unhandled server request: %s", event.Method)
|
b.logger.Printf("unhandled server request: %s", event.Method)
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
var params struct {
|
var params struct {
|
||||||
ThreadID string `json:"threadId"`
|
ThreadID string `json:"threadId"`
|
||||||
TurnID string `json:"turnId"`
|
ConversationID string `json:"conversationId"`
|
||||||
ItemID string `json:"itemId"`
|
TurnID string `json:"turnId"`
|
||||||
Reason string `json:"reason"`
|
ItemID string `json:"itemId"`
|
||||||
|
CallID string `json:"callId"`
|
||||||
|
ApprovalID string `json:"approvalId"`
|
||||||
|
Reason string `json:"reason"`
|
||||||
}
|
}
|
||||||
if err := json.Unmarshal(event.Params, ¶ms); err != nil {
|
if err := json.Unmarshal(event.Params, ¶ms); err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
if params.ThreadID == "" {
|
threadID := firstNonEmpty(params.ThreadID, params.ConversationID)
|
||||||
|
if threadID == "" {
|
||||||
return errors.New("approval request missing threadId")
|
return errors.New("approval request missing threadId")
|
||||||
}
|
}
|
||||||
thread, err := b.store.GetThreadByCodexID(ctx, params.ThreadID)
|
itemID := firstNonEmpty(params.ItemID, params.ApprovalID, params.CallID)
|
||||||
|
thread, err := b.store.GetThreadByCodexID(ctx, threadID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
@@ -1742,9 +1748,9 @@ func (b *Bot) handleCodexServerRequest(ctx context.Context, event codexapp.Event
|
|||||||
approval, err := b.store.UpsertPendingApproval(ctx, store.PendingApproval{
|
approval, err := b.store.UpsertPendingApproval(ctx, store.PendingApproval{
|
||||||
TelegramUserID: thread.TelegramUserID,
|
TelegramUserID: thread.TelegramUserID,
|
||||||
CodexRequestID: strconv.FormatInt(*event.ID, 10),
|
CodexRequestID: strconv.FormatInt(*event.ID, 10),
|
||||||
CodexThreadID: params.ThreadID,
|
CodexThreadID: threadID,
|
||||||
TurnID: params.TurnID,
|
TurnID: params.TurnID,
|
||||||
ItemID: params.ItemID,
|
ItemID: itemID,
|
||||||
Kind: kind,
|
Kind: kind,
|
||||||
PayloadJSON: string(pretty),
|
PayloadJSON: string(pretty),
|
||||||
})
|
})
|
||||||
@@ -1756,7 +1762,7 @@ func (b *Bot) handleCodexServerRequest(ctx context.Context, event codexapp.Event
|
|||||||
}
|
}
|
||||||
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, threadID, itemID, text, markup); err != nil {
|
||||||
return err
|
return err
|
||||||
} else if ok {
|
} else if ok {
|
||||||
return b.store.UpdatePendingApprovalMessage(ctx, approval.ID, msg.Chat.ID, msg.MessageID)
|
return b.store.UpdatePendingApprovalMessage(ctx, approval.ID, msg.Chat.ID, msg.MessageID)
|
||||||
@@ -1772,6 +1778,15 @@ func (b *Bot) handleCodexServerRequest(ctx context.Context, event codexapp.Event
|
|||||||
return b.store.UpdatePendingApprovalMessage(ctx, approval.ID, msg.Chat.ID, msg.MessageID)
|
return b.store.UpdatePendingApprovalMessage(ctx, approval.ID, msg.Chat.ID, msg.MessageID)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func firstNonEmpty(values ...string) string {
|
||||||
|
for _, value := range values {
|
||||||
|
if trimmed := strings.TrimSpace(value); trimmed != "" {
|
||||||
|
return trimmed
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
|
||||||
func (b *Bot) newOutputState(chatID int64) *outputState {
|
func (b *Bot) newOutputState(chatID int64) *outputState {
|
||||||
return &outputState{
|
return &outputState{
|
||||||
chatID: chatID,
|
chatID: chatID,
|
||||||
@@ -2722,6 +2737,9 @@ func approvalMarkup(id int64) *InlineKeyboardMarkup {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func approvalResponse(approval store.PendingApproval, decision string) any {
|
func approvalResponse(approval store.PendingApproval, decision string) any {
|
||||||
|
if isLegacyApprovalKind(approval.Kind) {
|
||||||
|
return map[string]any{"decision": legacyApprovalDecision(decision)}
|
||||||
|
}
|
||||||
if approval.Kind != "item/permissions/requestApproval" {
|
if approval.Kind != "item/permissions/requestApproval" {
|
||||||
return map[string]any{"decision": decision}
|
return map[string]any{"decision": decision}
|
||||||
}
|
}
|
||||||
@@ -2746,12 +2764,36 @@ func approvalResponse(approval store.PendingApproval, decision string) any {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func isLegacyApprovalKind(kind string) bool {
|
||||||
|
switch kind {
|
||||||
|
case "execCommandApproval", "applyPatchApproval":
|
||||||
|
return true
|
||||||
|
default:
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func legacyApprovalDecision(decision string) string {
|
||||||
|
switch decision {
|
||||||
|
case "accept":
|
||||||
|
return "approved"
|
||||||
|
case "acceptForSession":
|
||||||
|
return "approved_for_session"
|
||||||
|
case "decline":
|
||||||
|
return "denied"
|
||||||
|
case "cancel":
|
||||||
|
return "abort"
|
||||||
|
default:
|
||||||
|
return decision
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func renderApprovalHTML(kind string, raw json.RawMessage, status string) string {
|
func renderApprovalHTML(kind string, raw json.RawMessage, status string) string {
|
||||||
title := "Codex approval requested"
|
title := "Codex approval requested"
|
||||||
if strings.Contains(kind, "commandExecution") {
|
if strings.Contains(kind, "commandExecution") || kind == "execCommandApproval" {
|
||||||
title = "Codex requests command approval"
|
title = "Codex requests command approval"
|
||||||
}
|
}
|
||||||
if strings.Contains(kind, "fileChange") {
|
if strings.Contains(kind, "fileChange") || kind == "applyPatchApproval" {
|
||||||
title = "Codex requests file change approval"
|
title = "Codex requests file change approval"
|
||||||
}
|
}
|
||||||
if strings.Contains(kind, "permissions") {
|
if strings.Contains(kind, "permissions") {
|
||||||
@@ -2764,7 +2806,7 @@ func renderApprovalHTML(kind string, raw json.RawMessage, status string) string
|
|||||||
if reason, _ := params["reason"].(string); reason != "" {
|
if reason, _ := params["reason"].(string); reason != "" {
|
||||||
lines = append(lines, "", reason)
|
lines = append(lines, "", reason)
|
||||||
}
|
}
|
||||||
for _, key := range []string{"command", "cwd", "grantRoot", "permissions"} {
|
for _, key := range []string{"command", "cwd", "grantRoot", "permissions", "fileChanges"} {
|
||||||
if value, ok := params[key]; ok {
|
if value, ok := params[key]; ok {
|
||||||
lines = append(lines, fmt.Sprintf("%s: %s", argumentLabel(key), conciseValue(value)))
|
lines = append(lines, fmt.Sprintf("%s: %s", argumentLabel(key), conciseValue(value)))
|
||||||
}
|
}
|
||||||
@@ -2806,7 +2848,7 @@ func renderApprovalDetailsHTML(kind string, raw json.RawMessage) string {
|
|||||||
}
|
}
|
||||||
parts = append(parts, FieldHTML(label, text))
|
parts = append(parts, FieldHTML(label, text))
|
||||||
}
|
}
|
||||||
for _, key := range []string{"command", "cwd", "grantRoot", "permissions", "reason"} {
|
for _, key := range []string{"command", "cwd", "grantRoot", "permissions", "fileChanges", "parsedCmd", "reason"} {
|
||||||
if value, ok := params[key]; ok {
|
if value, ok := params[key]; ok {
|
||||||
appendValue(argumentLabel(key), value)
|
appendValue(argumentLabel(key), value)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -84,6 +84,34 @@ func TestApprovalResponseForPermissions(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestApprovalResponseForLegacyApproval(t *testing.T) {
|
||||||
|
approval := store.PendingApproval{Kind: "execCommandApproval"}
|
||||||
|
response, ok := approvalResponse(approval, "accept").(map[string]any)
|
||||||
|
if !ok {
|
||||||
|
t.Fatal("legacy response should be a map")
|
||||||
|
}
|
||||||
|
if response["decision"] != "approved" {
|
||||||
|
t.Fatalf("legacy accept = %v, want approved", response["decision"])
|
||||||
|
}
|
||||||
|
response, ok = approvalResponse(approval, "decline").(map[string]any)
|
||||||
|
if !ok {
|
||||||
|
t.Fatal("legacy decline response should be a map")
|
||||||
|
}
|
||||||
|
if response["decision"] != "denied" {
|
||||||
|
t.Fatalf("legacy decline = %v, want denied", response["decision"])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestRenderLegacyApprovalDetails(t *testing.T) {
|
||||||
|
raw := json.RawMessage(`{"conversationId":"thr_1","callId":"call_1","command":["git","remote","-v"],"cwd":"/workspace/project","reason":"Need remote details"}`)
|
||||||
|
text := renderApprovalHTML("execCommandApproval", raw, "")
|
||||||
|
for _, want := range []string{"Codex requests command approval", "Need remote details", "language-bash", "git", "CWD"} {
|
||||||
|
if !strings.Contains(text, want) {
|
||||||
|
t.Fatalf("legacy approval render missing %q in %q", want, text)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestEditReplyMarkupClearsInlineKeyboard(t *testing.T) {
|
func TestEditReplyMarkupClearsInlineKeyboard(t *testing.T) {
|
||||||
markup := editReplyMarkup(nil)
|
markup := editReplyMarkup(nil)
|
||||||
if markup == nil {
|
if markup == nil {
|
||||||
|
|||||||
Reference in New Issue
Block a user