Repository navigation
Description
Summary
A caller-supplied task TTL reaches tasks.Store.Start with only a ttl <= 0 → operator default rule and no ceiling, at every entry point:
internal/daemon/server.go:893—/v1call tasks:s.startCallTask(req.Task.TTL, ...)internal/daemon/routes_exec.go:238-245— exec tasks:req.Options.Task.TTL(internal/execsvc/execsvc.go:92-93, "milliseconds the result is kept, as MCP spells it")internal/daemon/routes_proto.go:422-425— proto ask tasks:req.TTL
internal/tasks/tasks.go:355-357 substitutes the default (tasks.ttl, 10m; proto.askTTL, 15m) only when ttl <= 0. Any positive value is accepted verbatim, and tasks.go:389 converts it unsafely:
time.AfterFunc(time.Duration(ttl)*time.Millisecond, func() { ... delete ...; cancel() })Two failure modes follow:
- Overflow → instant self-destruct.
time.Durationis int64 nanoseconds, so anyttl > ~9.22e12ms (~292 years in ms — a plain JSON number like10000000000000) makestime.Duration(ttl) * time.Millisecondwrap negative.time.AfterFuncwith a negative duration fires immediately: the task is deleted from the store and its context is cancelled at birth, before the work can run. The caller gets a 202 handle for a task that can never be collected. - Huge-but-valid → retention the operator never chose.
ttl = 1e12ms (~31.7 years) is stored verbatim, so task results — tool-call and exec results, the payloads the retention exists to bound — are kept effectively forever, silently overriding the operator'stasks.ttl(10m default). This is the same shape as the artifact-PUT ceiling gap in mcpx(daemon): artifact PUT ttl has no ceiling — a per-request TTL overrides the operator's artifacts.ttl (24h) up to ~292 years, so GC never collects the artifact #2327, in the task store instead of the artifact store.
Evidence (main 5a34fa70, laptop probe with the real tasks.Store)
Probe test in laptop worktree ~/git/fleet/probe-2026-10-29-laneB-taskttl/pkgs/mcpx/internal/tasks/probe_taskttl_test.go (go test -run TestProbeTaskTTLOverflow -v):
- Control
ttl=60000: work (150ms) completes, task present with statuscompletedafter 400ms. ttl=10000000000000(1e13): after 400ms the task is gone (Get→ not present) — deleted before its 150ms of work finished. Probe asserts this as the reproduced bug and fails as designed.ttl=1000000000000(1e12): accepted and stored verbatim (stored TTL=1000000000000 ms).
Why it matters
The task handle is the contract for long-running tool/exec calls: the caller polls or collects the result later. An overflowing TTL silently breaks that contract in the worst way — accepted with a 202, then instantly uncollectable, with the work cancelled — and a merely huge TTL defeats the retention bound that keeps results (potentially sensitive tool/exec output) from living in the daemon's store and SQLite DB forever. Both are one JSON field from any local caller, and neither produces an error anywhere.
Acceptance criteria
- A per-request task TTL is validated at the daemon boundary (or in
tasks.Store.Start): values above a defined ceiling are rejected with a 400-class error or clamped to the ceiling — triage pins which, and the ceiling's relationship to the operator'stasks.ttl/proto.askTTL. - No accepted TTL can overflow the
time.Duration(ttl) * time.Millisecondconversion (regression test at the boundary value, e.g.ttl = 1e13, asserting the task survives its work and completes). - The rule applies uniformly to all three entry points (
server.gocall tasks,routes_exec.goexec tasks,routes_proto.goask tasks) — a single guard inStartis the obvious way to guarantee that. - Existing task tests pass; the persisted
expiresAtMs = createdMs + ttlMsload path (tasks.go:210) stays consistent with the pinned rule.
Related: #2327 (the artifact-store sibling: per-request TTL with no ceiling overriding the operator default), #1653 (distinct — legitimate TTLs are clocked from creation, so a task is cancelled mid-run when its TTL lapses; this issue is about unbounded/overflowing TTL values, not the clock start).
Found by new-issue-creator-code-b (lane B) this run.
Activity
developing-today commentedon Oct 29, 2026
CODE-1195
dezren39 commentedon Oct 29, 2026
🤖 Agent note · filed
dezren39 commentedon Nov 1, 2026
🤖 Agent note · picked up
dezren39 commentedon Nov 1, 2026
🤖 Agent note · triaged — ALREADY FIXED upstream (clamp rule landed via #2498 / PR #2869); closing with evidence, no follow-up packet
d85e4098cf113ec7c3add1a74c0c03fd9840d429(gh api contents, 2026-11-01):pkgs/mcpx/internal/tasks/tasks.go—Startdelegates toStartWithDeadline, which substitutes the operator default forttl <= 0and then clamps any requested TTL to the store'sMaxTTL(if ttl > maxTTL.Milliseconds() { ttl = maxTTL.Milliseconds() }, :613-622) before anytime.Duration(ttl) * time.Millisecondconversion.MaxTTLis wired fromprotoTasks.maxTTLby the seams; unset meansDefaultMaxTTL = time.Hour(:456). Because the clamp lives in the singleStartguard, it covers all three entry points (daemon call tasks, exec tasks, ask tasks) uniformly — exactly the fix shape this triage was asked to route.14ac81c14(2026-10-30) "mcpx: task-store limits — requestor key, TTL clamp, per-requestor cap, tasks/* window, 128-bit IDs (mcpx: Phase 1 of #2497 — limits in the task store: requestor key, TTL clamp, per-requestor cap, tasks/* window, 128-bit IDs #2498)", carried through PR mcpx: task-store limits — requestor key, TTL clamp, per-requestor cap, tasks/* window, 128-bit IDs (#2498) #2869 (queue task 3985) with review-round follow-ups (2026-10-31, incl. the deadline-task retention floor at two poll intervals, :603-611). This issue was filed 2026-10-29 and was overtaken by that work two days later; nobody closed it.ttl=1e13request is clamped to ≤3.6e6 ms before conversion ✔, with regression coverageTestStartClampsTTLToTheStoreMax(tasks_test.go:202: above-max clamps on the returned task, under-max untouched); (3) uniform across entry points via the single guard ✔; (4) load path (tasks.go:272-280) computesexpiresAtMsin the int64 millisecond domain from the persistedttl_ms— pre-clamp rows were bounded by the old code's own behaviour (values past ~9.22e12 ms self-deleted at birth under the old AfterFunc too), so no surviving legacy row can overflow the load conversion in practice ✔.