Repository navigation
fix(release): fail closed on unsafe release tags #3452
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 10 commits
e0242b9
dadca65
b626e4b
7497e55
cbe0235
324a347
3fddc52
c543d36
0ca463f
97a2b60
8215856
9d6c91d
1b0335e
382d1c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -120,3 +120,34 @@ | |
| - Affected files: src/ui/components/TopBar/DownloadsIndicator.spec.tsx. | ||
| - Reference: PR #3443 (introduced the animated percentage slot), PR #3449 (documented after CI | ||
| runs surfaced the divergence). | ||
|
|
||
| ## Jest test run sits near Node's default 2 GB heap ceiling | ||
|
|
||
| - Status: Mitigated, not fixed. The ceiling is raised; the underlying accumulation remains. | ||
| - Symptom: The `Test` step dies with | ||
| `FATAL ERROR: Ineffective mark-compacts near heap limit - JavaScript heap out of memory` | ||
| and `exit code 255`, typically on `check (macos-latest)` first, after most or all suites | ||
| have already reported PASS and none have FAILED. Observed at 2038.0/2051.6 MB with | ||
| `current mu = 0.040` (V8 spending ~96% of its time in GC before giving up). | ||
| - Root cause: `@kayahr/jest-electron-runner` spawns a detached Electron child per test | ||
| target and registers a teardown closure for each one in a module-level `DISPOSABLES` set | ||
| that is only drained at the very end of the run. With `--maxWorkers=1` (required by the | ||
| runner) every suite shares one process, so nothing is reclaimed by worker recycling, and | ||
| each retained closure keeps its child's context reachable. `--coverage` inflates each | ||
| suite's retained set further, which is why `yarn test:coverage` (validate-pr) tips over | ||
| before `yarn test` (build-release) does. macOS fails first: slowest arm64 runner, largest | ||
| retained set. | ||
|
Comment on lines
+138
to
+139
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Replace subjective wording with measurable statements. Lines 138-139 describe the runner as “slowest” and having the “largest retained set” without measured comparisons. Lines 145-147 use “buys headroom” and “real fix”. State the observed result and required action directly. Proposed wording- macOS fails first: slowest arm64 runner, largest retained set.
+ In the referenced run, `macOS-latest` failed first. The observed heap values are listed above.
...
- raising the ceiling buys headroom, it does not stop the accumulation.
+ increasing the heap limit does not stop the accumulation.
- the real fix is upstream disposing each child
+ the required upstream fix is to dispose each childAs per coding guidelines: Also applies to: 145-147 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| - Workaround: `NODE_OPTIONS: --max-old-space-size=6144` on the `Test` step in | ||
| `.github/workflows/validate-pr.yml` and `.github/workflows/build-release.yml` | ||
| (GitHub-hosted runners have >= 7 GB), and `--detectOpenHandles` removed from the `test` / | ||
|
Comment on lines
+140
to
+142
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- relevant documentation ---'
cat -n docs/KNOWN_ISSUES.md | sed -n '125,160p'
printf '%s\n' '--- workflow files ---'
git ls-files '.github/workflows/*' | sort
printf '%s\n' '--- runner declarations and memory references ---'
rg -n -i --glob '.github/workflows/**' \
'runs-on|ubuntu-|windows-|macos-|max-old-space-size|7 GB|memory|runner' \
.github/workflows || trueRepository: RocketChat/Rocket.Chat.Electron Length of output: 7956 🌐 Web query:
💡 Result: The memory specifications for standard GitHub-hosted runners vary depending on whether the repository is public or private, and which operating system is used. As of August 11, 2026, the specifications are as follows [1][2]: Public Repositories: - Ubuntu-latest (Linux): 16 GB RAM (4 CPUs) [1][2] - Windows-latest (Windows): 16 GB RAM (4 CPUs) [1][2] - Macos-latest (macOS M1): 7 GB RAM (3 vCPUs) [1][2] Private Repositories: - Ubuntu-latest (Linux): 8 GB RAM (2 CPUs) [1][2] - Windows-latest (Windows): 8 GB RAM (2 CPUs) [1][2] - Macos-latest (macOS M1): 7 GB RAM (3 vCPUs) [1][2] The 7 GB RAM figure specifically applies to the standard GitHub-hosted macOS (M1/arm64) runners, which provide this amount of memory for both public and private repositories [1][2]. For Linux and Windows, the memory allocation is 16 GB for public repositories and 8 GB for private ones [1][2]. Larger memory options are available through GitHub-hosted large runners [3]. Citations:
Cite and qualify the runner-memory claim. The workflows use standard 🧰 Tools🪛 LanguageTool[uncategorized] ~140-~140: The official name of this software platform is spelled with a capital “H”. (GITHUB) [uncategorized] ~141-~141: The official name of this software platform is spelled with a capital “H”. (GITHUB) 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| `test:coverage` scripts — it retains async resource references by design in order to | ||
| report them. It is still available as `yarn test:debug` when actually debugging handles. | ||
| - Expect this to recur as the suite grows: raising the ceiling buys headroom, it does not | ||
| stop the accumulation. If it returns, do NOT just raise the number again — the real fix is | ||
| upstream disposing each child as its target finishes instead of at end-of-run. Note that | ||
| `workerIdleMemoryLimit` does NOT help: Jest recycles *worker processes* after a test, and | ||
| at `--maxWorkers=1` with this custom runner there is no worker to recycle. | ||
| - Affected files: .github/workflows/validate-pr.yml, .github/workflows/build-release.yml, | ||
| package.json, patches/@kayahr+jest-electron-runner+29.14.0.patch. | ||
| - Reference: PR #3452. Failing run 31515022528 (`check (macos-latest)`); green after the fix | ||
| in run 31515403323 (macOS 16m32s, 1721 tests, 0 failures). | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: RocketChat/Rocket.Chat.Electron
Length of output: 45050
🏁 Script executed:
Repository: RocketChat/Rocket.Chat.Electron
Length of output: 688
🏁 Script executed:
Repository: RocketChat/Rocket.Chat.Electron
Length of output: 257
🏁 Script executed:
Repository: RocketChat/Rocket.Chat.Electron
Length of output: 286
🏁 Script executed:
Repository: RocketChat/Rocket.Chat.Electron
Length of output: 1680
Disable checkout credential persistence in both workflows.
actions/checkoutdefaultspersist-credentialstotrueand storesgithub.tokenin the local Git configuration. No later authenticated Git operation requires this token. The pull-request workflow runs PR-controlled commands withpull-requests: write, which increases exposure impact.persist-credentials: falsebesidefetch-depth: 0in.github/workflows/pull-request-build.yml.with: persist-credentials: falsein.github/workflows/powershell-lint.yml.🧰 Tools
🪛 zizmor (1.29.0)
[warning] 39-42: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
📍 Affects 2 files
.github/workflows/pull-request-build.yml#L40-L42(this comment).github/workflows/powershell-lint.yml#L29-L30🤖 Prompt for AI Agents
Source: Linters/SAST tools