Skip to content

chore(tooling): typecheck root test/** with a dedicated tsgo lane#104475

Merged
steipete merged 10 commits into
mainfrom
claude/test-root-tsgo-lane
Jul 11, 2026
Merged

chore(tooling): typecheck root test/** with a dedicated tsgo lane#104475
steipete merged 10 commits into
mainfrom
claude/test-root-tsgo-lane

Conversation

@steipete

Copy link
Copy Markdown
Contributor

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 root test/**, and scripts/changed-lanes.mjs routed test/ edits to lint-only tooling checks. The concrete prior cost: the envDir type error in #104287 passed check:changed and 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:scripts lane (#104348): a tsconfig.test.root.json program over root test/** (fixtures and two Docker E2E clients that import built dist/** excluded, same rationale as the scripts/e2e exclusion), wired into tsgo:test/check:test-types, scripts/check.mjs, tsgo:all via a tsconfig.projects.json reference, 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 — zero any, zero @ts-expect-error — including 205 sibling .d.mts declaration files for scripts/**/*.mjs modules that test files import. Structured review then audited the declarations against their implementations and forced corrections in 51 of them (wrong never return, missing encoding-sensitive exec/spawn overloads, initializer-derived types, union gaps, missing fields); the lane already proved itself during the rebase by catching two upstream main commits whose new .mjs exports were missing from declarations.

User Impact

None at product runtime — developer/CI harness only. Test-harness type errors now fail check:changed locally/remotely and the CI test-types shard deterministically.

Evidence

  • node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.test.root.json exit 0 (from 1071 errors); tsgo:scripts, tsgo:core:test, and tsgo:all (with the new project reference) all exit 0.
  • Remote (Blacksmith Testbox through Crabbox, final lease run 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 scripts and the docker-e2e boundary guard green (the first declaration pass regressed type-aware lint in six .mjs implementations; fixed via precise declarations, not suppressions).
  • Structured autoreview (codex 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.mts files not routed; tsgo:all missing 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-existing GH_TOKEN: "${{ github.token }}" guard assertion and a renamed token: "secret-token" fixture); both were reviewed manually — typing-only param annotations and a 5-line fixture rename.
  • No runtime behavior changes; the only .mjs implementation edits are JSDoc/coercion cleanups justified in the fidelity commit, plus a structural (non-src-referencing) cast in scripts/e2e/npm-telegram-live-runner.ts to satisfy both the source-only program and the dist-import boundary guard.

@steipete
steipete requested a review from a team as a code owner July 11, 2026 12:45
@openclaw-barnacle openclaw-barnacle Bot added scripts Repository scripts docker Docker and sandbox tooling size: XL maintainer Maintainer-authored PR labels Jul 11, 2026
@clawsweeper

clawsweeper Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

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.

steipete added 9 commits July 11, 2026 13:52
- 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)
@steipete
steipete force-pushed the claude/test-root-tsgo-lane branch from 6b2af54 to a7dba4c Compare July 11, 2026 12:55
@clawsweeper

clawsweeper Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 11, 2026, 9:14 AM ET / 13:14 UTC.

