refactor(test): consolidate script declaration contracts#100992
Conversation
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper review |
|
Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 12:46 PM ET / 16:46 UTC. Summary PR surface: Source -16, Tests -170, Other +152. Total -34 across 8 files. Reproducibility: not applicable. as a bug reproduction: this PR is an internal cleanup. Source inspection does show current main still has the duplicated test-local script interfaces that the PR removes. Review metrics: none identified. 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. Next step before merge
Security Review detailsBest possible solution: Land this PR after normal maintainer review if the exact-head checks remain green, keeping the adjacent Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction: this PR is an internal cleanup. Source inspection does show current main still has the duplicated test-local script interfaces that the PR removes. Is this the best way to solve the issue? Yes. The PR uses the narrowest maintainable owner boundary by moving TypeScript consumers to the adjacent script declaration companions instead of preserving casts, central ambient declarations, or another parallel type layer. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 6b99fd9a94ce. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source -16, Tests -170, Other +152. Total -34 across 8 files. View PR surface stats
What 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
|
Related: #100977
What Problem This Solves
Script tests currently maintain shadow interfaces for internal
.mjshelpers because the adjacent.d.mtscompanions omit live exports. Those parallel contracts drift independently, require unsafe casts, and make script/test changes harder to review safely.Why This Change Was Made
Make the four affected script declaration companions the canonical owners for every live export used by TypeScript consumers. Tests now import those contracts directly, the stale ambient
ci-changed-scopedeclaration is removed, and shared Vitest config reuses the scheduling module's exported types. Runtime implementations and behavior are unchanged.User Impact
No user-visible behavior changes. Maintainers get one authoritative script contract per module, fewer unsafe casts, and compile-time detection when script exports and TypeScript consumers diverge.
Evidence
tbx_01kww0qbaeqcj0p2442c84dj68: focused script validation passed 314 tests total acrosssrc/scripts/test-projects.test.ts,src/scripts/docs-link-audit.test.ts,src/scripts/ci-changed-scope.test.ts,test/scripts/test-projects.test.ts, andtest/scripts/vitest-local-scheduling.test.ts.tbx_01kww0qbaeqcj0p2442c84dj68:env OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 corepack pnpm check:changedpassed all selected typecheck, lint, and guard lanes.tbx_01kww0qbaeqcj0p2442c84dj68:corepack pnpm exec oxfmt --check --threads=1passed for all eight changed files..agents/skills/autoreview/scripts/autoreview --mode local --prompt-file tmp/review-context.md --stream-engine-output: clean after resolving one accepted helper-input typing finding.git diff --check: passed.