refactor: simplification sweep#672
Merged
Merged
Conversation
Implements the low-risk "TOP 3"-style items from a parallel analysis sweep. No behavior change intended; build + typecheck pass. Buckets touched: - core: drop unused `PlatformError`/`SessionError` error classes + tests; drop unused `safeJsonStringify`. - connector-sdk: delete the dead legacy `IFeed` contract + `ParentFeedDefinition`, `SearchResult`, and the whole `FeedAuth*` family (zero refs; connectors use `ConnectorRuntime`/`ConnectorAuthSchema`). Trim the `index.ts` re-exports. - openclaw-plugin: delete the dead-by-default `memory-wiki-compat.ts` spike (~960 LOC) + its `memoryWikiCompat` config branch, manifest `wiki_*`/`memory_*` tool entries, and unit test wiring. - server (gateway): delete the unwired standalone gateway CLI (`gateway/cli/index.ts`, `startGateway`, `loadEnvFile`, `displayGatewayConfig`); drop the dead `FixedWindowRateLimiter`/`getClientIp` from `gateway/utils/rate-limiter.ts` (keep `sweepExpiredRateLimits`); delete the empty `TelegramResponseStrategy` subclass; collapse `resolveSettingsLookupUserId` now that `OAUTH_PLATFORMS` is empty; fold `createTokenVerifier` into `agent-ownership.ts` and delete the one-liner `token-verifier.ts`. - server (connect): drop the `example.invalid` placeholder dance in `oauth-providers.ts` — `resolveProviderConfig` now returns optional URLs and callers null-check the one they use. - server (misc dedup/cleanup): dedupe `pgBigintArray` in test-fixtures; drop the unused `resource` field from the invalidation `events/emitter.ts` shape; collapse the no-op `DEV_FALLBACK_FROM` record in `email/send.ts`. - no-dynamic-imports rule: convert `await import(...)` to static imports in the SDK namespaces (`knowledge`/`entities`/`watchers`), `watchers/reaction-executor.ts`, `server.ts` (lobu gateway / db / scheduled jobs), and `index.ts` (`/health/scheduler` per-request import); hoist `decrypt` import in `postgres-stores.ts`.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
buremba
added a commit
that referenced
this pull request
May 13, 2026
…lone typecheck (drop dead InvalidationEvent.resource sites + unused import left by #672)
buremba
added a commit
that referenced
this pull request
May 13, 2026
…lone typecheck (drop dead InvalidationEvent.resource sites + unused import left by #672)
buremba
added a commit
that referenced
this pull request
May 13, 2026
…lone typecheck (drop dead InvalidationEvent.resource sites + unused import left by #672)
buremba
added a commit
that referenced
this pull request
May 13, 2026
…lone typecheck (drop dead InvalidationEvent.resource sites + unused import left by #672)
buremba
added a commit
that referenced
this pull request
May 13, 2026
* fix: bug-fix sweep — sandbox action bypass, SSRF, OAuth hardening, leak/timeout/race fixes Security: - CRITICAL: sandbox action-key smuggling — createActionCaller spread caller input before forcing `action`, so a read-only query_sdk script could set `action: "delete"` and reach write/delete admin handlers. Now spreads input first then forces the discriminator (and strips any caller `action`); added a defensive read-mode check in __sdk_dispatch and a regression test. - HIGH: MCP-proxy SSRF — assertSafeUrl only ran in probeMcpServer; discoverTools /callTool/sendRequest fetched config.upstream_url unchecked. Now validated in sendRequest on every outbound fetch. - HIGH: OAuth profile:read auto-approval gated to first-party (canonical-origin) redirect URIs only; DCR registration rate-limiter on by default + fails closed. - secret-proxy: reject (don't warn) unauthenticated requests that name an agent. - cross-org tool-list bleed — MCP tool-discovery cache keyed by orgId:connectorKey. - getClientIP only trusts X-Forwarded-For / CF-Connecting-IP / X-Real-IP behind TRUSTED_PROXY; takes the rightmost trusted hop. - getLobuServiceToken fails closed without an organizationId. - AgentConnectionStore.deleteConnection / channel-binding list+deleteAll org-scoped. - insertEvent onConflict probe now org-scoped (defense against cross-tenant supersede). Other fixes: - core: sanitizeForLogging cycle guard (WeakSet → "[Circular]"); verifyWorkerToken rejects far-future timestamps; retryWithBackoff isolates throwing shouldRetry/onRetry. - agent-worker: custom-command execFile gets a 120s timeout + correct signal-kill exit code; heartbeat-failure abort resolves the in-flight turn before dispose(); worker exits when SSE reconnects are exhausted (no zombie); 60–120s timeouts on gateway/MCP fetches. - embeddings: getExtractor no longer caches a rejected model-load promise. - connectors: Google Calendar paginates to the trailing nextSyncToken (incremental sync now engages); Gmail uses epoch-second `after:` (no per-day re-emit); Google Play parseDate computed numerically. - connector-sdk: close the launched browser / CDP socket on partial-acquire failure; remove the goto timeout listener leak. - connector-worker: never auto-complete a watcher run as success in the daemon. - server: watcher next_run_at advances on terminal failure / materialize error (no per-minute retry storm); deliverToBotConnections called once per createNotificationForUsers (no per-admin duplicate sends); scaleDeployment(name,1) throws when the worker is gone so the consumer re-creates it; insert-event upsertEmbedding uses the txn handle; dotenv loaded before ./instrument so Sentry isn't silently disabled; test-db fixSchemaConstraints includes the 'task' run type. - openclaw-plugin: autoCapture moved to the agent_end hook (the worker drops before_prompt_build) and pairs the answer with the question that prompted it. * fix(review): enforce calendar maxResults while paging for sync token; keep per-client rate-limit key without TRUSTED_PROXY * fix(ci): move sandbox action-call test out of vitest glob; fix standalone typecheck (drop dead InvalidationEvent.resource sites + unused import left by #672)
4 tasks
buremba
added a commit
that referenced
this pull request
May 13, 2026
…694) #672 deleted getClientIp from rate-limiter.ts as dead code; #692 (hardening sweep) then added an import + call to it in secret-proxy.ts. The gateway fails to boot from source with "SyntaxError: ...does not provide an export named 'getClientIp'", and the prod bundle would silently call undefined() on the legacy header-swap path. Restore the helper. Also fix biome noEmptyBlockStatements lint errors in command-registry.test.ts (10 mock(async () => {}) bodies, all from #685) — these blocked the pre-commit hook on main.
5 tasks
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.
One cohesive sweep of safe, low-risk simplifications surfaced by a 15-analyst
pass: dead-code deletion, de-duplication, and collapsing needless indirection.
No behavior change intended.
make build-packages+bun run typecheckpass;bun testinpackages/core(254) andpackages/connector-sdk(11) green;plugin unit tests + manifest-contracts test green.
What changed (per area)
core
PlatformError/SessionErrorerror classes (zero refs) and their tests.safeJsonStringify.connector-sdk
IFeedcontract,ParentFeedDefinition,SearchResult, and the entireFeedAuth*family fromtypes.ts(zero refs anywhere — live connectors extendConnectorRuntime/ useConnectorAuthSchema). Pruned the matchingindex.tsre-exports. ~170 LOC + a dozen confusing duplicate exports gone.openclaw-plugin
memory-wiki-compat.ts(~960-LOC dead-by-default "spike", zero in-repo callers, undocumented) + its test, thememoryWikiCompatconfig branch & resolver inindex.ts, theMemoryWikiCompatConfigtype, the manifestwiki_*/memory_*contracts.toolsentries andmemoryWikiCompatconfig-schema block, and the manifest-sync test wiring. Net ≈ −1100 LOC. (Published-package change; recoverable from git if ever wanted.)server — gateway
gateway/cli/index.ts, plusstartGateway/startGatewayServer(and now-unusedhttpServervar +node:http/@hono/node-serverimports) ingateway/cli/gateway.ts, andloadEnvFile/displayGatewayConfig(+dotenvimport + deadDISPLAYconst) ingateway/config/index.ts. Prod runsserver.bundle.mjs; embedded usescreateGatewayApp— none of this was reachable.gateway/utils/rate-limiter.ts: dropped the deadFixedWindowRateLimiterclass andgetClientIp(onlysweepExpiredRateLimitsis live); deleted its test.TelegramResponseStrategy extends DefaultResponseStrategy {}subclass + comment;case "telegram"falls through to default.agent-ownership.ts: collapsedresolveSettingsLookupUserIdto the two-line form now thatOAUTH_PLATFORMSis permanently empty.createTokenVerifierintoagent-ownership.ts, deletedroutes/shared/token-verifier.ts, updated the 3 import sites.server — connect
connect/oauth-providers.ts:resolveProviderConfignow returns aResolvedProviderConfigwith optional endpoint URLs; callers null-check the URL they actually use. Deleted thehttps://example.invalid/...placeholder args at all three call sites.server — misc dedup/cleanup
pgBigintArraycopy in__tests__/setup/test-fixtures.ts(import fromdb/client).resourcefield from theevents/emitter.tsInvalidationEventshape.DEV_FALLBACK_FROMRecord<EmailCategory,string>(identical values) to a single string inemail/send.ts.no-dynamic-imports rule (repo policy)
await import(...)→ static imports insandbox/namespaces/{knowledge,entities,watchers}.ts,watchers/reaction-executor.ts,server.ts(./lobu/gateway,./db/client,./scheduled/jobs), andindex.ts(the per-request./scheduled/scheduler-healthimport in/health/scheduler).require('@lobu/core')fordecryptto a top-level static import inlobu/stores/postgres-stores.ts.Deferred (architectural / higher-risk — needs a dedicated PR)
packages/core/src/logger.ts— drop the deadUSE_WINSTON_LOGGERbranch +SentryTransport+ thewinstondep. (Need to confirm no prod env relies on JSON log output.)packages/core/src/api-types.ts— delete the dead parallel API contract (~225 LOC), keeping at mostModelOption/PrefillSkill/AgentInfo. (Touchesmodule-system.ts/token-service.tsre-declarations.)packages/core/src/modules.ts— deleteregisterAvailableModules(await import(packageName), never invoked with args). Touches agent-worker + server boot.packages/agent-worker/src/openclaw/worker.ts— extractrunAISession(~600-line god method) into named phases. Large, hottest path, package's vitest suite not in CI.packages/agent-worker/src/openclaw/processor.ts+gateway/gateway-integration.ts— collapse the triple stream-delta dedup into the processor. User-visible streaming.packages/agent-worker/src/gateway/sse-client.ts↔worker.ts— break the import cycle (pendingConfigNotifications→ own module) and kill the remainingawait imports.packages/cli/src/{eval/client.ts, commands/chat.ts}— unify the two parallel Agent-API/SSE clients intointernal/agent-api.ts.packages/cli/src/commands/memory/_lib/openclaw-auth.ts— collapse the memory-org/url wrappers + drop thelobu memory orgsubcommand (CLI-surface change).packages/cli/src/commands/_lib/apply/desired-state.ts— promote the hand-maintainedpackages/servermirrors (settings builder, slug/cron patterns, connector auth-schema) into@lobu/core/@lobu/connector-sdk. Cross-package, risky.packages/connectors/*— default the no-opexecute()onConnectorRuntime(or a shared helper) and delete the ~19 copies + the connector-schema fragment builders / review-scraper unification /whatsapp.ts+github.tssplits. Touches the catalog scanner / connector contract.packages/connector-worker/src/executor/runtime.ts+child-runner.ts— replace the__*-key IPC smuggling with a first-classmodefield; also the SDK↔workerFeedSyncResult/ContentItemunification. Hot path, both ends change together.packages/server/src/gateway/orchestration/{base-deployment-manager,impl/embedded-deployment}.ts— collapseBaseDeploymentManagerinto the only concreteEmbeddedDeploymentManager(embedded-only doctrine). Large mechanical diff on the worker-spawn path.packages/server/src/gateway/connections/chat-instance-manager.ts— extract the platform file-handler factories intoconnections/platform-file-handlers.ts(~340 LOC).packages/server/src/gateway/connections/interaction-bridge.ts—ExpiringClaimMap<T>to dedup the 3 TTL-claim maps.packages/server/src/gateway/routes/internal/device-auth.ts(846 LOC) — split secret-store CRUD + OAuth-endpoint resolution + device-auth flow out of the routes file. Wide blast radius (proxy, resume-after-oauth).packages/server/src/gateway/routes/public/agent.ts— split the ~300-linesendMessageRoutehandler (multipart parse / platform route / direct enqueue). Hot send path.packages/server/src/gateway/proxy/http-proxy.ts— pass grant/policy stores + egress judge explicitly instead of viasetProxyX()module-level mutable singletons. Security-critical.packages/server/src/gateway/services/core-services.ts(1041 LOC) — group the 50+ optional fields into cohesive sub-containers. Touches everything readingcoreServices.*.packages/server/src/db/client.ts— deletesimpleQuery()/DbQuery.simple?(no-op identity) — ~34 mechanical edits acrosstools/resolve_path.ts+workspace/multi-tenant.ts; deferred to avoid an error-prone regex pass in this sweep. Same forcreateDbClientFromEnv(env)→getDb()(27 call sites).packages/server/src/db/embedded-schema-patches.ts— teach the embedded path to run the dbmate migrations so this duplicate-of-every-migration file can go away. Touches every embedded user's DB on next boot.packages/server/src/watchers/{automation,run-completion}.ts— unify the duplicated watcher run-state mutation helpers intowatchers/run-state.ts.packages/server/src/notifications/{triggers,service}.ts— collapse the 4 "notify admins" clones behind anotifyOrgAdminshelper; fixdeliverToBotConnectionsper-user fan-out (channel posts N× per N admins).packages/server/src/auth/jwt-ish —utils/jwt.tsrewrite onnode:crypto(drop thecrypto.subtle+btoa/atobdance);oauth/utils.tscollapse the 7generate*token wrappers togenerateSecureToken+ the 3 prefixed ones; consolidate the 3 "resolve public base URL" helpers (auth/base-url.ts,utils/public-origin.ts,utils/url-builder.ts).packages/server/src/lobu/{gateway,stores/postgres-stores}.ts— type theanymodule-level singletons ingateway.ts(cascades into route handlers); foldresolveDefaultOrgIdinto themulti-tenantTtlCache; extract the 5×-copiedupsertChannelBinding; dedup the org-scoped/id-only query forking inpostgres-stores.ts; confirm-and-delete theenc:v1:legacy decrypt path.packages/server/src/{server,start-local,index}.ts— extract a sharedbootHttpServer()consumed by both entrypoints (kills the drifted ~120-line copy-paste); move the/api/:orgSlug/*block out ofindex.tsinto anorgApiRoutessub-router.packages/server/src/__tests__— relocatefixtures/fake-llm-server.test.tsout offixtures/; convert the 8bun:test__tests__files to vitest; wire the ~38 vitest integration files into CI.packages/landing/*— dedup the Cloudflarefunctions/PagesFunctiontype + collapse the twinmcpentrypoints; splitHeroProductCard.tsx(2948 LOC); replacegen-platform-configs.ts's regex-scrape ofpackages/serverZod source with an exported artifact.