full type coverage across user facing packages - #14319

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/type-coverage.yml:
- Around line 13-14: The workflow step name "Check TypeScript type coverage
(100%)" is misleading because the npm script ts-coverage:ci uses the flag
--at-least 90; either update the workflow step name to "Check TypeScript type
coverage (90%)" or change the ts-coverage:ci script to use --at-least 100 to
match the name—locate the step in .github/workflows/type-coverage.yml and the
npm script ts-coverage:ci in package.json (look for the --at-least 90 flag) and
make the labels/thresholds consistent.

In `@package.json`:
- Around line 34-39: Remove the unnecessary root-level runtime/dev dependencies:
delete "react", "react-dom", "semantic-ui-css", and "semantic-ui-react" from
package.json and instead ensure any test apps that need them declare them in
their own package.json (since root tsconfig excludes tools/modern-tests/**);
keep "mongodb" in the root deps because packages/npm-mongo/index.d.ts relies on
its types. Ensure package.json edits only remove those four packages and do not
touch "mongodb".

In `@packages/ddp/ddp.d.ts`:
- Line 9: The apply declaration's signature is wrong: change the apply<Result =
unknown>(method: string, ...parameters: EJSONable[]): Result; to accept an args
array and optional options/callback parameters so it matches the runtime API
used by apply(name, args, options, callback); specifically update the
declaration for apply to accept method: string, args: EJSONable[], options?:
unknown, callback?: (err: Error | undefined, result?: Result) => void and return
Result so callers and the implementation (apply) align.

In `@packages/email/email.d.ts`:
- Around line 3-6: The Address interface currently requires both name and
address; make the name property optional to match nodemailer by changing the
Address interface's name member to an optional string (e.g., name?: string) so
callers can supply just an address string or an object with only address; update
any related type references or usages of Address (e.g., in send functions,
MailOptions types) to accept the adjusted interface if they perform strict
construction checks.

In `@packages/mongo/mongo.d.ts`:
- Around line 357-359: The arrayFilters type is too narrow: replace the current
definition of arrayFilters (currently typed as Record<string,
NpmModuleMongodb.Filter<T>>[] | undefined) with the upstream-compatible type
NpmModuleMongodb.Document[] | undefined so array filter objects like {
"elem.status": "A" } or { "i.score": { $gte: 10 } } are accepted; update the
declaration where arrayFilters appears in the mongo.d.ts signature (the property
named arrayFilters) to use NpmModuleMongodb.Document[] instead of Record<string,
NpmModuleMongodb.Filter<T>>[].

In `@packages/mongo/polling_observe_driver.ts`:
- Around line 164-171: The catch block in polling_observe_driver.ts assumes the
caught value `e` has a `message` property and uses it directly when calling
this._multiplexer.queryError for the first poll; instead, normalize and validate
the error before using it: in the catch around the `first` check (where `err` is
currently cast), determine a safe error message by checking `e`'s type
(instanceof Error to use e.message, string to use e, or JSON/stringify fallback)
and include a fallback like "Unknown error" so `err.message` cannot be
undefined; then pass that safe message into the Error passed to
this._multiplexer.queryError (referencing this._cursorDescription and the
`first` branch) to avoid unsafe property access.

In `@packages/roles/definitions.d.ts`:
- Around line 29-32: Remove the conflicting interface declaration "interface
RolesCollection<T> { find(...): RolesCursor<T>; findOne(...): T | undefined; }"
so it no longer collides with the top-level exported type alias RolesCollection;
delete this interface block and any duplicate references to RolesCursor<T>
within it, and if those method signatures are required ensure they're present on
the exported RolesCollection type alias (or update callers to use the existing
exported type) so no usages are left pointing to the removed interface.
- Around line 7-27: The declared local types narrow core Meteor/Mongo types and
should be replaced with or extended from the public types: update RolesCursor<T>
to use Mongo.Cursor<T> (preserving generic), replace RolesUser with Meteor.User
(or extend Meteor.User if you need extra fields), and replace RolesSortSpecifier
and RolesFieldSpecifier with the official Mongo sort/field specifier types (or
type aliases to Record<string, 1 | -1> and Mongo.FieldSpecifier if available).
Concretely, import the canonical types from the Meteor/Mongo packages and change
the declarations for RolesCursor, RolesUser, RolesSortSpecifier, and
RolesFieldSpecifier to reference those exports (or extend them) so the package
surface matches the public Meteor/Mongo APIs.

In `@packages/routepolicy/routepolicy.d.ts`:
- Around line 6-10: The declaration currently exports a class but the runtime
exports a singleton instance; update the types to export the instance shape
instead of a constructor: replace the exported class RoutePolicy with an
exported constant/namespace type representing the instantiated object (matching
the API exposed by RoutePolicy = new RoutePolicyConstructor()), keeping members
like urlPrefixTypes: Record<string, RoutePolicyType> and any instance
methods/props from RoutePolicyConstructor; remove/avoid a constructor signature
so consumers cannot call new RoutePolicy().

In `@packages/server-render/server-render.d.ts`:
- Line 59: The Callback type was narrowed to Promise<void> | void which breaks
consumers that return values; revert or soften the change by restoring the
original return type (Promise<any> | any) or make Callback generic (e.g.,
Callback<T> = (sink: Sink) => Promise<T> | T) and provide a default of
unknown/any, or add an overload/alias that preserves the old signature and mark
the void variant as new/deprecated; update the exported type named Callback
accordingly so existing TypeScript callbacks that return values continue to
compile while the runtime still ignores return values.

In `@packages/tracker/tracker.d.ts`:
- Line 22: Update the TypeScript declarations to reflect runtime behavior:
change the type of firstRunPromise from Promise<void> to Promise<unknown> (to
match the actual resolved values returned by user async autoruns) and relax
onError's parameter type from Error to unknown (e.g., onError?: ((error:
unknown) => void) | undefined) so any thrown value is accepted; adjust the
declaration for the onError property and the firstRunPromise property in
tracker.d.ts to use these types and keep JSDoc/comments in sync.

In `@tools/fs/files.ts`:
- Around line 41-43: The current sourcemap const's TypeScript type is
incomplete: extend the type annotation for the imported require('source-map') to
include both SourceNode (the class used elsewhere) and the destroy() method on
SourceMapConsumer; specifically, update the sourcemap declaration so
SourceMapConsumer's constructor signature remains, originalPositionFor exists,
and add destroy(): void, and add a SourceNode type/interface matching how
SourceNode is instantiated/used in the file so TypeScript strict mode accepts
references to SourceNode and calls to SourceMapConsumer.destroy().

In `@tools/fs/watch.ts`:
- Line 278: The fromJSON signature's directories entry types declare contents as
string | null but DirectoryEntry.contents and toJSON use string[] | null; update
the fromJSON parameter type so directories: Array<{ absPath: string; include:
Array<string | { $regex: string; $options?: string }>; exclude: Array<string | {
$regex: string; $options?: string }>; names: string[] | null; contents: string[]
| null }> to match DirectoryEntry.contents and the toJSON output, ensuring type
consistency for the fromJSON method.

In `@tools/shell-client.ts`:
- Line 59: The type for the local variable info is missing the reason property,
causing a TypeScript error when later accessing info.reason; update the
declaration of info to include reason: string (e.g., change let info: { status:
string; port: number; key: string } to include reason: string) so all accesses
like info.reason compile correctly and reflect the expected shape.

In `@tools/tool-env/profile.ts`:
- Line 450: The callback typed for session.post('Profiler.stop') is incorrect:
make the second parameter optional and require the profile property; change the
signature for the callback passed to session.post('Profiler.stop') so it is
(err: Error | null, result?: { profile: { nodes?: unknown[]; samples?:
unknown[]; timeDeltas?: unknown[]; startTime?: number; endTime?: number } }) =>
void, updating the parameter name/result typing used in that anonymous callback
to reflect result as optional and profile as required.

In `@tools/utils/eachline.ts`:
- Around line 23-26: The catch block currently uses "error: unknown" and calls
done(error) which fails TypeScript strict typing; update the error handling in
the Transform._transform callback to narrow/convert the unknown to an Error
before passing to done — e.g., check if the caught value is an instance of Error
and call done(error) in that branch, otherwise call done(new
Error(String(error))) so done always receives Error | null | undefined; locate
this change around the catch handling that references done in eachline.ts.

---

Outside diff comments:
In `@packages/meteor/meteor.d.ts`:
- Around line 165-171: The current Meteor.call declaration disallows function
callbacks because the rest-arg type excludes functions; add an overload for
Meteor.call that accepts the same name and args but allows the final argument to
be an async callback (e.g., (error: Error | null, result?: Result) => void)
similar to Meteor.apply's asyncCallback parameter. Add a generic overload for
function call signature for Meteor.call(name: string, ...args: (EJSONable |
EJSONableProperty)[], callback: (error: Error | null, result?: Result) => void):
void (or equivalent) so callback-style usage type-checks while preserving the
existing promise-style/returning Result signature.

In `@packages/mongo/mongo.d.ts`:
- Around line 6-16: The type parameter constraint on UnionOmit is too strict and
requires K to be a key of T, breaking OptionalId when TSchema lacks an _id;
update the UnionOmit declaration so K is any property key (e.g., change "K
extends keyof T" to "K extends PropertyKey" or similar) so UnionOmit<T, '_id'>
works even if TSchema has no _id, leaving OptionalId as defined
(OptionalId<TSchema> = UnionOmit<TSchema, '_id'> & { _id?: string |
NpmModuleMongodb.ObjectId }) and keeping Selector/Modifier untouched.
- Around line 24-54: Update the Options generic to accept a second result type
parameter so transforms can change the output type: change Options<T> to
Options<T, U = T> and make the transform property use Transform<T, U>; ensure
related usages that rely on DispatchTransform still work with the new signature
(DispatchTransform, Transform, and the transform?: field in Options should
reference the two-parameter Transform<T, U> so find()/cursor type inference can
reflect transformed result types).

In `@packages/mongo/observe_multiplex.ts`:
- Around line 195-203: Replace the ad-hoc promise detection and incorrect
chaining: use Meteor._isPromise(result) instead of checking for 'catch' in
result when inspecting the value returned from the observeChanges callback
(refer to the variable result and callbackName), and remove the erroneous
handle.initialAddsSent.then(result) call since .then expects a function—either
delete that line or replace it with an appropriate function if there was
intended behavior; ensure any promise returned is handled via .catch as already
implemented for the promise case.

In `@packages/mongo/oplog_tailing.ts`:
- Around line 444-454: The idForOp function's return type and access are
incorrect: update the signature of idForOp to return string | Record<string,
unknown> (or string | Record<string,unknown> | undefined if you want to preserve
missing ids), use optional chaining when reading op.o2 (i.e. op.o2?._id) to
avoid runtime errors, and add a runtime guard that either throws when the
resolved _id is undefined or normalizes object _id values to strings (via
toString()/JSON.stringify() per caller expectations); ensure these changes are
applied in the idForOp function and any callers are adjusted if you change the
return type.

In `@tools/fs/safe-watcher-legacy.ts`:
- Around line 351-355: The cleanup handler on newStat's "stop" event currently
compares statWatchers[absPath] to statWatch (the stop function) which is wrong;
change the comparison to the watcher object variable (statWatcher) so the
identity check correctly verifies the registered watcher before deleting
statWatchers[absPath]; update the callback attached in newStat.on("stop", ...)
to use statWatcher in the equality test to avoid erroneous deletion.

---

Nitpick comments:
In @.github/workflows/type-coverage.yml:
- Around line 7-12: The CI currently runs the step "- run: npm install" without
caching; add an actions/cache step before that to cache npm's cache directories
(e.g., ~/.npm and .npm/_cacache) using actions/cache@v3, with a key that
includes runner.os, the node-version and a hash of package-lock.json (and a
sensible restore-keys fallback) so installs are restored from cache when
dependencies and Node version haven't changed; place this cache step immediately
before the "npm install" step in the workflow.

In `@packages/ddp-rate-limiter/ddp-rate-limiter.d.ts`:
- Line 10: The Matcher.type union is inconsistent: it allows any string but the
predicate variant only accepts the literal kinds 'method' or 'subscription';
update the Matcher.type declaration so the direct value is restricted to those
same literals (i.e., allow the literal kinds 'method' | 'subscription' or a
predicate that accepts 'method' | 'subscription') so both branches use the same
constrained type; locate the type in the ddp-rate-limiter.d.ts where
Matcher.type is declared and change its union accordingly.

In `@packages/ddp-server/session_collection_view.ts`:
- Around line 4-6: Replace the broad FieldValue type alias with the ejson
package's EJSONableProperty to align types with the EJSON type system: import
EJSONableProperty from 'ejson' and change the FieldValue declaration (or
references) to use EJSONableProperty (add "| undefined" if you need to preserve
the original allowance for undefined); this ensures EJSONableCustomType and
other EJSON-specific types are properly represented while preserving existing
runtime behavior handled by EJSON.clone().

In `@packages/ddp-server/session_document_view.ts`:
- Around line 1-2: The FieldValue type is duplicated; extract it into a single
shared module (create a types.ts that exports type FieldValue), replace the
inline type in session_document_view.ts and dummy_document_view.ts by importing
the exported FieldValue, and remove the duplicate definitions so both files
reference the same exported symbol; ensure the export name is exactly FieldValue
and update the import statements in any files that previously declared the type.

In `@packages/mongo-id/mongo-id.d.ts`:
- Around line 10-11: The ObjectID declaration exposes an underscored field _str
but lacks documentation that it's internal; update the declaration for class
ObjectID by marking _str as internal (either add a JSDoc comment like
"@internal" or "@private" above _str to indicate it's not part of the public
API, or change the visibility to private/protected if external access is not
intended) and, if you must keep it public for type-safe reasons, add a JSDoc
note clarifying its internal usage and that it should not be relied upon by
consumers so intent is clear.

In `@packages/mongo/cursor_description.ts`:
- Around line 32-33: Remove the // `@ts-ignore` and properly type
Mongo.Collection._rewriteSelector instead of suppressing the error: locate the
declaration for Mongo.Collection (or add a .d.ts in the mongo package) and add
an explicit signature for _rewriteSelector (e.g., accept the actual selector
input type and return the normalized selector type used by this.selector), or
alternatively cast the result to the expected selector type when assigning to
this.selector in cursor_description.ts; reference
Mongo.Collection._rewriteSelector and this.selector when making the change so
the compiler can validate types without using `@ts-ignore`.

In `@packages/mongo/cursor.ts`:
- Around line 102-105: The code casts methodName to string when calling the
method, which is misleading for symbol keys (e.g., Symbol.iterator); update the
call in the Cursor.prototype assignment so you access and invoke the property
using the actual methodName (no string cast) or use Reflect.get to obtain the
function and then call it with .apply, ensuring symbol properties are supported;
locate the assignment on Cursor.prototype that uses
setupAsynchronousCursor(this, methodName) and change the invocation to retrieve
and invoke the property using methodName (or Reflect.get) rather than casting to
string.
- Around line 66-69: The parameter type for _publishCursor currently uses the
broad Function type; replace it with precise callback signatures to reflect
Meteor subscription methods: use an object type where added: (collectionName:
string, id: string, fields: Record<string, unknown>) => void; changed:
(collectionName: string, id: string, fields: Record<string, unknown>) => void;
and removed: (collectionName: string, id: string) => void (or define a small
interface like SubscriptionCallbacks and use it). Update the _publishCursor
signature to accept that typed object (referencing _publishCursor and the sub
parameter) so callers and the call to Mongo.Collection._publishCursor remain
type-safe.

In `@packages/mongo/observe_multiplex.ts`:
- Around line 220-223: The destructuring currently binds an unused `_docId` from
`clonedDoc`; change the pattern to discard the id while keeping the rest as
`docFields` (e.g., use a throwaway binding) so the `_id` isn't assigned to an
unused variable; update the line that currently does `const { _id: _docId,
...docFields } = clonedDoc;` to discard `_id` (while leaving `docFields` intact)
in the same block where `clonedDoc` is created (the code that uses
`handle.nonMutatingCallbacks` and `EJSON.clone`).

In `@packages/mongo/oplog_tailing.ts`:
- Around line 326-328: Remove the inline ad-hoc type on the mongodb-uri require
in _startTailing and install the `@types/mongodb-uri` dev dependency; replace the
line "const mongodbUri: { parse: (uri: string) => { database: string } } =
require('mongodb-uri');" with a properly typed import (e.g., import * as
mongodbUri from 'mongodb-uri' or import mongodbUri = require('mongodb-uri')
depending on your tsconfig/moduleInterop) and keep the existing call to
mongodbUri.parse(this._oplogUrl) so the compiler uses the official type
definitions from `@types/mongodb-uri`.

In `@packages/mongo/oplog_v2_converter.ts`:
- Around line 58-61: The ArrayOperator interface is missing the s${number} index
for nested diffs; update the interface named ArrayOperator to include keys of
the form `s${number}` mapped to OplogDiff (or OplogDiff | OplogValue if both
appear), so it aligns with arrayOperatorKeyRegex and the runtime check that
inspects position[0] === 's' (this will remove the need for the cast around the
value at the spot where position is parsed). Ensure the template-literal index
signature for `s${number}` is added alongside the existing `u${number}`
signature in the ArrayOperator declaration.

In `@packages/mongo/polling_observe_driver.ts`:
- Line 73: The current brittle cast for creating this._taskQueue uses (Meteor as
unknown as { _AsynchronousQueue: new () => AsyncQueue })._AsynchronousQueue
which can break silently; replace this ad-hoc assertion by declaring a proper
internal type and/or augmenting the Meteor namespace (e.g., add an internal
types file that declares interface Meteor { _AsynchronousQueue: new () =>
AsyncQueue } or a dedicated InternalMeteor type) and then use that declared type
when instantiating _AsynchronousQueue (or wrap creation in a small factory that
checks existence at runtime and throws a clear error). Target symbols:
_taskQueue, Meteor, _AsynchronousQueue, AsyncQueue, and use the new
declaration/augmentation instead of the inline cast.
- Around line 113-114: Extract the repeated inline type assertion for
Package['facts-base'] into a reusable alias (e.g., FactsPackage) declared near
the top of polling_observe_driver.ts, then replace the duplicated casts like
(Package['facts-base'] as { Facts?: { incrementServerFact: (...) } } |
undefined) with a single cast using the alias (Package['facts-base'] as
FactsPackage) wherever used (notably in the incrementServerFact calls at the
observe-driver polling sites around the current usages and the occurrences at
lines referenced 209-210).

In `@packages/retry/retry.d.ts`:
- Line 38: The declaration exposes a supposedly-private method _timeout in the
public type surface; either make _timeout truly private in the source class (so
it is omitted from the generated .d.ts), remove it from the exported type, or
explicitly mark it as internal (e.g., add a /** `@internal` */ JSDoc) so the
declaration generator omits it; alternatively, if it is intended to be public,
rename/remove the underscore and add documentation. Locate the _timeout method
in the retry package (the class or interface declaring _timeout) and apply one
of these fixes, then regenerate the type declarations.

