Eliminate TypeScript Errors

[kriszyp]

Eliminate TypeScript Errors.

[claude]

Two statements on one line, and the surrounding block (lines 98–110) has severely misaligned indentation introduced by this PR. The brace count is actually balanced and the logic is correct, but this will confuse future editors and linters. Worth cleaning up before merge.

[claude]

Review: fix: Reduce TypeScript errors across the codebase

1 blocker found.

---

1. Resource.invalidate stub breaks dispatch — 500 instead of 405

File: resources/Resource.ts:473–475

What: invalidate was changed from an optional declaration (invalidate?) to a concrete base-class method that throws new Error('Not implemented'). The static invalidate transactional wrapper at line 212 dispatches via:

return resource.invalidate ? resource.invalidate(query) : missingMethod(resource, 'invalidate');

Because invalidate is now always defined on every instance (it's a concrete base method), resource.invalidate is always truthy. The missingMethod branch — which throws a ClientError(405) with the correct Allow header — can never be reached. Instead, resource.invalidate(query) is called unconditionally, throwing a plain Error('Not implemented').

Why it matters: Any non-Table Resource subclass that doesn't override invalidate will now return a 500 Internal Server Error instead of a properly handled 405 Method Not Allowed. Table overrides invalidate and is unaffected, but plain custom resources are broken.

Suggested fix: Replace throw new Error('Not implemented') with missingMethod(this, 'invalidate'):

invalidate(target: RequestTargetOrId): void | Promise<void> {
    missingMethod(this, 'invalidate');
}

---

Traced surfaces (for calibration)

• tsconfig.json typo fix (launchServiceSCripts → launchServiceScripts): correct, no concerns.
• @types/node@20 devDependency: devDep only, engines already requires Node ≥20, appropriate.
• ComponentV1.config Readonly<> removal: opens up mutation beyond construction; not blocking.
• ResourceInterface.ts: invalidate was already non-optional in the interface; PR aligns Resource, but the implementation is wrong (see blocker above).
• LMDBTransaction.commit() return type → any: erases Promise<CommitResolution> contract; no logic change.
• ImmediateTransaction.save/LMDBTransaction.save → ...args: any[]: erases parameter types; body accesses args[0] correctly.
• Resources.set() @ts-expect-error: Map subclass with extra params; acceptable suppression.
• blob.ts LMDBStore → RootDatabase: correct type substitution.
• bin/copyDb.ts openDB(name) → openDB({ name }): correct lmdb API call form.
• REST.ts formatting (lines 98–110): brace count balanced, logic unchanged, but indentation is mangled and two statements share line 105 — see inline comment.

[claude]

Debug console.log left in production code.

This fires on every HTTP POST request, polluting logs with internal implementation details.

Suggested change

	console.log('DEBUG static post', { resourcePost: !!resource.post, loadAsInstance: resource.constructor.loadAsInstance });
	static post = transactional(
	function (resource: any, query: RequestTarget, _request: Context, data: any) {
	if (resource.#id != null) resource.update?.(); // save any changes made during post
	return resource.constructor.loadAsInstance === false ? resource.post(query, data) : resource.post(data, query);
	},
	{ hasContent: true, type: 'create', method: 'post' }
	);

[claude]

invalidate changed from optional to concrete — breaks 405 response for unimplemented resources.

The check at line 212 is resource.invalidate ? resource.invalidate(query) : missingMethod(resource, 'invalidate'). With invalidate now always a concrete method, the ternary always takes the truthy branch and calls invalidate(query), which throws new Error('Not implemented'). That propagates as a 500 instead of the ClientError(405) that missingMethod produces, which also sets the Allow header.

Either keep it optional (invalidate?(target): void | Promise<void>) or throw via missingMethod:

Suggested change

	invalidate(target: RequestTargetOrId): void | Promise<void> {
	invalidate(target: RequestTargetOrId): void | Promise<void> {
	missingMethod(this, 'invalidate');
	}

[claude]

Review — PR #413: Reduce TypeScript errors across the codebase

3 blockers found. The overall approach (targeted as any casts + @ts-expect-error directives to make the build pass cleanly) is reasonable for a large type-error reduction pass. Most of the individual changes look correct. Three issues need to be addressed before merge.

---

1. Debug console.log left in production code

File: resources/Resource.ts:219
What: console.log('DEBUG static post', ...) fires on every HTTP POST request.
Why it matters: Pollutes logs in production and exposes internal implementation details (resource.post presence, loadAsInstance value) on every request.
Suggested fix: Remove the line. See inline comment.

---

2. invalidate changed from optional method to concrete throwing method — breaks 405 handling

File: resources/Resource.ts:474
What: invalidate?() (optional) became invalidate() { throw new Error('Not implemented') } (concrete). The dispatch guard at line 212 is resource.invalidate ? resource.invalidate(query) : missingMethod(resource, 'invalidate'). Because invalidate is now always a concrete method, the guard always takes the truthy branch and calls the method, which throws new Error('Not implemented').
Why it matters: missingMethod throws ClientError(405) and populates the Allow response header. The generic Error('Not implemented') will propagate as a 500. Any resource that doesn't explicitly override invalidate changes from a clean 405 Method Not Allowed to a 500 Internal Server Error.
Suggested fix: Either restore the optional signature, or delegate to missingMethod inside the concrete body. See inline comment.

---

3. @types/node devDependency added without a dependencies.md entry

File: package.json:126
What: "@types/node": "^20.19.39" was added to devDependencies; dependencies.md was not updated.
Why it matters: Repo convention requires an entry for every new package (the file explicitly notes dev deps still need some consideration). The lock also downgraded undici-types from ~7.18.0 to ~6.21.0 as a side-effect; confirm the @types/node@20 ceiling is intentional given the codebase may target Node 22+.
Suggested fix: Add a brief entry to dependencies.md explaining the need for explicit Node typings and the chosen version ceiling.

---

What I traced (not blockers)

• create() parameter order swap (ResourceInterface.ts, Resource.ts): the old interface had (newRecord, target) but the call site always passed (id, record), so the old types were inverted. The fix correctly aligns types with runtime behavior. Implementers whose TS code followed the old parameter names/types will see new compile errors — worth a note in the PR description.
• Formatting corruption in server/REST.ts and resources/LMDBTransaction.ts: braces and else clauses ended up on the same line as preceding statements in several hunks. Syntactically valid but difficult to read; worth a cleanup pass.
• Readonly<ComponentV1Config> removal: weakens the type contract on config to allow construction-time mutation. The readonly on the property itself still prevents reassignment; this is a pragmatic tradeoff.
• bin/copyDb.ts openDB call: openDB(INTERNAL_DBIS_NAME) → openDB({ name: INTERNAL_DBIS_NAME }) looks like a correct API conformance fix.
• Dep bumps (@harperfast/rocksdb-js 1.0.0→1.1.0, lmdb 3.5.3→3.5.4): minor/patch bumps, fine.

[claude]

The parameter order here was swapped from (newRecord, target) to (target, newRecord). The calling convention in Resource.ts (resource.create(id, record)) confirms that (target, newRecord) is the correct order — the old interface was wrong.

However, this is still a breaking public API change: any TypeScript code that implemented ResourceInterface and typed create(newRecord, target) per the old declaration will now have the semantics of its first arg silently change from Partial<Record> to RequestTargetOrId. It compiles, but the logic breaks at runtime.

This needs a changelog entry or migration note in the PR description so integrators know to swap the argument order in their create implementations.

[claude]

Review: fix: Reduce TypeScript errors across the codebase

1. Breaking API change — ResourceInterface.create parameter order

File: resources/ResourceInterface.ts:21-24

What: The create? method signature was swapped from (newRecord, target) to (target, newRecord). The calling convention in Resource.ts (resource.create(id, record)) confirms the new order is correct — the old interface was documenting the wrong order.

Why it matters: This is a public interface contract change. Any TypeScript implementor who declared create(newRecord: Partial<Record>, target: RequestTargetOrId) per the old interface will silently receive a RequestTargetOrId where they expected their record data and vice versa — it compiles but breaks at runtime. The PR description doesn't call this out.

Suggested fix: Add a note to the PR body (and ideally a CHANGELOG entry) that ResourceInterface.create callers need to swap their parameter order. Even though the old declaration was factually wrong relative to actual dispatch, downstream implementors followed it.

---

No other blockers found. Here's what I traced:

• tsconfig.json typo fix (launchServiceSCripts → launchServiceScripts): correct, low-risk.
• @types/node@20 added to devDependencies: devDep, so no dependencies.md entry required. The version pin to 20 is narrower than current LTS but acceptable for type checking.
• ComponentV1.config Readonly removal: allows constructor mutation, appears intentional.
• Resources.ts ResourceEntry.Resource: any: weakens type safety; not a correctness regression since the field is annotated any in neighboring fields too, but worth watching.
• ResourceBridge.upsertRecords now awaits insertUpdateValidate: the function is synchronous, so await is a no-op. Harmless but unnecessary.
• isSaving return type corrected (string → Promise<void>): the old declared type was wrong; the fix is correct.
• LMDBTransaction.ImmediateTransaction.timestamp setter added: correctly pairs with the existing getter; @ts-expect-error is the right suppression here.
• REST.ts formatting corruption: indentation is mangled and target = {} as any; if (...) appears on one line. TypeScript/JS is whitespace-insensitive so the logic is preserved, but this should be cleaned up before merge.
• bin/copyDb.ts openDB call: changed from string arg to object arg — both are valid lmdb-js API forms; the typed form is cleaner.
• Security: no new auth paths, no new secret handling, no injection surfaces introduced.

[claude]

Behavioral regression: non-implementing subclasses now get 500 instead of 405.

Previously invalidate was optional (invalidate?), so resource.invalidate was undefined on subclasses that didn't override it. The transactional wrapper at line 212 checks resource.invalidate ? and falls back to missingMethod() (which returns a proper 405 Method Not Allowed) when it's falsy.

Now that invalidate is a concrete method on the base class, resource.invalidate is always truthy — the missingMethod branch is dead for every resource that doesn't override it. Calling invalidate on such a resource will throw new Error('Not implemented'), which the framework will serialise as a 500 Internal Server Error instead of the previous 405.

Either keep the ? optional declaration (and remove this body), or update the transactional wrapper to check a different sentinel rather than truthiness.

[claude]

Breaking public API change: create() argument order swapped.

This PR flips create(newRecord, target) → create(target, newRecord) in both ResourceInterface and Resource. Any consumer that implements create (or calls it directly) against this interface will silently pass the wrong values into each parameter — no type error because the types overlap. This needs a migration note in CHANGELOG / CLAUDE.md and ideally a major-version bump, or the flip must be coordinated with all in-tree callers before merging.

Also check Resource.ts line 204 (resource.create ? await resource.create(id, record)) — that call still passes (id, record) in the old order and will now mismatch the new signature.

[claude]

Formatting corruption: two statements collapsed onto one line.

This line is target = {} as any; immediately followed by if (request?.user?.role?.permission?.super_user) { with only whitespace between them. While syntactically valid, this is clearly a corruption introduced during the PR (the original had two separate lines). The surrounding block (lines 97–110) also has severely misaligned try/catch/if braces that make it very hard to audit correctness. Please restore proper formatting before merging.

[claude]

Debugging artifact committed to the repository.

errors.txt should not be checked in. It records a TS compiler error (serverUtilities.ts(474,1): error TS1128: Declaration or statement expected) that appears to have been resolved, but leaving the file in-tree is confusing and will show up in future git status / IDE searches. Please delete this file.

[claude]

Silent behavioral change: logger parameter is now ignored.

The previous code was this.#logger = logger || loggerWithTag(this.#appName), which used a caller-supplied logger when provided. This PR drops the fallback and always calls loggerWithTag. Any caller (including tests using a mock logger) that passes a logger argument now has it silently discarded, which breaks the contract without changing the constructor signature. If logger is no longer part of the intended API, remove it from the constructor parameter list and update all call sites; otherwise restore the || logger fallback.

[claude]

Blocker — module.exports in an ES module

import statements appear at lines 13–22, making this file an ES module. module is not defined in ES modules — this assignment throws ReferenceError: module is not defined at runtime.

Replace with named ESM exports:

Suggested change

	module.exports = {
	export { verifyPerms, verifyPermsAST, ... };

or add export keyword to each function/const declaration individually and remove this block.

[claude]

Circular dependency regression — static import defeats the late-load workaround

The original harper_logger.js used a require() call at the very end of the file (line 909) specifically to avoid this circular chain:

harper_logger → RootConfigWatcher → configUtils → harper_logger

The late require() worked because by line 909, module.exports had already been set (line 267 runs first), so when configUtils re-entered harper_logger via its own require(), it received a fully populated exports object with logger.error, logger.trace, etc. defined.

A static import declaration, wherever it appears in the source, is hoisted to the top of the compiled CJS output by TypeScript. The compiled file will look like:

const { RootConfigWatcher } = require('../../config/RootConfigWatcher.js'); // moved to TOP
// ...
// module.exports = { notify, fatal, ... } hasn't run yet

When RootConfigWatcher loads configUtils, configUtils does const logger = require('../utility/logging/harper_logger.js') and receives an empty partial export because module.exports = {...} at line 267 hasn't been reached yet. Every subsequent logger.error(...) / logger.trace(...) call in configUtils will throw TypeError: logger.error is not a function at runtime.

Suggested fix: Convert back to a dynamic require inside a lazy function, matching the original intent:

let _RootConfigWatcher: typeof import('../../config/RootConfigWatcher.js').RootConfigWatcher | undefined;
function getRootConfigWatcher() {
    if (!_RootConfigWatcher) {
        _RootConfigWatcher = require('../../config/RootConfigWatcher.js').RootConfigWatcher;
    }
    return _RootConfigWatcher;
}

Then use getRootConfigWatcher() wherever RootConfigWatcher is currently used in the file.

[claude]

Still unfixed from the prior review. The comment above this line says "we have to load this at the end to avoid circular dependencies problems" — but a static import defeats that intent. TypeScript compiles this to a require() at the top of the CJS output in dist/, before module.exports is set. configUtils receives an empty object and every logger.error(…) call throws TypeError at startup.

Replace with a lazy require() inside the function body that actually uses RootConfigWatcher, or a dynamic import() inside an async initializer called after exports are assigned.

[claude]

Debug console.log left in — logs harperBridge's keys on every SQLSearch import. Remove before merging.

Suggested change

	console.log("HARPER BRIDGE IN SQLSEARCH", Object.keys(harperBridge)); export default SQLSearch;
	export default SQLSearch;

[claude]

Debug console.log left in production code — fires on every import of SQLSearch, i.e. on every SQL search. Remove it:

Suggested change

	console.log("HARPER BRIDGE IN SQLSEARCH", Object.keys(harperBridge)); export default SQLSearch;
	export default SQLSearch;

[claude]

Static import hoisting defeats the circular-dependency workaround. The comment says this must load last, but static ES module imports are hoisted before any module code runs — the load order is the same regardless of where the import statement appears in the file.

Replace with a lazy require() inside the function body that uses RootConfigWatcher, or a dynamic import() inside an async initializer called after exports are set.

[claude]

Scratch test script committed accidentally. Should be deleted and added to .gitignore.

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	typescript-eslint@​8.59.0 ⏵ 8.58.0
	@​aws-sdk/​client-s3@​3.1037.0 ⏵ 3.1042.0	+1

View full report

[claude]

Debug console.log still present — this fires on every import of SQLSearch. The previous review flagged this; the latest commit only reformatted the line. Please remove it entirely.

Suggested change

	console.log('HARPER BRIDGE IN SQLSEARCH', Object.keys(harperBridge));
	export default SQLSearch;

[claude]

Static ES module imports are hoisted — placement at the end of the file has no effect on load order. The circular-dependency workaround the comment describes requires a lazy require() inside the function body that uses RootConfigWatcher, or a dynamic import() inside an async initializer called after exports are established.

[claude]

module.exports on line 17 is not in scope in an ES module — the static import here makes Node.js treat this file as ESM. This throws ReferenceError: module is not defined in ES module scope at startup. Fix: replace module.exports = function ... with export default function ....