Skip to content

fix(licenses): regenerate the notices after the bundled-font declaration - #3155

Merged
Astro-Han merged 1 commit into
apache:mainfrom
Joob1n:fix/regenerate-third-party-notices
Aug 17, 2026
Merged

fix(licenses): regenerate the notices after the bundled-font declaration#3155
Astro-Han merged 1 commit into
apache:mainfrom
Joob1n:fix/regenerate-third-party-notices

Conversation

@Joob1n

@Joob1n Joob1n commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

npm run check:third-party-notices currently fails on main. This regenerates the file it checks.

Cause

#3140 added the Geist OFL declaration to LICENSE. The notices generator folds LICENSE into its output, so THIRD_PARTY_NOTICES.txt went stale the moment that merged — my PR, my omission.

Why CI did not catch it

The Windows package job is the only caller of check:third-party-notices. It runs on pull requests only, and its trigger paths do not include LICENSE:

07:30  #3140 merges — touches LICENSE alone, so this job never runs
   ↓
       main is now stale, and nothing on main re-runs the check
   ↓
08:38  #3148 touches electron-builder.config.mjs, which *is* a trigger path
       → the job wakes up and fails on debt from a different PR

So the failure surfaced on an unrelated PR, two paths and an hour removed from what caused it. Worth noting for anyone deciding whether that job should also run on main, or whether LICENSE belongs in its trigger paths — I have not changed either here, since both are policy calls.

The change

Generator output only. 107 lines added, none removed, all of them the OFL text #3140 introduced.

Verified with npm ci rather than an incremental install, so the dependency closure matches what CI resolves; check:third-party-notices passes afterward.

Sent separately from #3154, which makes this class of failure self-describing — it is what localized this one, in one line, to LICENSE content rather than a dependency change.

Unblocks #3148.

apache#3140 added the Geist OFL declaration to LICENSE, and the notices generator
folds LICENSE into its output, so the committed file has been stale since that
merged. `npm run check:third-party-notices` has failed on `main` ever since.

Nothing caught it because the Windows `package` job is the only caller of this
check, it runs on pull requests alone, and its trigger paths do not include
LICENSE — so the PR that caused the drift never ran the job that would have
said so. The next PR to touch one of those paths inherited the failure.

Regenerated output only: 107 lines added, none removed, all of them the OFL
text apache#3140 introduced. Verified under `npm ci` so the dependency closure
matches what CI resolves.

Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
@hqhq1025
hqhq1025 requested a lite review from Copilot August 17, 2026 08:58
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a905624a-f5c1-494e-b4b0-9d1f9dff46a6

📥 Commits

Reviewing files that changed from the base of the PR and between 48e95e1 and abc0c6b.

📒 Files selected for processing (1)
  • apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt

Included review availability: Your plan includes up to 3 reviews per rolling hour; 0 remain after this review.


📝 Walkthrough

What this PR solves

THIRD_PARTY_NOTICES.txt was stale after the Geist OFL declaration was added to LICENSE. The PR regenerates the file and adds 107 lines for the Geist and Geist Mono fonts, including the SIL Open Font License 1.1 text. This restores consistency with the notices generator and resolves the npm run check:third-party-notices failure.

Source of truth and scope

The PR extends the existing generated notices file from LICENSE. It does not create a parallel licensing path.

The change is the smallest coherent solution. It updates only the generated notices file and adds no runtime, policy, CI, or public API changes.

Deletions and simplification

No code or tests can be deleted or simplified without weakening the notices check or removing required license text.

Risks and validation

The change adds licensing documentation for bundled fonts. It does not change application behavior or exported entities.

Validation included:

  • npm ci
  • Successful npm run check:third-party-notices

The final required-check status remains unverified unless confirmed directly in the repository or CI results.

Review-relevant risks

The current diff has a licensing effect because it adds the Geist and Geist Mono SIL Open Font License 1.1 notices. Any material licensing change requires independent human review under repository policy.

No other protected-area effect was identified in the current diff. The person performing the merge reviews the final diff, and a maintainer makes the final determination.

Walkthrough

Added third-party notices for the bundled Geist and Geist Mono variable webfonts, including attribution details and the complete SIL Open Font License 1.1 text.

Changes

Font licensing

Layer / File(s) Summary
Geist font license notice
apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt
Added attribution, version, copyright, redistribution context, and the SIL Open Font License 1.1 text for the Geist and Geist Mono webfonts.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to abc0c

This PR only regenerates the third-party notices file to include the bundled-font license text; no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

Suggested reviewers: astro-han

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the cause, change, CI context, and verification, but it omits the required AI use and checklist sections. Add the required Summary, Verification, AI use, and Checklist sections, and select the applicable options.
Ai Use Disclosure ⚠️ Warning The PR description selects neither AI-use declaration; the only PR commit has no valid Generated-by trailer. Select exactly one disclosure. Update the PR description and review CONTRIBUTING.md's “Human ownership and AI attribution” section for responsibility, provenance, licensing, and tool-output obligations; preserve any required trailer through squash or amend.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: regenerating license notices after adding the bundled-font declaration.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the desktop app’s committed third-party notices output to reflect the recently added Geist / Geist Mono bundled webfont license declaration, fixing the current check:third-party-notices drift on main.

Changes:

  • Regenerated THIRD_PARTY_NOTICES.txt to include the SIL OFL 1.1 notice text for the bundled Geist fonts (as emitted .woff2 assets).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the regeneration — I ran the actual generator (npm run generate:third-party-noticesscripts/generate-third-party-notices.mjs) against the PR head tree and byte-compared the output with the committed file: zero bytes differ. The change is exactly what the canonical flow produces (the Geist OFL block lands inside the @ai-sdk/provider-utils@5.0.27 section via the --- VERSION-PINNED LICENSE TEXT OVERRIDE ---, since the generator folds repo LICENSE into that package's section — same mechanism as the existing trycua/cua entry). I also cross-checked the other notices artifacts (cargo generator, THIRD_PARTY_LICENSES.txt for vendored SVGs, git NOTICE) — none needed updating.

Conclusion: PASS — no blocking findings (no P0/P1/P2).

P3 (optional): the generated npm notices path (apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt) is missing from the trigger list in .github/workflows/release-windows-check.yml:22-36 (only the cargo notices and git NOTICE are listed), so this PR's own change never ran the owning package job — the body transparently defers the LICENSE-trigger gap, but the npm path itself is also outside the triggers. A one-line trigger addition would keep the next LICENSE-adjacent drift visible; otherwise fine as a stated deferral. Also, the body says "107 lines added"; the actual diff is 106 additions.


AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on ollama-cloud/deepseek-v4-flash). The subagent executed the real generator at the PR head tree and verified byte-identical output; the P3 CI-trigger point is from static workflow inspection. Please weigh these findings with your own judgment.

中文摘要(AI 辅助审查)

结论:PASS,无阻塞项。subagent 实际运行生成脚本(npm run generate:third-party-notices)并与 PR 提交的文件逐字节比对——零差异,改动就是规范流程的输出(Geist OFL 块经 VERSION-PINNED LICENSE TEXT OVERRIDE 落入 @ai-sdk/provider-utils 段,与既有 trycua/cua 条目同机制)。其他 notices 制品(cargo 生成器、SVG 的 THIRD_PARTY_LICENSES.txt、git NOTICE)均无需更新,无遗漏。P3(可选):npm notices 路径不在 release-windows-check.yml 的 trigger 列表(只列了 cargo notices 和 git NOTICE),本 PR 自身改动从未跑过 package job——body 已显式延后 LICENSE trigger 缺口,但 npm 路径本身也在 trigger 外,建议补一行或显式声明延后;另外 body 说 "107 lines" 实际是 106。

@Astro-Han

Copy link
Copy Markdown
Contributor

LGTM. Merging!

@Astro-Han
Astro-Han merged commit 85410e7 into apache:main Aug 17, 2026
13 checks passed
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.

3 participants