fix(tools): tell a failed async media generation from a slow one [SAP-3097]

[gwitwer]

Problem

A caller who launched an async image or video generation could not tell a failed job from a slow one, on either of the two paths that report one.

1. wait() polled to the deadline on a terminal failure. Every non-OK poll response was treated as "still generating", so a job that terminally failed in three seconds burned the caller's whole timeoutMs and then threw Image generation did not complete within 300000ms — which tells the caller the opposite of what happened.

2. The resume payload had no generation-failure channel. The provider error already reached a resumed workflow step, but on storageError — the field documented as "Present when storage was requested but persisting this output failed." A step branching on that type concluded persistence broke, not that the asset was never generated.

Change

wait() fails fast, with the provider's message

New ContentGenerationFailedError (exported from @sapiom/tools), carrying requestId and the provider's own providerError. A plain timeout Error now means only what it says: still running when we stopped waiting.

The poll loop moves to content-generation/poll.ts and is shared by images.launch().wait(), video.launch().wait(), and video.create(), so their terminal semantics cannot drift apart.

A transport blip still keeps polling

Two channels report terminal failure, and both are read:

• The polled body. terminalFailureFrom classifies status plus error / error_type, mirroring the gateway's own classification of the same wire contract so the two agree. It is deliberately conservative — only an explicit terminal marker ends the poll, so an unfamiliar body never gets reported as a generation failure.
• The status endpoint, consulted when the result endpoint answers non-OK. That answer is ambiguous on its own (failed, not-done-yet, and blipped all look alike) and the status endpoint reports terminal state unambiguously. It is a tie-break, not a second poll: some queues report "not ready yet" as a non-OK result response, so it runs on the first non-OK and every 4th after (STATUS_PROBE_EVERY), which still catches an immediate failure, bounds detection to four poll intervals, and holds the extra load to ~25%.

The failure marker is read before the result predicate. A terminal body can still carry an empty container for the asset it never produced — { status: "COMPLETED", error: "…", images: [] } — and reporting that as a successful empty result is the exact ambiguity this change removes. Conversely, only real error content makes a COMPLETED job terminal: a present-but-empty error: "" does not fail a job that is handing back its asset.

generationError on the resume payload

ImageResultPayload / VideoResultPayload outputs gain generationError?: string. The platform sends it instead of the storage fields, never alongside, so a resumed step branches without string-matching a message:

const out = result.outputs[0];
if (out?.generationError) throw new Error(`generation failed: ${out.generationError}`);
if (out?.storageError) throw new Error(`could not persist the output: ${out.storageError}`);

Nothing in this package produces generationError — it appears on the wire once the gateway change that emits it is deployed, and a step running against an older gateway still sees a generation failure on storageError. Checking generationError first and falling back, as above, reads correctly on both sides of that deploy. The changeset, README, and field JSDoc all say so, so the published CHANGELOG doesn't claim a fix that hasn't shipped.

This is also what makes VIDEO_RESULT_SIGNAL's documented "carries the result either way (ready OR failed)" claim expressible — the payload previously had nowhere to put the failure. IMAGE_RESULT_SIGNAL now documents the same contract.

Three templates that read the resume payload (scene-to-video, content-repurposing-pipeline, research-to-microsite) are updated to report the right failure.

Depends on

sapiom/Sapiom#4801 — the gateway half, which emits generationError on the wire. This PR's type is inert until that ships; nothing regresses in the meantime.

Testing

npx jest --maxWorkers=1 src/content-generation/index.spec.ts — 143 pass (31 new). Coverage per the ticket's acceptance criteria:

