refactor: remove duplicated and dead code found by the codebase slop audit

[georgi]

What changed

An audit of web/ and packages/ found the same rule, contract or entity declared in two to five places, and several Python-port layers that no production code reaches. This branch removes the duplication at its root rather than patching the copies: provider retry and polling becomes one implementation instead of one per vendor per layer; the frontend ui_* tool contracts move into @nodetool-ai/protocol so the browser registry and the headless eval bridges cannot drift; workflow persistence gets one service that REST and tRPC both call; node props take their type and default from the descriptor instead of any plus a second inline default; and the dead adapter, storage, auth and settings layers are deleted with their tests.

Several changes are bug fixes rather than cleanups, because the copies had already diverged.

Bugs fixed, each with a reproduction watched failing first

Bug	Consequence before the fix
AssetStore.add seeded the query cache under ["assets", id] while every reader looks up ["asset", id]	One dead cache entry per asset on every folder listing; the singular cache never warmed
castValue returned NaN in the KIE and Topaz factories	Topaz was sent the literal string "NaN" for a paid API call
nodetool_assets.timeline_id was absent from the Postgres schema declaration though the column exists and asset.ts filters on it	Found by a new dialect-parity test
Track audio effects were reported as picture Canvas 2D cannot draw	A false "your preview differs from the export" warning in two surfaces
SummarizerNode, ExtractorNode, ClassifierNode declared image/audio props, listed them in inputFields, and never sent them	The inputs were silently ignored
StructuredOutputGeneratorNode.max_tokens was never passed to the provider	All four sibling generators pass it
kie-dynamic never passed the run's AbortSignal	A cancelled run kept polling a paid API and kept being billed
assets get printed an empty url column	Neither data source has that field; replaced with created_at
setGetNodeStore had no production caller	The "injectable" store getter was permanently undefined
Four dead crypto.randomUUID fallbacks, one emitting non-UUID ids	—
ThreadMessageList dropped audio/video/document blocks	They now render through the chat's own renderer
A chroma-key default had drifted three ways (0 in the web UI, 0.5 in the shader and the writer)	Settled at 0.5, which is what the rendered frame actually shows

Verification

Run locally on the merged tree at the head of this branch. CI is the stronger evidence and it is green — see the CI section below, along with the caveat about why a local pass on this branch was worth less than it looked.

Check	Result
npm run build:packages	62/62
npm run typecheck (web, electron)	clean
npm run lint	0 errors
npm run test:packages	exit 0
web Jest	1294 suites, 14418 tests
electron Jest	63 suites, 690 tests
nodetool harness gate --base origin/main --json	exit 0 (see correction below)
npm run capabilities:check	current (268 capabilities)
npm run backend:smoke	server.mjs booted, answered /health

Three results need stating precisely rather than as a tick.

Mobile typecheck fails, and not because of this branch. mobile/ is deliberately outside the workspace and its own lockfile pins the published @nodetool-ai/protocol@0.7.0-rc.11 from the registry, so it compiles against a version that lags this repo. This branch touches zero files under mobile/. The same failure reproduces on main.

One CLI test flakes on my machine only. tests/run-dsl.test.ts spawns the built CLI and asserts within 30 s; the whole file takes 22 s standalone here, so under turbo's full parallelism in this container it tips over. It passes standalone every time (775/775) and the whole package pass is green at --concurrency=4. CI's own cli leg passes, so this is a property of this container, not of the branch — recorded because I spent time on it, not because it needs a fix. Not papered over with a larger timeout.

Mutation gates were not run. npm run test:mutation across the eight gated packages is hours of CPU and is CI's job.

Corrections to earlier claims on this PR

