Skip to content

feat(tools): deadlineMinutes on the coding + model run specs, awaiting_capacity in RunStatus [SAP-3201] - #839

Open
gwitwer wants to merge 3 commits into
mainfrom
feat/SAP-3201
Open

feat(tools): deadlineMinutes on the coding + model run specs, awaiting_capacity in RunStatus [SAP-3201]#839
gwitwer wants to merge 3 commits into
mainfrom
feat/SAP-3201

Conversation

@gwitwer

@gwitwer gwitwer commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

What

Gives the SDK a typed way to say "I can wait N minutes" on a run, and teaches the run-status union about the deferred state the server will start returning.

  • deadlineMinutes?: number on CodingRunSpec and ModelRunSpec, sent on the wire as deadline_minutes by both codingLaunch/codingRun and models.run/models.launch.
  • RunStatus gains awaiting_capacity — added to the union and to RUN_STATUSES (so codingResultSchema accepts it), and deliberately not to TERMINAL.

Why

Today every run dispatches immediately and is billed at the most expensive lane, because an author has no way to express slack. This is the author-facing half of the label + deadline vocabulary from plans/model-execution-surface/interfaces.md: the author states the kind of call (model) and how long they can wait (deadlineMinutes); the platform derives the billing lane (run_now / priority / standard / flex). The caller never names a lane.

Two details worth checking in review

Omitted means absent, not zero. deadline_minutes: spec.deadlineMinutes leaves the value undefined when unset, so JSON.stringify drops the key entirely. The server has to distinguish "no deadline" from "zero minutes", and an existing caller's request body stays byte-identical to before this change. Covered by a test on each surface.

awaiting_capacity is non-terminal. TERMINAL stays {"completed", "failed"}. run()'s poll loop and a launch() handle's wait() only stop on those two, so a deferred run keeps polling instead of resolving with a null result. There's a test that drives a run through two awaiting_capacity polls into completed and asserts the loop kept going.

Scope

agents.run (AgentRunSpec) is left alone. It dispatches a deployed orchestration rather than an LLM call, so it has no billing lane to derive — the ticket lists that as a nice-to-have "if it costs nothing", and it doesn't.

ModelRunStatus does gain awaiting_capacity, reversed from this PR's first revision after review. The original reasoning was that the ticket scopes the new status to RunStatus and the gateway doesn't emit the deferred state on the model-run surface. That was the wrong way round: deadlineMinutes ships on ModelRunSpec (an explicit acceptance criterion), and the only reason to send a deadline is for the platform to defer — so the day it does, a union too narrow to hold the value mis-types handle.status() and makes modelRunResultSchema.parse reject a real payload. Reserving the member costs a consumer one branch that is currently unreachable; omitting it costs a silent type lie. It's kept out of MODEL_TERMINAL exactly as on the coding side, and its doc comment says it is reserved rather than mirrored.

Ships on its own

The server ignores deadline_minutes until SAP-3202 lands, so this is zero behavior change. Includes a minor changeset for @sapiom/tools — the wire ticket's execution-surface.contract.json fixture is checked against the published package, so this has to publish before that ticket can go green.

Verification

  • pnpm build, pnpm typecheck, pnpm lint — all clean across the workspace.
  • npx jest --maxWorkers=1 src/models/{launch,run-launch,coding-result,resume-payload}.spec.ts — 40 passed. (Full suite left to CI per repo policy.)
  • provider-neutral-copy-check and agent-studio-terminology-check pass.

Closes SAP-3201

🤖 Generated with Claude Code

https://claude.ai/code/session_019s65cc2Q5ravSTn9cZUj7e

…atus

Authors have no way to say "I can wait" on a run, so every run is dispatched
immediately and billed at the most expensive lane. This adds the author-facing
half of the label + deadline vocabulary: the author states the kind of call
(`model`) and how long they can wait (`deadlineMinutes`), and the platform
derives the billing lane (run_now / priority / standard / flex) from that. No
lane is ever named by the caller.