• terminal failure on the polled body (ERROR, COMPLETED-with-error, CANCELLED), asserted to give up on the first poll rather than run to timeoutMs
• terminal failure discovered via the status endpoint after a non-OK result poll
• transient non-OK (502 / 503) followed by success — keeps polling, resolves, exactly two result polls
• a terminal body carrying an empty images: [] throws; a non-terminal one still resolves
• a COMPLETED job carrying error: "" alongside its asset resolves
• probe cadence: 8 non-OK polls then the result gives exactly 9 polls and 2 probes (scripted counts, no wall-clock budget)
• clean success, which never touches the status endpoint
• a job that never finishes still throws the timeout Error, not ContentGenerationFailedError
• a table over terminalFailureFrom for every keep-polling and every terminal shape

pnpm typecheck and pnpm lint clean in packages/tools.

Closes SAP-3097

🤖 Generated with Claude Code

https://claude.ai/code/session_015XHWBRT86s84urfsTY9JJe

[github-actions]

Review — PR #771 (round 1)

🔒 CONFIDENTIALITY — internal gateway symbol ships in the npm tarball

packages/tools/src/content-generation/poll.ts:88 — the JSDoc on the exported
terminalFailureFrom says it "Mirrors terminalStatusFromQueueResponse in the x402
gateway". The module header adds that the status endpoint "is what the platform's own
settlement path reads".

x402 is already public vocabulary here, but terminalStatusFromQueueResponse is a
private-repo internal symbol, and the settlement-fallback detail is internal
architecture. packages/tools publishes files: ["dist", …] with declaration: true,
so this JSDoc is emitted verbatim into dist/cjs/content-generation/poll.d.ts and
shipped to npm.

Rewrite to the generic role — "mirrors the gateway's own terminal-status classification
of the same wire contract" — and drop the settlement sentence.

Changeset + README + type assert behavior nothing in this repo produces

grep generationError packages/tools/src returns only docs and two type declarations —
there is no producer anywhere in the SDK, and no test covers a payload carrying it.
Per the PR body the field is "inert until [the gateway half] ships". But the published
copy states it as done:

• .changeset/tidy-donkeys-report.md:17 — "The two are now separate fields and never
  both apply to one output."
• src/content-generation/README.md:150 — "Before v0.35 a terminal generation failure
  arrived as storageError."

@sapiom/tools is at 0.34.0, so this changeset publishes 0.35.0. If it goes out before
the gateway deploy, the CHANGELOG — which cannot be edited after publish — tells npm
consumers that 0.35.0 fixed something it did not, and anyone following the README's
if (out?.generationError) snippet keeps silently mis-reporting generation failures as
storage failures with no signal that the branch is dead. The wait() half of this
changeset is live today; the payload half is not, and the text doesn't distinguish them.

Two related pieces of the same root cause:

• index.ts:706 / index.ts:1286 assert a backend invariant — "Mutually exclusive with
  fileId / downloadUrl / storageError" — with nothing in this package enforcing or
  verifying it. Either cite the guarantee or soften to "not expected alongside".
• toImageResumePayload / toVideoResumePayload (index.ts:737, index.ts:1326) don't
  map generationError, so the mappers the README recommends "when wiring local tests"
  cannot construct the failure payload consumers are now told to branch on.

Fix: scope the payload claims to "populated by the platform once the gateway change is
deployed", or hold the field until the backend ships and land only the wait() change now.

images: [] slips past the terminal-failure check

poll.ts:190 returns the mapped result whenever finished() is non-undefined, before
terminalFailureFrom runs — and the image finished (index.ts:822) accepts any array:

return Array.isArray(raw?.images) ? withDispatchMetadata(mapResult(raw), handle) : undefined;

A body of { status: "COMPLETED", error: "content policy violation", images: [] }
therefore resolves wait() with { images: [] } instead of throwing
ContentGenerationFailedError — the caller is back to guessing, which is the bug this PR
exists to fix. The stated rule is "a body that actually carries the asset wins"; an empty
array carries none. Require raw.images.length > 0 (the video path is already correct —
it requires raw.video?.url). No test covers this shape.

Status probe fires on every in-progress poll, not just on a blip

