Skip to content

fix: abort hung OpenAI changelog fetches - #3615

Open
SebTardif wants to merge 2 commits into
openclaw:mainfrom
SebTardif:fix/changelog-openai-timeout
Open

SebTardif wants to merge 2 commits into
openclaw:mainfrom
SebTardif:fix/changelog-openai-timeout

Conversation

@SebTardif

@SebTardif SebTardif commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Publish changelog preview calls OpenAI with no deadline. If api.openai.com stalls, .catch(() => null) never runs because fetch never rejects. The publish UI and publish action wait until Convex kills the action.

Evidence

Live bun run of generateWithOpenAI against this branch. Fetch hung until the abort signal fired. The call rejected with AbortError in 33ms (proof used a 30ms abort stand-in for the 10s product timeout).

$ bun proof-f009.mts
{
  "timeoutMs": 10000,
  "outcome": "AbortError",
  "elapsedMs": 33
}

Unfixed main generateWithOpenAI has no signal on the OpenAI fetch. Sibling convex/lib/embeddings.ts already aborts after 10s.

Real behavior proof

  • Behavior or issue addressed: Changelog OpenAI fetch now has a 10s abort. A hung provider rejects instead of holding the publish action forever. Callers already fall back to the local changelog text.
  • Real environment tested: Windows 11, Bun 1.4.1, worktree fix/changelog-openai-timeout on origin/main 0b34234f, live bun calling generateWithOpenAI with a hung fetch.
  • Exact steps or command run after this patch: bun proof-f009.mts from the patched worktree (proof helper not committed).
  • Evidence after fix: terminal output copied below.
$ bun proof-f009.mts
{
  "timeoutMs": 10000,
  "outcome": "AbortError",
  "elapsedMs": 33
}
  • Observed result after fix: The hung fetch aborted. Outcome is AbortError. Elapsed time is 33ms, not the Convex action limit. Product timeout remains 10000ms.
  • What was not tested: A live OpenAI outage during a real publish in production.

Summary

Uses AbortSignal.timeout(10_000) on the existing OpenAI changelog request. Existing .catch(() => null) plus fallback notes stay in place.

Tracker

Ref #3675

That issue stays open if this PR is closed without landing on main.

generateWithOpenAI fetched api.openai.com with no deadline. A stalled
response held publish preview until the Convex action limit. Pass
AbortSignal.timeout(10s) and keep the existing fallback changelog.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
@clawsweeper

clawsweeper Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@vercel

vercel Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

@SebTardif is attempting to deploy a commit to the OpenClaw Foundation Team on Vercel.

A member of the Team first needs to authorize it.

Assert on the captured spy instead of AbortSignal.timeout so
typescript(unbound-method) stays clean in ci:static.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
@clawsweeper clawsweeper Bot added P2 Normal backlog priority with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 5, 2026
@clawsweeper

clawsweeper Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed September 11, 2026, 10:09 AM ET / 14:09 UTC (Revision 2).

ClawSweeper review

What this changes

Adds a 10-second timeout to OpenAI changelog requests used by publishing and previews, with tests for successful responses and cancellation.

Merge readiness

Blocked before merge - 2 items remain

The fix remains necessary: current main and v0.23.3 still lack the deadline. No blocking patch defects were found, but the previous real-transport and fallback proof request remains unresolved.

Priority: P2
Reviewed head: c9a50516bb7f75d91b4867624e4082221eff3b64

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The patch is focused and source-supported, but the runtime evidence does not yet demonstrate the complete recovery behavior.
Proof confidence 🦐 gold shrimp (3/6) Needs stronger real behavior proof before merge: The captured Bun transcript exercises generateWithOpenAI with a substituted timer and observes rejection after 33ms, but does not establish a real transport request or a production caller returning fallback notes. The prior successful-response and stalled-request recovery proof request remains applicable; a redacted production-path terminal trace is sufficient, without a production outage or video. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: The captured Bun transcript exercises generateWithOpenAI with a substituted timer and observes rejection after 33ms, but does not establish a real transport request or a production caller returning fallback notes. The prior successful-response and stalled-request recovery proof request remains applicable; a redacted production-path terminal trace is sufficient, without a production outage or video. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 9 items Current main still needs the fix: The shared OpenAI fetch has no signal or deadline. All three callers catch rejection and return fallback notes, but an unresolved request cannot reach those fallbacks.
Release check: The v0.23.3 version of the changelog helper also lacks a request deadline; no shipped fix was established.
Introduced patch and fallback boundary: The verified merge-base-to-head patch adds the signal at the shared fetch and two focused tests. Existing catch-and-fallback behavior remains intact. Read callers in skills.ts, packages.ts and skillPublish.ts, plus openaiResponse.ts and embeddings.ts.
Findings None None.
Security None None.

