[freeqaz]

Problem

A survey of the tests added or changed on 2026-10-18 (outside files that open PRs touch) found three gaps:

• apps/controller/src/console/agents/list.mjs: the delayed row fill checks context.isCurrent() && reading.isConnected. No test pinned the context.isCurrent() half. Dropping it passed all 12 row-reading tests in console-agents-list-return.
• tests/integration/workspace-files-native.test.mjs still compared Number(counter.stdout.trim()) with 0, so empty counter output passed (Number("") === 0). Require an explicit zero in runtime listener tests #2838 fixed the same pattern in the native listener test.
• Two of today's waits were time-based:
  • kubernetes-real-kubectl's two-teardown late-cleanup test ordered its steps with 20/20/50 ms sleeps.
  • returnWithHeldRows().reveal() polled page.evaluate every 20 ms against a hand-rolled deadline.

Change

This PR changes tests only.

• console-agents-list-return
  • New case: a refocus holds the same list on screen while its own list read revalidates. The held list is inert and still connected, but no longer current. A reread that landed and is waiting out its 150 ms must not fill a held row. The case advances the page clock past the fill and checks the held rows still read "Reading deployment…".
  • reveal() now uses page.waitForFunction with the same 5 s bound in place of the poll loop.
• kubernetes-real-kubectl
  • The two-teardown test now orders its steps with promises plus one event-loop turn (setImmediate). Late cleanups drain in microtasks, so that turn is enough.
  • It now asserts that the second teardown's cleanup runs while the first teardown's cleanup is still blocked, and then checks the final order.
  • Both after() throws now check the stopsSetup message. That regex moved to module scope.
  • A finally releases the first cleanup, so a failing run never hangs the file.
• workspace-files-native: assert.equal(counter.stdout.trim(), "0").

Verification

• Test names are unchanged except for the one added case. No new files, so the suite mapping is unchanged.

• Hand-made mutants in list.mjs, run against the row-reading tests (13 with the new case):

  Mutant	Before	After
  render + 250 ms estimate	4 fail	5 fail
  hold 150 ms when never shown	2 fail	3 fail
  animationstart	5 fail	6 fail
  drop reading.isConnected	1 fail	1 fail
  drop context.isCurrent() in the timer	0 fail	1 fail

  Each run's "After" result was the same before and after the reveal() change.

• Mutants in tests/helpers/kubernetes-real.mjs, run against the two-teardown test:

  Mutant	Before	After
  file-wide count gates the drain (the original bug)	killed	killed
  missing throw to setup	killed	killed
  one file-wide serial drain queue	survived	killed

  A FIFO late order survives both versions here; the earlier late-cleanup test covers ordering.

• 20/20 on 2 CPUs (taskset) for console-agents-list-return and kubernetes-real-kubectl. workspace-files-native needs prepared runtime images, so it is left to its CI lane.

• Prettier and ESLint are clean on the touched files.

Risks

Test-only. The new browser case uses the page clock and the hold pattern this file already uses. workspace-files-native and kubernetes-real-kubectl keep their CI lanes.

[clawsweeper]

🦞👀
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.

[clawsweeper]

Codex review: needs maintainer review before merge.

What this changes

This PR strengthens stale-row and cleanup-order tests, replaces timing-based waits, and requires explicit zero model-call output.

Example: The native workspace test receives empty counter output.

• Before: Number("") evaluates to 0, so the assertion passes.
• After: The assertion fails because the output must explicitly contain "0".

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	Useful, focused test hardening supported by specific mutation results and successful checks for the touched suites.
Proof confidence	🐚 platinum hermit (4/6)	This maintainer-authored PR changes only tests; its reported mutation comparisons and head-linked CI are proportional validation, and no new shipped behavior requires separate live proof.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: Regression tests could miss stale writes into a held list, serialized cleanup queues, or absent model-counter observations.
Reason: The maintainer-authored direction adds independently useful assertions to existing suites without expanding product scope.

Merge readiness

✅ Ready for maintainer review

Keep open: this PR adds useful, distinct regression coverage without changing product behavior. No actionable introduced defect was found.

Priority: P3
Reviewed head: fe6378f5e9c42181d5b7cacdbbb7201be8184e7f

Before merge

None.

Findings

None.

Agent review details

How this fits together

