chore(tooling): typecheck root test/** with a dedicated tsgo lane#104475
Conversation
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: chore(tooling): typecheck root test/** with a dedicated tsgo lane This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
- test/tsconfig/tsconfig.test.root.json: root-test program (strict unused checks, fixtures excluded; two Docker E2E clients that import built dist/** stay out, same rationale as the scripts/e2e exclusion in tsconfig.scripts.json) - tsgo:test:root wired into tsgo:test, check:test-types, scripts/check.mjs, and the ci.yml test-types shard, mirroring the tsgo:scripts lane (#104348) - changed-lane routing: test/**/*.ts (excluding fixtures) and the lane tsconfig now trigger 'typecheck test root' in check:changed; previously test/ paths ran lint only, so harness type errors surfaced first in CI (#104287 envDir case) - burn down all 1071 latent type errors in the program: precise param/local types across test/scripts, test/vitest, test/e2e, and transitive scripts/e2e program members; 205 sibling .d.mts declaration files for imported .mjs modules (committed separately); zero any, zero ts-expect-error - resolve the pre-existing testing star-export ambiguity in scripts/e2e/parallels/common.ts with an explicit re-export Closes #104388
- re-derive 51 .d.mts files from implementation data flow instead of initializers: fix a wrong never return (runTestProjectsDelegation returns the child), add encoding-sensitive exec/spawn overloads (plain-gh), restore the full release profile union, make parsed paths string | null, add missing parseArgs fields via help/non-help unions, add a missing sibling declaration (budget-number-args), drop 15 unused lint directives - precise install-record/tuple typing removes the type-aware oxlint regressions the first declarations caused in scripts/e2e implementations - route .mts declaration edits under test/ to the testRoot lane and reference the test-root project from tsconfig.projects.json so tsgo:all covers it (closes both review findings against the lane wiring)
6b2af54 to
a7dba4c
Compare
|
Codex review: needs maintainer review before merge. Reviewed July 11, 2026, 9:14 AM ET / 13:14 UTC. Summary Reproducibility: yes. at source level. Current Review metrics: 2 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Land the single source-only root-test lane with its two Docker-owned exclusions, keeping the compiler project, changed-file planner, aggregate build reference, CI task, declarations, and focused routing guards aligned as one canonical path. Do we have a high-confidence way to reproduce the issue? Yes at source level. Current Is this the best way to solve the issue? Yes. A dedicated project wired through the existing tsgo wrapper, project references, centralized changed planner, and CI test-types task is the narrowest maintainable solution. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against e681646834e6. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: chore(tooling): typecheck root test/** with a dedicated tsgo lane This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
|
Merged via squash.
|
…enclaw#104475) * chore(types): add declaration files for scripts/lib and scripts/e2e modules * chore(types): add declaration files for top-level script modules (a-m) * chore(types): add declaration files for top-level script modules (n-z) * test: use a non-secret-shaped gateway token fixture * test: type ci workflow guard helpers for the root test lane * chore(tooling): typecheck root test/** with a dedicated tsgo lane - test/tsconfig/tsconfig.test.root.json: root-test program (strict unused checks, fixtures excluded; two Docker E2E clients that import built dist/** stay out, same rationale as the scripts/e2e exclusion in tsconfig.scripts.json) - tsgo:test:root wired into tsgo:test, check:test-types, scripts/check.mjs, and the ci.yml test-types shard, mirroring the tsgo:scripts lane (openclaw#104348) - changed-lane routing: test/**/*.ts (excluding fixtures) and the lane tsconfig now trigger 'typecheck test root' in check:changed; previously test/ paths ran lint only, so harness type errors surfaced first in CI (openclaw#104287 envDir case) - burn down all 1071 latent type errors in the program: precise param/local types across test/scripts, test/vitest, test/e2e, and transitive scripts/e2e program members; 205 sibling .d.mts declaration files for imported .mjs modules (committed separately); zero any, zero ts-expect-error - resolve the pre-existing testing star-export ambiguity in scripts/e2e/parallels/common.ts with an explicit re-export Closes openclaw#104388 * chore(types): correct declaration fidelity per structured review - re-derive 51 .d.mts files from implementation data flow instead of initializers: fix a wrong never return (runTestProjectsDelegation returns the child), add encoding-sensitive exec/spawn overloads (plain-gh), restore the full release profile union, make parsed paths string | null, add missing parseArgs fields via help/non-help unions, add a missing sibling declaration (budget-number-args), drop 15 unused lint directives - precise install-record/tuple typing removes the type-aware oxlint regressions the first declarations caused in scripts/e2e implementations - route .mts declaration edits under test/ to the testRoot lane and reference the test-root project from tsconfig.projects.json so tsgo:all covers it (closes both review findings against the lane wiring) * chore(scripts): keep telegram runner dist typing structural for the boundary guard * chore(types): declare runtime pack and gateway readiness exports added on main * test: pin the importTargetPlan form of the plugin-contract plan import The guard expectation still referenced the raw await import( form that 7250680 (openclaw#103975) replaced with the importTargetPlan fallback helper; the assertion fails on current main.
Closes #104388
What Problem This Solves
Fixes an issue where TypeScript errors in the test harness (all 111 Vitest lane configs,
test/scripts/**,test/helpers/**,test/e2e/**) were invisible to both the changed gate and, for most files, CI entirely: no tsgo lane compiled roottest/**, andscripts/changed-lanes.mjsroutedtest/edits to lint-only tooling checks. The concrete prior cost: theenvDirtype error in #104287 passedcheck:changedand died in CI — and only because that one config is transitively imported; a probe showed 1071 latent type errors hiding in the uncompiled tree.Why This Change Was Made
Mirrors the just-landed
tsgo:scriptslane (#104348): atsconfig.test.root.jsonprogram over roottest/**(fixtures and two Docker E2E clients that import builtdist/**excluded, same rationale as thescripts/e2eexclusion), wired intotsgo:test/check:test-types,scripts/check.mjs,tsgo:allvia atsconfig.projects.jsonreference, the ci.yml test-types shard, and changed-lane routing (test/**TS-family files →typecheck test root). All 1071 errors were burned down with precise types — zeroany, zero@ts-expect-error— including 205 sibling.d.mtsdeclaration files forscripts/**/*.mjsmodules that test files import. Structured review then audited the declarations against their implementations and forced corrections in 51 of them (wrongneverreturn, missing encoding-sensitive exec/spawn overloads, initializer-derived types, union gaps, missing fields); the lane already proved itself during the rebase by catching two upstreammaincommits whose new.mjsexports were missing from declarations.User Impact
None at product runtime — developer/CI harness only. Test-harness type errors now fail
check:changedlocally/remotely and the CI test-types shard deterministically.Evidence
node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.test.root.jsonexit 0 (from 1071 errors);tsgo:scripts,tsgo:core:test, andtsgo:all(with the new project reference) all exit 0.pnpm test:changed(7 shards green, including the guard tests that caught two earlier regressions in this branch: the crestodian dist-import contract and the parallels barrel contract),pnpm check:changed(which now self-applies the new lane:lanes=scripts, testRoot, tooling),pnpm check:workflows.node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.scripts.json scriptsand the docker-e2e boundary guard green (the first declaration pass regressed type-aware lint in six.mjsimplementations; fixed via precise declarations, not suppressions).gpt-5.6-sol, xhigh) per commit: the three declaration commits were initially judged incorrect with concrete fidelity findings — all confirmed against implementations and fixed in the follow-up commit, which reviews clean (0.93); the lane-wiring review's two findings (.d.mtsfiles not routed;tsgo:allmissing the project) are fixed and re-reviewed clean; the final boundary-guard fix reviews clean (0.98). Two commits are inaccessible to the review helper by design (its secret scanner fails closed on a pre-existingGH_TOKEN: "${{ github.token }}"guard assertion and a renamedtoken: "secret-token"fixture); both were reviewed manually — typing-only param annotations and a 5-line fixture rename..mjsimplementation edits are JSDoc/coercion cleanups justified in the fidelity commit, plus a structural (non-src-referencing) cast inscripts/e2e/npm-telegram-live-runner.tsto satisfy both the source-only program and the dist-import boundary guard.