• The CodeQL alerts grew from six to twelve. All twelve were checked line by line against main and none is introduced here; the mechanism and the two corroborating signals are under CI state below.
• An intermediate checkpoint commit captured a deliberate sabotage of openai-provider.ts that an agent used to prove its guard test could fail. The tip is correct — the only diff from main in that file is a 4-line import move, and the 180-test provider-contract suite passes — but the bad hunk exists in intermediate history and matters if this branch is squashed selectively.
• One commit message lost three backtick-quoted phrases to shell substitution, leaving two sentences incoherent. Not amended, because the commit is pushed.
• I reported the harness gate as passing three times, and it was failing. Its human-readable output ends with Gate: 18/18 selfchecks passed, and I read that as success without checking the exit code. The gate also fails on unmapped code files and was exiting 1 on eight of them throughout. They are packages/deploy/src/*, a directory UNCLAIMED_PATHS already documents as having no harness — the registry's own audit honors that entry, the gate never consulted it, so the first diff to touch such a directory failed with no way to satisfy it. Fixed by having the gate read the same table, verified by removing the entry and watching it go back to exit 1. Related trap: piping that command truncates its JSON at 64KB on a diff this size, which looks like a crash and is not one.

CI state, and the one check still red

Green on ac52ec90: every check except CodeQL — the whole Quality Gate
(build, lint, typecheck, deps, bundle, docker, harness-gate,
test-app, and all four test-packages-* legs), plus Workflow Runner Browser E2E, Workflow Integration Tests, the browser workflow suite, route loading,
and fixture-mode codegen.

CodeQL: 12 alerts, none introduced here, and nothing left for me to do.
Its own summary carries the mechanism — "Alerts not introduced by this pull
request might have been detected because the code changes were too large."
Deleting runtime/src/type-predicates.ts meant a one-line import change in 56
files, and CodeQL attributes a touched file's pre-existing alerts to the PR
that touched it; two carried old ReDoS findings. The last four arrived the same
way: one added test in kie-dynamic.test.ts surfaced the four
includes("cdn.example.com") lines its fetch mock has always had, all four
unchanged on main. All 12 were checked line by line against main. Two
further signals agree: the three Analyze jobs all succeed (it is the
aggregate alert-count gate that fails), and the count is identical — 11 high,
1 medium — across a66c411e, 842c7063 and ac52ec90, three commits with
different diffs. This repo runs code scanning through GitHub's default
setup, so there is no per-query or per-path exclusion to reach for; a false
positive is dismissed in the Security UI, which needs repo admin.

How the browser E2E was fixed, and my own wrong measurement

All 27 specs were timing out on page.waitForFunction(workflowRunnerReady). My
first attempt chased a vite build failure — the wrong measurement, because
playwright.config.ts boots the Vite dev server, so the Rollup error I was
fixing is not the error CI sees. Loading the harness page against the dev
server and reading pageerror gave the real one at once:

Browser stub: node:module.createRequire not supported
  at .../@openclaw_fs-safe.js:1057

and, past that, process is not defined from the same module. Both are at
module scope in @openclaw/fs-safe's native-binding loader, so merely having
it in a browser graph throws before any code calls into it.

It is in the graph because of this branch, and rolldown says so while building
web:

packages/storage/src/index.ts is dynamically imported by
packages/runtime/src/context.ts but also statically imported by
packages/runtime/src/storage-workspace.ts, packages/runtime/src/testing.ts

main's runtime defined its own FileStorageAdapter in context.ts;
consolidating onto @nodetool-ai/storage's made storage-workspace.ts import
that package statically, defeating the deliberate await import in context.ts
whose comment says storage reaches node:fs.

Stubbed at the same seam both configs already use for Node-only modules.
root() is called only from FileStorageAdapter's constructor and a browser
has no local directory to point one at, so the stub throws on use like every
other one there. web's bundle carried the real package until now — the
chunks pulling it are lazy node bundles, so it was latent rather than a load
failure, but it was there.

Measured and deliberately not shipped: making the createRequire stub
return a throwing require instead of throwing itself. It fixes the first
error on its own, but with fs-safe stubbed nothing reaches it, so there is no
failing case to justify the edit.

A local green is not a CI green on this branch

The websocket leg is the sharp example. FileStorageAdapter's constructor
calls mkdirSync(root, { recursive: true }), so it created /home/user/ws the
first time I ran those tests, and every later local run passed against a
directory my own earlier run had made. I reported that package fixed twice on
the strength of runs that were measuring my container rather than the change.
Treat every "verified locally" line above as weaker evidence than the CI leg
that covers it.

Correction on attribution. I previously wrote that I could not attribute the
browser E2E failure to this branch, and reasoned that the chat timeout might be
CI contention. Both were wrong to leave standing: the Test workflow on main
at 5aa9e98f passed every job. Every check red here was this branch's, and each
has now been traced to a specific change in it.

One hypothesis checked and discarded rather than shipped: adding
@nodetool-ai/storage to packages/runtime looked like it could perturb npm
hoisting and make the bundler resolve OpenTelemetry's node build. It does not —
packages/runtime/node_modules/@opentelemetry/otlp-exporter-base@0.221.0 is
nested identically in main's lockfile and this branch's.

Things the audit got wrong

Roughly a third of the smaller findings did not reproduce and were corrected rather than "fixed": assetStorage is not vestigial (seven hosts set it to a different adapter than storage); receive_clipboard is a live database column; the packages/storage file list named files that do not exist; and several derivations the audit asked for were already done.

Known follow-ups, not done here

• GET /api/users/validate_username was dropped by the REST-to-tRPC migration as a side effect, with no replacement, and is restored here — its suite came back with the merge and was red. It remains a stub: it regex-checks the name and returns available: true unconditionally, never consulting storage, and no caller exists in web, electron or mobile. Worth retiring deliberately, which is not the same as losing it to a refactor.
• Listing a workspace now creates it. Consolidating the two FileStorageAdapter classes changed the constructor from this.rootDir = resolve(rootDir) (main's, in runtime/src/context.ts) to one that calls mkdirSync(root, { recursive: true }) (packages/storage's). So constructing an adapter to read a workspace has a filesystem side effect it did not have, and fails outright where the process cannot write — which is exactly how CI caught it. I did not change storage's constructor: callers may rely on the root existing after construction, and that is a call for whoever owns the storage seam. The two tests that exposed it now use writable temp roots and name the mkdir in a comment.
• Two static imports defeat a deliberate lazy one. context.ts reaches @nodetool-ai/storage through await import on purpose — its comment says the package reaches node:fs — but storage-workspace.ts and testing.ts import it statically, so the runtime barrel cannot be loaded off Node without a bundler alias. The alias is what this branch adds; removing the static imports is the real fix and needs a decision about where createLocalWorkspace lives, since StorageAdapter.uriForKey is synchronous and cannot be deferred behind a dynamic import.
• codegen:dsl:check is platform-sensitive: ALL_BASE_NODES gates the Apple automation nodes behind process.platform === "darwin", so regenerating on Linux drops that whole namespace. The generated output here was left as main has it; main fails the same check on Linux.
• Two schema items are allowlisted with reasons rather than fixed, because either needs a new migration: nodetool_team_tasks never reaches Postgres, and nodetool_assets.size is INTEGER on disk versus real in Drizzle.
• protocol and node-sdk now both document themselves as the workspace's single home for type predicates, and three signatures differ.
• A frontend component fetches /api/models/3d, which nothing serves. Pre-existing; main does not serve it either.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GLpT1DT5CC3beHvsGbsw46

[georgi]

Quality Gate (npm run check) / lint is red on 12ca1b5 with 12 errors and 70 warnings. This is the PR's own failure, not an unrelated one, and I am not re-running it — a re-run would fail identically.

The commit is a deliberate mid-flight checkpoint: parallel agents were still editing the tree when it was taken, so it was pushed to preserve the work, not because it was verifiable. No fix is pushed with this comment because a push now would capture another partial state; the next push will be the verified one.

The errors are worth recording, because they are the anti-slop ratchet doing exactly its job. Three trees that were at zero for a rule got put back on the board by newly added files:

File	Rule	Count
packages/node-sdk/src/manifest-node-values.ts	no-runtime-typeof	5
packages/protocol/src/predicates.ts	no-unknown-returns	1
packages/websocket/src/lib/workflow-service.ts	no-conditional-empty-object-spread	1

Each is a new file introduced by this work, and in each case the rule is pointing at something real rather than a style preference. The node-sdk helper branches on typeof value when the manifest already declares the field's type, which is the domain value it should branch on. The protocol predicate would export a function returning unknown from the package that is meant to be the wire contract. The workflow service expresses partial-column semantics as a conditional spread, which is the construct that makes a column silently stop being written.

The remaining errors and the unused-import warnings are in the same set of files. Fixes are with the agents that own them, and the verification checklist in the description will be filled in with real results before this is ready for review.

---

Generated by Claude Code

[georgi]

Two things on the current red state, one of which needs a human with Security-tab access.

CodeQL: 1 high + 1 medium, cause not yet established. I cannot read the annotations from this session — the check run exposes only a summary (output.text is empty) and I have no code-scanning API access here, so I could not identify either alert. What I did instead:

• Enumerated the 16 source files this branch actually adds and scanned them for the patterns CodeQL rates high in TS: dynamic-URL network calls, path construction from input, exec/spawn, and regexes with nested quantifiers. None present.
• The alert count has been identical (1 high, 1 medium) on every checkpoint since the first, including commits that touched entirely different areas, which is not the signature of something a specific change introduced.
• CodeQL's own summary says: "Alerts not introduced by this pull request might have been detected because the code changes were too large." This diff is 411 files, +10,269/−25,037.

So my working conclusion is misattribution against an oversized diff rather than a vulnerability this branch adds — but that is a hypothesis I could not confirm, and it should not be taken on trust. Whoever can open the Security tab should read the two alerts; if either is genuinely in this branch's code I will fix it ahead of any remaining refactor.

The branch is 44 commits behind main. Merge base is b298c5df, origin/main is now e3634858. CI has therefore been running this work against a stale base the whole time, and a diff this deletion-heavy has real conflict potential with four dozen commits of upstream change. I have not merged yet only because agents are still editing the tree and a merge mid-write would corrupt their in-flight edits. It is the first thing I will do once they finish, before the verification pass.

One earlier claim of mine needs correcting for the record: I previously listed files like ModeChips.tsx and permission-gate.ts as added by this branch. That was wrong — I had computed the diff against a stale local origin/main, which pushed the merge base too far back. Those files come from d3d45b33 on main and are not part of this work.

---

Generated by Claude Code

[georgi]

Resolved: none of the six CodeQL alerts is introduced by this branch. Correcting my previous comment — I said the annotations were unreadable from here, and that was wrong. Fetching the check-run page directly returned all six.

#	Rule	Severity	Location	Introduced here?
1-4	Polynomial regex on uncontrolled data	High	packages/integration-nodes/src/nodes/kie-dynamic.ts:181,261,283,293	No — all four regexes are byte-identical on main
5	Polynomial regex on uncontrolled data	High	packages/protocol/src/custom-providers.ts:103	No — this branch does not modify that file at all
6	Information exposure through a stack trace	Medium	packages/websocket/src/http-api.ts:1960	No — the res.end(JSON.stringify({ detail })) construct is present on main

Method, so this can be re-checked rather than taken on trust: for each flagged line I extracted the exact source line at HEAD and grepped for it verbatim in git show origin/main:<file>. Four of the five ReDoS sites and the stack-trace site are present on main unchanged; the fifth sits in a file with no diff on this branch. CodeQL surfaced them because the enclosing files were touched, which is the behaviour its own summary warns about: "Alerts not introduced by this pull request might have been detected because the code changes were too large."

The jump from 1 high to 5 high between b4760ee6 and 28ee641d is corroborating rather than alarming: those two commits differ only by two test files (28 insertions), so a four-alert delta cannot be attributable to the code change.

I am not fixing them here. They are real issues worth a separate PR — the kie-dynamic.ts regexes parse model-supplied markdown, so uncontrolled input genuinely reaches them — but fixing pre-existing findings inside a 411-file refactor would widen a diff that is already too large to review well. One will likely resolve itself regardless: http-api.ts:1960 sits in the legacy handleApiRequest dispatcher this PR is deleting.

The actionable item on this PR remains that it is 44 commits behind main.

---

Generated by Claude Code

[georgi]

CodeQL went from 6 alerts to 8 on 7d3212b4. The two new ones are also not introduced by this branch, and the growth has a mechanism worth naming.

#	Rule	Severity	Location	Introduced here?
7	Polynomial regex on uncontrolled data	High	packages/runtime/src/providers/base-provider.ts:1882	No — byte-identical at origin/main:1880
8	Polynomial regex on uncontrolled data	High	packages/runtime/src/providers/ollama-provider.ts:119	No — byte-identical at origin/main:119

Same method as before: extract the exact source line at the scanned commit, grep for it verbatim in git show origin/main:<file>. Alert 7 is the auth-error classifier /\b401\b|\b403\b|unauthorized|forbidden|invalid.*api.*key|authentication/i; alert 8 is the trailing-slash trim apiUrl.replace(/\/+$/, ""). Neither line is modified by this branch.

Why the count is climbing. CodeQL attributes a file's pre-existing alerts to a PR when the PR touches that file, which its own summary warns about. This branch deleted packages/runtime/src/type-predicates.ts and repointed all 56 importers at @nodetool-ai/protocol — a one-line import change in each. Two of those 56 files happened to contain pre-existing ReDoS findings, so both surfaced. Expect the number to keep rising as the remaining agents touch more files; a rising count here is not evidence of new risk, and each new alert still gets checked against main individually rather than waved through.

The medium alert moved from http-api.ts:1960 to :1953 for the same undramatic reason: this branch removed seven declarations above it that the REST-to-tRPC migration had orphaned.

Correcting my previous comment: it closed by saying the open item was that the branch sat 44 commits behind main. That is done — origin/main was merged in at 4cfa6c2a, with three conflicts resolved and three further build breaks that no textual conflict could expose. The branch is now level with main.

Recent commits carry [skip ci] deliberately, because they are mid-flight checkpoints taken while parallel agents are still editing the tree; running the full gate on a knowingly-partial state wastes it. The final push will not carry it, and the verification checklist in the description will be filled in with real results before this is ready for review.

---

Generated by Claude Code

[georgi]

CodeQL went 8 → 12. The four new ones are the mechanism I described in the previous comment, and I checked them rather than assuming that.

All four are urlStr.includes("cdn.example.com") in packages/integration-nodes/tests/kie-dynamic.test.ts — the fetch mock's branch for the fake result CDN. The construct appears 4 times on origin/main and 4 times here, unchanged. They surfaced because I added a test to that file, which is enough for CodeQL to attribute the file's existing alerts to the PR.

Worth noting what did not get flagged: the line I actually added is String(url).includes("kie.ai"), the same shape, and CodeQL left it alone. So this is not a rule that fires on the pattern in general.

The medium alert moved from http-api.ts:1953 to :1895 — same res.end(JSON.stringify({ detail })), still present on main, shifted by deletions above it.

Running total: 12 alerts, 0 introduced by this branch, each checked line by line against main. The PR description carries the full verification table.

---

Generated by Claude Code

[georgi]

Correcting my own attribution, because I had it backwards and said so publicly.

I wrote earlier that I could not attribute the Workflow Runner Browser E2E failure to this branch, and reasoned that test-packages-core's timeout might be runner contention. Both were wrong to leave standing. The Test workflow on main at 5aa9e98f ran today and every job passed — Workflow Runner Browser E2E, test-packages-core, test-packages-websocket, harness-gate, docker, bundle, the lot. So every check red on this PR is this branch's doing, not pre-existing and not the runner.

Since then:

• test-packages-websocket is fixed — 2802 of 2802. I had called the last two a "seam decision"; that was me stopping one level too early. Both listFiles cases write real files into a real temp directory and assert real sizes, but the module mock replaced readdir with a bare vi.fn() resolving to undefined. The old adapter never called it; the consolidated one does. The mock was only ever workable because the code under test never reached it. The fix needed one extra step: putting the real implementation in the mock factory works for exactly one test, because afterEach's restoreAllMocks strips it — so the binding lives in beforeEach. Removing the readdir line puts both cases straight back to red.
• harness-gate is fixed, and it had been failing on every commit while printing Gate: 18/18 selfchecks passed. It also fails on unmapped code files and was exiting 1 on eight packages/deploy/src/* files — a directory UNCLAIMED_PATHS already documents as having no harness. The registry's own audit honors that entry; the gate never consulted it.

Two remain, and I have not isolated either cause:

• Browser E2E — seven genuinely missing stub exports fixed and reproduced, but it now stops on @opentelemetry/otlp-exporter-base's node http transport importing bare stream in a browser graph. Adding stream to BARE_STUBBED is the obvious move and is wrong: the config records that it makes the stubs import themselves.
• test-packages-core — packages/chat's cold-import smoke test exceeds its 60s budget. Locally it is 8.8s before my changes and 9.09s after, so the dependency-direction change did not address it.

One hypothesis I checked and discarded rather than shipping: adding @nodetool-ai/storage to packages/runtime looked like it could perturb hoisting and make the bundler pick OpenTelemetry's node build. It does not — that package is nested under packages/runtime/node_modules identically in main's lockfile and this branch's.

The PR description carries the full verification table and the corrections.

---

Generated by Claude Code

[georgi]

CI is green on ac52ec90 except CodeQL. Every other check passes, including the whole Quality Gate, Workflow Runner Browser E2E, docker, harness-gate, and the two legs that took several attempts (test-packages-websocket, test-packages-core).

CodeQL: 12 alerts, none introduced by this PR — and no fix to port.

CodeQL's own summary carries the mechanism: "Alerts not introduced by this pull request might have been detected because the code changes were too large." That is what happened here. Deleting runtime/src/type-predicates.ts meant a one-line import change in 56 files, and CodeQL attributes a touched file's pre-existing alerts to the PR that touched it. Two of those files carried old ReDoS findings. The last four arrived the same way: adding one test to kie-dynamic.test.ts surfaced the four includes("cdn.example.com") lines its fetch mock has always had, all four unchanged on main.

All 12 were checked line by line against main. Two further signals point the same way:

• The three Analyze jobs (javascript-typescript, actions, python) all succeed. It is the aggregate alert-count gate that reports failure.
• The count is identical — 11 high, 1 medium — across a66c411e, 842c7063 and ac52ec90, three commits with different diffs. ac52ec90 added two stub .js files and two Vite config blocks; nothing CodeQL rates high-severity. An alert this branch introduced would have moved that number.

There is no fix to port and no re-run worth spending: CodeQL is reporting a stable, reproducible alert set, not flaking. Per AGENTS.md, this repo runs code scanning through GitHub's default setup, so a .github/codeql/*.yml config file is inert and there is no per-query or per-path exclusion to reach for — a false positive is dismissed in the Security UI, which needs repo admin. That is the one action left on this check and it is not mine to take.

Full detail, along with the browser-E2E root cause and the follow-ups this PR deliberately does not take on, is in the PR description.

---

Generated by Claude Code