feat(kiloclaw) improve openclaw test harness - #4877
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryReviewed the incremental changes (surfacing the websocket close reason for gateway transport errors and clarifying the org-credit/1008 documentation) and found no high-confidence bugs, security issues, or logic errors in the changed lines. Files Reviewed (4 files)
Previous Review Summary (commit 04dbbb0)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 04dbbb0)Status: No Issues Found | Recommendation: Merge Executive SummaryReviewed the OpenClaw smoke-test harness changes (org-credit preflight, gateway warm-up gating, transport-error retry, and docs) and found no high-confidence bugs, security issues, or logic errors in the changed lines. Files Reviewed (4 files)
Reviewed by claude-sonnet-5 · Input: 26 · Output: 6.8K · Cached: 747.1K Review guidance: REVIEW.md from base branch |
…guidance The live smoke discarded the websocket close reason on a failed agent turn, so the string that names the cause never reached the operator. Print it for transport errors: it is generated by OpenClaw's own loopback gateway inside the container, not by the model provider, so it carries no credentials. Provider error bodies stay suppressed. This surfaced the real cause of a failing turn: 'pairing required: device is asking for more scopes than currently approved'. OpenClaw's in-container gateway auto-approves the CLI device at operator.read, then refuses its upgrade to operator.write and closes with 1008. That also disproves the guidance added earlier in this branch, which told the reader a 1008 meant a missing organization id. It does not; a 1008 reproduces with the org id correctly set, and the credential symptom is a 402. Correct the preflight message and the README to describe the two separately, and stop printing the 'message suppressed' caveat when a reason was in fact shown.
1 participant
Summary
The credentialed live smoke (
smoke-live-provider.sh) could fail for reasonsthat had nothing to do with the image under test, and it suppressed the error
text, so those failures were hard to tell apart from real ones. This fixes three
causes and makes the remaining failures name themselves.
Organization scope was silently dropped on the documented path. The org id
was only read from the Kilo CLI config when the token also came from that config.
Exporting
KILOCODE_API_KEY, which is what the README, the validator, and theimage checks all tell you to do, skipped org discovery entirely and ran with no
org scope. A personal token in an organization then spends personal credits
rather than org credits, and the live turn fails inside the gateway as
402 add credits, a websocket1008close, or "provider rejected the requestschema". None of those name the real cause, and all of them read as a broken
image.
The readiness check gated on the wrong thing.
wait_for_readypolls thecontroller's
/_kilo/health, which reportsreadyas soon as the controller isup. The gateway is still loading provider plugins and running model discovery for
several seconds after that, and a call landing in that window dies with a
websocket
1006abnormal closure.Failures were unattributable. On error the smoke printed
command failedandsuppressed the whole body, so a startup race and a real provider rejection looked
identical.
Changes:
from, and report which source it came from.
invalid token exits immediately by name. A token belonging to an organization
with no
KILOCODE_ORGANIZATION_IDset stops the run, lists the org names andids from the profile, and prints the export line to fix it.
ALLOW_NO_ORG_SCOPE=trueoverrides it for the case where the token genuinely has personal credits.
wait_for_gateway_serving(), which pollsopenclaw gateway call statusuntil the gateway actually answers. It runs after first boot and again after
assert_hook_config_self_heal, whose own wait also only returns the controllerto ready.
1006warm-up signature, never on othercloses such as
1008policy violations and never on provider errors, so realincompatibilities still fail on the first attempt.
type,kind,code) on failure whilekeeping the message body suppressed, since provider errors can carry live
credentials.
and the image checks footer.
Verification
Run against prebuilt OpenClaw images on a local machine. Nothing here runs in CI.
28 passed, 0 failed, repeated three times. Both readiness gates fired
(
gateway serving after 1sandafter 2s) and no retries were needed.at the live turn with
gateway_transport_error closed 1006, and passed whena 45 second delay was inserted before the turn.
readyat 3s while thegateway did not answer a
statuscall until several seconds later.on the first attempt and now reports
error: gateway_transport_error closed 1008instead ofcommand failed.message instead of building an image and failing later.
Reviewer Notes.
Visual Changes
N/A
Reviewer Notes
Two things I did not verify and would flag:
stops the run) has not been exercised against a real org token. Only the
invalid token branch and the syntax were tested.
1006is defense in depth and was not exercised by any passingrun. The readiness gate alone closed the window in every case, so retry never
triggered. If reviewers would rather not carry untested retry logic, dropping
it and keeping only the readiness gate is reasonable, and the smoke would still
pass the runs above.
The preflight adds a network call to
https://api.kilocode.ai/api/profilebeforethe smoke starts. It is overridable with
KILOCODE_API_BASE, and an unreachableendpoint warns and continues rather than blocking the run.