fix(core): add missing closing braces in formatDateForContext test block#4863
Merged
Conversation
PR #4798 introduced a `describe('formatDateForContext')` test block in environmentContext.test.ts but omitted the closing `});` for both the `it` case and the `describe` block, causing a TS1005 syntax error that broke `tsc --build` and failed CI on all three platforms (ubuntu, macos, windows) plus lint.
Collaborator
Author
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a TypeScript syntax error in the core test suite by properly closing the formatDateForContext it(...) block and its surrounding describe(...) block in environmentContext.test.ts, preventing the test block from spilling into the next describe and breaking tsc --build / CI.
Changes:
- Add the missing
});to closeit('should use current date when no date provided'). - Add the missing
});to closedescribe('formatDateForContext'), restoring correct test file structure.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
|
Thanks for the quick hotfix @yiliang114! 🙏 Template: mostly complete ✓ — "Why it's needed" heading is replaced by "Root Cause" (same info, different name) and "Linked Issues" is absent. Not blocking for a 3-line hotfix, but worth matching the template headings next time for consistency. On direction: this is a clear-cut CI hotfix. PR #4798 was merged with red CI and broke On approach: scope is minimal and correct — two closing braces, nothing else. The timeline table and root-cause analysis are thorough. The branch-protection discussion is a valuable process note (separate concern, not part of this fix). Moving on to code review and test verification. 🔍 中文说明感谢快速修复 @yiliang114！🙏 模板：基本完整 ✓ — "Why it's needed" 标题被 "Root Cause" 替代（信息相同，名称不同），缺少 "Linked Issues"。对于 3 行热修复不构成阻塞，但建议下次使用模板的标准标题以保持一致。 方向：这是一个明确的 CI 热修复。PR #4798 在 CI 失败的情况下被合入，导致 main 分支 方案：范围最小且正确——仅两个闭合大括号，无其他改动。时间线表格和根因分析很充分。分支保护讨论是有价值的流程建议（独立议题，不属于本修复范围）。 进入代码审查和测试验证。🔍 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewDiff is exactly as described: two closing braces ( Before (main — broken)
After (with PR fix applied)
Summary
中文说明代码审查Diff 与描述完全一致：两个闭合大括号（ 修复前（main — 已损坏）
修复后（应用本 PR）
总结
— Qwen Code · qwen3.7-max |
Collaborator
|
Stepping back: this is about as straightforward as a hotfix gets. Main is broken because a PR with red CI was merged — the fix is two closing braces. The independent proposal I'd write before seeing the diff is identical to what's here. No simpler path exists. The before/after is conclusive: The branch-protection discussion in the PR body is a good process note worth following up on separately — it's the real systemic fix to prevent this class of regression. Approving. ✅ 中文说明退一步看：这个热修复简单直接。main 因为一个 CI 失败的 PR 被合入而损坏——修复就是两个闭合大括号。我在看 diff 之前独立提出的方案与 PR 完全一致，不存在更简单的路径。 前后对比结论明确： PR 正文中的分支保护讨论值得单独跟进——这才是防止此类回归的系统性修复。 批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
previously approved these changes
Jun 8, 2026
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
…est.ts PR #4798 added a new vi.mock with importOriginal but left the old manual mock in place. Vitest uses the last vi.mock call, so the old mock won without formatDateForContext, causing 77 test failures. Merge both into a single importOriginal-based mock that preserves all overrides.
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
wenshao
reviewed
Jun 8, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅ Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
wenshao
approved these changes
Jun 8, 2026
Collaborator
✅ Verification Report — PR #4863 (fix CI breakage from #4798)Verdict: PASS — recommend merge. This PR genuinely restores
Claim vs. diff
MethodBuilt nothing user-visible — this is a CI-health fix, so the surface is the build + the affected test files. In a worktree on PR head Steps & Evidence1. ✅ Build fix — The before state is the exact TS1005 that is red on 2. ✅ The undisclosed
The duplicate's winning mock dropped the 3. ✅ Both changed files together: 188 tests pass; lint clean. Findings
RecommendationMerge — it correctly un-breaks Verified locally on Linux: worktree on PR head |
tanzhenxin
approved these changes
Jun 8, 2026
tanzhenxin
left a comment
Collaborator
There was a problem hiding this comment.
Verified: this closes the unbalanced-brace syntax error introduced by #4798 that broke the core package build/typecheck (and the bundle/sandbox image build), and consolidates the now-duplicate environmentContext mock in the client test after the system-reminder refactor. CI is green across macOS/Windows/Linux. LGTM.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Adds two missing closing braces (
});) inpackages/core/src/utils/environmentContext.test.ts— one for theitblock and one for thedescribe('formatDateForContext')block. Without them the file has a syntax error (TS1005: '}' expectedat line 599) that failstsc --buildand breaks CI on main.Why it's needed
PR #4798 (
fix(core): inject current date on every user query to prevent stale date, commit6a99ed150) added a newdescribe('formatDateForContext')test block but omitted the closing});for the lastitcase and the outerdescribe. This merged the new block into the nextdescribe('startup reminder builders'), producing a TypeScript parse error. The PR's own CI was already red (Lint ❌, Test ubuntu/macos/windows ❌) but it was approved and merged, breaking main immediately.Timeline:
6a99ed150)tsc --buildfails with TS1005Process note: the repo currently allows merging PRs with failing required checks. See #4864 for a proposal to enable required status checks on main branch protection.
Reviewer Test Plan
How to verify
Evidence (Before & After)
N/A — non-user-visible fix (two missing closing braces in a test file). The change is a 3-line diff adding
});,});, and a blank line.Tested on
Environment (optional)
npx tsc --buildandnpx vitest runlocally.Risk & Scope
Linked Issues
Fixes the CI breakage introduced by #4798.
See #4864 for the branch protection improvement to prevent recurrence.
中文说明
这个 PR 做了什么
在
packages/core/src/utils/environmentContext.test.ts中补上了两个缺失的闭合括号（});）——一个闭合it块，一个闭合describe('formatDateForContext')块。缺失这两个括号导致 TypeScript 语法错误（TS1005: '}' expected），tsc --build失败，main 分支 CI 全线挂掉。为什么需要
PR #4798（
fix(core): inject current date on every user query to prevent stale date，commit6a99ed150）新增了describe('formatDateForContext')测试块，但遗漏了最后一个it用例和外层describe的闭合});，导致新块与下方的describe('startup reminder builders')合并，产生 TypeScript 解析错误。该 PR 自身的 CI 已经全红（Lint ❌，三个平台 Test ❌），但仍被 approve 并合入，直接导致 main 分支 CI 挂掉。风险与范围
关联 Issue
修复 #4798 引入的 CI 问题。
参见 #4864 关于开启 main 分支保护规则的建议。