The existing browser, conformance, and native-runtime suites exercise console view lifetimes, test-resource cleanup, and workspace operations, then assert DOM state, cleanup ordering, and model-call observations.

flowchart TB
  T[Test suites] --> C[Console view lifecycle]
  C --> D[Held row assertions]
  T --> H[Bounded teardown helper]
  H --> O[Independent cleanup assertions]
  T --> N[Native workspace runtime]
  N --> Z[Explicit zero assertion]

Loading

Technical review

Best possible solution:

Retain the focused assertions and deterministic coordination in the existing suites.

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

Source inspection confirms that empty counter output passes the old numeric assertion and that the old cleanup assertion permits serialization; the PR also reports targeted mutation comparisons for the added browser case.

Is this the best way to solve the issue?

The patch strengthens existing test owners without adding production seams, duplicating a capability, or changing the runtime contract.

AGENTS.md: found and applied where relevant.

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

Provenance checked

• Console row-delay coverage keeps the original intent (3ac90c1: fix(console): count the row loading text's 150 ms from the frame it shows in #2837 measures the loading hold from the animation's actual end; this PR preserves that behavior and tests the separate held-view guard.)
• Bounded late-cleanup coverage keeps the original intent (83d12d1: test: end bounded teardown promptly when setup outlives its test #2796 and test: see the file's end when a file hook fails, and stop late setup safely #2799 require bounded late cleanup and cooperative setup termination; the revised test preserves those requirements and checks independent draining.)
• Native workspace model-call observation keeps the original intent (f8c4083: fix(workspace): follow the embedded native Agent identity #1956 verifies native workspace operations without model execution; requiring explicit zero strengthens the existing observation.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

• Pinned test-only introduction: The verified introduction changes only the three test files: 66 additions and 22 deletions, with no production, dependency, workflow, or persistence changes. (fe6378f5e9c4)
• Distinct held-view coverage: The new case holds list revalidation while the old rows remain connected and inert, advances beyond their delayed fill, and checks that all three remain unfilled; this specifically protects the existing context.isCurrent() guard. (tests/browser/console-agents-list-return.test.mjs:608, fe6378f5e9c4)
• Independent cleanup queues: The revised assertion requires the second teardown's cleanup to complete while the first remains blocked, matching the helper's per-teardown drain ownership and catching a shared serial queue that the previous sorted-result assertion allowed. (tests/conformance/kubernetes-real-kubectl.test.mjs:481, fe6378f5e9c4)
• Related explicit-zero repair: The merged Require an explicit zero in runtime listener tests #2838 repairs the same numeric-coercion weakness in native-listener tests; this PR applies the focused assertion repair to the separate workspace-native case. (tests/integration/workspace-files-native.test.mjs:351, fe6378f5e9c4)
• Current main retains these gaps: Comparison against fetched main confirms that the three focused test improvements remain unique to this PR. (6f62cbd9ea28)
• Head-linked validation: CI run https://github.com/openclaw/openclaw-enterprise/actions/runs/39128809188 identifies the reviewed head; Console Browser 2, Checks and Conformance 1, Image Runtime Startup, and CI Required report success, while other checks remain in progress. (fe6378f5e9c4)

Review metrics

Metric	Value	Why it matters
Change scope	Production +0/-0; tests +66/-22	The growth adds distinct held-view and concurrent-cleanup assertions within existing suites.

Labels

Label changes:

• add P3: This is focused test reliability and coverage maintenance with no shipped behavior change.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

• P3: This is focused test reliability and coverage maintenance with no shipped behavior change.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 18, 2026, 9:01 AM ET / 13:01 UTC.

[freeqaz-openai]

I would keep this change. I checked that the new teardown test catches a real loss of coverage: in a disposable copy, I made two teardowns share one late-cleanup queue. The test on main passed with that defect. This version failed because the second cleanup had not run while the first was held, then passed with the actual per-teardown queues. The finally also releases the held cleanup if the assertion fails.

The other two changes make sense: the browser test observes the old list while it is still present but inert, and the native workspace test no longer mistakes empty output for an observed zero.

I verified the hosted results on the merge preview of 6f62cbd9 + 2b85ca7b: 339 browser, 2,579 conformance/baseline, and 18 runtime-startup cases passed, including all three changed cases. My review covers source and those results; the visible ClawSweeper verdict still names the previous head, so I am leaving final review and merge with the owner.