chore(typescript): bump to TypeScript 7.0.2 (native port) - #1262
Conversation
|
✅ Deterministic PR hygiene checks passed. |
📝 WalkthroughWalkthroughThe PR updates TypeScript, ties execution payload types to discriminated message cases, adds serialization tests, and normalizes binary forwarding for live-sideband upstream frames. It also updates TypeScript contract checks for version 7 compatibility. ChangesCompatibility updates
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
TypeScript 7 is the Go-based native port (8-12x faster full builds). Bump the root devDependency from 5.9.3 to 7.0.2 and fix the three type-level strictness differences it surfaces: - native-exec-common: make execBytes generic over the message case so the value type is checked consistently (removes the as never hack) - server: sendUpstreamFrame copies Buffer frames into an ArrayBuffer- backed Uint8Array for WebSocket.send (BufferSource rejects SharedArrayBuffer-backed buffers) - server: readyState CLOSED comparisons use the numeric literal 3 where Bun's type narrows readyState to 0|1|2 Adds focused regression tests for the execBytes serialization (3 cases: mcpResult, requestContextResult, diagnosticsResult). Verified: bun run typecheck (TS 7.0.2), bun audit clean, 70 focused tests pass.
a1a52fd to
4c986e4
Compare
- install-scripts: the hardcoded devDependency expectation for typescript moves 5.9.3 -> 7.0.2 with the bump - translator-budget: TS 7 refuses files-on-command-line when a tsconfig is present (TS5112); pass --ignoreConfig so the fixture is typechecked standalone and the TS2554 contract assertion still fires
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/cursor-native-exec-common.test.ts`:
- Around line 65-80: Extend the test around diagnosticsResult and the decoded
value from AgentClientMessageSchema to assert the diagnostics result
discriminator is "error" and its payload contains path "/tmp/x" and error
"unsupported". Keep the existing outer message-case assertions unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 42db1251-8c6a-4b79-8bb0-8831f359d7ae
📒 Files selected for processing (3)
tests/cursor-native-exec-common.test.tstests/install-scripts.test.tstests/translator-budget.test.ts
| test("diagnosticsResult round-trips an error result", () => { | ||
| const bytes = execBytes( | ||
| execServerMessage({ case: "diagnosticsArgs", value: create(DiagnosticsResultSchema, {}) }), | ||
| "diagnosticsResult", | ||
| create(DiagnosticsResultSchema, { | ||
| result: { | ||
| case: "error", | ||
| value: create(DiagnosticsErrorSchema, { path: "/tmp/x", error: "unsupported" }), | ||
| }, | ||
| }), | ||
| ); | ||
|
|
||
| const decoded = fromBinary(AgentClientMessageSchema, bytes); | ||
| expect(decoded.message.case).toBe("execClientMessage"); | ||
| expect(decoded.message.value.message.case).toBe("diagnosticsResult"); | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Assert the decoded diagnostics error payload.
The test only verifies the outer message cases. It does not verify result.case, path, or error. A serializer that drops the diagnostics payload would still pass. Assert the error discriminator and both fields.
Proposed fix
const decoded = fromBinary(AgentClientMessageSchema, bytes);
+ const result = decoded.message.value.message.value.result;
expect(decoded.message.case).toBe("execClientMessage");
expect(decoded.message.value.message.case).toBe("diagnosticsResult");
+ expect(result.case).toBe("error");
+ if (result.case !== "error") {
+ throw new Error(`Expected diagnostics error result, got ${result.case}`);
+ }
+ expect(result.value.path).toBe("/tmp/x");
+ expect(result.value.error).toBe("unsupported");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test("diagnosticsResult round-trips an error result", () => { | |
| const bytes = execBytes( | |
| execServerMessage({ case: "diagnosticsArgs", value: create(DiagnosticsResultSchema, {}) }), | |
| "diagnosticsResult", | |
| create(DiagnosticsResultSchema, { | |
| result: { | |
| case: "error", | |
| value: create(DiagnosticsErrorSchema, { path: "/tmp/x", error: "unsupported" }), | |
| }, | |
| }), | |
| ); | |
| const decoded = fromBinary(AgentClientMessageSchema, bytes); | |
| expect(decoded.message.case).toBe("execClientMessage"); | |
| expect(decoded.message.value.message.case).toBe("diagnosticsResult"); | |
| }); | |
| test("diagnosticsResult round-trips an error result", () => { | |
| const bytes = execBytes( | |
| execServerMessage({ case: "diagnosticsArgs", value: create(DiagnosticsResultSchema, {}) }), | |
| "diagnosticsResult", | |
| create(DiagnosticsResultSchema, { | |
| result: { | |
| case: "error", | |
| value: create(DiagnosticsErrorSchema, { path: "/tmp/x", error: "unsupported" }), | |
| }, | |
| }), | |
| ); | |
| const decoded = fromBinary(AgentClientMessageSchema, bytes); | |
| const result = decoded.message.value.message.value.result; | |
| expect(decoded.message.case).toBe("execClientMessage"); | |
| expect(decoded.message.value.message.case).toBe("diagnosticsResult"); | |
| expect(result.case).toBe("error"); | |
| if (result.case !== "error") { | |
| throw new Error(`Expected diagnostics error result, got ${result.case}`); | |
| } | |
| expect(result.value.path).toBe("/tmp/x"); | |
| expect(result.value.error).toBe("unsupported"); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/cursor-native-exec-common.test.ts` around lines 65 - 80, Extend the
test around diagnosticsResult and the decoded value from
AgentClientMessageSchema to assert the diagnostics result discriminator is
"error" and its payload contains path "/tmp/x" and error "unsupported". Keep the
existing outer message-case assertions unchanged.
Ingwannu
left a comment
There was a problem hiding this comment.
The focused implementation looks coherent, and I independently passed the 23 touched tests, bun run typecheck, and bun run privacy:scan on ccf85d3c with CPU affinity limited to two cores.
I am holding approval for two merge gates rather than a code defect:
- this dependency/native-toolchain change is now 13 commits behind current
dev(d55b903d), beyond the repository readiness window, so please rebase it onto the current integration head; - the macOS required check is still pending, and this TypeScript package ships platform-native binaries, so that platform result is part of the dependency-boundary review.
After the rebase, please rerun the focused serialization/install/translator tests plus typecheck/privacy and let the full cross-platform CI complete. I will re-review the exact new head once those gates are green.
Summary
typescript5.9.3 → 7.0.2 — the Go-based native port (8-12x faster full builds).native-exec-common.ts: makeexecBytesgeneric over the message case so the value type is checked consistently (removes theas neverescape hatch).server/index.ts: newsendUpstreamFramehelper copiesBufferframes into anArrayBuffer-backedUint8ArraybeforeWebSocket.send(Bun'sBufferSourcerejectsSharedArrayBuffer-backed buffers); the live-sideband frame sends now use it.server/index.ts:readyStateCLOSED checks use the numeric literal 3 (with explanatory comments) where Bun's type narrowsreadyStateto 0|1|2.<6.1.0peer bound); TS 7 support there waits on typescript-eslint 9.x.Validation
bun run typecheck— pass (TS 7.0.2)bun audit— pass, 0 vulnerabilitiesbun testfocused (cursor native exec + websocket live transport) — pass, 115 tests / 0 failReview notes
execBytesis called from native-exec.ts and native-exec-tools.ts withcreate(SomeSchema, {...})values; the generic signature type-checks each call site exactly.readyStatenumeric-literal comparisons are runtime-identical toWebSocket.CLOSED(which is 3).Summary by CodeRabbit
Bug Fixes
Compatibility