`deadlineMinutes` is optional on both `CodingRunSpec` and `ModelRunSpec` and
rides the wire as `deadline_minutes`, matching the module's existing snake_case
mapping. When unset it stays `undefined`, so `JSON.stringify` drops the key —
the server has to be able to tell "no deadline" from a `0` or a `null`, and an
existing caller's request is byte-identical to before.

`RunStatus` gains `awaiting_capacity`, the state a deferred run reports while
it waits for a lane. It is deliberately kept out of `TERMINAL`: `run()`'s poll
loop and a `launch()` handle keep polling through it, so a deferred run is
never resolved with a null result.

The server ignores `deadline_minutes` until the wire ticket (SAP-3202) lands,
so this ships with zero behavior change. `agents.run` is left alone — it
dispatches a deployed orchestration, not an LLM call, so it has no billing lane
to derive.

Refs SAP-3201, SAP-3195

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019s65cc2Q5ravSTn9cZUj7e
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Review — PR #839

1. run() throws on any deadline longer than its default timeout

packages/tools/src/models/index.ts:450 / :770

codingRun (:473) and run (:793) call handle.wait() with no options, so the timeout is the
hardcoded default: 20 min for coding, 10 min for model runs. A run parked in
awaiting_capacity is non-terminal, so the loop keeps polling — and then hits
Date.now() > deadline and throws coding run <id> timed out after 1200000ms (last status: awaiting_capacity).

Concretely: agent.coding.run({ task, deadlineMinutes: 60 }) — the exact call the feature exists
for — is guaranteed to throw at 20 minutes if the platform actually defers it. run() takes no
timeoutMs, so a caller on that surface has no way out except rewriting to launch() +
wait({ timeoutMs }).

This also makes the published copy wrong. README :32 and the changeset both say "run keeps
polling through it and a launch handle keeps waiting, so a deferred run is never resolved with a
null result" — true about resolving, but it does not survive the deadline it was given.

Fix: derive the poll timeout from the spec, e.g. timeoutMs = Math.max(default, spec.deadlineMinutes * 60_000 + slack), and add a test that a 60-minute deadline held in
awaiting_capacity past 20 minutes does not throw. The existing test (launch.spec.ts:110) only
proves the loop iterates three times.

2. The changeset asserts billing behavior the released server does not implement

.changeset/deadline-minutes-run-specs.md

"The platform derives the billing lane (run_now / priority / standard / flex) from it, so
a run you can wait on costs less."

Present tense, unconditional — and per the PR body the server ignores deadline_minutes until the
follow-up ticket lands. A consumer who reads this in @sapiom/tools' CHANGELOG.md, sets
deadlineMinutes: 120, and expects a smaller bill gets identical dispatch at identical cost.
Changesets are compiled into the shipped CHANGELOG and cannot be retracted. Reword to what is true
on this version: the field is accepted and sent on the wire; lane derivation arrives in a later
release. Same for README :32 ("buys a cheaper run").

Separately, priority / standard / flex are new public vocabulary. Only run_now appears in
the shipped surface today (packages/tools/src/llm/index.ts:362, as an e.g.), and the API neither
accepts nor returns any of the four. Enumerating a taxonomy the caller can never name — the copy
says so itself — reads as a roadmap leak into the tarball. Drop the list and describe the effect.

3. deadlineMinutes on ModelRunSpec creates a state the model types cannot represent

packages/tools/src/models/index.ts:522 (spec field), :498 (ModelRunStatus), :635 (schema)

The PR deliberately keeps awaiting_capacity out of ModelRunStatus because the server does not
emit it there — but it still ships deadlineMinutes on ModelRunSpec and sends it on the wire. The
only reason to send it is for the server to defer, and the moment it does:
handle.status() returns a string outside its declared union (a silent type lie in published
types), and modelRunResultSchema.parse fails with "status must be a valid ModelRunStatus".

