Skip to content

test(runtime): keep export parity current - #16773

Closed
brennanb2025 wants to merge 2 commits into
mainfrom
brennanb2025/shared-ci-export-parity-fix
Closed

brennanb2025 wants to merge 2 commits into
mainfrom
brennanb2025/shared-ci-export-parity-fix

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Aug 27, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​5 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​4
Prod 0 0 0 0

Summary

  • update the runtime export-parity inventory for the terminal degradation exports now published by runtime-types
  • unblock unrelated PR merge refs that fail shard 10 under Node 24 and Node 26

Verification

@brennanb2025

brennanb2025 commented Aug 27, 2026 •

Copy link
Copy Markdown
Contributor Author

Review verification (exact head b56a1ef1cee30562b03acc2091a24432eaf37a05):

  • Live PR head and local HEAD match the required full SHA; no push or merge performed.
  • The diff remains limited to src/shared/runtime-client-export-parity.test.ts (test inventory only). The pushed follow-up adds the previously omitted Runtime.RuntimeTerminalUnavailableReason type, so the prior P2 coverage finding is resolved; all eight runtime value exports are listed.
  • Focused parity test passes locally (2/2) under Node 26.5.0; pnpm tc passes; git diff --check passes. The changed-quality wrapper cannot parse the package manager Node 26 engine warning, an environment/tooling caveat.
  • Review threads: 0 total / 0 unresolved; no submitted reviews or actionable automated comments.
  • PR Checks run 33087590430 is currently red only because Node 26 shard 14 hit an unrelated unhandled TypeError: store.getSettings is not a function in attach-main-window-services.test.ts; the dependent verify gate also failed. All other checks (including static analysis, typecheck, Node 24 shards, remaining Node 26 shards, compatibility, packages, and shell contracts) passed. Rerun/green CI is required before merge.

Verdict: review-clean with the previous P2 fixed; parity change itself is validated, but the PR is not merge-ready until the unrelated flaky CI pair is green.

@coderabbitai

coderabbitai Bot commented Aug 27, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e8e83fe3-4b15-4d31-bc56-dafeb53a627a

📥 Commits

Reviewing files that changed from the base of the PR and between d774e21 and b56a1ef.

📒 Files selected for processing (1)
  • src/shared/runtime-client-export-parity.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The Runtime export parity test now includes RuntimeTerminalUnavailableReason in its type inventory. It also expects TERMINAL_PTY_DEGRADATION_CAPABILITY, TERMINAL_UNAVAILABLE_ERROR_CODE, and terminalUnavailableMessage. Existing value exports are reordered to maintain alphabetical order.

Merge Risk: ⚪ Minimal · up to b56a1

This test-only update keeps expected runtime exports aligned with the published runtime types and does not change production behavior; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the change and includes useful verification details. It does not follow the required template because it omits the ELI5, What Changed, Why, Linked Issue, Visual Proof, Review,… Complete the required template sections. Add a linked issue after “Fixes #”. Add N/A with a reason for Visual Proof because there is no UI or behavior change. Record the applicable testing, review, compatibility, and checklist items. Incl…
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: updating the runtime export parity test. It is concise and specific.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the change and includes useful verification details. It does not follow the required template because it omits the ELI5, What Changed, Why, Linked Issue, Visual Proof, Review, Notes, Agent skill upstream boundary, and Checklist sections. The required linked issue is also missing.

Resolution

Complete the required template sections. Add a linked issue after “Fixes #”. Add N/A with a reason for Visual Proof because there is no UI or behavior change. Record the applicable testing, review, compatibility, and checklist items. Include the Agent skill upstream boundary statement and AI Disclosure information when applicable.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@brennanb2025

brennanb2025 commented Aug 27, 2026 •

Copy link
Copy Markdown
Contributor Author

Non-UI QA evidence for exact head b56a1ef1cee30562b03acc2091a24432eaf37a05:

  • Focused runtime export parity test: pass (2/2) locally on Node 26.5.0.
  • pnpm tc: pass locally.
  • git diff --check origin/main...HEAD: pass.
  • Targeted attach-main-window-services.test.ts: pass locally (30/30), indicating the CI Node 26 shard failure (store.getSettings is not a function) is unrelated/flaky and outside this one-line inventory change.
  • Live PR Checks run 33087590430: all checks pass except Node 26 shard 14 and its dependent verify gate; E2E is correctly skipped. Rerun/green CI is required before merge.
  • This is a pure test-contract/inventory PR with no Electron, mobile, or user-visible behavior change, so no UI validation or manual image is required.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Added the missing Runtime.RuntimeTerminalUnavailableReason entry to RuntimeTypeInventory, keeping the export-parity inventory complete and alphabetically ordered.

Validated with the focused parity test on Node 24.18.0 and Node 26.5.0 (2/2 tests passed on each), plus pnpm tc, focused oxlint, oxfmt --check, and git diff --check.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Closing as fully superseded. The entire diff here (the export-parity inventory refresh for the terminal-unavailability exports) landed on main via #16810 (9253634) — main's copy of src/shared/runtime-client-export-parity.test.ts is now byte-identical to this branch's version, so merging this would be a no-op.

The remaining red on this PR (tests node 26 14/16 + verify) is unrelated to this change: the shard had 420 passing / 0 failing tests and failed only on an unhandled rejection from a deferred worktree-base watcher timer leaked by attach-main-window-services.test.ts. That pollution is addressed separately by #16836.

If I've missed anything unique this branch still carries, please reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant