Skip to content

[codex] add gateway stall diagnostics#77542

Closed
steipete wants to merge 5 commits into
mainfrom
codex/gateway-stall-diagnostics
Closed

[codex] add gateway stall diagnostics#77542
steipete wants to merge 5 commits into
mainfrom
codex/gateway-stall-diagnostics

Conversation

@steipete

@steipete steipete commented May 4, 2026

Copy link
Copy Markdown
Contributor

Summary

This adds higher-signal gateway stall diagnostics for slow Telegram turns where the transport is healthy but the Gateway/embedded Codex run stops making useful progress.

Changes:

  • Add a lightweight diagnostic phase ring and emit diagnostic.phase.completed for Gateway and CLI startup spans.
  • Include the current phase, recent completed phase spans, and bounded active/waiting/queued work labels in diagnostic.liveness.warning events and logs.
  • Preserve phase/work attribution in the diagnostic stability recorder and bundle parser.
  • Mark stalled embedded runs with terminalProgressStale=true when the last bridge progress looked terminal, such as rawResponseItem/completed, but the Gateway still considers the embedded run active.
  • Add OPENCLAW_TRACE_SYNC_IO=1 support in scripts/run-node.mjs to pass Node --trace-sync-io through the normal source runner for startup I/O investigations.
  • Enable OPENCLAW_TRACE_SYNC_IO=1 by default for pnpm gateway:watch Gateway children, while preserving explicit overrides like OPENCLAW_TRACE_SYNC_IO=0.
  • Document the new liveness/stall fields and sync-I/O trace behavior.
  • Improve Slack socket-mode startup retry logs so missing SDK error payloads report unknown error instead of undefined.

Why

The Telegram path was not the bottleneck: transport probes were fast, but Gateway sessions could remain active after the Codex bridge had already reported terminal-looking progress. Existing liveness warnings only showed aggregate counts (active, waiting, queued) plus event-loop/CPU numbers, so diagnosing the next stall required manual tmux/log archaeology.

With this change, the next warning should say which phase was active, what recent startup/runtime spans completed, which session/work label is blocking, and whether the bridge emitted terminal progress without closing the embedded run. Gateway watch also records sync I/O traces by default so local dev stalls have file/module-loader evidence without needing to remember the env flag.

Operator impact

No behavior change to scheduling, Telegram delivery, recovery thresholds, or restart policy.

Manual one-off source-runner tracing remains available:

OPENCLAW_TRACE_SYNC_IO=1 pnpm openclaw gateway --force

Gateway watch enables the same trace mode by default for the watched child. To suppress Node sync I/O trace output in watch mode:

OPENCLAW_TRACE_SYNC_IO=0 pnpm gateway:watch

Verification

  • pnpm test src/logging/diagnostic.test.ts
  • pnpm test src/infra/run-node.test.ts
  • pnpm test src/infra/watch-node.test.ts
  • pnpm test src/infra/gateway-watch-tmux.test.ts
  • pnpm tsgo:core
  • pnpm tsgo:core:test
  • pnpm format:docs:check
  • pnpm docs:check-mdx
  • pnpm tsgo:extensions:test
  • pnpm tsgo:extensions
  • pnpm test extensions/slack/src/monitor/provider.reconnect.test.ts
  • git diff --check

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation gateway Gateway runtime cli CLI command changes scripts Repository scripts size: M maintainer Maintainer-authored PR labels May 4, 2026
@clawsweeper

clawsweeper Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Keep open: the protected maintainer label prevents conservative cleanup close, and the branch is not merge-ready because it reintroduces default-on gateway-watch sync-I/O tracing, lacks real behavior proof, and is dirty/uneditable while current main already carries the central diagnostics with opt-in tracing.

Canonical path: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

So I’m closing this here because the remaining work is already tracked in the canonical issue.

Review details

Best possible solution:

Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

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

Not applicable: this is a diagnostic feature/default-change PR, not a bug report with a current-main failing reproduction path. The blockers come from source comparison, live PR state, Codex source inspection, and missing real behavior proof.

Is this the best way to solve the issue?

No, not as submitted. Current main already carries the central diagnostics while keeping gateway-watch tracing opt-in, so the maintainable path is a refresh or replacement that preserves that default unless maintainers explicitly approve changing it.

Security review:

Security review cleared: No concrete security or supply-chain concern found; the diff changes diagnostics, script-runner flags, docs, tests, and Slack log formatting without dependency, workflow, secret, permission, or download changes.

AGENTS.md: found and applied where relevant.

What I checked:

  • stale F-rated PR: PR was opened 2026-05-04T21:35:33Z, is older than 60 days, and the latest review rated it F.
  • proof blocker: real behavior proof is missing and proof tier is F, so this branch is not merge-ready without contributor follow-up.
  • no human follow-up: live comments and timeline hydrated by apply contain no non-automation activity after the ClawSweeper review.

Likely related people:

  • steipete: Peter Steinberger authored this PR branch and the current-main commit that added the central Gateway stall diagnostic implementation. (role: introduced diagnostic behavior; confidence: high; commits: e84d4b27f44c, 659cc50f209e; files: src/logging/diagnostic.ts, src/logging/diagnostic-phase.ts, src/infra/diagnostic-events.ts)
  • kevinslin: Kevin Lin authored the merged follow-up that restored gateway-watch sync-I/O tracing to opt-in behavior with docs, tests, and real behavior proof. (role: watch default follow-up owner; confidence: high; commits: e984a99c7e00; files: scripts/watch-node.mjs, src/infra/watch-node.test.ts, docs/help/debugging.md)
  • Vincent Koc: Recent current-main history shows Vincent Koc maintaining watch-node shutdown behavior in the same script surface. (role: recent watch-node contributor; confidence: medium; commits: 6585cb3b442b; files: scripts/watch-node.mjs)

Codex review notes: model internal, reasoning high; reviewed against 010b61746379.

@steipete
steipete force-pushed the codex/gateway-stall-diagnostics branch from 1ae7805 to cc1e702 Compare May 4, 2026 22:02
@steipete
steipete marked this pull request as ready for review May 4, 2026 22:02
@openclaw-barnacle openclaw-barnacle Bot added the channel: slack Channel integration: slack label May 4, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. labels Jun 14, 2026
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Closing as stale and superseded by current diagnostics. Current main contains the phase/liveness diagnostics through a792068d9da while keeping sync-I/O tracing opt-in; this branch would reintroduce noisy default-on gateway-watch tracing and lacks the required live proof.

@steipete steipete closed this Jul 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: slack Channel integration: slack cli CLI command changes docs Improvements or additions to documentation gateway Gateway runtime maintainer Maintainer-authored PR merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. scripts Repository scripts size: M 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