Repository navigation
test(cloud-agent-next): run the shared e2e scenarios against a deployed stack - #6284
Merged
eshurakov merged 2 commits intoSep 22, 2026
Merged
Conversation
Contributor
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (4 changed files)
Fix these issues in Kilo Cloud Previous Review Summaries (3 snapshots, latest commit f692114)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit f692114)Status: 8 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (55 files)
Fix these issues in Kilo Cloud Previous review (commit 4cf15e9)Status: 8 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (55 files)
Fix these issues in Kilo Cloud Previous reviewStatus: 6 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (55 files)
Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
eshurakov
force-pushed
the
eshurakov/jovial-starling
branch
2 times, most recently
from
September 17, 2026 20:31
9211a9e to
e453de4
Compare
eshurakov
force-pushed
the
eshurakov/jovial-starling
branch
from
September 17, 2026 21:01
e453de4 to
4cf15e9
Compare
eshurakov
force-pushed
the
eshurakov/jovial-starling
branch
from
September 21, 2026 11:29
4cf15e9 to
f692114
Compare
…ed stack Give the harness one implementation per scenario that runs under either the local or the deployed profile, and drive it through an e2e-only Worker surface that uses only mechanisms production also has. - Deployed profile: E2E_PROFILE=deployed drives a real Cloudflare stack. Auth resolves env-first and then from an ordinary personal-token file, so E2E_USER_TOKEN + FAKE_LLM_ADMIN_TOKEN alone are enough; the path never reads .dev.vars, root env files or Postgres, and never mints a credential. assertDeployedProfileEnv owns source precedence and resolveStreamTicket owns the connected-stream credential for both profiles. - Shared scenarios: cold-hot, unknown-model, auth-reject, chunked-streaming, empty-response, waiters-clean, llm-error, the three callback scenarios and nine queue/continuity/micro scenarios now have one shared definition, so 19 of the 34 scenarios run under either profile. scenario-capabilities.ts owns the gate (a missing injected environment, and a local profile without the mandatory `sandbox` capability, are errors before any side effect; a declared-but-absent capability is an explicit `unsupported`; a throwing capability stays a failure) and run.ts owns the 1/2/0 exit policy. - E2e surface: an e2e-only entrypoint exposing /__e2e/*, reachable only behind the e2e-scoped INTERNAL_API_SECRET sent as x-internal-api-key, compared constant-time and failing closed when unset, with the caller's JWT still required on every route except the callback ingest route, which is authenticated by its own unguessable path token. The production entrypoint and its routes are untouched and the surface is unreachable there by construction, not by a flag. Callback bodies are read as raw bytes and cancelled past 64 KiB, bounded to 50 records with a one-hour TTL. - Less duplicated ownership: worktree ownership is read from getSession instead of a second direct cli_sessions_v2 query, the transcript reduction skips CLI progress parts marked `kilocode.lifecycle == transient` so the echoed payload stays at the end of the observed text, gate tags are run-scoped with one release/leak owner, and API selection has one owner that fails a conflicting pin instead of skipping it. - deploy-e2e-worker.sh no longer requires INTERNAL_API_SECRET on redeploy: wrangler never clears an existing Worker secret, so a supplied value now means "rotate" and its absence reports that the deployed value is kept. Honest gaps: test/e2e/** is outside the package typecheck; the retryable provider-error path stays uncovered because llm-error now asserts the terminal failure the product actually produces (its retry premise was stale); the Docker profile's full scenario remains unverified locally for an egress-proxy reason (dialing 127.0.0.1:3000 while this worktree's Next.js is on 8300); and the deployed matrix currently fails five interrupt/queue scenarios. That cause is now established - a cross-machine deadline comparison in the product Worker - and is fixed in #6281. No behaviour change in this commit: the harness and dev-launcher files are also reformatted to satisfy the repo-wide `oxfmt --list-different .` check, which the package-level formatter (src only) does not cover.
- chunked-streaming / empty-response: hoist the stream and close it in a `finally`, so a throwing `requireContainer` cannot leave an open, subscribed WebSocket behind - hot: track the warm-up stream so the existing `finally` closes it on the allocation-failure path too - fetchStreamTicket: bound the ticket POST with AbortSignal.timeout(10s) so a backend that accepts but never responds cannot hang the scenario past its deadline, and report a stalled body-read as a timeout rather than as invalid JSON; the bound is injectable so tests can use a small value
eshurakov
force-pushed
the
eshurakov/jovial-starling
branch
from
September 21, 2026 14:13
f692114 to
b59387d
Compare
pandemicsyn
approved these changes
Sep 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Gives the e2e harness one shared implementation per scenario that runs under either the local or the deployed profile, and drives the deployed profile through an e2e-only Worker surface built only from mechanisms production also has.
Base is
eshurakov/deployed-e2e-infra, which supplies the deployed fake LLM Worker and its Durable Object. Merge that one first.E2E_PROFILE=deployeddrives a real Cloudflare stack. Authentication resolves environment-first and then from an ordinary personal-token file, soE2E_USER_TOKEN+FAKE_LLM_ADMIN_TOKENalone are enough. The path never reads.dev.vars, root env files or Postgres, and never mints a credential. Container identity is observed through the e2e surface instead of a local Docker daemon.cold-hot,unknown-model,auth-reject,chunked-streaming,empty-response,waiters-clean,llm-error, the three callback scenarios, and nine queue/continuity/micro scenarios. The other 15 remain local-only because they need Docker to inspect or kill containers.scenario-capabilities.tsdecides whether a scenario can run before any side effect: a missing injected environment is an error, a conflicting API pin is an error, a local profile without the mandatorysandboxcapability is an error, a declared-but-absent capability is an explicitunsupportedrather than a silently weakened assertion, and a throwing capability stays a failure.run.tsowns the exit policy:1any failure,2any unsupported,0clean./__e2e/*, reachable only behind the e2e-scopedINTERNAL_API_SECRETsent asx-internal-api-key, compared constant-time and failing closed when unset, with the caller's JWT still required on every route except the callback ingest route, which is authenticated by its own unguessable path token. The production entrypoint and its routes are untouched, and the surface is unreachable there by construction rather than by a flag. Callback bodies are read as raw bytes, bounded to 50 records with a one-hour TTL, and a body past 64 KiB is cancelled.e2e:deployed(smoke-deployed.ts) runs every shared scenario against a deployed Worker, with a tolerant per-scenario cleanup backstop so one stuck session cannot hide the rest, and reports passed / failed / unsupported as distinct categories..github/workflows/e2e-deployed.ymlis aworkflow_dispatch-only runner around it.getSessioninstead of a second directcli_sessions_v2query; transcript reduction skips CLI progress parts markedkilocode.lifecycle == transientso the echoed payload stays at the end of the observed text; gate tags are run-scoped behind one release/leak owner; and API selection has one owner that fails a conflicting pin instead of skipping it.deploy-e2e-worker.shno longer requiresINTERNAL_API_SECRETon redeploy: wrangler never clears an existing Worker secret, so a supplied value now means "rotate" and its absence reports that the deployed value is kept.local-httpprofile (E2E_LOCAL_HTTP=1), so the local Worker can be driven with deployed-style auth and the HTTP-only capability set.Verification
Manually verified against the deployed stack:
Summary: 14 passed, 5 failed, 0 unsupported— 19 shared scenarios, serial, 23m24s wall clock.cleanupDeadlineAtis minted on the Worker's clock and then validated against the Durable Object's clock andSandboxControl's clock; because the minted value already contained the timeout, the upper bound reduced toworkerNow > doNow, so ordinary skew rejected every interrupt. Evidence: 9Stop cleanup deadline is invalidrejections, each 62-184 ms afterStarting session interruption, and nosession.abortever reached the wrapper. It is fixed in fix(cloud-agent-next): own the stop cleanup deadline per hop #6281, and the matrix should go green once that fix is onmainandcloud-agent-e2e-testis redeployed from it.cloud-agent-nextrecorded 8,862interruptSessioncalls over 7 days with 0 rejections, because it pins both functions and Durable Objects to one region. The e2e deployment does not share that incidental protection.Not verified here:
test:e2e-workersMiniflare suite and the deployed matrix are not wired into PR CI —ci.ymlruns unit and integration only. The workflow is dispatched by hand.127.0.0.1:3000, Next.js listens on 8300).empty-responsetwice in 13 local runs) is not root-caused.llm-errorasserts the terminal failure the product actually produces, because its original retry premise was stale.Visual Changes
N/A
Reviewer Notes
Automated coverage:
test:e2e-workers(Miniflare entry/surface tests intest/e2e/workers/),test/unit/e2e/**(deployed auth, capability gate, config renderer, shared scenario definitions, cleanup, client), and thetest:fake-llmsuite from the first PR.workflow_dispatchworkflow must exist on the default branch before GitHub offers it. It becomes runnable once this merges.kilocode-sessionsR2 bucket and production service bindings — and its endpoints are public with valid-token admission only, with no per-user isolation. Dedicated Worker names keep the stack addressable separately; they do not isolate its resources.test/e2e/README.mdnames the Workersfake-llm-e2e-testandcloud-agent-next-e2e-testand uses a<sub>.workers.devplaceholder at line 439, whiledeploy/README.mdhardcodesengineering-e11and the deployment actually exercised is reached at a custom domain. TheE2E_USER_IDrow also reads as an access-control restriction; it is a plane selector (*by default), not an access-control boundary.