Skip to content

Commit 2c3a0e2

Browse files
garethxclaude
andcommitted
fix: report real counts, and mark every page that is not the whole story
From the Hermes plugin's closed PRs. Of the five I had not already reviewed, one applied directly, two were already handled, and two do not exist here. hookdeck_recent_deliveries returned a capped page of open Issues with no total and no marker. Asked "what needs attention?", a model sees twenty and has nothing to tell it there are four hundred — so it reports what it can see, which for a tool whose job is saying what is true is the worst way to be wrong. It now returns `openIssuesTotal` from the count endpoint, plus `openIssuesTruncated` when the page is short of it, and `localTruncated` for the local records. hookdeck_status.deadLetters is the local log's true size, but the log evicts oldest-first at its cap, so a full log reports a floor rather than a total. `deadLettersIsAtLeast: true` now says so. hookdeck_status's issue count and hookdeck_issues' total were already counted rather than measured. Not applicable: the CI gate reading the legacy commit-status API (we gate on the workflow itself, and have no release gate to get wrong) and the dashboard bundle tests (there is no dashboard here). The tunnel escalation from #7 is the same change already made. Also borrowed the practice rather than a fix: every safety-critical guard was mutation-checked by breaking it and confirming the suite fails. All eight are covered — the pause ceiling, the allowMutations gate, the read-only refusal, the attempt-count ceiling, the pre-verification dead-letter gate, the backoff reset rule, the retry-rule coverage check and the new project-mismatch check — each caught by between one and four tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 25fbfd6 commit 2c3a0e2

6 files changed

Lines changed: 144 additions & 6 deletions

File tree

docs/agent-tools.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,11 @@ Payload text from a webhook is third-party input, and the tools treat it that wa
8585
- The delivered body is **opt-in** (`includeBody`), truncated at 4,000 characters, and labelled as data rather than presented as something addressed to the reader.
8686
- The `hookdeck listen` child's output is scrubbed of the API key as it is captured, not as it is read — that output is surfaced by `hookdeck_status` and we do not write it, so a future CLI version echoing a key into a banner would otherwise land it in a model's context with nothing here having changed.
8787
- A test asserts that no configured secret appears in *any* tool's result, so the next tool added inherits the check.
88+
89+
## Counts are counted
90+
91+
A status tool that returns a page and lets the reader infer a total is worse than one that says nothing: a model asked "what needs attention?" will report what it can see as though it were everything.
92+
93+
- `hookdeck_status.openIssues` and `hookdeck_issues`' `total` come from Hookdeck's count endpoint, not from the length of a page.
94+
- `hookdeck_recent_deliveries` returns `openIssuesTotal` beside the page it shows, and an `openIssuesTruncated` note whenever the two differ. The local records get the same treatment via `localTruncated`.
95+
- `hookdeck_status.deadLetters` is the local log's true size, but the log evicts oldest-first at its cap — so once it is full, `deadLettersIsAtLeast: true` says the number is a floor. A floor reported as a floor beats a ceiling reported as a total.

src/store/deadletter.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ export interface DeadLetterLog {
6363
): Promise<DeadLetterRecord>;
6464
list(limit?: number): DeadLetterRecord[];
6565
count(): number;
66+
/** The cap at which the log evicts oldest-first, so callers can say "at least". */
67+
capacity(): number;
6668
close(): Promise<void>;
6769
stats(): { entries: number; persistence: PersistenceState };
6870
}
@@ -143,6 +145,10 @@ export async function createDeadLetterLog(
143145
return store.values().length;
144146
},
145147

148+
capacity() {
149+
return maxEntries;
150+
},
151+
146152
async close() {
147153
await store.close();
148154
},

