Skip to content

fix(engine): replay downtime-queued pending messages and retry infra failures - #8

Merged
ranxianglei merged 4 commits into
masterfrom
2026-09-17_pending-replay-infra-retry
Sep 17, 2026
Merged

ranxianglei merged 4 commits into
masterfrom
2026-09-17_pending-replay-infra-retry

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Closes #5

Problem (verified)

The ework#4 message was queued at 00:15Z while the VM was down for disk expansion. After reboot, the stale-pending guard measured age from created_at (32 min over the 30-min limit), marked the message failed with expired: stale pending message not replayed, and the user input was lost forever. Separately, infrastructure-class failures (web unreachable / ENOSPC / child killed by signal) were treated as terminal content failures.

Changes

1. Pending age on the engine's active clock (opencode.ts, op.ts, db.ts)

  • New messages.pending_since: when the message (re-)entered pending; the stale guard now ages from pending_since ?? created_at.
  • recover() shifts pending_since to now for this daemon's owned issues only at boot — downtime does not age the queue; other daemons' queues keep their original clock (multi-machine safe).

2. Infra-class failures retry on a dedicated budget (opencode.ts, config.ts)

  • New classification: web unreachable, ENOSPC, child killed by signal (exit >= 128).
  • Dedicated infra_attempts budget (WORK_INFRA_RETRY_MAX, default 3) with exponential backoff WORK_INFRA_RETRY_BASE_MS * 2^(attempt-1) (default base 15s); the retry_after hold is enforced atomically in claim/pickup paths.
  • Content-failure attempts stays untouched by infra retries; terminal failed only after the budget is exhausted. A visible notice comment is posted on the first requeue.

3. Tests (stale-pending.test.ts, new infra-retry.test.ts)

  • Downtime scenario: a backdated-over-limit pending message is replayed after restart, not expired; the clock shift never touches another daemon's queue.
  • Infra ladder: N consecutive infra failures -> terminal failed only after budget exhaustion (infraAttempts == limit, content attempts == 1); success after one infra failure -> done (content attempts stay 0); infraRetryMax=0 disables retries; web-unreachable gate path retries before failing with gate:.

Test harness hermeticity (prevents recurrence of the incident that blocked this requeue): tests/setup.ts pins a per-PID temp DB under os.tmpdir() regardless of inherited WORK_DB_PATH/DAEMON_DB_PATH, drops inherited WORK_WAKE_LOGINS, replaces hardcoded /tmp fixtures with tmpdir(), and pins a dead gitea url in reopen-heal so its wake-logins fetch fails closed deterministically.

Verification

  • bun run check (tsc --noEmit): clean
  • bun test (packages/daemon): 241 pass / 0 fail across 30 files, run against the temp DB pinned by setup.ts

中文摘要:修复停机期间排队的 pending 消息被 stale-expiry 吞掉的问题(重启时只把本 daemon 名下队列的计时器拨回现在,停机时间不计入老化),并新增基础设施类失败(网关不可达 / ENOSPC / 子进程被信号杀死)的独立重试预算与指数退避(默认 3 次),重试耗尽才终态失败;已补重启重放与完整重试阶梯测试,全套 241 个测试通过。

…failures

ework#5: a message queued while the daemon was down was expired by the
stale-pending guard (age measured from created_at) and never ran again.

- age pending messages from pending_since instead of created_at; recover()
  shifts pending_since to now for this daemon's owned issues so time spent
  down does not age the queue (multi-machine safe: other daemons' queues
  keep their original clock)
- new infra-failure class (web unreachable, ENOSPC, child killed by
  signal): dedicated infra_attempts budget (WORK_INFRA_RETRY_MAX, default
  3) with exponential backoff (WORK_INFRA_RETRY_BASE_MS * 2^attempt);
  content-failure attempts stay untouched; terminal failure only after the
  budget is exhausted; retry_after hold enforced atomically in claim paths
- migration + backfill for pending_since / infra_attempts / retry_after
- tests: restart-replay scenario (stale-pending.test.ts) and the full
  infra-retry ladder: budget exhaustion, success-after-retry, disabled
  budget, web-unreachable gate path (infra-retry.test.ts)
- test harness hermeticity: per-PID temp DB under os.tmpdir() pinned in
  tests/setup.ts regardless of inherited WORK_DB_PATH/DAEMON_DB_PATH,
  drop inherited WORK_WAKE_LOGINS, replace hardcoded /tmp fixtures with
  tmpdir(), pin a dead gitea url in reopen-heal so the wake-logins fetch
  fails closed deterministically
@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework

[bot] 🏷 收到。这是 [PR] 类 issue,对应 GitHub PR #8(分支 2026-09-17_pending-replay-infra-retry → master,open)。我不会重做实现,现在拉取分支核对代码、跑 bun run check + bun test(packages/daemon)、查 CI 状态,稍后在本线程给出审查结论。

…; hermetic web test db

Review fixes for the pending-replay / infra-retry change:

- op.ts/opencode.ts: requeue infra failures with a single atomic UPDATE that
  sets status=pending, pending_since and retry_after together. The previous
  two-statement sequence (updateMessageStatus then setRetryAfter) left a
  window where the message was pending but its hold was not yet visible
  (status transitions clear retry_after), so a racing daemon or the global
  drain could claim it before the backoff elapsed.

- db.ts: restrict the boot-time pending_since backfill to rows still in
  pending state; transitions out of pending null the clock on purpose, so
  historical done/failed rows must not be re-stamped on every boot.