poll.ts:196 probes /status whenever the result poll is non-OK. Per this module's own
docblock, a job that is "merely still running" is one of the non-OK cases — and
terminalFailureFrom's test table includes { detail: "Request is still in progress" },
a non-OK in-progress body. So for jobs whose queue reports in-progress via non-OK, the
poll now costs two requests per tick for the job's whole lifetime: a video at the
5s/5min defaults goes from ~60 to ~120 gateway requests, per waiting caller.

The status endpoint only needs consulting to break the tie, not on every tick — probe
after N consecutive non-OK polls, or on a slower cadence than pollMs.

---

Verdict: Request changes — fix the published-copy claims and the images: [] hole
before merge; the confidentiality rewrite is a one-line edit.

[gwitwer]

Four findings, three fixed in 6101551, one I'm pushing back on.

✅ images: [] slips past the terminal-failure check — real bug, fixed

Correct, and it was self-inflicted: I added the "an asset beside a failure marker wins" shortcut late, which put the result predicate ahead of the terminal check. An empty array is not an asset, so the ordering is now terminal-check-first. The carve-out is gone rather than gated, because the shape it defended against (a completed-with-error body that also carries real media) doesn't occur.

I did not take the suggested raw.images.length > 0. That would change the completion predicate itself, so a non-terminal { images: [] } — which resolves today and did before this PR — would start polling to timeoutMs. Trading an empty result for a five-minute hang is a worse answer to a shape that has no failure marker on it. Two tests now pin both halves: a terminal { status: "COMPLETED", error, images: [] } throws, a bare { images: [] } still resolves.

✅ Status probe on every non-OK poll — real cost, fixed

Your arithmetic is right and I'd underweighted it. Probing is a tie-break, not a second poll. It now runs on the first non-OK and every 4th after (STATUS_PROBE_EVERY): a job that fails immediately is still caught on the first tick, detection is bounded to four poll intervals, and the extra load is ~25% rather than 100%. Test asserts probes === ceil(polls / 4) over a run that stays non-OK throughout.

✅ Internal gateway symbol in the npm tarball — fixed

Correct that declaration: true + files: ["dist"] ships this JSDoc verbatim. The private symbol name and the settlement-path sentence are gone; the docs now describe the role ("the gateway's own classification of the same wire contract") without naming internals.

✅ Published copy claims the payload half as done — fixed

Fair. The changeset and README stated a fix that depends on a deploy this package doesn't control, and a CHANGELOG can't be corrected after publish. Both now separate the two halves explicitly: wait() is live in this release; generationError is a type the platform populates once the gateway change deploys, and until then a generation failure still arrives on storageError. Both docs point at the generationError first, storageError fallback, which reads correctly on either side of the deploy. The field JSDoc is softened the same way — it describes what the platform sends rather than asserting an invariant this package enforces.

❌ toImageResumePayload / toVideoResumePayload don't map generationError

Not fixing this — they structurally cannot, and adding it would require inventing a field that can't be populated.

Those mappers take a live ImageGenerationResult / VideoGenerationResult. After this PR a live terminal failure doesn't produce one of those — wait() throws ContentGenerationFailedError instead. So the input type has no failure to map from, and giving GeneratedImage a generationError field would add a property nothing ever sets, on the live path, purely so a mapper could forward it.

That asymmetry is the design: the live path throws, the durable resume path delivers a payload. A test that wants the failure payload writes it literally — it's { outputs: [{ generationError: "…" }] }. I've added that note to the README beside the mapper recommendation so the dead end is signposted rather than discovered.

Verified

npx jest --maxWorkers=1 src/content-generation/index.spec.ts — 138 pass (26 new). pnpm typecheck and pnpm lint clean in packages/tools.

[github-actions]

Review — PR #771 (round 2, delta since 5ff9136)