Pick one: hold deadlineMinutes off ModelRunSpec until the model-run surface can defer, or widen
ModelRunStatus and MODEL_TERMINAL's neighbours the same way the coding side was widened. The
current split ships an option whose success case the types reject.

4. Widening RunStatus is a typecheck break on a minor

packages/tools/src/models/index.ts:53

Adding a member to an exported union breaks consumers with an exhaustive switch +
default: assertNever(status). Minor is the right level for this repo, but the changeset body
should carry the one-line heads-up so a consumer sees why their build reddened.


Verdict: request changes. Finding 1 is a functional bug that makes the headline feature throw on
realistic inputs; findings 2 and 3 are unretractable-copy and public-type-surface problems that are
cheaper to fix before publish than after.

… model runs

Review round 1 on #839.

A deferred run polls until it reaches a terminal state, but `wait()`'s default
budget was a fixed 20 min (coding) / 10 min (model). `run()` takes no
`timeoutMs`, so `coding.run({ task, deadlineMinutes: 60 })` — the exact call
this feature exists for — would have thrown at 20 minutes the moment the
platform actually deferred it. The handle's default is now derived from the
spec's deadline (deadline + the surface default as slack, since the deadline
bounds when the run finishes, not when it starts). It only ever widens, and an
explicit `wait({ timeoutMs })` still wins.

`ModelRunSpec` accepts a deadline, so `ModelRunStatus` has to be able to hold
the state that deadline produces. It gains `awaiting_capacity` too — otherwise
the success case of an option we ship would mis-type `handle.status()` and make
`modelRunResultSchema.parse` reject a real payload. It stays out of
`MODEL_TERMINAL`, same as the coding side.

Changeset and README no longer claim a deadline makes a run cheaper today: the
platform doesn't honor the field until SAP-3202 lands, and a changeset is
compiled into a published CHANGELOG that can't be retracted. The changeset also
now warns consumers with exhaustive switches that a new union member is a
compile error on their side.

Refs SAP-3201

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019s65cc2Q5ravSTn9cZUj7e
@gwitwer

gwitwer commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks — finding 1 is a real bug and finding 3 is a real inconsistency this PR introduced. Both fixed in d50f5b3, along with 2 and 4. One partial disagreement, on the taxonomy half of 2.

1. run() throwing past its default timeout — fixed

Correct, and it was the headline call: coding.run({ task, deadlineMinutes: 60 }) would have thrown at 20 minutes the moment the platform actually deferred it, with no way out on a surface that takes no timeoutMs.

Fixed one level up from where you suggested. Rather than patching codingRun/run, the handle's default is now derived from the spec, so launch() + wait() gets the same protection as run():

async wait({ timeoutMs = defaultWaitMs(spec.deadlineMinutes, 20 * 60_000), pollMs = 3_000 } = {})

defaultWaitMs returns deadlineMinutes * 60_000 + surfaceDefaultMs — the deadline bounds when the run finishes, so the surface default rides on top as slack for dispatch latency and poll granularity rather than replacing it. It only ever widens (a deadline under the default, or a non-positive one, leaves the default alone), and an explicit wait({ timeoutMs }) still wins.

Three tests per surface, driving a run through awaiting_capacity while a stubbed Date.now jumps 25 minutes past the coding default (15 past the model one): the deadline case completes, the no-deadline case still throws timed out after 1200000ms (last status: awaiting_capacity), and an explicit timeoutMs: 1000 still throws at 1000ms. You were right that the old test only proved the loop iterates.

3. deadlineMinutes on ModelRunSpec with no deferred state to land in — fixed

