Skip to content

Commit d89e7f3

Browse files
authored
Merge pull request #38 from lesquel/fix/codex-permission-dashboard
fix(dashboard): make Codex permission requests visible in the dashboard
2 parents c37c881 + 66cfcc0 commit d89e7f3

6 files changed

Lines changed: 320 additions & 22 deletions

File tree

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
// codex-permission-alias.test.ts — TDD for the Codex permission dashboard alias fix.
2+
//
3+
// Bug: Codex emits `pilot.permission.pending` / `pilot.permission.resolved` but
4+
// the dashboard only handled `permission.requested` / `permission.resolved`.
5+
// Fix: add named constants for the pilot event types and handle them as aliases
6+
// in the SSE handleEvent switch.
7+
//
8+
// These tests run against pure modules (no browser APIs needed):
9+
// - constants.js — verifies the new constants are defined with correct values
10+
// - permission-normalize.js — verifies the pure normalization helper works for
11+
// both native OpenCode payload shapes and Codex payload shapes
12+
13+
import { describe, expect, test } from "bun:test"
14+
import { EVENTS } from "../constants.js"
15+
import {
16+
normalizePermissionPending,
17+
normalizePermissionResolved,
18+
} from "../sse/permission-normalize.js"
19+
20+
// ── Step 1: constants ─────────────────────────────────────────────────────────
21+
22+
describe("constants.js — pilot permission alias event types", () => {
23+
// FAILING until constants are added: EVENTS.PERMISSION_PENDING_PILOT and
24+
// EVENTS.PERMISSION_RESOLVED_PILOT must exist with the exact strings that
25+
// Codex emits.
26+
27+
test("EVENTS.PERMISSION_PENDING_PILOT equals 'pilot.permission.pending'", () => {
28+
expect(EVENTS.PERMISSION_PENDING_PILOT).toBe("pilot.permission.pending")
29+
})
30+
31+
test("EVENTS.PERMISSION_RESOLVED_PILOT equals 'pilot.permission.resolved'", () => {
32+
expect(EVENTS.PERMISSION_RESOLVED_PILOT).toBe("pilot.permission.resolved")
33+
})
34+
35+
test("native PERMISSION_REQUESTED constant is unchanged (no regression)", () => {
36+
expect(EVENTS.PERMISSION_REQUESTED).toBe("permission.requested")
37+
})
38+
39+
test("native PERMISSION_RESOLVED constant is unchanged (no regression)", () => {
40+
expect(EVENTS.PERMISSION_RESOLVED).toBe("permission.resolved")
41+
})
42+
})
43+
44+
// ── Step 2: normalization helper ──────────────────────────────────────────────
45+
46+
describe("normalizePermissionPending — Codex payload shape", () => {
47+
// The exact payload Codex emits in handlers.ts ~line 275:
48+
// { permissionID, title: `Codex: ${tool_name}`, sessionID, permissionType, metadata }
49+
const codexEvent = {
50+
type: "pilot.permission.pending",
51+
properties: {
52+
permissionID: "uuid-123",
53+
title: "Codex: BashTool",
54+
sessionID: "sess-abc",
55+
permissionType: "codex-tool",
56+
metadata: { tool_name: "BashTool", source: "codex-hook" },
57+
},
58+
}
59+
60+
test("normalizePermissionPending exists and handles Codex payload", () => {
61+
const normalized = normalizePermissionPending(codexEvent)
62+
expect(normalized.id).toBe("uuid-123")
63+
expect(normalized.permissionID).toBe("uuid-123")
64+
expect(normalized.title).toBe("Codex: BashTool")
65+
expect(normalized.sessionID).toBe("sess-abc")
66+
expect(normalized.type).toBe("codex-tool")
67+
expect(normalized.metadata).toEqual({ tool_name: "BashTool", source: "codex-hook" })
68+
})
69+
70+
test("normalizePermissionPending handles native OpenCode payload shape", () => {
71+
// Native OpenCode events carry the same fields under .properties
72+
const nativeEvent = {
73+
type: "permission.requested",
74+
properties: {
75+
permissionID: "native-uuid-456",
76+
title: "Shell: rm -rf",
77+
sessionID: "sess-native",
78+
permissionType: "shell",
79+
pattern: "/tmp/**",
80+
metadata: { command: "rm -rf /tmp/foo" },
81+
},
82+
}
83+
const normalized = normalizePermissionPending(nativeEvent)
84+
expect(normalized.id).toBe("native-uuid-456")
85+
expect(normalized.title).toBe("Shell: rm -rf")
86+
expect(normalized.pattern).toBe("/tmp/**")
87+
})
88+
89+
test("normalizePermissionPending handles .data shape (EventSource named-event path)", () => {
90+
// Named SSE events wrap the payload in .data
91+
const namedEvent = {
92+
type: "permission.requested",
93+
data: {
94+
permissionID: "evt-data-789",
95+
title: "Read file",
96+
sessionID: "sess-data",
97+
permissionType: "read",
98+
},
99+
}
100+
const normalized = normalizePermissionPending(namedEvent)
101+
expect(normalized.id).toBe("evt-data-789")
102+
expect(normalized.sessionID).toBe("sess-data")
103+
})
104+
105+
test("pattern field is undefined (not present) for Codex events — safe default", () => {
106+
// Codex does NOT send a pattern field. The normalize should produce
107+
// pattern: undefined (not throw) so the banner gracefully shows no detail.
108+
const normalized = normalizePermissionPending(codexEvent)
109+
expect(normalized.pattern).toBeUndefined()
110+
})
111+
})
112+
113+
describe("normalizePermissionResolved — Codex payload shape", () => {
114+
const codexResolved = {
115+
type: "pilot.permission.resolved",
116+
properties: {
117+
permissionID: "uuid-123",
118+
action: "allow",
119+
source: "remote",
120+
},
121+
}
122+
123+
test("normalizePermissionResolved exists and handles Codex payload", () => {
124+
const normalized = normalizePermissionResolved(codexResolved)
125+
expect(normalized.id).toBe("uuid-123")
126+
expect(normalized.permissionID).toBe("uuid-123")
127+
})
128+
129+
test("normalizePermissionResolved handles client_disconnected reason", () => {
130+
const disconnectEvent = {
131+
type: "pilot.permission.resolved",
132+
properties: {
133+
permissionID: "uuid-disconnected",
134+
action: "deny",
135+
source: "remote",
136+
reason: "client_disconnected",
137+
},
138+
}
139+
const normalized = normalizePermissionResolved(disconnectEvent)
140+
expect(normalized.id).toBe("uuid-disconnected")
141+
expect(normalized.permissionID).toBe("uuid-disconnected")
142+
})
143+
144+
test("normalizePermissionResolved handles timeout reason", () => {
145+
const timeoutEvent = {
146+
type: "pilot.permission.resolved",
147+
properties: {
148+
permissionID: "uuid-timeout",
149+
action: "deny",
150+
source: "remote",
151+
reason: "timeout",
152+
},
153+
}
154+
const normalized = normalizePermissionResolved(timeoutEvent)
155+
expect(normalized.id).toBe("uuid-timeout")
156+
})
157+
})

