chore(typecheck): burn down advisory typecheck debt to zero (1438 → 0)#232
Open
jsugg wants to merge 36 commits into
Open
chore(typecheck): burn down advisory typecheck debt to zero (1438 → 0)#232jsugg wants to merge 36 commits into
jsugg wants to merge 36 commits into
Conversation
JSDoc @param/@typedef annotations plus type-only boundary casts at the JSON/gh-API edges. No runtime change, no `any`. Strict typecheck errors 1438 -> 1423; adds burn-down log + pattern to docs/typecheck-debt.md.
Add DOM to tsconfig.checkjs lib so Node's global fetch/Response/Headers resolve (34 'Cannot find name fetch' errors, repo-wide), and type the parseArgs arg-objects + JSON/boundary casts in four scripts/github pages/reporting scripts (14 errors). Strict typecheck 1423 -> 1372. No runtime change except compose-pages-site publishPath defaulting to '' instead of null (safer). 32 related unit tests green.
@param {unknown} on the config coercion + provider-override helpers,
import('joi').Root + env callback types, and type-only boundary casts
on the parsed-YAML object. Strict typecheck 1372 -> 1360. No runtime
change; providerOverrides unit tests green.
@typedef the GitHub dependency-graph compare API model (DependencyChange/Vulnerability/VulnerableChange) plus the CLI ParsedArgs/ValidatedArgs shapes, annotate every helper's params and returns, and cast once at the parseArgs validation boundary. Strict typecheck 1360 -> 1311 (scripts/github/review-dependencies.js 49 -> 0). No runtime change; 23 related unit tests green.
@typedef the CLI ParsedArgs/ValidatedArgs and the App API InstallationLookupResponse/InstallationTokenResponse; fetchGitHubJson now returns unknown with each caller casting at the boundary; the header map is Record<string,string> for the conditional Content-Type. Strict typecheck 1311 -> 1277 (create-github-app-installation-token.js 34 -> 0). No runtime change; 12 related unit tests green.
@typedef the duck-typed collaborators (Describer/AsyncDescriber/ ScraperService/ImageDescriberFactory) and domain shapes (ImageDescription/ ProviderJob/SettledDescription); make supportsAsyncJobs a type predicate so the async-only describer methods narrow; @template the concurrency mapper; type-only cast of this.constructor to preserve dynamic dispatch. Strict typecheck 1277 -> 1230 (PageDescriptionService.js 47 -> 0). No runtime change; 8 related unit tests green.
@typedef the job model (Job/JobSeed/JobError/ProviderJob), collaborators (Describer/JobStore/ImageDescriberFactory), and the ResolveDescriptionResult union; type the now/sleep constructor deps; cast this.constructor static access (replace_all across the buildExpirationIso calls) and job.providerJobId once at the refresh boundary. Strict typecheck 1230 -> 1188 (DescriptionJobService.js 42 -> 0). No runtime change; 7 unit tests green.
@typedef collaborators (Logger/HttpClient/ProviderConfig/RequestOptions/ ImageAsset) and axios-style HttpError; error helpers take unknown and cast to HttpError internally so catch-vars flow without caller changes; document the sleep constructor dep; type-only this.constructor cast; cast Set.has args where the value is optional/null. Strict typecheck 1188 -> 1146 (OpenAiCompatibleVisionDescriberService.js 42 -> 0). No runtime change; 6 related unit tests green.
@typedef Logger/ProviderConfig/RequestOptions/Prediction/ReplicateClient/ ProviderJob; type the SDK-shaped predictions client; replace_all the this.constructor.normalizePrediction static-access casts; boundary-cast the unknown error message. Strict typecheck 1146 -> 1104 (ReplicateDescriberService.js 42 -> 0). No runtime change; 4 unit tests green.
Sibling of DescriptionJobService: @typedef PageJob/DescriptionJob/JobStore/ Logger/PageDescriptionServiceLike/DescriptionJobServiceLike/PageResolveResult; cast the optional descriptionJobService once at the resolver boundary; type the inner recursive waitForTerminalDescriptionJob arrow; replace_all the this.constructor static casts; Error & { code } casts in the job-error builders; buildFailedJob takes unknown + internal cast. Strict typecheck 1104 -> 1039 (PageDescriptionJobService.js 65 -> 0). No runtime change; 13 related unit tests green.
Azure/Ollama/Scraper describer+scraper services: per-file Logger/HttpClient/ ProviderConfig/RequestOptions typedefs, AzureError shape cast for the axios-error skip-check, type-only this.constructor casts, URLSearchParams.set/ new URL arg casts (constructor-guaranteed endpoint), Scraper images-accumulator cast + typed outbound-policy. Strict typecheck 1039 -> 1004; src/services area now 0 (from 273 baseline). No runtime change; 31 related unit tests green.
The concrete JobStore (in-memory + Redis): @typedef StoredJob/RedisClient/ RedisTransaction/DescriptionJobsConfig; annotate the module helpers and both stores' methods; double-cast the real redis client to a loose RedisClient at the boundary (avoids fighting the generic redis types); type the initialize options object so the destructuring-default stops hiding config/logger. Strict typecheck 1004 -> 972 (src/infrastructure/descriptionJobStore.js 32 -> 0). No runtime change; 6 related unit tests green.
…rs to zero OpenAiCompatibleProviderOptions typedef for the options bag; type the inner buildEnvSchema/buildConfig/isConfiguredInConfig/createRuntime arrows; cast the runtime constructor arg via ConstructorParameters<typeof Service>[0]. Root-caused the never[] errors to helpers.js's dependentEnvNames = [] default and typed validateApiKeyBackedProviderEnv + the shared helpers. Strict typecheck 972 -> 931 (buildOpenAiCompatibleProvider 33 -> 0, helpers 8 -> 0). No runtime change; 51 provider unit tests green.
Express controller: minimal duck-typed collaborators (factory + 3 service-likes) and ControllerRequest/ControllerResponse typedefs (query/params as Record<string,string>, chainable res); @param appended to each handler's existing @Swagger block; replace_all the two uniform catch-error casts (error.message -> (Error), cause: error -> (object)). Strict typecheck 931 -> 898 (descriptionController.js 33 -> 0). No runtime change; 24 controller unit tests green.
Bare `npx jest` used the root config, which never loaded tests/setup/jest.setup.js, so the QE-009 env restore only ran in lane configs. Any suite that mutates process.env leaked between tests when run directly (validateEnvVars: 12 false failures).
Typedef the external Newman report shapes (report/execution/assertion/ failure) and the internal aggregate model; replace bare-object JSDoc params, type the Map indexes and reduce seed, cast at the JSON boundaries. scripts/postman/newman-summary.js 60 -> 0; total 823 -> 750 (includes prior second-order batch). Ledger rows added for both. No runtime change.
Model the Postman collection JSON (PostmanCollection/CollectionItem/ CollectionEvent/CollectionHeader) once in collection-utils and reuse it across lint-collection and newman-runner; type the provider-validation scope resolution against a ProviderValidationInfo shape cast at the providerCatalog boundary; type the app-server env bag, port allocator results, and newman-runner failure plumbing. Drops the dead desiredScope !== 'all' guard (unreachable after the early return). scripts/postman 116 -> 0 (area from 185 baseline); total 750 -> 609 including second-order wins in importers. No runtime change; 106 tests across 13 suites green.
Type the server lifecycle plumbing: generic configureServer over net.Server, RequestListener app params, Promise resolve hints, duck- typed logger/cluster/process collaborators (ClusterLike, FatalLogger, ShutdownHandler), and injectable-dependency shapes for the runtime bootstrap. src/server 82 -> 0; total 609 -> 529. No runtime change; 23 tests across 6 server suites green.
Type the provider definitions (Joi/env/config arrow params, createRuntime deps, ConstructorParameters ctor casts, replicate ESM-shaped-types double cast), the shared caption/image helpers (unknown params with one cast after narrowing), and the runtime registry (RuntimeProvider typedef). src/providers 70 -> 0; total 529 -> 465. No runtime change; 51 provider suites green.
Type the final 54 src/api errors (the tail of the 87 baseline, after descriptionController's 33). Express ships no bundled types, so add an ambient types/express.d.ts shim (declare module 'express') wired into the tsconfig include: this keeps the express boundary `any` while local JSDoc typedefs carry the handler/request/response shapes, and clears the 5 TS7016 require sites on its own. - healthController/scraperController: duck-typed HttpRequest/HttpResponse (chainable status/json/send), ScraperServiceLike, Error cast on the catch. - access-control/error-handler: AccessRequest (get/path), ErrorRequest/ ErrorResponse; asyncHandler and createAccessControlMiddleware use an explicit function-type @returns so the returned handler's params are contextually typed (no inline casts). - rate-limiters: express-rate-limit is ESM-shaped for a CJS require, so double-cast to (options: object) => Function (same shape as pino/replicate); a RateLimiterOptions bag with optional skip clears the status-limiter TS2345. - request-filter/routes: Filter* (headers/log/router) and route controller typedefs; loggers typed (...args: any[]) => void so pino's generic Logger<{...}> stays assignable (an unknown[] shape fails the contravariant param check — this fixed a 2-error createApp regression). Type-only; no runtime change. src/api area 0; total 465 -> 407. Verified (verifier-lite): tsc src/api empty, total 407, related controller/middleware unit tests green, eslint clean. Claude-Session: https://claude.ai/code/session_01C2NqWYBoePEb1yP8PmcRT9
…Asset The src/providers fetchImageAsset typing (04c53f3) gave its httpClient param a concrete Record<string, PolicyRequestFn> shape (2-arg (url, options?) calls). The Azure/Ollama/OpenAiCompatibleVision describer services genuinely call httpClient.post(url, body, config) (3-arg) internally, so their local HttpClient type cannot structurally be that Record — an irreconcilable arity mismatch. Fixed at the consumer (never loosen the new export): - Cast this.httpClient to `any` once at each fetchImageAsset call site so the internal .post(url, body, config) typing stays intact. - Tighten each outboundUrlPolicy dep from opaque Function to (value: any) => Promise<URL> (matches fetchImageAsset + ScraperService). Type-only; no runtime change. src/services area back to 0; total 407 -> 401. Verified: tsc src/services empty; 16 tests across the 3 service suites green. Claude-Session: https://claude.ai/code/session_01C2NqWYBoePEb1yP8PmcRT9
Finishes src/ (utils + composition root). All type-only.
src/utils:
- helmet & swagger-ui-express are ESM-shaped for a CJS require: helmet
double-cast to its ['default']; swagger-ui-express gets an ambient
declare-module shim (docs UI is lazy-required behind /api-docs).
- express.Router() is `any` under the express shim, so the router helpers
cast the router to a local typed shape (ValidationRouter/SwaggerRouter)
whose handler signature contextually types the (req, res[, next]) params.
- getUpstreamErrorSummary: @param {unknown} on the axios-error/header
helpers, UpstreamErrorLike + HeaderBag duck typedefs cast once past the
guards, Record<string, unknown> reduce seed; lazy swagger lets typed any.
src/createApp.js (composition root):
- CreateAppOptions typedef makes every injectable dep optional (clears the
non-defaulted destructure errors) and duck-types them `any` — except
config, kept precise as typeof import('../config'). This also clears the 3
pino Logger<{...}>-vs-service-Logger assignability errors at logger:
appLogger. Return type left inferred so app.js keeps resolving .app.
src/app.js:
- Cast the node cluster module to `any` at the runtimeBootstrap boundary
(TS2559: the minimal { isPrimary? } param shares no properties with the
module type), and double-cast the startRuntimeFn arrow to
() => Promise<void> through unknown (its Promise<object> is consumed as void).
No runtime change. src/ tree now 0; total 401 -> 349.
Verified (verifier-lite): tsc src/ empty, total 349, createRouter +
getUpstreamErrorSummary + createApp unit suites and the api integration
suite green.
Claude-Session: https://claude.ai/code/session_01C2NqWYBoePEb1yP8PmcRT9
With src/ already at zero, the whole non-scripts tree now typechecks.
All type-only.
- swagger-jsdoc: ambient declare-module shim (dev-only spec-regen fallback).
- providerCatalog.js: a ProviderDefinition typedef cast once onto
getProviderCatalog().slice() types every .forEach/.filter/.map/.flatMap
callback; validateEnv -> string[] and getStartupWarnings ->
{provider?,message?}[] so validateEnvVars keeps resolving; a
ProviderValidationDefinition intersection narrows the validation-scope
filter so .providerValidation.scopeKey is non-optional; ProviderOverrides
record; Record<string, object> sections seed.
- descriptionJobStore.js / rateLimitStore.js: the env / loosely-parsed-value
bags are typed Record<string, any> / any at the parse boundary — this
clears the {}-index and Set.has/Array.includes literal-union errors while
preserving the existing inferred return shapes (no consumer churn);
redisTopology typed string clears the 'external'-literal TS2367; the kind
resolvers get { mode?, redisUrl? } option bags.
No runtime change. config area 0 (src also 0); total 349 -> 302.
Verified: tsc config+src empty; 39 tests across 6 config suites green;
eslint clean.
Claude-Session: https://claude.ai/code/session_01C2NqWYBoePEb1yP8PmcRT9
scripts/openapi area 63 -> 0; total 302 -> 239. The whole area is comment-only JSDoc typing - no runtime change. - spec-utils.js: export shared external-JSON typedefs OpenApiNode (Record<string, any> - the document is external JSON, nodes stay loose) and OpenApiSpec (only the top-level sections the gates walk are named: openapi/info/paths/components/security, & OpenApiNode). loadSpec returns OpenApiSpec; listOperations yields operation: OpenApiNode; canonicalize reduce seed cast to Record<string, any>; resolveRef pointer-walk seed bridged to any. - validate-spec.js: import OpenApiSpec; Violation typedef for the rule reports; every bare-object spec @param -> OpenApiSpec (clears the TS2339-on-object property reads); writeReport stats/violations typed; Map reduce seed cast to Map<string, Violation[]>; isNonEmptyString takes unknown; two catch-site (Error) casts. - diff-contract.js: import OpenApiSpec/OpenApiNode; ContractChange typedef; pathsOf/schemasOf return Record<string, OpenApiNode>; methodsOf takes unknown (guards narrow it); @type on the parseArgs literal so base: null widens to string|null; Map seed cast; two catch casts. - check-freshness.js: DriftSummary/FreshnessResult typedefs; keysOf(unknown)/difference(string[]); driftLines' pair table cast to [string, string[]][] so the destructured items keep their array type; one DriftSummary boundary cast at the non-fresh call site; two catch casts. Exposed 2 pre-existing baseline TS2352s in scripts/postman/provider-validation-scope.js (second-order from the providerCatalog typing in eae331e, missed by the config-only grep) - next batch. Verified (verifier-lite): tsc scripts/openapi grep empty, total 239; 46 tests across the 3 openapi gate suites green; eslint clean on all 4 files. Claude-Session: https://claude.ai/code/session_01C2NqWYBoePEb1yP8PmcRT9
Total 239 -> 237. The two boundary casts onto the local ProviderValidationProvider duck stopped "sufficiently overlapping" once eae331e gave getProviderValidationProviders()/ getProviderValidationByScope() their rich ProviderValidationDefinition types (providerValidation: { scopeKey } & Record<string, any> vs the duck's required typed fields). Fixed at the consumer: bridge both casts through unknown, keeping the local precise duck as the source of truth. Comment-only - no runtime change. Verified (verifier-lite): tsc scripts/postman/ grep empty, total 237; 24 providerValidationScope tests green; eslint clean. Claude-Session: https://claude.ai/code/session_01C2NqWYBoePEb1yP8PmcRT9
Area 45 -> 0; total 237 -> 192 (exactly -45, no second-order). Comment-only JSDoc typing - no runtime change. - RESOLVED_ENV_VALUE_BUILDERS: one @type Record<string, (input: { host, ports: Record<string, number> }) => string | null> on the const contextually types all 12 builder arrows (23 TS7031s) and the [entry.key] lookup. - runNewman: hoisted the inline options type into a RunNewmanOptions typedef with envPath required (always supplied by the wrapper; clears the (string|undefined)[] args TS2322); destructure default cast to it (TS2739); runHarnessNewman wrapper takes Partial<RunNewmanOptions>. - @type annotations: fixedPorts Record<string, number>; parsed Newman env clone { values?: { key, value? }[] } & Record<string, any>; childDiagnosticLogs; authAppServer ChildProcess | undefined; the three provider-credential lets string | null | undefined (real-provider mode assigns process.env reads). - Boundary casts: buildAppServerEnv results -> Record<string, string> at both spawnLogged sites; frozen LOW_COST scopes -> string[] (TS4104); signal-handler pair -> [string, () => void] tuple (TS2345 on process.off); catch (Error) cast at emitHarnessFailureDiagnostics. - (value?: unknown) => void resolve hint clears the TS2810 new Promise(); Uint8Array[] chunks accumulator (Buffer[] trips the TS6 Uint8Array<ArrayBufferLike> generic on Buffer.concat). Verified (verifier-lite + direct): tsc harness grep empty, total 192; 8 runPostmanHarness tests green; eslint clean. Claude-Session: https://claude.ai/code/session_01C2NqWYBoePEb1yP8PmcRT9
Area 8 -> 0; total 192 -> 184 (exactly -8, no second-order).
Comment-only JSDoc typing - no runtime change.
run-postman-deploy.js:
- sleep: @param {number} + @returns {Promise<void>} (the return
annotation contextually types the executor, so Promise<unknown>
never leaks into waitForStableDeploy's sleepFn default).
- Frozen LOW_COST_PROVIDER_VALIDATION_SCOPES -> string[] cast
(TS4104, same pattern as the harness).
- requestDeployProbe catch: error -> (Error) cast (TS2322 unknown).
- collectDeployStabilizationIssues input: the read jsonBody widened
unknown -> any (optional-chained .code read on external JSON).
- JSDoc on the recursive attempt arrow's destructured input
({ attemptCount, consecutiveSuccesses, lastIssues }).
scripts/postman/live-provider-validation.js:
- ensureReportsDir's `= null` default inferred `null | undefined`,
rejecting the `string | null` dir every caller passes; inline
@type {string | null | undefined} on the param corrects the
under-annotation at the source (clears run-postman-live's one
error).
Verified (verifier-lite): tsc deploy/live/live-provider grep empty,
total 184; 34 tests across runPostmanDeploy + runPostmanLive green;
eslint clean on all 3 files.
Claude-Session: https://claude.ai/code/session_01C2NqWYBoePEb1yP8PmcRT9
Area 29 -> 0; total 184 -> 156 (-29 here, +1 second-order in
scripts/perf/run-perf-smoke.js from the now-typed createFixtureApp -
folded into the upcoming perf batch). Comment-only JSDoc typing - no
runtime change.
- Express is `any` under the ambient shim: cast the app once to a
local FixtureApp duck whose get/post handler signature
(FixtureHandler over FixtureRequest/FixtureResponse) contextually
types all 12 (req, res) pairs - same pattern as the src routers.
- createOpenAiCompatibleStubHandler returns FixtureHandler instead
of the shim-less import('express').RequestHandler (TS2694).
- TS6 Buffer gotcha: the JSDoc-resolved Buffer type does not
sufficiently overlap Uint8Array<ArrayBufferLike> (Buffer.compare's
parameter type), so the three comparison fixtures
(ASSET_{A,B,PROVIDER_FAILURE}_PNG) and the raw-body accumulator
are typed Uint8Array once at their declarations via
unknown-bridged double-casts; captionForBuffer takes
@param {Uint8Array}.
- Replicate stub body param widened Record<string, unknown> ->
Record<string, any> (optional-chained .input.image read on
external JSON).
Verified (verifier-lite + direct): tsc fixture-server grep empty,
total 156; 4 postmanFixtureServer tests green; eslint clean.
Claude-Session: https://claude.ai/code/session_01C2NqWYBoePEb1yP8PmcRT9
Area 45 -> 0; total 156 -> 111 (-45: 44 in the two perf files + the +1 second-order in run-perf-smoke.js folded from the fixture-server batch, now that createFixtureApp is typed). Comment-only JSDoc typing plus one ambient shim - no runtime change. - supertest ships no bundled types and no @types package is installed, so a `declare module 'supertest'` shim in types/supertest.d.ts (same approach as the express shim) clears the TS7016 and keeps the request/agent boundary `any` while local JSDoc types the callers. - performanceBudget.js gains a PerformanceSample typedef (the input duck) and an EvaluatedSample typedef (the enriched result), threaded through evaluatePerformanceSample, summarizePerformanceReport, and formatSampleLine. - run-perf-smoke.js: standard param annotations (toNumber unknown, resolveAcceptedBudgets string|undefined, median number[], the FastStubDescriber methods string). @returns {Promise<void>} on listen/closeServer contextually types their executors, clearing the no-arg resolve() TS2810 and the close-callback implicit-any error; reservePort @returns {Promise<number>} with server.address() cast to import('node:net').AddressInfo for the .port read. - The createApp option bag is `any`, so requestLogger's (req, res, next) take inline @type {any} and the app-taking helpers (secureGet / settlePageDescription / measurePageFanOut / measureDocsSteadyState) take @param {any} app. The two this.constructor.buildResult calls cast this.constructor to typeof FastStubDescriber. The folded second-order is http.createServer(/** @type {any} */ (createFixtureApp(...))). Verified: tsc perf grep empty, total 111; 8 performanceBudget tests green; eslint clean. Claude-Session: https://claude.ai/code/session_01C2NqWYBoePEb1yP8PmcRT9
Area 42 -> 0; total 111 -> 69 (exactly -42, no second-order). The four
production-ref / audit CLIs. Comment-only JSDoc typing - no runtime
change.
Three of them (promote-to-production, rollback-production,
update-deployment-status) share one root cause: each parseArgs seeds a
partial `const args = { ...defaults }` and fills the rest in the arg
loop, but the literal's inferred type held only the seeded keys, so
every `args.repo = ...` write, every later read, and the `return args`
failed against the full @returns shape. Fix per file:
- Hoist the inline @returns object into a named typedef (PromoteArgs /
RollbackArgs / DeploymentStatusArgs).
- Annotate the accumulator /** @type {Partial<T>} */ so the
incremental writes are legal.
- Assert the return /** @type {T} */ (args) after the required-field
guard already proves completeness - the guard enforced it at runtime
before this change.
parse-security-audit-report.js was the same parseArgs shape (typedef
AuditArgs + Partial + return assert) plus missing @params (argv
string[], reportFile string, appendOutput's outputFile/key/value). Its
report parsing got an AuditReport / AuditVulnerabilities external-JSON
duck typedef so the parseError catch-branch property and the
vulnerabilities.{critical,high,low,moderate} reads resolve, and the two
catch (error) blocks narrow `error instanceof Error ? error.message :
...`.
Verified: tsc github grep empty, total 69; 35 tests across the four
suites green; eslint clean.
Claude-Session: https://claude.ai/code/session_01C2NqWYBoePEb1yP8PmcRT9
Area 31 -> 0; total 69 -> 38 (exactly -31, no second-order). The manual
TLS-trust diagnostic CLI. Comment-only JSDoc typing plus two type-only
casts - no runtime change.
Every error was TS7006 implicit-any on a param; no structural issues.
- Added three duck typedefs: CertName (string | { CN?: string } -
X509Certificate yields a string subject/issuer, the synthesized
trust-anchor candidate uses a { CN } object), InspectedCertificate
(the chain-item shape), and DoctorTlsArgs.
- parseArgs seeds targetUrl: undefined then assigns the positional arg,
so the accumulator is annotated { ..., targetUrl: string | undefined }
and the return asserted DoctorTlsArgs (targetUrl guaranteed by the
usage guard) so main sees a plain string.
- splitPemCertificates typed @returns {string[]}, which contextually
types the four downstream .map/.find/.forEach callback params
(fingerprint/chain building) with no annotation of their own.
- probeUrl / inspectCertificateChain got param-only annotations: their
new Promise return type is inferred from the resolved values, not the
params, so main's probe-result and chain usage is unchanged.
- printChain's two defensive .CN fallbacks (dead for string subjects
but needed for the { CN } form) read through a local
/** @type {any} */ cast.
Verified: tsc doctor-tls grep empty, total 38; no test references this
manual CLI, so node --check + eslint clean.
Claude-Session: https://claude.ai/code/session_01C2NqWYBoePEb1yP8PmcRT9
Areas 29 + 9 -> 0; total 38 -> 0. npm run typecheck now exits 0 across src/, config/, and scripts/ - the ledger reaches zero. Comment-only JSDoc typing plus type-only casts - no runtime change. scripts/reporting (29 -> 0), the Allure history/metadata scripts: - Three accumulators seeded with a subset of keys then extended with optional string props (environmentProperties, executor, manifest) are annotated with their Record<string, string> / Record<string, string | number> return type so the conditional writes are legal. - resolve-allure-history-policy's two policy builders had stale camelCase @returns typedefs while the code actually returns snake_case GitHub-Actions output keys (history_artifact_name, ...); corrected both to @returns {Record<string, string>} (how toOutputLines and the outer resolver already consume them), clearing 8 TS2561s. The resolveAllureHistoryPolicy tests confirm the runtime snake_case keys are unchanged. - Two required-prop option bags with = {} defaults cast the default = /** @type {T} */ ({}). - restore: typed the inner collectPage accumulator to the rich GitHub-API artifact shape (matching listArtifactsForName's @returns - a narrow first guess triggered a second-order TS2322 at the selectNewestArtifact consumer), completed the options typedef with the three missing test-seam impl overrides (downloadArtifactArchiveImpl / extractArchiveImpl / listArtifactsForNameImpl - typing these properly is what surfaced the artifact-shape mismatch), and the TS6 Buffer->ArrayBufferView gotcha at fs.writeFile reads through an unknown-bridged Uint8Array cast. - fetch: result.content (flat, non-discriminated @returns so always string | undefined) cast string at the restored-branch write. scripts/docs (9 -> 0), the Markdown validator: TS7006/TS7031 implicit-any params/binding-elements plus one unknown catch. @param annotations (filePath/dirPath string, files string[], argv string[]), a DocViolation typedef threaded through validateMarkdownFile's @returns and writeReport's destructured { files, violations } param (which contextually types the inner forEach violation), and the main catch narrows error instanceof Error ? error.message : error. Verified: tsc grep empty, total 0 (npm run typecheck exit 0); 43 reporting tests + 6 validateDocs tests green; eslint clean. Claude-Session: https://claude.ai/code/session_01C2NqWYBoePEb1yP8PmcRT9
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
jsugg
changed the title
The Allure-history restore path wrote a downloaded artifact straight to
disk (`fs.writeFile(dest, Buffer.from(await res.arrayBuffer()))`). Those
bytes are untrusted network input, which CodeQL flags as
js/http-to-file-access ("network data written to file").
Add explicit validation before the write:
- cap the archive at 100 MiB, checked against both the Content-Length
header (fail fast, pre-buffer) and the actual byte length, so a
corrupted or hostile response cannot exhaust the disk;
- require the ZIP "PK" magic bytes so only a recognized archive format
can reach the filesystem and the downstream unzip;
- reject empty payloads.
assertArchivePayload returns a real Uint8Array, which also drops the
unknown-bridge cast the write previously needed to type-check.
Covered by new unit tests for assertArchivePayload (valid/empty/
oversized/non-ZIP) and downloadArtifactArchive (writes a valid ZIP,
refuses oversized Content-Length, refuses non-ZIP).
| } | ||
|
|
||
| const archiveBytes = assertArchivePayload(await archiveResponse.arrayBuffer()); | ||
| await fs.writeFile(destinationPath, archiveBytes); |
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
2 participants
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.
Summary
Burns the advisory strict-JSDoc typecheck gate (
npm run typecheck—tsconfig.checkjs.json,checkJs+strict, TypeScript 6) down to zeroacross
src/,config/, andscripts/. Measured baseline was 1438 errors;npm run typechecknow exits 0.Every commit is comment-only JSDoc typing plus type-only casts — no runtime
change, and the committed strict profile is never weakened. Work landed
area by area, highest error counts first, each batch verified independently
(tsc clean for the area + the related Jest suites + ESLint) before commit.
Method
@typedefexternal-JSON / duck shapes; cast once atvalidation boundaries (double-cast through
unknownonly where TS reportsinsufficient overlap).
narrow
@returns, a snake_case output map mislabeled camelCase), never byloosening an exported type.
export. Two were caught and folded in-flight (a fixture-app cast into the
perf smoke; an impl-seam typing that surfaced an under-specified artifact
@returns).Verification
npm run typecheck→ 0 errors (exit 0).touched file; the one file with no test coverage (
scripts/doctor-tls.js,a manual CLI) checked with
node --check.docs/typecheck-debt.md.Follow-up (not in this PR)
Promotion of the gate from advisory to required is staged separately and is
sequenced after this merges (so
mainis green first): removecontinue-on-errorfrom the CItypecheckjob and add thetypecheckcontext to
config/github/required-checks.json, then a maintainer applies thelive branch-protection / ruleset change per
docs/required-checks.md.