You've got the better argument here and I've taken it. ModelRunSpec.deadlineMinutes is a hard requirement of the ticket (there's an explicit acceptance criterion for models.run({ prompt, deadlineMinutes: 30 })), so holding it off isn't available — which leaves widening, and widening is the right side to err on anyway. A union that's too wide costs a consumer one unreachable branch; one that's too narrow silently mis-types a real runtime value and makes modelRunResultSchema.parse throw on a real payload.

ModelRunStatus now has awaiting_capacity, kept out of MODEL_TERMINAL exactly as on the coding side, and the inline status list in modelRunResultSchema.parse matches. Tested: a deferred model run keeps polling to completion, and parse accepts the status.

2. Changeset copy — fixed as to tense; keeping the lane vocabulary

The tense point is right and worth more than it looks, because a changeset compiles into a CHANGELOG that can't be retracted. Both the changeset and the README now say plainly that the field is accepted and sent on the wire, that the platform does not honor it yet, and that setting it does not change dispatch or price until the server ticket lands.

On dropping priority / standard / flex: keeping them, deliberately. Naming that vocabulary is an explicit requirement of SAP-3201 ("Doc comment on the field states the vocabulary: the deadline derives the billing lane (run_now / priority / standard / flex)"), and the point of stating it is precisely that the caller doesn't name a lane — it tells an author what the knob they're turning controls, so they can reason about why 5 minutes and 5 hours price differently. run_now is also already public on this surface (CodingRunOutcome.lane, ModelRunOutcome.lane), so the axis isn't new, only the rest of its range. Happy to revisit if someone with the pricing surface wants it held back.

4. Exhaustive-switch break — added

The changeset now carries the heads-up, with the guidance a consumer needs at the break: handle awaiting_capacity as non-terminal, the run is still in flight.

Verification: pnpm build, pnpm typecheck, pnpm lint clean (only pre-existing unused-import warnings in unrelated packages). npx jest --maxWorkers=1 src/models/ — 46 passed across the four spec files, up from 40. Copy and terminology checks pass.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #839 (delta since 17a8d03c)

Round 1's findings 1, 3 and 4 are fixed, and the handle-level defaultWaitMs is a better fix than
the one suggested. Two new items, one deliberate non-fix, one correction to round 1.

New — the widened budget polls at a fixed interval, with no way to tune it

packages/tools/src/models/index.ts:471, :809

wait()'s budget is now deadlineMinutes * 60_000 + surfaceDefault, but pollMs is still a flat
3_000 / 2_000 and there is no backoff while the run sits in awaiting_capacity. run() takes
no pollMs, so the interval is unreachable on the surface the deadline feature is aimed at.

models.run({ prompt, deadlineMinutes: 480 }) parked for its window issues ~14,700 GETs on
/models/v1/runs/{id} for one run; the coding equivalent ~10,000. Before this commit the ceiling
was 300–400 polls. That is a consumer's own rate limit and request bill spent waiting.

Fix: back off (or step pollMs up) once the status is awaiting_capacity, and expose pollMs on
run() alongside it.

New — the PR body and the ModelRunStatus JSDoc still say the gateway doesn't emit this

The Scope section still reads "ModelRunStatus does not gain awaiting_capacity … claiming a
state the server doesn't emit would be a lie in the published types" — contradicted by d50f5b3b,
and it lands in the merge commit. Meanwhile index.ts:523 keeps "mirrored from the gateway's
ModelRunStatus" while adding a member the author has said the gateway does not send on that
surface. Cite the backend guarantee, or say plainly that the member is reserved for when the
model-run surface starts deferring.

Not fixed — deliberately

priority / standard / flex remain in the changeset, README :32, and both deadlineMinutes
JSDoc blocks. Author reaffirmed with the ticket requirement; noting only that three of the four
lanes still appear nowhere in the API and ship in an unretractable CHANGELOG. Their call.

Round 1 got this wrong

Round 1 claimed run_now appears only in llm/index.ts:362 as an e.g.. It is also public on
CodingRunOutcome.lane (models/index.ts:133) and ModelRunOutcome.lane (:595), so the lane
axis was already part of this module's surface — the author's rebuttal was correct on that point.


Verdict: approve with comments. Nothing here blocks publish; the poll interval is worth fixing
before the server starts honoring deadlines. Tests not run locally (jest invocation not permitted
in this environment); relying on the author's reported 46 passing.

Review round 2 on #839.

Widening wait()'s budget to cover the deadline (previous commit) raised the
poll ceiling with it: at a flat 2s interval, a `deadlineMinutes: 480` model run
parked for its whole window would issue ~14,700 GETs for one run, against ~300
before. That is the caller's rate limit and request bill spent learning nothing,
since a parked run has nothing to report until the platform dispatches it.

The interval now doubles while the status is `awaiting_capacity`, capped at 60s
— a few hundred polls for an 8-hour deadline. Every other status, `running`
included, snaps straight back to the caller's `pollMs`, so a run that is
actually moving is still observed at full cadence and a terminal transition is
caught promptly.

Also corrects the `ModelRunStatus` doc comment, which still said the union was
mirrored from the gateway while declaring a member the gateway does not emit on
that surface. It is reserved, not mirrored, and now says so and why.

Refs SAP-3201

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019s65cc2Q5ravSTn9cZUj7e
@gwitwer

gwitwer commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

Round 2 addressed in 7cfd75e. Both new items were fair; the poll-count one was self-inflicted by round 1's fix.

New 1 — flat poll interval against a widened budget: fixed

Right, and it's the direct cost of the round-1 change: widening the budget raised the poll ceiling with it. Your arithmetic holds — a deadlineMinutes: 480 model run parked for its window went from ~300 polls to ~14,700.

The interval now doubles while the status is awaiting_capacity, capped at 60s:

function nextPollMs(status: string, currentMs: number, callerPollMs: number): number {
  if (status !== "awaiting_capacity") return callerPollMs;
  return Math.min(currentMs * 2, DEFERRED_POLL_CAP_MS);
}

That's ~484 polls for the 8-hour case, back in the same order as the old ceiling. Every other status — running included — snaps straight back to the caller's pollMs, so a run that's actually executing is still observed at full cadence and the terminal transition is caught within one interval. Worst case a parked run notices dispatch up to 60s late, which is nothing against a deadline measured in hours.

Two tests, recording the delay each sleep asks for while firing it immediately: [2, 4, 8, 1, 1] across awaiting_capacity ×3 → running ×2 → completed (doubling and snap-back), and [60_000, 60_000] from a 40s pollMs (the clamp).

Declining the second half — exposing pollMs on run(). That changes the public signature of codingRun/run, which the ticket doesn't ask for, and the escape hatch already exists: launch() + wait({ pollMs }). The backoff is the part that actually needed fixing, because it helps run() callers who by definition can't pass anything. If someone wants tunable polling on the blocking surface, that's worth its own ticket with a considered options shape rather than a parameter bolted onto the end of (spec, transport, baseUrl).

New 2 — stale copy in the PR body and the JSDoc: fixed

Both were genuinely contradictory, and the PR body one would have landed in the merge commit.

The Scope section is rewritten to say ModelRunStatus does gain the member, that this reversed the first revision after review, and why the original reasoning was backwards.

The JSDoc now says the member is reserved, not mirrored — the gateway does not emit it on this surface today; it's declared because this surface accepts a deadline, and the only reason to send a deadline is for the platform to defer. I've stated it that way rather than citing a backend guarantee, because there isn't one to cite yet: SAP-3202 hasn't landed. Reserving costs one unreachable branch, omitting costs a silent type lie.

On the lanes and the round-1 correction

Noted, and thanks for the correction on CodingRunOutcome.lane / ModelRunOutcome.lane. Your standing objection to priority / standard / flex in an unretractable CHANGELOG is recorded here for whoever owns the pricing surface; if they want the range held back to run_now, it's a one-line edit before publish and I'd take that patch.

Verification: pnpm build, pnpm typecheck, pnpm lint clean. npx jest --maxWorkers=1 src/models/ — 48 passed across the four spec files (46 → 48). Copy and terminology checks pass.

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.

1 participant