Skip to content

perf(server): push the agent filter into listJobs for /triggers/runtime (#535) - #603

Merged
edspencer merged 2 commits into
mainfrom
perf/535-triggers-runtime-pushdown
Aug 13, 2026
Merged

perf(server): push the agent filter into listJobs for /triggers/runtime (#535)#603
edspencer merged 2 commits into
mainfrom
perf/535-triggers-runtime-pushdown

Conversation

@edspencer

@edspencer edspencer commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Closes #535.

Note

Unblocked and rebased-by-merge (2026-08-04). The agents?: string[] filter this needed (edspencer/herdctl#432) shipped in @herdctl/core@5.28.0, and main has since moved its floor to ^5.31.0 — above what this needs. So origin/main has been merged in (not rebased) and the branch's own @herdctl/core bump has been dropped: the diff no longer touches packages/server/package.json or package-lock.json at all. Re-verified on the merged branch against the installed 5.31.0: 1,657 server tests pass, typecheck clean.


The bug

listRunsForAgents asked core for every job record with neither filter nor limit, then filtered and sliced in JS:

const { jobs } = await listJobs(jobsDir).catch(...);   // no filter, no limit
const filtered = jobs.filter((j) => wanted.has(j.agent));
return limit > 0 ? filtered.slice(0, limit) : filtered;

With filter.limit === undefined, core sets retain = matches — so every record in the shared, never-pruned, fleet-wide jobs directory is read, YAML-parsed and Zod-validated on every call, then thrown away.

It was written that way for a reason, and the comment said so: ListJobsFilter had agent (exactly one) and no multi-agent form, while this route needs a project's agent plus every scoped trigger-<slug>-<name> agent.

Measured

2,016-record jobs dir, three consecutive calls per process, index warm, each mode in its own process:

call 1 (cold index) call 2 call 3
before 1,226 ms 1,047 ms 1,081 ms
after 1,245 ms 83 ms 69 ms

~16× warm. The old shape never warmed — that's the real finding. It paid full price on every request, and /api/projects/:slug/triggers/runtime is polled every 10 s while the Triggers tab is open.

(This reproduces the design note's 1,106 ms warm figure closely. An earlier run of mine showed ~2,400 ms; that was contention, and the per-process measurement above is the clean one.)

Why route (a) — fix it upstream — rather than the Paddock-side workaround

The alternative was calling listJobs(dir, {agent, limit}) once per agent and merging, with no upstream dependency. I measured that before choosing, and it doesn't hold up:

agents per-agent calls, merged
1 144 ms
2 149 ms
3 289 ms
4 582 ms
6 888 ms
8 1,059 ms
(unfiltered — today) 1,615 ms

Each call re-stats the whole directory, so the cost is O(agents × dirsize). A project's agent set is keeper + one per trigger — so a trigger-heavy project regresses back toward the number we're trying to remove, and the "typically a handful" assumption is exactly where this route ends up as triggers get used more. That's a scaling cliff hidden behind an average.

Route (a) is also one Set lookup in core's matches, preserves ordering by construction (one global sort over one candidate list, unchanged), and fixes the more impactful victim named in herdctl#418 — herdctl's own dashboard /api/jobs, a paginated endpoint that was hydrating the entire directory per request.

Releasing both repos is routine here, so I didn't contort the design to avoid it.

Correctness — this is the one of the three with real semantics

Order is what's at risk, not just membership: the old shape sorted the entire directory then filtered; the new one filters first and lets core sort the survivors. Those agree only because core sorts the full candidate set before paging — so I asserted it rather than reasoning about it.

test/unit/herdctl-runs-for-agents.test.ts keeps a literal reimplementation of the pre-#535 function as an oracle and diffs against it, over records deliberately interleaved across agents and timestamps so any per-agent grouping would reorder the result. Also covered: the truncating-limit case (the cap must apply after filtering, or a chatty agent evicts everything wanted), most-recent-first ordering, multi-agent spanning, a repeated agent name (an unscoped schedule trigger legitimately runs under the project agent, so the caller's list can repeat it), empty input, missing jobs dir, and limit <= 0.

On the real corpus: returned ids identical, n=200.

These tests fail against a core older than 5.28.0 — 4 of 8 — which is the intended signal. That's why this relies on the dependency floor (now ^5.31.0, inherited from main) rather than re-filtering defensively in Paddock: on an older core agents is an unknown key and is silently ignored, so a defensive re-filter would return plausible but wrong results (the newest 200 across all agents, then filtered) instead of surfacing the mismatch. See the Tests section for the re-verified failure output.

Unchanged by design

The pre-existing hazard noted in herdctl#418 — a chatty agent can push a rarely-run trigger out of the 200 window, showing it as never-run — is preserved exactly. The limit still applies after filtering to the requested agents. Neither caused nor fixed here; worth its own issue if "newest job per agent" should be a first-class query.

Tests

1,657 server tests pass (133 files) on the merged branch against the installed @herdctl/core@5.31.0. Typecheck clean on both packages.

The "4 of 8 fail on an older core" claim was re-verified on this merge rather than taken on trust. Simulating what a pre-5.28.0 core does — agents is an unknown key, silently ignored — by dropping it from the listJobs filter gives exactly 4 failures of 8:

FAIL > returns the identical set AND order as the pre-#535 scan-and-filter
FAIL > agrees with the legacy shape when the limit truncates
FAIL > spans several agents in one pass and excludes the rest
  AssertionError: expected [ 'sweeper-alpha', …(2) ] to deeply equal [ 'trigger-alpha-nightly', …(1) ]
FAIL > treats limit <= 0 as no cap, still filtered to the agents
  AssertionError: expected [ … ] to have a length of 15 but got 30

The other four (most-recent-first ordering, repeated agent name, empty input, missing jobs dir) are contract tests that hold either way by design.

🤖 Generated with Claude Code

…me (#535)

`listRunsForAgents` — behind the Triggers tab's per-trigger last-run column —
called core's `listJobs(jobsDir)` with NEITHER filter nor limit, then filtered
to the agents it wanted and sliced in JS. That defeats core's job index
completely: with `filter.limit === undefined` core sets `retain = matches`, so
every record in the shared, never-pruned, fleet-wide jobs directory is read,
YAML-parsed and Zod-validated on every call, then discarded.

It was written that way for a reason — `ListJobsFilter` had `agent` (exactly
one) and no multi-agent form, and this route needs a project's agent PLUS every
scoped `trigger-<slug>-<name>` agent. herdctl#418 adds `agents?: string[]`, so
the filter can now go where the index can use it.

Measured on the 2,016-record jobs dir, three calls per process, index warm:

    call             1 (cold)      2        3
    before            1,226 ms   1,047 ms  1,081 ms
    after             1,245 ms      83 ms     69 ms

~16× warm. The old shape NEVER warmed — that is the point. It paid full price
on every request, and /api/projects/:slug/triggers/runtime is polled every 10 s
while the tab is open.

Both `agents` and `limit` are load-bearing: `agents` alone leaves `limit`
undefined, so `retain = matches` and every match is hydrated anyway.

ORDER is what is at risk here, not just membership — the old shape sorted the
whole directory then filtered; the new one filters first and lets core sort the
survivors. So the regression test diffs the new call against a literal
reimplementation of the old one, over records interleaved across agents and
timestamps so any per-agent grouping would reorder the result. Verified on the
real corpus too: ids identical for n=200.

Those tests FAIL against @herdctl/core 5.27.0 (4 of 8), which is the intended
signal. The dependency floor is raised to ^5.28.0 rather than re-filtering
defensively in Paddock — a defensive filter would mask a version mismatch by
returning plausible-but-wrong results instead of surfacing it.

BLOCKED on herdctl#418 (edspencer/herdctl#432) shipping in a core release.

Co-Authored-By: Claude <noreply@anthropic.com>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 1, 2026

Copy link
Copy Markdown

Deploying paddock with  Cloudflare Pages  Cloudflare Pages

Latest commit: c230ea5
Status: ✅  Deploy successful!
Preview URL: https://e413e0e8.paddock-7u2.pages.dev
Branch Preview URL: https://perf-535-triggers-runtime-pu.paddock-7u2.pages.dev

View logs

@edspencer

Copy link
Copy Markdown
Owner Author

Unblocked. @herdctl/core@5.30.0 is published and carries the API this PR needs.

herdctl#432 is merged (434996aa), the version PR herdctl#440 is merged, and 5.30.0 is on npm. I verified the published tarball rather than the version number — package/dist/state/job-metadata.d.ts now declares:

agents?: string[];

Worth flagging why the version number alone would have misled: 5.29.1 was already published when this PR was parked, which is past the 5.28.0 the description asks for — but agents was not in it. herdctl#432 hadn't merged; the versions had moved for unrelated reasons. Anyone bumping the lockfile on "5.29.1 > 5.28.0" would have got the same red CI with a newer dependency.

To land this: bump @herdctl/core to ^5.30.0, refresh the lockfile, take it out of draft, and CI should go green (the description records 1,422 server tests passing locally against a build of the herdctl branch).

Note main has moved a long way since 1 August — it is now v0.55.0 — so expect to rebase.

Drops the branch's own `@herdctl/core` floor bump: main is already on
^5.31.0, above the ^5.28.0 the pushdown needs, so the branch no longer
carries a dependency change.
@edspencer
edspencer marked this pull request as ready for review August 4, 2026 20:41
@edspencer
edspencer merged commit 0087124 into main Aug 13, 2026
5 checks passed
@edspencer
edspencer deleted the perf/535-triggers-runtime-pushdown branch August 13, 2026 12:45
@github-actions github-actions Bot mentioned this pull request Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf(server): listRunsForAgents defeats core's job index by passing no filter — /triggers/runtime costs 1.1s, 76% discarded

1 participant