Remove unused and redundant platform code - #10
Conversation
Delete the unused Redis command APIs, the empty Control body mode, the D1 query function overload, the unreachable Gateway pattern namespace gate, the sortPatterns predicate guard, and the RegExp.escape wrapper; index Auth issue templates once at module load, trust same-isolate Control reload result types, reuse the DO invoke envelope decoder, and express the documented S3 cleanup one-page contract as a guarded branch. Retain every observable contract: Redis reply validation, empty-body rejection, reserved-namespace 404s, local envelope error wording, checkpoint bounds, and template shape enforcement now covered by a static contract test. Signed-off-by: Lu Zhang <lu@wdl.dev>
Inline the single workflow due-promotion flow into its ready-tick owner, flatten workflow definition verification, specialize the scheduler workflow tick, validate KV HMGET plans once at their construction boundary, drop the unused supervisor timeout alias, and compile the BTreeMap metric-key formatter only for its parity tests. Retain shard discovery, bounded overfetch, malformed-token cleanup, the fenced Lua move, Redis reply validation, byte-budget accounting, and the disabled-host tick no-op; record why the bounded DEK cache is kept for a future remote KMS provider. Signed-off-by: Lu Zhang <lu@wdl.dev>
Replace the test-only shell tokenizer, environment assignment parser, fallback syntax, and quote round-trip with explicit argv arrays, and drop the PATHEXT lookup branch that the repository does not support. Use argv-native Compose and Redis helpers, and model the one Redis missing-key fallback with ordinary error handling. Signed-off-by: Lu Zhang <lu@wdl.dev>
Signed-off-by: Lu Zhang <lu@wdl.dev>
There was a problem hiding this comment.
Pull request overview
This PR removes unused/redundant platform code across the JS and Rust sides, consolidates ownership boundaries (notably Gateway WebSocket lifecycle state), and simplifies integration helpers by switching from shell-ish strings to explicit argv arrays.
Changes:
- Extract Gateway WebSocket lifecycle admission/reconciliation + atomic route/rollout reads into
gateway/websocket-lifecycle.js, and update style contracts + docs to reflect the new ownership split. - Remove unused Redis command-surface APIs/fakes and related tests, plus simplify request-body and delegated-issue-template handling.
- Refactor integration helpers to avoid shell parsing by using argv arrays throughout; inline/trim some Rust workflow/scheduler helper abstractions.
Reviewed changes
Copilot reviewed 70 out of 70 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/style-contracts.test.js | Update gateway entrypoint style-contract expectations for new owner modules. |
| tests/unit/redis-session.test.js | Remove tests for deleted Redis helper APIs. |
| tests/unit/integration-environment.test.js | Update tests for argv-based sh() behavior. |
| tests/unit/integration-cli-helper.test.js | Replace shell-parsing tests with argv-forwarding coverage. |
| tests/unit/gateway-runtime.test.js | Add wiring for gateway/websocket-lifecycle.js in unit harness. |
| tests/unit/gateway-lib.test.js | Remove escapeRegex test after inlining RegExp.escape. |
| tests/unit/gateway-dispatch.test.js | Remove test for pattern reserved-ns rejection path. |
| tests/unit/fake-redis.test.js | Drop expectations for removed fake-redis command. |
| tests/unit/do-runtime-protocol.test.js | Add coverage for local-envelope wording on decode errors. |
| tests/unit/control-shared.test.js | Update readJsonBody behavior: empty body always invalid. |
| tests/unit/control-reload-handler.test.js | Add regression test for reload response when repair fails. |
| tests/unit/control-delete-handler.test.js | Remove unused Redis API assertion in delete handler tests. |
| tests/unit/auth-lib.test.js | Shift delegated template checks to module-load contract behavior. |
| tests/integration/scheduler-shutdown-drain.test.js | Convert docker invocations to argv arrays. |
| tests/integration/redis-conformance.test.js | Convert redis CLI helpers to argv arrays. |
| tests/integration/manual/websocket-hang-repro.manual.mjs | Convert docker invocations to argv arrays. |
| tests/integration/log-tail.test.js | Convert redis eval invocation to argv array. |
| tests/integration/helpers/structured-logs.js | Remove shell quoting; use argv docker logs call. |
| tests/integration/helpers/stack.js | Convert compose/ps/restart/log calls to argv arrays. |
| tests/integration/helpers/shell-quote.js | Remove obsolete shell-quoting helper. |
| tests/integration/helpers/runtimes.js | Convert runtime compose orchestration to argv arrays + env opts. |
| tests/integration/helpers/redis.js | Convert all redis-cli invocations to argv arrays; simplify fallbacks. |
| tests/integration/helpers/queue-scenarios.js | Convert scheduler restart call to argv array. |
| tests/integration/helpers/index.js | Update exports to new compose helper + remove shellQuote export. |
| tests/integration/helpers/d1-runtime.js | Convert compose invocations to argv arrays + env opts. |
| tests/integration/helpers/compose.js | Make compose helpers argv-based; adjust profile env handling. |
| tests/integration/helpers/cli.js | Replace shell parser with argv-based sh() wrapper. |
| tests/integration/gateway.test.js | Convert gateway restart to argv array. |
| tests/integration/gateway-websocket.test.js | Convert docker commands to argv arrays. |
| tests/integration/durable-objects-ownership.test.js | Convert kill command to argv array + profile env. |
| tests/integration/durable-objects-alarms.test.js | Convert kill/up commands to argv arrays + profile env. |
| tests/integration/delete-api.test.js | Convert XRANGE probe to argv array. |
| tests/integration/d1-storage-shared-localdisk.test.js | Convert docker/inspect/kill calls to argv arrays + env. |
| tests/integration/d1-binding.test.js | Convert docker logs call to argv array. |
| tests/integration/cron-triggers.test.js | Convert scheduler restart to argv array. |
| tests/helpers/mocks/fake-redis.js | Remove fake Redis session method that’s no longer used. |
| tests/helpers/load-auth-index.js | Pre-index delegated templates once; simplify resolver wrapper. |
| system-workers/s3-cleanup/src/index.js | Simplify paging loop (single page per run) while preserving bounds/checkpointing. |
| shared/redis.js | Remove unused typedef export. |
| shared/redis-session.js | Remove unused Redis session pipeline + unused multi helper. |
| shared/redis-resp.js | Remove unused HGETEX/HSETEX arg builders and unused typedef. |
| shared/redis-command-surface.js | Remove unused command-surface methods; tighten return typing. |
| shared/redis-command-client.js | Remove unused convenience APIs/typedefs. |
| scripts/integration-environment.js | Simplify PATH command resolution used by integration tooling. |
| rust/workflows/src/tests.rs | Update string-based guard for renamed workflow verifier. |
| rust/workflows/src/api/tick/ready.rs | Inline due-promotion logic into ready tick path. |
| rust/workflows/src/api/sharded_dispatch.rs | Remove now-unused generic due-promotion helper types/functions. |
| rust/workflows/src/api/active_export.rs | Consolidate workflow def verification into a single entrypoint. |
| rust/workflows/src/api.rs | Remove unused imports from sharded_dispatch. |
| rust/supervisor/src/config.rs | Remove chained env lookup helper; use single env name. |
| rust/scheduler/src/workflows.rs | Switch to workflow-specific tick helper. |
| rust/scheduler/src/remote_tick.rs | Specialize remote tick posting to workflows tick path. |
| rust/redis-proxy/src/secrets.rs | Document bounded DEK cache rationale for future KMS support. |
| rust/redis-proxy/src/kv.rs | Validate grouped HMGET plan once and pass expected cardinality through. |
| rust/common/src/metrics.rs | Limit metric key formatting helper to tests only. |
| gateway/websocket-lifecycle.js | New module: Gateway WS lifecycle admission/reconcile + snapshot reads. |
| gateway/runtime.js | Delegate lifecycle behavior to new websocket lifecycle manager module. |
| gateway/lib.js | Move shared routing-unavailable error + inline RegExp.escape usage. |
| gateway/dispatch.js | Remove redundant reserved-ns check for pattern hits. |
| gateway/config.capnp | Embed new gateway-websocket-lifecycle module. |
| docs/source-map.zh.md | Update module ownership map for new gateway lifecycle module. |
| docs/source-map.md | Update module ownership map for new gateway lifecycle module. |
| docs/modules/gateway.zh.md | Document new split: lifecycle vs transport modules. |
| docs/modules/gateway.md | Document new split: lifecycle vs transport modules. |
| do-runtime/protocol.js | Centralize invoke envelope decode + preserve envelope-specific wording. |
| d1-runtime/router.js | Remove overload-style options API; use structured options object. |
| control/json-body.js | Remove allow-empty mode; always reject empty bodies. |
| control/handlers/reload.js | Tighten typing and avoid inventing publish results on repair failure. |
| auth/lib.js | Index delegated templates once at module load; simplify resolver. |
| auth/index.js | Remove runtime template-map construction; resolve directly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Restore the PATHEXT lookup that integration CLI resolution has had since the initial release. The repository declares no POSIX-only requirement and scripts/run-parallel.js still selects npm.cmd on win32, so dropping it narrowed an undeclared platform surface rather than removing dead code. Advance the transitive brace-expansion lockfile entry to 5.0.9 within the range minimatch already declares, resolving GHSA-rgw5-rvv9-x895 without touching package.json; eslint is already at its latest release. Signed-off-by: Lu Zhang <lu@wdl.dev>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 69 out of 70 changed files in this pull request and generated no new comments.
Suppressed comments (1)
rust/supervisor/src/config.rs:103
shutdown_timeout_ms()no longer honors the legacy*_WORKERD_STOP_TIMEOUT_MSenv var (previously accepted via the chained lookup). If any deployed configs still setD1_WORKERD_STOP_TIMEOUT_MS/DO_WORKERD_STOP_TIMEOUT_MS, this change will silently fall back to the default and could shorten/lengthen stop behavior.
If the intent is to remove this alias, it should be called out explicitly as an operationally observable change; otherwise, keep the compatibility fallback.
pub(crate) fn shutdown_timeout_ms(config: &SupervisorConfig) -> u64 {
positive_int_env(
config.env_prefix,
"SHUTDOWN_TIMEOUT_MS",
DEFAULT_SHUTDOWN_TIMEOUT_MS,
)
}
Return compose no-build selection as argv from the shared preflight helper and route the four remaining hand-assembled compose up invocations through composeUp/composeProfileUp, retiring the composeUpNoBuildArgs adapter and its public export. Drop the shell-escaped payload exception from the testing standard now that the shell-quote helper is gone, and list gateway/websocket-lifecycle.js alongside gateway/runtime.js as a rollout lifecycle contract owner. Signed-off-by: Lu Zhang <lu@wdl.dev>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
RegExp.escapewrapper.sortPatternsmakes unreachable, thesortPatternspredicate guard thattypecheck:strictrejects at build time, and per-request revalidation of the in-repo Auth issue templates, now indexed once at module load behind a static contract test.gateway/websocket-lifecycle.js, and point the entrypoint style contract at the manager factory that would carry that state.Behavior
No observable contract changes. Redis reply validation, empty-body rejection, reserved-namespace 404s, local envelope error wording, S3 checkpoint bounds, shard discovery, bounded overfetch, malformed-token cleanup, the fenced Lua move, and the disabled-host tick no-op are all retained. The bounded DEK cache is kept deliberately, with its rationale recorded for a future remote KMS provider.
Validation
npm run lint,npm run typecheck,npm run typecheck:strict,npm run lint:unusednpm run compile:workerdnpm run test:unit(2,285 tests)cargo fmt --all --checkcargo clippy --locked --workspace --all-targets -- -D warningscargo test --locked --workspace(428 tests)npm run test:integration(55 integration files, 0 failures)The consolidated branch produces a byte-identical tree to the unsquashed history it replaces (
c5503ad).🤖 Generated with Claude Code