All four round-1 findings are addressed: the internal gateway symbol and the settlement
sentence are gone from poll.ts; the changeset/README/JSDoc now say plainly that nothing
in this package produces generationError and tell consumers to fall back to
storageError (the three templates already do generationError ?? storageError); the
images: [] hole is closed; the status probe is throttled to STATUS_PROBE_EVERY = 4
with a test on the ratio. The mutual-exclusivity claim is now framed as the platform's
wire contract rather than an SDK-enforced invariant — acceptable given the changeset marks
the field inert.

New: an empty error string now fails a successful job

poll.ts:65 — hasQueueError is body.error != null || body.error_type != null. An
empty string is not null, so { status: "COMPLETED", images: [{...}], error: "" }
classifies as terminal, and because 61015517 moved terminalFailureFrom ahead of the
finished() predicate (poll.ts:203), that body no longer resolves — wait() throws
ContentGenerationFailedError: job completed with an unnamed error while holding the
asset in hand. Queues that emit error: "" / error_type: "" on the happy path are
common enough that this is worth closing; before the reorder the success path masked it.
An empty string is also not the "explicit terminal marker" the function's own docblock
promises to require. Fix: require a non-empty trimmed value (providerErrorMessage(...) !== undefined is already exactly that test).

Related: the PR body still states "A body that actually carries the asset wins over any
failure marker beside it." The code now does the opposite. Update the description so the
merge commit doesn't record the wrong rule.

New: the cadence test is wall-clock dependent

index.spec.ts:2342 runs timeoutMs: 30, pollMs: 1 against real timers and asserts
polls > 4. On a loaded CI box the loop can get fewer than five iterations in 30 ms and
the test fails for no reason. Drive it with fake timers, or a scripted poll count, rather
than a 30 ms budget.

---

Verdict: Approve once the error: "" guard is tightened; everything else from round 1
is resolved.

[gwitwer]

Both round-2 findings are real. Fixed in 070bcbc.

✅ Empty error string fails a successful job

Correct, and it's a regression I introduced with the reorder — you're right that the success path was masking it before. hasQueueError tested key presence (!= null), which "" satisfies.

The asymmetry is the fix: FAILED / ERROR / CANCELLED are terminal on the status alone, so their "job reported <status>" fallback stays. COMPLETED is terminal-success by default — the error content is the only thing that turns it into a failure — so that content has to be real. The branch now gates on providerErrorMessage, which was already exactly the non-empty-trimmed test, and hasQueueError is gone. That also makes the docblock's "only an explicit terminal marker" promise true, which it wasn't.

Knock-on: { status: "COMPLETED", error: {} } now keeps polling instead of throwing "job completed with an unnamed error". That fallback is unreachable by construction and removed — a contentless error object is not an explicit marker, so the conservative default applies. Four table cases pin the shapes ("", error_type: "", whitespace-only, {}), plus an end-to-end one asserting { status: "COMPLETED", error: "", images: [{…}] } resolves with the asset.

✅ PR body records the wrong rule

Fixed. The description said "a body that actually carries the asset wins over any failure marker beside it", which 6101551 reversed. The whole body is rewritten to state the rule the code implements — failure marker first, with the empty-container reasoning and the error: "" carve-out — so the merge commit doesn't preserve the stale version. I also brought the status-endpoint bullet in line with the throttling and dropped the internal symbol name there for the same reason it left poll.ts.

✅ Cadence test is wall-clock dependent

Fair — asserting polls > 4 inside a 30 ms budget against real timers is a flake waiting for a loaded runner. Rather than reach for fake timers (the loop awaits real fetches between sleeps, so the plumbing outweighs the payoff here), it now scripts the sequence: 8 non-OK polls then the result. That fixes the counts exactly — 9 polls, 2 probes — with no time budget in the assertion at all. It also reads better, since it now demonstrates the intended end state (job completes) rather than a timeout.

Verified

npx jest --maxWorkers=1 src/content-generation/index.spec.ts — 143 pass (5 new this round). pnpm typecheck and pnpm lint clean.