src/tools/deliveries.ts

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@ export async function recentDeliveriesHandler(
2020

2121
// Filter first, then limit: filtering a pre-truncated page silently returns
2222
// fewer rows than asked for.
23-
const local = deps.deadLetter
23+
const matching = deps.deadLetter
2424
.list(500)
25-
.filter((r) => params.routeId === undefined || r.routeId === params.routeId)
26-
.slice(0, limit);
25+
.filter(
26+
(r) => params.routeId === undefined || r.routeId === params.routeId,
27+
);
28+
const local = matching.slice(0, limit);
2729

2830
// Joined rather than returned separately: Hookdeck's view and ours disagree
2931
// precisely when something interesting happened.
@@ -54,6 +56,7 @@ export async function recentDeliveriesHandler(
5456
// place to be wrong.
5557
let issues: ReturnType<typeof summariseIssue>[] | null = null;
5658
let issuesNote: string | undefined;
59+
let openIssuesTotal: number | null = null;
5760
if (deps.client !== undefined) {
5861
const result = await deps.client.listIssues({
5962
status: "OPENED",
@@ -62,6 +65,12 @@ export async function recentDeliveriesHandler(
6265
if (result.ok) {
6366
const names = await resolveConnectionNames(deps.client, result.data);
6467
issues = result.data.map((i) => summariseIssue(i, names));
68+
69+
// Counted, never inferred from the page. A tool that returns twenty of
70+
// four hundred issues without saying so leaves a model to guess at the
71+
// total, and it will guess from what it can see.
72+
const total = await deps.client.countIssues({ status: "OPENED" });
73+
if (total.ok) openIssuesTotal = total.data;
6574
} else {
6675
issuesNote = `Could not read Hookdeck Issues: ${result.message}`;
6776
}
@@ -80,13 +89,30 @@ export async function recentDeliveriesHandler(
8089
source: deps.source,
8190
// The real DLQ.
8291
openIssues: issues,
92+
/**
93+
* How many there actually are, counted rather than measured from the page.
94+
* Null when it could not be counted, which is different from zero.
95+
*/
96+
openIssuesTotal,
97+
...(issues !== null &&
98+
openIssuesTotal !== null &&
99+
issues.length < openIssuesTotal
100+
? {
101+
openIssuesTruncated: `Showing ${issues.length} of ${openIssuesTotal} open issues. Raise limit, or use hookdeck_issues.`,
102+
}
103+
: {}),
83104
/**
84105
* Failures Hookdeck cannot see, because we had already answered 2xx when
85106
* they happened. No Issue will ever open for these.
86107
*/
87108
unreportedFailures: postAck,
88109
/** Local mirror of failures Hookdeck also recorded; prefer the Issue. */
89110
locallyRecorded: rows.filter((r) => r.hookdeckVisible === true),
111+
...(matching.length > local.length
112+
? {
113+
localTruncated: `Showing ${local.length} of ${matching.length} local records. Raise limit to see more.`,
114+
}
115+
: {}),
90116
...(issuesNote !== undefined ? { note: issuesNote } : {}),
91117
...(issues !== null && issues.length === 0 && postAck.length === 0
92118
? {

src/tools/status.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,14 @@ export async function statusHandler(
6868
: {}),
6969
},
7070
deadLetters: deps.deadLetter.count(),
71+
/**
72+
* The log evicts oldest-first at its cap, so a count sitting at the cap is
73+
* a floor rather than a total. Said out loud: a model reporting "500
74+
* failures" as the number is worse than one reporting "at least 500".
75+
*/
76+
...(deps.deadLetter.count() >= deps.deadLetter.capacity()
77+
? { deadLettersIsAtLeast: true }
78+
: {}),
7179
retryCancellations: deps.retryCancels?.() ?? null,
7280
transport: {
7381
mode: deps.config.transport.mode,

test/config-parse.test.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,9 @@ describe("project pinning", () => {
447447
const result = parseHookdeckConfig({
448448
signingSecret: "whsec",
449449
projectId: "tm_abc",
450-
routes: { a: { source: "a", dispatch: { mode: "wake", sessionKey: "m" } } },
450+
routes: {
451+
a: { source: "a", dispatch: { mode: "wake", sessionKey: "m" } },
452+
},
451453
});
452454
expect(result.ok).toBe(true);
453455
if (result.ok) expect(result.config.projectId).toBe("tm_abc");
@@ -457,9 +459,12 @@ describe("project pinning", () => {
457459
const result = parseHookdeckConfig({
458460
signingSecret: "whsec",
459461
transport: { mode: "cli", cliConfigPath: "/custom/config.toml" },
460-
routes: { a: { source: "a", dispatch: { mode: "wake", sessionKey: "m" } } },
462+
routes: {
463+
a: { source: "a", dispatch: { mode: "wake", sessionKey: "m" } },
464+
},
461465
});
462466
expect(result.ok).toBe(true);
463-
if (result.ok) expect(result.config.transport.cliConfigPath).toBe("/custom/config.toml");
467+
if (result.ok)
468+
expect(result.config.transport.cliConfigPath).toBe("/custom/config.toml");
464469
});
465470
});

test/tools.test.ts

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1509,3 +1509,88 @@ describe("doctor diagnoses a CLI/API-key project mismatch", () => {
15091509
).toBeUndefined();
15101510
});
15111511
});
1512+
1513+
describe("counts are counted, never measured from a page", () => {
1514+
// A tool that returns twenty of four hundred without saying so leaves a
1515+
// model to guess the total, and it will guess from what it can see.
1516+
it("reports the real open-issue total alongside the page", async () => {
1517+
const d = await deps({
1518+
client: fakeClient({
1519+
countIssues: vi.fn(async () => ({ ok: true as const, data: 400 })),
1520+
}),
1521+
});
1522+
const result = await recentDeliveriesHandler(d, {});
1523+
1524+
expect(result.openIssuesTotal).toBe(400);
1525+
expect(result.openIssues).toHaveLength(1);
1526+
expect(String(result.openIssuesTruncated)).toMatch(/Showing 1 of 400/);
1527+
});
1528+
1529+
it("does not claim truncation when the page holds everything", async () => {
1530+
const d = await deps();
1531+
const result = await recentDeliveriesHandler(d, {});
1532+
expect(result.openIssuesTotal).toBe(1);
1533+
expect(result.openIssuesTruncated).toBeUndefined();
1534+
});
1535+
1536+
it("distinguishes an uncountable total from zero", async () => {
1537+
const d = await deps({
1538+
client: fakeClient({
1539+
countIssues: vi.fn(async () => ({
1540+
ok: false as const,
1541+
code: "api_error",
1542+
message: "boom",
1543+
})),
1544+
}),
1545+
});
1546+
expect((await recentDeliveriesHandler(d, {})).openIssuesTotal).toBeNull();
1547+
});
1548+
1549+
it("says how many local records it left out", async () => {
1550+
const d = await deps();
1551+
for (let i = 0; i < 30; i += 1) {
1552+
await d.deadLetter.record({
1553+
eventId: `evt_${i}`,
1554+
routeId: "stripe",
1555+
code: "c",
1556+
reason: "r",
1557+
retriesCancelled: false,
1558+
lastAttempt: true,
1559+
});
1560+
}
1561+
const result = await recentDeliveriesHandler(d, { limit: 5 });
1562+
expect(String(result.localTruncated)).toMatch(/Showing 5 of 30/);
1563+
});
1564+
1565+
it("marks a full dead-letter log as a floor, not a total", async () => {
1566+
// The log evicts oldest-first at its cap, so "500" is "at least 500".
1567+
const d = await deps();
1568+
const { createDeadLetterLog } = await import("../src/store/deadletter.js");
1569+
d.deadLetter = await createDeadLetterLog({ ttlHours: 168, maxEntries: 3 });
1570+
for (let i = 0; i < 5; i += 1) {
1571+
await d.deadLetter.record({
1572+
eventId: `evt_${i}`,
1573+
code: "c",
1574+
reason: "r",
1575+
retriesCancelled: false,
1576+
lastAttempt: true,
1577+
});
1578+
}
1579+
1580+
const result = await statusHandler(d, {});
1581+
expect(result.deadLetters).toBe(3);
1582+
expect(result.deadLettersIsAtLeast).toBe(true);
1583+
});
1584+
1585+
it("does not mark a log below its cap", async () => {
1586+
const d = await deps();
1587+
await d.deadLetter.record({
1588+
eventId: "evt_1",
1589+
code: "c",
1590+
reason: "r",
1591+
retriesCancelled: false,
1592+
lastAttempt: true,
1593+
});
1594+
expect((await statusHandler(d, {})).deadLettersIsAtLeast).toBeUndefined();
1595+
});
1596+
});

0 commit comments

Comments
 (0)