Skip to content

test(runtime-host): give async wait helpers explicit time budgets - #4511

Open
qianzhu18 wants to merge 1 commit into
apache:mainfrom
qianzhu18:test/runtime-host-wait-budgets
Open

test(runtime-host): give async wait helpers explicit time budgets#4511
qianzhu18 wants to merge 1 commit into
apache:mainfrom
qianzhu18:test/runtime-host-wait-budgets

Conversation

@qianzhu18

Copy link
Copy Markdown

Summary

The same failure class as #4383, applied to the packages/runtime-host test suite. Several helpers poll for an asynchronous condition with a fixed count of macrotask ticks (or immediates). A tick count is not a time budget: on a loaded CI runner the underlying work — turn settlement over real UDS connections, agent-graph wake-run persistence, peer mesh roster propagation over real serve/join networking, usage and artifact persistence — can span more ticks than the loop allows, so the helper gives up before the condition is genuinely false and the failure cannot be distinguished from a real regression.

This converts the fixed-tick loops to the shared waitFor primitive from @maka/core/test-only/async-primitives with an explicit 5s wall-clock deadline and a 10ms poll interval:

  • execution-model-composition.test.ts — graph-wake loop, startTurn, waitForTerminal, waitForUsage, waitForCanonicalAttempts, waitForCaptureArtifacts, waitForAutomaticMemoryRequestsToSettle
  • plan-two-client-uds.test.tswaitForTerminal
  • runtime-policy-coordinator.test.ts — inline turn-settlement loop (now via the file's existing pollFor import)
  • peer-mesh.test.ts — roster propagation loop (previously a 200ms budget)
  • peer-native.test.tswaitForRequestCount (previously 10 immediates)

Each helper keeps its failure message; waitForCanonicalAttempts and waitForAutomaticMemoryRequestsToSettle keep their detailed diagnostics by building them from the final state when the wait times out. The in-memory setImmediate helpers (waitForPending, waitForGoalRun) are intentionally left alone — they poll in-memory facades where a tick budget is effectively unbounded. Production behavior is unchanged; this is test-infrastructure only.

Fixes#4510

Verification

  • npm run --workspace @maka/runtime-host build — clean (no TS errors)
  • node --test on all five converted files: 69/69 pass on macOS arm64 (28 in execution-model-composition, 11 plan-two-client-uds, 4 peer-native, 19 peer-mesh, 17 runtime-policy-coordinator — same totals as the pre-change baseline)
  • biome check on the five files — clean

Not run: the repository-wide CI matrix (Windows-only suites noted in #4390 do not affect these files; none of the converted files is Windows-gated except execution-model-composition, which skips on win32 at the describe level).

AI use

  • Generative tooling made a substantive contribution

Several runtime-host test helpers poll for asynchronous conditions
(turn settlement over UDS, agent-graph wake-run persistence, peer mesh
roster propagation, usage and artifact persistence) with a fixed count
of macrotask ticks. A tick count is not a time budget: on a loaded CI
runner the underlying work can span more ticks than the loop allows,
so the helper gives up before the condition is genuinely false.
Convert the fixed-tick loops to the shared waitFor primitive from
@maka/core/test-only/async-primitives with a 5s wall-clock deadline
and a 10ms poll interval, keeping each helper's failure message and
diagnostics. In-memory setImmediate helpers are intentionally left
alone. Production behavior is unchanged.
Fixesapache#4510
@qianzhu18

Copy link
Copy Markdown
Author

Friendly ping — this has been open ~5 days while the queue moves quickly, so it may have slipped past triage.

Two pointers that may help: it's the runtime-host counterpart of #4383 (the same fixed-tick wait-budget class that #4387 fixed for packages/runtime), and it closes #4510 with the full inventory listed there. I noticed #4510 was assigned after this PR was opened — no duplicate work is in flight (as noted on the issue).

Happy to split it per-file or adjust scope if that makes review easier.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A note on current state, not a technical verdict: this PR is currently CONFLICTING and its merge-base sits 183 commits behind origin/main, and it has never had test checks run on it. Any line-level review now would not survive the rebase.

Could the author please rebase onto current main and let CI run? Happy to review once it is rebased and green — there is already one substantive observation queued for then (a time-budget change that also substantially loosens one budget, plus a suggestion to carry with the rebase).


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

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

Labels

effort/MUnder 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(runtime-host): wait helpers poll on fixed tick budgets and can flake under load

2 participants

@qianzhu18@Astro-Han