src/dashboard/constants.d.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// Type declarations for the subset of constants.js exports consumed by .test.ts files.
2+
// constants.js is browser vanilla JS — kept that way because the dashboard ships
3+
// .js to the browser as static files. This .d.ts gives TypeScript visibility
4+
// for the symbols our tests touch without forcing a build step.
5+
6+
export declare const EVENTS: {
7+
readonly SESSION_UPDATED: string
8+
readonly SESSION_CREATED: string
9+
readonly SESSION_DELETED: string
10+
readonly MESSAGE_CREATED: string
11+
readonly MESSAGE_UPDATED: string
12+
readonly MESSAGE_PART_UPDATED: string
13+
readonly MESSAGE_PART_DELTA: string
14+
readonly PERMISSION_REQUESTED: string
15+
readonly PERMISSION_RESOLVED: string
16+
readonly PERMISSION_PENDING_PILOT: string
17+
readonly PERMISSION_RESOLVED_PILOT: string
18+
readonly STATUS_CHANGED: string
19+
readonly TOOL_COMPLETED: string
20+
readonly PILOT_TOOL_COMPLETED: string
21+
readonly TODO_UPDATED: string
22+
readonly REFERENCES_READY: string
23+
readonly VCS_BRANCH_UPDATED: string
24+
readonly LSP_UPDATED: string
25+
readonly PILOT_SUBAGENT_SPAWNED: string
26+
}
27+
28+
export declare const LIMITS: {
29+
readonly SESSIONS_META_FETCH: number
30+
readonly TITLE_MAX_CHARS: number
31+
readonly PROMPT_MAX_CHARS: number
32+
readonly SSE_BACKOFF_MIN_MS: number
33+
readonly SSE_BACKOFF_MAX_MS: number
34+
readonly MCP_POLL_INTERVAL_MS: number
35+
readonly AGENT_BADGE_MAX_CHARS: number
36+
readonly BASH_CMD_PREVIEW_CHARS: number
37+
readonly TOOL_ARG_PREVIEW_CHARS: number
38+
readonly SCROLL_BOTTOM_THRESHOLD_PX: number
39+
}
40+
41+
export declare const STATUS_CLASS: {
42+
readonly idle: string
43+
readonly busy: string
44+
readonly error: string
45+
}
46+
47+
export declare const AGENT_BADGE_CLASS: {
48+
readonly plan: string
49+
readonly build: string
50+
readonly custom: string
51+
readonly dynamic: string
52+
readonly compact: string
53+
}
54+
55+
export declare const STORAGE_KEYS: {
56+
readonly FOLDER_COLLAPSED: string
57+
readonly ACTIVE_DIRECTORY: string
58+
readonly MV_PANELS: string
59+
readonly MV_ACTIVE: string
60+
readonly SUBAGENTS_COLLAPSED: string
61+
readonly RIGHT_PANEL_COLLAPSED: string
62+
readonly COST_HISTORY: string
63+
readonly COST_BUDGET_WARNED: string
64+
readonly PINNED_TODOS: string
65+
readonly PROJECT_TABS: string
66+
readonly ACTIVE_PROJECT_ID: string
67+
}
68+
69+
export declare const AGENT_COLOR: {
70+
readonly SATURATION: number
71+
readonly LIGHTNESS: number
72+
}

