fix(hooks): store a hook delivered twice only once
A doubled hook registration (plugin + leftover checkout install) POSTs every event twice with a byte-identical payload, so the timeline showed each row twice and token/alert/lane side effects ran twice. POST /api/hooks/event now skips a payload identical to the session's LATEST event from the last 10s (findRecentDuplicateEvent). Latest-only keeps two legitimately identical Stop events of consecutive turns, which always have other events between.
This commit is contained in:
+5
-3
@@ -159,9 +159,11 @@ npm run install-hooks
|
|||||||
> **Skip this entirely when the `agenthub` plugin is installed.** The plugin declares
|
> **Skip this entirely when the `agenthub` plugin is installed.** The plugin declares
|
||||||
> the same eight hooks itself (inline in `.claude-plugin/plugin.json`, each
|
> the same eight hooks itself (inline in `.claude-plugin/plugin.json`, each
|
||||||
> running `${CLAUDE_PLUGIN_ROOT}/scripts/hook-handler.js`). Running both means
|
> running `${CLAUDE_PLUGIN_ROOT}/scripts/hook-handler.js`). Running both means
|
||||||
> every event is POSTed twice — events carry no id, so ingest cannot deduplicate
|
> every event is POSTed twice. Events carry no id, so ingest can only drop a
|
||||||
> them and every token and cost figure doubles — as does every event in the feed
|
> POST whose payload is byte-identical to the session's LATEST event from the last
|
||||||
> and every subagent row (one `Agent` call producing two cards). `scripts/plugin-bootstrap.js`
|
> 10 seconds (`findRecentDuplicateEvent`, since v1.8.1) — that stops the doubled
|
||||||
|
> feed rows, token/cost figures and subagent cards, but the second hook process
|
||||||
|
> still runs on every event, so remove the duplicate registration anyway. `scripts/plugin-bootstrap.js`
|
||||||
> removes checkout-installed entries on session start (backup:
|
> removes checkout-installed entries on session start (backup:
|
||||||
> `~/.claude/settings.json.agenthub-bak`) — and since v1.6.17 a settings file that
|
> `~/.claude/settings.json.agenthub-bak`) — and since v1.6.17 a settings file that
|
||||||
> still carries them also blocks the bootstrap's fast path (`hasLegacyHooks`),
|
> still carries them also blocks the bootstrap's fast path (`hasLegacyHooks`),
|
||||||
|
|||||||
@@ -216,13 +216,13 @@ describe("Event-driven alert evaluation", () => {
|
|||||||
const ruleId = created.body.rule.id;
|
const ruleId = created.body.rule.id;
|
||||||
|
|
||||||
const sessionId = `alerts-burst-${Date.now()}`;
|
const sessionId = `alerts-burst-${Date.now()}`;
|
||||||
await postHook("BurstProbe", { session_id: sessionId });
|
await postHook("BurstProbe", { session_id: sessionId, seq: 1 });
|
||||||
await postHook("BurstProbe", { session_id: sessionId });
|
await postHook("BurstProbe", { session_id: sessionId, seq: 2 });
|
||||||
|
|
||||||
let feed = await fetch("/api/alerts");
|
let feed = await fetch("/api/alerts");
|
||||||
assert.equal(feed.body.alerts.filter((a) => a.rule_id === ruleId).length, 0);
|
assert.equal(feed.body.alerts.filter((a) => a.rule_id === ruleId).length, 0);
|
||||||
|
|
||||||
await postHook("BurstProbe", { session_id: sessionId });
|
await postHook("BurstProbe", { session_id: sessionId, seq: 3 });
|
||||||
feed = await fetch("/api/alerts");
|
feed = await fetch("/api/alerts");
|
||||||
const fired = feed.body.alerts.filter((a) => a.rule_id === ruleId);
|
const fired = feed.body.alerts.filter((a) => a.rule_id === ruleId);
|
||||||
assert.equal(fired.length, 1);
|
assert.equal(fired.length, 1);
|
||||||
|
|||||||
@@ -617,6 +617,37 @@ describe("Hook Event Processing", () => {
|
|||||||
assert.equal(res.body.error.code, "MISSING_SESSION");
|
assert.equal(res.body.error.code, "MISSING_SESSION");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("stores a hook delivered twice only once (double registration)", async () => {
|
||||||
|
const hook = {
|
||||||
|
hook_type: "PreToolUse",
|
||||||
|
data: {
|
||||||
|
session_id: "hook-dup-1",
|
||||||
|
tool_name: "Bash",
|
||||||
|
tool_use_id: "toolu_dup_1",
|
||||||
|
tool_input: { command: "ls", description: "List files" },
|
||||||
|
},
|
||||||
|
};
|
||||||
|
const first = await post("/api/hooks/event", hook);
|
||||||
|
const second = await post("/api/hooks/event", hook);
|
||||||
|
assert.equal(first.status, 200);
|
||||||
|
assert.equal(second.status, 200);
|
||||||
|
assert.equal(second.body.duplicate, true);
|
||||||
|
const { n } = db
|
||||||
|
.prepare("SELECT COUNT(*) AS n FROM events WHERE session_id = 'hook-dup-1'")
|
||||||
|
.get();
|
||||||
|
assert.equal(n, 1);
|
||||||
|
|
||||||
|
// A DIFFERENT payload in the same session still lands.
|
||||||
|
await post("/api/hooks/event", {
|
||||||
|
...hook,
|
||||||
|
data: { ...hook.data, tool_use_id: "toolu_dup_2" },
|
||||||
|
});
|
||||||
|
const after = db
|
||||||
|
.prepare("SELECT COUNT(*) AS n FROM events WHERE session_id = 'hook-dup-1'")
|
||||||
|
.get();
|
||||||
|
assert.equal(after.n, 2);
|
||||||
|
});
|
||||||
|
|
||||||
it("should auto-create session and main agent on first PreToolUse", async () => {
|
it("should auto-create session and main agent on first PreToolUse", async () => {
|
||||||
const res = await post("/api/hooks/event", {
|
const res = await post("/api/hooks/event", {
|
||||||
hook_type: "PreToolUse",
|
hook_type: "PreToolUse",
|
||||||
|
|||||||
@@ -172,7 +172,7 @@ describe("hook ingestor — sessions.name sync from transcript", () => {
|
|||||||
]);
|
]);
|
||||||
await req("POST", "/api/hooks/event", {
|
await req("POST", "/api/hooks/event", {
|
||||||
hook_type: "Stop",
|
hook_type: "Stop",
|
||||||
data: { session_id: sid, cwd, transcript_path: tpath },
|
data: { session_id: sid, cwd, transcript_path: tpath, turn: 1 },
|
||||||
});
|
});
|
||||||
assert.equal(stmts.getSession.get(sid).name, "keep-me");
|
assert.equal(stmts.getSession.get(sid).name, "keep-me");
|
||||||
|
|
||||||
@@ -184,7 +184,7 @@ describe("hook ingestor — sessions.name sync from transcript", () => {
|
|||||||
]);
|
]);
|
||||||
await req("POST", "/api/hooks/event", {
|
await req("POST", "/api/hooks/event", {
|
||||||
hook_type: "Stop",
|
hook_type: "Stop",
|
||||||
data: { session_id: sid, cwd, transcript_path: tpath },
|
data: { session_id: sid, cwd, transcript_path: tpath, turn: 2 },
|
||||||
});
|
});
|
||||||
assert.equal(stmts.getSession.get(sid).name, "renamed-explicitly");
|
assert.equal(stmts.getSession.get(sid).name, "renamed-explicitly");
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1476,6 +1476,17 @@ const stmts = {
|
|||||||
AND updated_at < strftime('%Y-%m-%dT%H:%M:%fZ', 'now', '-' || ? || ' minutes')`
|
AND updated_at < strftime('%Y-%m-%dT%H:%M:%fZ', 'now', '-' || ? || ' minutes')`
|
||||||
),
|
),
|
||||||
|
|
||||||
|
// A second delivery of one hook: the session's LATEST event, a few seconds
|
||||||
|
// old, with the same type and a byte-identical payload (the duplicate guard
|
||||||
|
// in routes/hooks.js). Latest only, never "any recent": two turns can end in
|
||||||
|
// identical Stop payloads, but real work always lands between them, while a
|
||||||
|
// double delivery arrives back to back.
|
||||||
|
findRecentDuplicateEvent: db.prepare(
|
||||||
|
`SELECT 1 FROM (SELECT event_type, data, created_at FROM events
|
||||||
|
WHERE session_id = ? ORDER BY id DESC LIMIT 1)
|
||||||
|
WHERE event_type = ? AND data = ?
|
||||||
|
AND created_at >= strftime('%Y-%m-%dT%H:%M:%fZ', 'now', '-10 seconds')`
|
||||||
|
),
|
||||||
insertEvent: db.prepare(
|
insertEvent: db.prepare(
|
||||||
"INSERT INTO events (session_id, agent_id, event_type, tool_name, summary, data, created_at) VALUES (?, ?, ?, ?, ?, ?, strftime('%Y-%m-%dT%H:%M:%fZ', 'now'))"
|
"INSERT INTO events (session_id, agent_id, event_type, tool_name, summary, data, created_at) VALUES (?, ?, ?, ?, ?, ?, strftime('%Y-%m-%dT%H:%M:%fZ', 'now'))"
|
||||||
),
|
),
|
||||||
|
|||||||
@@ -1144,6 +1144,20 @@ router.post("/event", (req, res) => {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The same hook delivered twice — AgentHub's hooks registered twice (plugin
|
||||||
|
// + a leftover checkout install, two marketplaces), or two servers on one DB
|
||||||
|
// that both got the POST — carries a byte-identical payload. Store it once,
|
||||||
|
// and skip the lane/alert side effects too, or every timeline row, status
|
||||||
|
// flip and alert doubles.
|
||||||
|
// ponytail: DB lookup, so two servers committing the same hook in the same
|
||||||
|
// instant can still both win; a unique payload-hash column closes that.
|
||||||
|
if (
|
||||||
|
data.session_id &&
|
||||||
|
stmts.findRecentDuplicateEvent.get(data.session_id, hook_type, JSON.stringify(data))
|
||||||
|
) {
|
||||||
|
return res.json({ ok: true, duplicate: true });
|
||||||
|
}
|
||||||
|
|
||||||
const result = processEvent(hook_type, data);
|
const result = processEvent(hook_type, data);
|
||||||
if (!result) {
|
if (!result) {
|
||||||
return res.status(400).json({
|
return res.status(400).json({
|
||||||
|
|||||||
Reference in New Issue
Block a user