In `@packages/routepolicy/routepolicy.d.ts`:
- Line 1: The type alias RoutePolicyType is currently internal; export it so
downstream modules can reuse the union type. Update the declaration for
RoutePolicyType (the existing "type RoutePolicyType = 'network' |
'static-online';") to be exported (export type RoutePolicyType = ...), ensure
any references in this file (and consumers) import the symbol rather than
duplicating the union, and run TS checks to confirm no further changes are
needed.

In `@packages/webapp-hashing/webapp-hashing.d.ts`:
- Around line 1-8: The ManifestResource interface is currently internal; export
it so other packages can import and reuse the type. Update the declaration by
converting the local interface into an exported type (export interface
ManifestResource) so consumers (e.g., callers referencing ManifestResource) can
import it for manifest-related typings and ensure consistent cross-package
declarations.

In `@packages/webapp/webapp.d.ts`:
- Line 30: The manifest property is currently typed as Record<string,
unknown>[], losing the stronger shape defined by ManifestResource in
webapp-hashing; update the manifest declaration to use ManifestResource[]
instead and import that type (using an `import type { ManifestResource } from
'webapp-hashing'` or the appropriate package export) so the webapp.d.ts export
aligns with the shared interface, and ensure any existing references compile
after replacing `manifest: Record<string, unknown>[]` with `manifest:
ManifestResource[]`.

In `@tools/fs/watch.ts`:
- Around line 300-305: The helper reFromJSON currently parses unvalidated regex
strings (and WatchSet.fromJSON isn't referenced), creating dead code and a
potential ReDoS risk; either delete reFromJSON (and any WatchSet.fromJSON
entrypoints) if the JSON deserialization is unused, or harden it: make
reFromJSON a defensive API that validates inputs (e.g., validate type, max
pattern length, reject known-dangerous constructs or excessive nested/adjacent
quantifiers, and wrap new RegExp in try/catch) and update WatchSet.fromJSON to
call this safe parser so any external use is protected.

In `@tsconfig.json`:
- Around line 9-13: The tsconfig currently sets both "outDir" and "noEmit";
remove the redundant "outDir" field from the config (delete the "outDir":
"./dist", entry) since "noEmit": true disables emission, leaving only "noEmit"
(and other options like "allowJs"/"checkJs"/"moduleResolution") to avoid
configuration noise; ensure no build scripts rely on outDir elsewhere before
removing.