src/dashboard/constants.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,14 @@ export const EVENTS = Object.freeze({
3131
// This is what drives the typewriter effect; MESSAGE_PART_UPDATED only
3232
// carries snapshots (pending → running → completed) without the delta.
3333
MESSAGE_PART_DELTA: 'message.part.delta',
34-
// Permissions
34+
// Permissions — native OpenCode SDK events
3535
PERMISSION_REQUESTED: 'permission.requested',
3636
PERMISSION_RESOLVED: 'permission.resolved',
37+
// Permissions — Codex bridge events (alias: same handler, different type string)
38+
// Codex emits these types; renaming the emit would break Telegram/push consumers,
39+
// so the dashboard aliases them here instead.
40+
PERMISSION_PENDING_PILOT: 'pilot.permission.pending',
41+
PERMISSION_RESOLVED_PILOT: 'pilot.permission.resolved',
3742
// Status / tool
3843
STATUS_CHANGED: 'status.changed',
3944
TOOL_COMPLETED: 'tool.completed',
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// Type declarations for permission-normalize.js — consumed by .test.ts files.
2+
// The module is browser vanilla JS; this .d.ts gives TypeScript visibility
3+
// without forcing a build step.
4+
5+
export type NormalizedPermissionPending = {
6+
id: string | undefined
7+
permissionID: string | undefined
8+
description: string | undefined
9+
title: string | undefined
10+
sessionID: string | undefined
11+
type: string | undefined
12+
pattern: string | undefined
13+
metadata: Record<string, unknown> | undefined
14+
}
15+
16+
export type NormalizedPermissionResolved = {
17+
id: string | undefined
18+
permissionID: string | undefined
19+
}
20+
21+
export declare function normalizePermissionPending(ev: Record<string, unknown>): NormalizedPermissionPending
22+
export declare function normalizePermissionResolved(ev: Record<string, unknown>): NormalizedPermissionResolved
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// permission-normalize.js — Pure helpers that normalize SSE permission events
2+
// to the stable shape expected by handlePermissionRequested / handlePermissionResolved.
3+
//
4+
// Extracted so the logic is testable without importing browser-API-dependent modules.
5+
// Both native OpenCode events (`permission.requested` / `permission.resolved`) and
6+
// Codex bridge events (`pilot.permission.pending` / `pilot.permission.resolved`)
7+
// carry their payload under `ev.properties`. Named-event SSE wrappers may carry
8+
// the payload under `ev.data` instead — both paths are handled.
9+
10+
/**
11+
* Normalize any permission-pending SSE event (native or Codex) to the stable shape
12+
* consumed by handlePermissionRequested in permissions.js.
13+
*
14+
* @param {object} ev Raw SSE event object: { type, properties?, data? }
15+
* @returns {{ id, permissionID, description, title, sessionID, type, pattern, metadata }}
16+
*/
17+
export function normalizePermissionPending(ev) {
18+
const d = ev.data ?? ev
19+
const props = ev.properties ?? d ?? {}
20+
return {
21+
id: props.permissionID ?? props.id,
22+
permissionID: props.permissionID ?? props.id,
23+
description: props.title ?? props.description,
24+
title: props.title,
25+
sessionID: props.sessionID,
26+
type: props.permissionType ?? props.type,
27+
pattern: props.pattern,
28+
metadata: props.metadata,
29+
}
30+
}
31+
32+
/**
33+
* Normalize any permission-resolved SSE event (native or Codex) to the stable shape
34+
* consumed by handlePermissionResolved in permissions.js.
35+
*
36+
* @param {object} ev Raw SSE event object: { type, properties?, data? }
37+
* @returns {{ id, permissionID }}
38+
*/
39+
export function normalizePermissionResolved(ev) {
40+
const d = ev.data ?? ev
41+
const resolvedProps = ev.properties ?? d ?? {}
42+
return {
43+
id: resolvedProps.permissionID ?? resolvedProps.id,
44+
permissionID: resolvedProps.permissionID ?? resolvedProps.id,
45+
}
46+
}

src/dashboard/sse/sse.js

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { isFileEditingToolEvent } from '../components/files-changed.js'
1717
import { debouncedRefreshFilesChanged } from '../components/files-changed-bridge.js'
1818
import { buildApiUrl } from '../api/api.js'
1919
import { EVENTS, LIMITS } from '../constants.js'
20+
import { normalizePermissionPending, normalizePermissionResolved } from './permission-normalize.js'
2021
import { playNotifySound } from '../ui/notif-sound.js'
2122

2223
let eventSource = null
@@ -77,6 +78,12 @@ const SSE_EVENTS = [
7778
EVENTS.MESSAGE_PART_DELTA,
7879
EVENTS.PERMISSION_REQUESTED,
7980
EVENTS.PERMISSION_RESOLVED,
81+
// Codex bridge alias events — same handler paths, different type strings.
82+
// The server does not send named `event:` SSE lines today (all events arrive
83+
// as unnamed `data:` messages handled by onmessage), but the list must stay
84+
// complete so named-event subscriptions are correct if the server ever does.
85+
EVENTS.PERMISSION_PENDING_PILOT,
86+
EVENTS.PERMISSION_RESOLVED_PILOT,
8087
EVENTS.STATUS_CHANGED,
8188
EVENTS.TODO_UPDATED,
8289
]
@@ -458,32 +465,21 @@ async function handleEvent(ev) {
458465
}
459466
}
460467

461-
if (t === EVENTS.PERMISSION_REQUESTED) {
462-
// Pilot events carry payload under .properties (see notifications.ts).
463-
// Normalize to a stable shape for handlers that don't know about .properties.
464-
const props = ev.properties ?? d ?? {}
465-
const normalized = {
466-
id: props.permissionID ?? props.id,
467-
permissionID: props.permissionID ?? props.id,
468-
description: props.title ?? props.description,
469-
title: props.title,
470-
sessionID: props.sessionID,
471-
type: props.permissionType ?? props.type,
472-
pattern: props.pattern,
473-
metadata: props.metadata,
474-
}
468+
// Handle both native OpenCode permission events and Codex bridge aliases.
469+
// Codex emits `pilot.permission.pending` / `pilot.permission.resolved` — renaming
470+
// those would break Telegram/push consumers, so we alias here instead.
471+
if (t === EVENTS.PERMISSION_REQUESTED || t === EVENTS.PERMISSION_PENDING_PILOT) {
472+
// Payload may be under .properties (pilot events) or .data (named-event path).
473+
// normalizePermissionPending handles both shapes identically.
474+
const normalized = normalizePermissionPending(ev)
475475
handlePermissionRequested(normalized)
476476
// Dispatch for push-notifications module
477477
window.dispatchEvent(new CustomEvent('pilot:permission:pending', { detail: normalized }))
478478
}
479479

480-
if (t === EVENTS.PERMISSION_RESOLVED) {
481-
// pilot.permission.resolved also carries payload under .properties.
482-
const resolvedProps = ev.properties ?? d ?? {}
483-
const resolvedNormalized = {
484-
id: resolvedProps.permissionID ?? resolvedProps.id,
485-
permissionID: resolvedProps.permissionID ?? resolvedProps.id,
486-
}
480+
if (t === EVENTS.PERMISSION_RESOLVED || t === EVENTS.PERMISSION_RESOLVED_PILOT) {
481+
// Payload may be under .properties (pilot events) or .data (named-event path).
482+
const resolvedNormalized = normalizePermissionResolved(ev)
487483
handlePermissionResolved(resolvedNormalized)
488484
}
489485

0 commit comments

Comments
 (0)