- web/tests/setup.ts: pin the per-PID temp DB under os.tmpdir() instead of
  hardcoded /tmp (read-only on some hosts), mirroring the daemon-side
  setup.ts fix already in this PR. No behavior change on runners where
  tmpdir() is /tmp.
@ranxianglei

ranxianglei commented Sep 17, 2026 •

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

Review complete — I found 3 real problems, fixed them directly on the branch (commit 35a5242), and the fresh CI run is fully green. Verdict: mergeable.

What I verified against the PR description

  • Code matches the claims: pending_since clock with boot-time shift scoped to this daemon's owned issues (shiftPendingSinceForOwned joins through op_sessions→issues.owner_daemon_id, so other daemons' queues keep their original clock); infra classification (web unreachable / ENOSPC / exit ≥ 128) on a dedicated infra_attempts budget with base * 2^(attempt-1) backoff; retry_after hold enforced in the pickup paths and atomically inside claimMessage; content-failure attempts untouched; notice comment on first requeue.
  • Reproduced locally on the branch: bun run check clean; packages/daemon 241 pass / 0 fail across 30 files (exactly the claimed numbers, incl. the new infra-retry.test.ts ladder and the downtime-replay cases).

Original CI failure — transient, not caused by this PR

The failed job died in packages/web/tests/csp.test.ts: its spawned server never came up within the 20s poll (web did not come up on …). This PR touches zero files under packages/web, every other server-spawning web test passed in the same run, and master's CI was green. After pushing my fixes, the same job passed on the new commit — confirming flake. All three jobs (type-check+unit, Docker smoke, Docker E2E) are now green at 35a5242; PR mergeable state is clean.

Fixes I committed directly (35a5242)

  1. Claim-window race in infra requeue (op.ts, opencode.ts): requeueInfraFailure did updateMessageStatus(pending) then setRetryAfter(...) as two separate writes — and status transitions clear retry_after, so between the two statements the message was pending and claimable with no hold visible. A racing daemon (or the global drain) could pull it out before the backoff elapsed, defeating the retry budget's pacing. Replaced with a single atomic UPDATE (Store.requeueWithBackoff) that sets status, pending_since, retry_after, error together.
  2. Boot backfill over-stamped historical rows (db.ts): the migration backfill ran SET pending_since = created_at WHERE pending_since IS NULL on every row — including done/failed history whose clock was deliberately nulled on transition — re-stamping the whole table on every boot. Scoped to AND status = 'pending'.
  3. Web test DB path hardcoded to /tmp (packages/web/tests/setup.ts): same defect class your daemon-side setup.ts fix in this PR already addressed; /tmp is read-only on some hosts (including this review environment, which is what blocked local verification of the web suite). Now uses os.tmpdir() — identical path on runners where tmpdir() is /tmp, so no CI behavior change.

Findings reported only (non-blocking, pre-existing)

  • bun.lock drift: lockfile pins @ework/aio 0.5.80 while packages/aio/package.json says 0.5.81 — bun install rewrites the lockfile on any machine. Unrelated to this PR; worth a small separate chore commit.
  • 9 web tests fail locally only when running the full suite (wake-logins.test.ts ×7, upstream-sync.test.ts ×2). Proven pre-existing/environmental: identical failures on origin/master with the same tmpdir fix applied; they pass in CI and pass locally in isolation/pairs; ruled out inherited-env leakage (stripped every daemon-ish var, same result) and port collision (bun test runs all files in one process → identical pid offsets, distinct port bases). Only remaining delta vs CI is local bun 1.4.0 vs the CI-pinned 1.3.14. Nothing here blocks the PR.

中文摘要:审查通过——代码与描述一致、本地 241/0 测试复现成功;原 CI 失败是 web 包(本 PR 未改动)的偶发启动超时,重跑已全绿;我直接在分支上修了 3 个问题(infra 重试入队的竞态窗口改为单条原子 UPDATE、启动回填限定 pending 行、web 测试库路径去掉硬编码 /tmp),当前 CI 三项全过,可以合并。

… web reconciliation

- recover(): boot-time scan of all non-terminal messages (running/interrupted/pending), age-blind, requeued per session (restart = clock break)
- hasRecoveryDelivery: only bot replies posted strictly after promptTime and free of in-progress wording count as delivery; used by both boot recovery and the stranded-interrupted sweep (fixes the acp-kernel#310 misjudged-done class)
- reconcileWebComments: web comment stream is the source of truth; comments consumed during the failure window but never persisted are requeued, deduped by comment id, floored at the newest recorded message, wake policy mirrored (fixes the bc#853 consumed-not-delivered class)
- per-boot recovery report (interrupted/requeued/delivered/backfilled/deferred/queued_now) as a [system] admin notice, WORK_RECOVERY_REPORT=0 mutes the prefix
- tests: restart-recovery harness simulating hard death via lease expiry; covers zero-touch respawn with delivery, misjudged-done, downtime-pending replay (composed with #5), consumed-not-delivered backfill; unit tests for looksLikeInProgress and hasRecoveryDelivery
…nified

daemon: unified restart recovery — boot reconciliation, event replay, strict delivery check
@ranxianglei
ranxianglei merged commit 1d5057f into master Sep 17, 2026
3 checks passed
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.

daemon: 停机期间 pending 消息被 stale-expiry 吞掉,重启后不重放(ework#4 实锤)

1 participant