Summary
Adds a dedicated tsgo project for root test/**, integrates it into aggregate, changed-file, and CI checks, and supplies the declarations and type corrections needed for the existing test harness to compile.

Reproducibility: yes. at source level. Current main has no dedicated root-test tsgo project or changed-file typecheck route, and the branch's current-head check-test-types job passes.

Review metrics: 2 noteworthy metrics.

  • Declaration contracts audited: 206 implementation pairs. The new declaration files were compared with their JavaScript modules because inaccurate declarations could create false type confidence.
  • Source-only exclusions: 2 Docker clients. Both exclusions are narrow and retain separate Docker execution owners rather than silently losing validation.

Stored data model
Persistent data-model change detected: persistent cache schema: package.json, persistent cache schema: scripts/build-all.d.mts, persistent cache schema: test/tsconfig/tsconfig.test.root.json, serialized state: scripts/check-no-conflict-markers.d.mts, unknown-truncated-pull-files, vector/embedding metadata: scripts/check-release-metadata-only.d.mts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #104388
Summary: This PR is the direct implementation candidate for the canonical root-test typecheck issue; the two merged PRs are adjacent precedent and observed-failure context rather than replacements.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Next step before merge

  • No automated repair is needed; the remaining action is normal explicit maintainer review and landing of the protected PR.

Security
Cleared: The diff adds no dependencies, third-party action refs, permissions, secret access, install hooks, downloads, or package-resolution changes; CI invokes only repository-owned tooling.

Review details

Best 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 main has no dedicated root-test tsgo project or changed-file typecheck route, and the branch's current-head check-test-types job passes.

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 changes

Label changes:

  • add P2: The PR fixes a deterministic test-harness and CI validation gap with meaningful maintainer impact but no product-runtime outage.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-fix Blacksmith Testbox results for the new lane, changed gate, workflow checks, and affected tests, while live current-head CI independently confirms the test-types lane passes.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides after-fix Blacksmith Testbox results for the new lane, changed gate, workflow checks, and affected tests, while live current-head CI independently confirms the test-types lane passes.

Label justifications:

  • P2: The PR fixes a deterministic test-harness and CI validation gap with meaningful maintainer impact but no product-runtime outage.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides after-fix Blacksmith Testbox results for the new lane, changed gate, workflow checks, and affected tests, while live current-head CI independently confirms the test-types lane passes.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-fix Blacksmith Testbox results for the new lane, changed gate, workflow checks, and affected tests, while live current-head CI independently confirms the test-types lane passes.
Evidence reviewed

What I checked:

  • Current-main gap: Current main has scripts, core-test, and extension-test typecheck lanes but no project or changed-check command covering root test/**. (scripts/check-changed.mjs:394, e681646834e6)
  • Centralized lane wiring: The branch classifies non-fixture root-test TypeScript-family paths into testRoot using the repository's existing changed-lane owner. (scripts/changed-lanes.mjs:18, 3efd6e916ed9)
  • Changed-check integration: The centralized changed-check planner invokes tsgo:test:root when the new lane is selected. (scripts/check-changed.mjs:397, 3efd6e916ed9)
  • Aggregate and direct coverage: The branch adds the direct package script, includes it in tsgo:test, and adds the project to build-mode tsgo:all. (package.json:1966, 3efd6e916ed9)
  • CI integration: The test-types CI task invokes the new repository-owned script and preserves the historical-target compatibility behavior used by the adjacent scripts lane. (.github/workflows/ci.yml:1475, 3efd6e916ed9)
  • Source-only exclusions: The two excluded built-dist clients have dedicated Docker shell entrypoints and test-project routing coverage, so the narrow exclusions do not orphan their functional validation. (test/tsconfig/tsconfig.test.root.json:10, 3efd6e916ed9)

Likely related people:

  • steipete: Prior merged work introduced the analogous scripts tsgo lane and recently repaired the centralized changed-run routing used by this PR. (role: introduced adjacent typecheck and changed-lane behavior; confidence: high; commits: 2e2366b6d394, 8cd092620d72; files: scripts/changed-lanes.mjs, scripts/check-changed.mjs, scripts/check.mjs)
  • Vincent Koc: Recent history shows substantial work on the central CI workflow and check fanout, making this a useful secondary routing candidate for CI integration review. (role: recent CI area contributor; confidence: medium; commits: e085fa1a3ffd, e85e6bc4fbb8, de4429ceb331; files: .github/workflows/ci.yml)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper

clawsweeper Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

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.

The guard expectation still referenced the raw await import( form that
7ae5996 (#103975) replaced with the importTargetPlan fallback helper;
the assertion fails on current main.
@steipete
steipete merged commit fe261b0 into main Jul 11, 2026
87 checks passed
@steipete
steipete deleted the claude/test-root-tsgo-lane branch July 11, 2026 13:15
@steipete

Copy link
Copy Markdown
Contributor Author

Merged via squash.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels Jul 11, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 12, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docker Docker and sandbox tooling maintainer Maintainer-authored PR P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: XL status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test harness: root test/** has no tsgo typecheck lane (1071 latent type errors; changed gate runs no typecheck for test/ paths)

1 participant