How this fits together

ClawHub builds changelogs from uploaded readme content and file differences. Publishing and preview actions request OpenAI-generated notes and use local fallback notes when generation fails.

flowchart TD
  A[Readme and file changes] --> B[Publish or preview action]
  B --> C[OpenAI request with deadline]
  C --> D{Request succeeds?}
  D -->|Yes| E[Generated notes]
  D -->|No or timeout| F[Local fallback notes]
  E --> G[Returned changelog]
  F --> G
Loading

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: The captured Bun transcript exercises generateWithOpenAI with a substituted timer and observes rejection after 33ms, but does not establish a real transport request or a production caller returning fallback notes. The prior successful-response and stalled-request recovery proof request remains applicable; a redacted production-path terminal trace is sufficient, without a production outage or video. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Complete next step (P2) - Add real transport evidence for successful generation and stalled-request cancellation returning fallback notes. Redact keys, IP addresses and private endpoints; update the PR body to trigger re-review, or ask a maintainer to comment @clawsweeper re-review if needed.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
LOC +77/-1 across 2 files; production +4/-0, tests +73/-1 Production growth is limited to the deadline and test exports, with two focused regression tests.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #3675
Summary: This PR is the proposed implementation for the open changelog-timeout tracker.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Technical review

Best possible solution:

Bound the shared OpenAI request while preserving successful generated notes, user-supplied changelogs and existing fallback behavior.

Do we have a high-confidence way to reproduce the issue?

Yes, source establishes the failure path: a stalled OpenAI fetch prevents the awaiting callers from reaching fallback notes. No current-main runtime reproduction was executed.

Is this the best way to solve the issue?

Yes, the shared request is the narrowest fix location; caller-specific timers would duplicate policy, and a promise race would leave the request running. Runtime recovery proof remains outstanding.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against cbfee7343ddc.

Labels

Label justifications:

  • P2: This is a bounded publishing reliability improvement for stalled changelog requests, without evidence of a current widespread outage.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The captured Bun transcript exercises generateWithOpenAI with a substituted timer and observes rejection after 33ms, but does not establish a real transport request or a production caller returning fallback notes. The prior successful-response and stalled-request recovery proof request remains applicable; a redacted production-path terminal trace is sufficient, without a production outage or video. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Current main still needs the fix: The shared OpenAI fetch has no signal or deadline. All three callers catch rejection and return fallback notes, but an unresolved request cannot reach those fallbacks. (convex/lib/changelog.ts:98, cbfee7343ddc)
  • Release check: The v0.23.3 version of the changelog helper also lacks a request deadline; no shipped fix was established. (convex/lib/changelog.ts:98, 87ca030c30f3)
  • Introduced patch and fallback boundary: The verified merge-base-to-head patch adds the signal at the shared fetch and two focused tests. Existing catch-and-fallback behavior remains intact. Read callers in skills.ts, packages.ts and skillPublish.ts, plus openaiResponse.ts and embeddings.ts. (convex/lib/changelog.ts:112, c9a50516bb7f)
  • Captured proof and review continuity: The supplied snapshot, sourceRevision 89d94ac76dcf1e97aef86a339cfad1608fad17ad95a3c171ee82c3947d2d4a67, records a Windows/Bun call rejecting with AbortError after 33ms using a substituted timer. The helper is unavailable, so real transport use is unknown, not presumed mocked. Neither returned fallback notes nor a real successful request is demonstrated. The live REST body matches this evidence, and the earlier review at the same head requested those observations. (c9a50516bb7f)
  • Concrete runtime dependency: The Convex preview action invokes the changed helper, establishing that Convex's fetch and AbortSignal implementation is relevant. The adjacent embeddings helper already uses an AbortController and a request timer. (convex/skills.ts:10207, c9a50516bb7f)
  • Convex runtime dependency contract: The runtime installs AbortSignal from abortcontroller-polyfill; its package manifest pins version 1.7.6. The fetch implementation forwards requests through the production asynchronous fetch operation. (npm-packages/udf-runtime/package.json:11, 7ee142085fd2)

Likely related people:

  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Provide redacted runtime evidence showing a successful response and real stalled-request cancellation followed by returned fallback notes.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)
  • reviewed 2026-09-05T16:47:24.759Z sha c9a5051 :: needs real behavior proof before merge. :: none

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal backlog priority with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant