Skip to content
This repository was archived by the owner on Jun 8, 2026. It is now read-only.

Commit 8db7ef9

Browse files
author
Andrew Eye
committed
FEA-1444: Add Codex hook event dedup + renderer opt-in toggle
Layer 2 — Event-level dedup against rollout-tail: - New filterEventsAlreadyCapturedByHooks in codex-import.js. Runs before importSession in importCodexSession; queries existing events for rows matching (session_id, event_type, COALESCE(tool_name, ''), created_at truncated to whole seconds) and drops duplicates. Hook-handler-inserted rows land first (real-time); the ~5s-later rollout-tail no-ops on match. - The dedup statement is cached per-dbModule via WeakMap so a batch of many changed sessions compiles the SQL once, not once per session. - Best-effort: any prepare/query failure is swallowed and the event is kept. False negatives (cosmetic duplicates) are preferred over false positives (silently dropped events). Sub-second timestamp drift handled by the substr(?, 1, 19) match. Layer 3 — Renderer opt-in toggle: - apps/desktop/src/main/app.ts: new IPC handlers desktop:{get,set}-agent-monitor-codex-hooks-opt-in that call the already-exported isAgentMonitorCodexHooksOptIn / setAgentMonitorCodexHooksOptIn. Gated on the master Agent Dashboard flag, matching the Claude pair shape exactly. - apps/desktop/src/main/preload.ts: mirror bridge methods exposing the two channels to the renderer. - apps/desktop/src/renderer/index.html: new #codexDashConsent toggle row directly below the existing #claudeDashConsent. Same visibility semantics (shown only on agent-settings route), same disabled-when- master-off behavior, same hint text reset on master flag re-enable (Codex hint reset now mirrors Claude's after a review-flagged gap). refreshCodexHooksToggle is called when the user toggles the Claude hooks toggle so the sibling's on/off badge stays in sync. Test: - apps/desktop/test/agent-session-event-dedup.test.ts (new): 4 unit tests on the filter against a sandbox SQLite DB. Covers (1) duplicate filter with sub-second drift, (2) non-duplicate survival across mixed events, (3) best-effort behavior on DB errors, (4) empty-events no-op. Uses the conditional-skip pattern (skipReason ? { skip: ... } : undefined) because Node test runner treats skip: null as "skip with no reason" rather than "don't skip" — gotcha learned during debugging. Independent code review (second pass on the cumulative diff) surfaced: - Medium: Codex hint stale after master flag re-enable (Claude hint was reset in the else branch, Codex hint wasn't). Fixed. - Low: Comment inaccuracy about which toggle "unblocks the master flag" in wireClaudeHooksToggle. Rewritten. - Low: Dedup statement prepared once per session inside the batch loop instead of once per process. Cached via WeakMap. All three addressed in this commit. Testing: - pnpm -C apps/desktop typecheck: clean - pnpm -C apps/desktop lint: clean - pnpm -C apps/desktop build:agent-monitor: clean (Layer 1 hard-gates + sidecar SQLite gate continue to pass) - pnpm -C apps/desktop test (full suite): 1945/1945 pass, 0 fail - New agent-session-event-dedup.test.ts: 4/4 pass - agent-monitor-hooks-core.test.ts (from Layer 1): 10/10 pass (no regression) Risks: - Cross-second timestamp drift between hook and rollout-tail can defeat dedup. Documented design tradeoff: false negatives are cosmetic duplicates; false positives would silently drop events. Will revisit with a content-hash dedup key in a follow-up if cosmetic duplication becomes visible in practice. - IPC handler test coverage deferred. Both new handlers are shallow plumbing that call already-tested functions (covered by Layer 1's agent-monitor-hooks-core.test.ts). Typecheck validates the signatures + preload bridge types. - Renderer JS duplicates the Claude/Codex toggle pattern. Per CLAUDE.md this is the kind of duplication worth extracting, but inline renderer JS makes a shared helper awkward; deferred to a separate cleanup ticket if the pattern repeats with a third harness.
1 parent 87a601c commit 8db7ef9

5 files changed

Lines changed: 421 additions & 3 deletions

File tree

apps/desktop/scripts/agent-monitor-codex/codex-import.js

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,16 @@ const catchupCache = createCatchupCache({ persistPath: ingestCachePath("codex")
3030
* or { skipped: true } when the file has no usable content.
3131
*/
3232
function importCodexSession(dbModule, session) {
33+
// FEA-1444 dedup: if the user opted into Codex hooks, the same logical
34+
// event will already be in `events` (inserted ~5s earlier by the hook
35+
// handler). Without this filter the rollout-tail importer would create a
36+
// duplicate row for every Codex event after the user opts in. Match on
37+
// (session_id, event_type, tool_name, created_at-truncated-to-second) —
38+
// hooks and rollout-tail timestamps usually agree within sub-second
39+
// granularity for the same logical event. False negatives are tolerable
40+
// (cosmetic duplicates); false positives would silently drop events, so
41+
// the match is intentionally narrow.
42+
filterEventsAlreadyCapturedByHooks(dbModule, session);
3343
const result = importSession(dbModule, session);
3444
// Stamp the harness regardless of skipped/backfilled — cheap, idempotent,
3545
// and self-heals rows imported before the `harness` column existed.
@@ -42,6 +52,73 @@ function importCodexSession(dbModule, session) {
4252
return { sessionId: session.sessionId, result, reactivated };
4353
}
4454

55+
/**
56+
* FEA-1444: filter session.events in place, removing any whose
57+
* (session_id, event_type, COALESCE(tool_name, ''), created_at-rounded-to-second)
58+
* already exists in the `events` table. The hook handler inserts in real time;
59+
* rollout-tail catches up ~5s later — so the dedup query consistently sees
60+
* hook-sourced rows first when both paths are active.
61+
*
62+
* No-op when session.events is empty or the events query fails (best-effort;
63+
* never block the import on a dedup-time error).
64+
*/
65+
// FEA-1444 dedup-stmt cache: the filter runs per-session inside
66+
// importCodexSession, which itself runs inside the importBatch transaction
67+
// loop. Caching the prepared statement on the dbModule keeps us at O(1)
68+
// compilations per process instead of O(sessions) per batch. The cache is
69+
// keyed by dbModule so a fresh DatabaseSync in tests gets a fresh stmt.
70+
const dedupStmtCache = new WeakMap();
71+
function getDedupCheckStmt(dbModule) {
72+
let stmt = dedupStmtCache.get(dbModule);
73+
if (stmt) return stmt;
74+
// Truncate created_at to seconds via `substr(?, 1, 19)` so sub-second
75+
// timestamp drift between the hook payload and the rollout file doesn't
76+
// produce a false negative.
77+
stmt = dbModule.db.prepare(
78+
"SELECT 1 FROM events " +
79+
"WHERE session_id = ? AND event_type = ? " +
80+
"AND COALESCE(tool_name, '') = COALESCE(?, '') " +
81+
"AND substr(COALESCE(created_at, ''), 1, 19) = substr(COALESCE(?, ''), 1, 19) " +
82+
"LIMIT 1",
83+
);
84+
dedupStmtCache.set(dbModule, stmt);
85+
return stmt;
86+
}
87+
88+
function filterEventsAlreadyCapturedByHooks(dbModule, session) {
89+
if (!session || !Array.isArray(session.events) || session.events.length === 0) {
90+
return;
91+
}
92+
let checkStmt;
93+
try {
94+
checkStmt = getDedupCheckStmt(dbModule);
95+
} catch {
96+
return; // schema mismatch or db locked — fall through, accept duplicates
97+
}
98+
const filtered = [];
99+
for (const ev of session.events) {
100+
if (!ev || typeof ev !== "object") {
101+
filtered.push(ev);
102+
continue;
103+
}
104+
try {
105+
const hit = checkStmt.get(
106+
session.sessionId,
107+
ev.event_type ?? null,
108+
ev.tool_name ?? null,
109+
ev.created_at ?? null,
110+
);
111+
if (hit) {
112+
continue; // already captured by the hook handler — drop the duplicate
113+
}
114+
} catch {
115+
// best-effort — on any per-row failure, keep the event
116+
}
117+
filtered.push(ev);
118+
}
119+
session.events = filtered;
120+
}
121+
45122
/**
46123
* Parse + import every discovered Codex rollout file. Designed to be cheap on
47124
* repeat runs: importSession skips already-imported sessions (or backfills
@@ -107,4 +184,9 @@ async function importAllCodexSessions(dbModule, opts = {}) {
107184
return { imported, skipped, errors };
108185
}
109186

110-
module.exports = { importAllCodexSessions, importCodexSession };
187+
module.exports = {
188+
importAllCodexSessions,
189+
importCodexSession,
190+
// Exposed for regression coverage of FEA-1444 dedup.
191+
filterEventsAlreadyCapturedByHooks,
192+
};

apps/desktop/src/main/app.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,9 @@ import { DesktopWindow } from "./window.js";
6969
import { AgentMonitorSidecar } from "./agent-monitor-sidecar.js";
7070
import { AgentSessionSyncService } from "./agent-session-sync-service.js";
7171
import {
72+
isAgentMonitorCodexHooksOptIn,
7273
isAgentMonitorHooksEnabled,
74+
setAgentMonitorCodexHooksOptIn,
7375
setAgentMonitorHooksEnabled,
7476
syncAgentMonitorHooksOnBoot,
7577
} from "./agent-monitor-hooks.js";
@@ -2499,6 +2501,26 @@ export class DesktopApplication {
24992501
return setAgentMonitorHooksEnabled(enabled === true);
25002502
},
25012503
);
2504+
// FEA-1444: opt-in toggle for Codex hooks. Surfaced as a sibling of the
2505+
// Claude hooks toggle in the Agent Dashboard view. Gated on the master
2506+
// Agent Dashboard flag for the same reason the Claude toggle is — the
2507+
// sidecar must be running to receive the forwarded hook events.
2508+
ipcMain.handle("desktop:get-agent-monitor-codex-hooks-opt-in", () =>
2509+
this.isAgentMonitorEnabled() && isAgentMonitorCodexHooksOptIn(),
2510+
);
2511+
ipcMain.handle(
2512+
"desktop:set-agent-monitor-codex-hooks-opt-in",
2513+
(_event, optIn: boolean) => {
2514+
if (!this.isAgentMonitorEnabled()) {
2515+
return {
2516+
ok: false,
2517+
enabled: false,
2518+
error: "Agent Dashboard is disabled in Settings.",
2519+
};
2520+
}
2521+
return setAgentMonitorCodexHooksOptIn(optIn === true);
2522+
},
2523+
);
25022524
ipcMain.handle("desktop:get-logs", () => gatewayLog.getEntries());
25032525
ipcMain.handle("desktop:clear-logs", () => {
25042526
gatewayLog.clear();

apps/desktop/src/main/preload.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,16 @@ const desktopApi = {
136136
"desktop:set-agent-monitor-hooks-enabled",
137137
enabled,
138138
) as Promise<{ ok: boolean; enabled: boolean; error?: string }>,
139+
// FEA-1444: Codex hook opt-in. Mirrors the Claude pair above.
140+
getAgentMonitorCodexHooksOptIn: () =>
141+
ipcRenderer.invoke(
142+
"desktop:get-agent-monitor-codex-hooks-opt-in",
143+
) as Promise<boolean>,
144+
setAgentMonitorCodexHooksOptIn: (optIn: boolean) =>
145+
ipcRenderer.invoke(
146+
"desktop:set-agent-monitor-codex-hooks-opt-in",
147+
optIn,
148+
) as Promise<{ ok: boolean; enabled: boolean; error?: string }>,
139149
getAllFlags: () =>
140150
ipcRenderer.invoke("desktop:get-all-flags") as Promise<unknown>,
141151
onFlagsChanged: (callback: () => void) => {

apps/desktop/src/renderer/index.html

Lines changed: 103 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3404,6 +3404,17 @@ <h1>See every coding agent you use — in one place</h1>
34043404
<span>Enable Claude Code session tracking</span>
34053405
<span id="claudeDashHooksHint" style="color:var(--muted-foreground,#888)">— off (no changes to your global Claude config)</span>
34063406
</div>
3407+
<!-- FEA-1444: opt-in toggle for Codex hook ingestion. Sibling of the
3408+
Claude toggle above. Requires `[features].codex_hooks = true` in
3409+
~/.codex/config.toml on the Codex side. -->
3410+
<div id="codexDashConsent" style="display:none;align-items:center;gap:10px;padding:10px 14px;border-bottom:1px solid var(--border,#2a2a2a);font-size:13px">
3411+
<label class="toggle-switch" title="Installs Codex CLI hooks in ~/.codex/hooks.json so this dashboard receives live session events from Codex (in addition to the rollout-tail watcher that already runs). Requires `[features].codex_hooks = true` in ~/.codex/config.toml. Off by default; fully removable.">
3412+
<input type="checkbox" id="codexDashHooksToggle" />
3413+
<span class="toggle-track"></span>
3414+
</label>
3415+
<span>Enable Codex CLI session tracking (experimental)</span>
3416+
<span id="codexDashHooksHint" style="color:var(--muted-foreground,#888)">— off (rollout-tail watcher continues)</span>
3417+
</div>
34073418
<div id="claudeDashStatus" class="dash-loading" aria-live="polite"></div>
34083419
<iframe id="claudeDashFrame" title="Claude Code Agent Monitor" style="display:none;width:100%;border:0;background:#fff"></iframe>
34093420
</section>
@@ -3971,6 +3982,11 @@ <h3 class="settings-group-title">Labs</h3>
39713982
if (consent) {
39723983
consent.style.display = id === "agent-settings" ? "flex" : "none";
39733984
}
3985+
// FEA-1444: sibling Codex toggle gets the same visibility treatment.
3986+
const codexConsent = document.getElementById("codexDashConsent");
3987+
if (codexConsent) {
3988+
codexConsent.style.display = id === "agent-settings" ? "flex" : "none";
3989+
}
39743990
startClaudeDashboard();
39753991
navigateAgentRoute(item.route);
39763992
} else {
@@ -4030,13 +4046,26 @@ <h3 class="settings-group-title">Labs</h3>
40304046
const status = document.getElementById("claudeDashStatus");
40314047
const toggle = document.getElementById("claudeDashHooksToggle");
40324048
const hint = document.getElementById("claudeDashHooksHint");
4049+
// FEA-1444: same disable-when-master-off treatment for the Codex toggle.
4050+
const codexToggle = document.getElementById("codexDashHooksToggle");
4051+
const codexHint = document.getElementById("codexDashHooksHint");
40334052

40344053
if (toggle) {
40354054
toggle.disabled = !cachedAgentMonitorEnabled;
40364055
if (!cachedAgentMonitorEnabled) {
40374056
toggle.checked = false;
40384057
}
40394058
}
4059+
if (codexToggle) {
4060+
codexToggle.disabled = !cachedAgentMonitorEnabled;
4061+
if (!cachedAgentMonitorEnabled) {
4062+
codexToggle.checked = false;
4063+
}
4064+
}
4065+
if (codexHint && !cachedAgentMonitorEnabled) {
4066+
codexHint.textContent =
4067+
"— enable Agent Dashboard in Settings to manage session tracking";
4068+
}
40404069

40414070
if (!cachedAgentMonitorEnabled) {
40424071
stopClaudeDashboardPoll();
@@ -4059,8 +4088,16 @@ <h3 class="settings-group-title">Labs</h3>
40594088
activateTab("settings");
40604089
activateSettingsTab("relay-gateway");
40614090
}
4062-
} else if (hint) {
4063-
hint.textContent = "— off (no changes to your global Claude config)";
4091+
} else {
4092+
if (hint) {
4093+
hint.textContent = "— off (no changes to your global Claude config)";
4094+
}
4095+
// FEA-1444: mirror Claude hint reset for Codex so the stale
4096+
// "enable Agent Dashboard in Settings..." text doesn't linger
4097+
// after the user turns the master flag back on.
4098+
if (codexHint) {
4099+
codexHint.textContent = "— off (rollout-tail watcher continues)";
4100+
}
40644101
}
40654102

40664103
// Render the initial view once the enabled-state is known on boot.
@@ -4110,6 +4147,62 @@ <h3 class="settings-group-title">Labs</h3>
41104147
} catch (_) { /* leave as-is */ }
41114148
}
41124149

4150+
// FEA-1444: Codex hook opt-in toggle. Mirrors the Claude pair above.
4151+
async function refreshCodexHooksToggle() {
4152+
const toggle = document.getElementById("codexDashHooksToggle");
4153+
const hint = document.getElementById("codexDashHooksHint");
4154+
if (!toggle) return;
4155+
if (!cachedAgentMonitorEnabled) {
4156+
toggle.disabled = true;
4157+
if (hint) {
4158+
hint.textContent =
4159+
"— enable Agent Dashboard in Settings to manage session tracking";
4160+
}
4161+
return;
4162+
}
4163+
try {
4164+
const optedIn = await api.getAgentMonitorCodexHooksOptIn();
4165+
toggle.checked = !!optedIn;
4166+
toggle.disabled = false;
4167+
if (hint) {
4168+
hint.textContent = optedIn
4169+
? "— on (Codex hooks installed in ~/.codex/hooks.json; also set [features].codex_hooks = true in ~/.codex/config.toml)"
4170+
: "— off (rollout-tail watcher continues)";
4171+
}
4172+
} catch (_) { /* leave as-is */ }
4173+
}
4174+
4175+
(function wireCodexHooksToggle() {
4176+
const toggle = document.getElementById("codexDashHooksToggle");
4177+
const hint = document.getElementById("codexDashHooksHint");
4178+
if (!toggle) return;
4179+
toggle.addEventListener("change", async () => {
4180+
const want = toggle.checked;
4181+
toggle.disabled = true;
4182+
if (hint) hint.textContent = want ? "— enabling…" : "— disabling…";
4183+
try {
4184+
const res = await api.setAgentMonitorCodexHooksOptIn(want);
4185+
if (!res || !res.ok) {
4186+
toggle.checked = !want;
4187+
if (hint) {
4188+
hint.textContent =
4189+
"— failed: " + ((res && res.error) || "unknown error");
4190+
}
4191+
} else {
4192+
await refreshCodexHooksToggle();
4193+
}
4194+
} catch (e) {
4195+
toggle.checked = !want;
4196+
if (hint) {
4197+
hint.textContent =
4198+
"— failed: " + (e && e.message ? e.message : "error");
4199+
}
4200+
} finally {
4201+
toggle.disabled = false;
4202+
}
4203+
});
4204+
})();
4205+
41134206
(function wireClaudeHooksToggle() {
41144207
const toggle = document.getElementById("claudeDashHooksToggle");
41154208
const hint = document.getElementById("claudeDashHooksHint");
@@ -4125,6 +4218,11 @@ <h3 class="settings-group-title">Labs</h3>
41254218
hint.textContent = "— failed: " + ((res && res.error) || "unknown error");
41264219
} else {
41274220
await refreshClaudeHooksToggle();
4221+
// FEA-1444: refresh sibling Codex toggle state so its on/off
4222+
// badge stays in sync after a successful Claude-toggle change.
4223+
// (The master `agentMonitorEnabled` flag, not this toggle,
4224+
// controls the Codex toggle's disabled state.)
4225+
await refreshCodexHooksToggle();
41284226
}
41294227
} catch (e) {
41304228
toggle.checked = !want;
@@ -4161,6 +4259,9 @@ <h3 class="settings-group-title">Labs</h3>
41614259
return;
41624260
}
41634261
void refreshClaudeHooksToggle();
4262+
// FEA-1444: also refresh the sibling Codex toggle on dashboard mount
4263+
// so its on/off badge reflects the persisted state.
4264+
void refreshCodexHooksToggle();
41644265
if (claudeDashLoaded) { sizeClaudeFrame(); return; }
41654266
renderDashLoading();
41664267

0 commit comments

Comments
 (0)