fix: attribute the adapted opencode sources and the models.dev snapshot - #3325
Conversation
31a9326 to
8aa2935
Compare
3eab973 to
e4bdb38
Compare
7e64456 to
8dca808
Compare
e4bdb38 to
78dbe2c
Compare
c34f687 to
8311320
Compare
8311320 to
b3684bd
Compare
b3684bd to
9466fa2
Compare
jackwener
left a comment
There was a problem hiding this comment.
Automated review of exact head 9466fa2fc5958128bb5f5932410e78d8e9f40b77 against current main@d62857a8357e9160926726a2a13096bc2dc2b91d.
Both defects remain on main: the adapted opencode files and generated models.dev payload have no durable attribution, and the Apache-text override copies Maka's entire root LICENSE—including THIRD-PARTY COMPONENTS—into the provider-utils notice. This revision records the two non-npm origins at the source/release authority, makes the override fail closed when the delimiter is absent, removes the leaked appendix from both generated notices, and wires the regression test into check:release/the CI plan. The generator remains the single notice authority. I found no actionable P0-P2 implementation defect; all reported required checks are green.
Required conclusions:
- Optimal for the actual problem: yes for the current missing attributions and notice leak.
- Production code to delete: the leaked generated notice sections are correctly deleted; no further production code identified.
- Tests to delete/replace: none identified; the notice-section assertion fails under the original generator and the missing-delimiter branch fails closed in code.
- Deeper refactor: not required for this PR. A future per-artifact declaration should guard non-npm attributions against accidental removal, but it need not duplicate the notice generator here.
- Ready to merge: implementation/check-wise yes, but this automated review is not approval.
- Residual risks/gaps: copyright/source-chain wording, mixed-origin scope, and the
LICENSE/NOTICEconclusion are licensing/provenance judgments; they require independent human/PPMC review. PR #2907 should rebase after this lands and drop its overlapping generator/test/generated-notice changes.
The code origin audit (#2907) found two pieces of third-party material that ship in every release and appear in none of Maka's attribution surfaces, both for the same structural reason: neither is an npm package, and the notice generator walks npm production dependency trees only. `tool-output.ts` and `edit-replace.ts` already said in prose that they adapt opencode, and the adaptation is real — the escape-normalizing replacer's regular expression and its nine branches are carried over unchanged. Prose is not the notice MIT requires. Each file now records source, revision, license, and copyright, and LICENSE carries the permission notice so it travels with the release. Both files are mixed-origin, so the notice is scoped to the adapted material and neither gets a whole-file SPDX identifier. The LICENSE entry records the whole upstream chain rather than stopping at the nearest one. opencode's edit.ts credits cline diff-apply and the gemini-cli editCorrector; that file is Copyright 2025 Google LLC under Apache-2.0, and its escape set shares its first eight branches, in order, with the replacer here. opencode added the ninth and changed the pattern. The entry states the chain and leaves any conclusion about it to a licensing review. The models.dev entry states what is copied instead of characterising it. The earlier draft said the individual records were uncopyrightable facts and that what came from upstream was the selection and arrangement. Both halves were wrong: `sync-model-metadata.mjs` copies `model.name` and `model.description` verbatim, and the snapshot carries 1779 upstream-authored descriptions; while the selection and the ordering are the generator's, not upstream's. Upstream copyright lines were checked against the upstream LICENSE files rather than transcribed: both are `Copyright (c) 2025 opencode` and `Copyright (c) 2025 models.dev` verbatim. The generated headers name their payload. models.dev serves a rolling document with no version, so the generator stamps the sha256 of the data it extracted — not of api.json, which would move whenever any of the ~40 dropped providers changed and produce header-only diffs that say nothing about the file they sit in. The digest covers every export derived from the catalog rather than the two largest: a digest over a subset would sit in a header claiming to describe the file while an upstream change to an uncovered export left it unmoved. Verified by regenerating from the pinned payload, where the data is byte-identical and only the digest line moves, and by confirming that mutating a field reaching only `GENERATED_MODELS_DEV_PROVIDER_FACTS` changes it. Closes the first exit criterion of #3270. Generated-by: Claude Code
#2907 landed the truncation that keeps Maka's own THIRD-PARTY COMPONENTS section out of the `@ai-sdk/provider-utils` notice, and the regression test that guards it. Two gaps remain. `split` returns the whole string when the delimiter is absent, so renaming or dropping the section header silently reinstates the exact bug that was just fixed — and `--check` would then demand the wrong output be committed. Require the delimiter rather than assume it. The test itself was not wired anywhere. It now runs under `check:release`, and `LICENSE` and both generator files are CLI-package inputs to the CI planner, so a pull request touching any of them rebuilds the CLI release candidate and regenerates its notice. The desktop notice stays covered by `check:release` and the manually dispatched ASF source-candidate workflow. Generated-by: Claude Code
9466fa2 to
e6858c1
Compare
|
Rebased after #2907 merged. That PR carried the same generator fix this branch's first commit did, so that commit is dropped — main already has the truncation, the identical regression test, and both cleaned notices. Two commits remain:
The automated review above predates this rebase. @M4n5ter @likun666661 — you both reviewed #2907; this is its sibling change, and the judgment I'd most value is whether dropping the first commit was right and whether the LICENSE wording holds up. The copyright wording, the mixed-origin scoping, and the upstream chain recorded for opencode are licensing judgments that need independent human review. |
M4n5ter
left a comment
There was a problem hiding this comment.
English
I reviewed exact head e6858c1ec444d87ff61fec79cb1ccdb27a436510.
Verdict: Request changes.
1. [Blocking] Do not classify the Apache-origin matcher code as opencode/MIT
packages/runtime/src/edit-replace.ts:21-41 labels the adapted matcher region as opencode/MIT, while LICENSE:304-316 acknowledges the Gemini Apache-2.0 origin and then states that the MIT license applies to “the adapted material.”
The fixed upstream history shows that this is a mixed-license region:
- The line-trimmed matcher came through Cline’s Apache-2.0 implementation, introduced into opencode by fc7af31.
- The first eight escape branches came from Gemini CLI’s Apache-2.0 implementation, including its
Copyright 2025 Google LLCnotice. Opencode subsequently added the ninth branch and changed the pattern.
The current wording therefore collapses independently Apache-licensed material into a single MIT scope and does not preserve the Google notice in the source header. It also points indirectly to mutable upstream main paths—one Cline path has already disappeared—so the statement that the full chain is recorded is not reproducible.
Please keep tool-output.ts scoped to opencode/MIT, but describe edit-replace.ts by material scope:
- opencode-owned MIT additions/modifications;
- fixed Cline revision, Apache-2.0;
- fixed Gemini CLI revision, Apache-2.0, preserving the Google copyright notice.
The root license already contains Apache-2.0, and none of these upstreams supplies a NOTICE, so this does not require duplicating the Apache license or adding unrelated NOTICE content. This is also consistent with ASF’s treatment of third-party works.
2. [Important] Give the snapshot digest one canonical meaning—or remove it
The new digest is neither a stable identity for the generated content nor an identity for the original rolling source:
directoryis hashed inapi.jsoninsertion order, but emitted in sorted provider-ID order. Reversing only the input provider keys produces byte-identical generated bodies while changing both header digests (c27d…→b644…).- Metadata-only changes alter the pricing file’s header, and pricing-only changes alter the metadata file’s header, because both outputs share one aggregate digest.
- Changes to unused/unmapped upstream data can change the raw source while leaving the digest unchanged.
This creates exactly the header-only churn that the new comment says it avoids, and “which exact payload produced it” has no single verifiable interpretation.
The cleanest fix is subtraction: keep the source/license attribution but remove the derived digest. Git already fixes the committed generated payload. GENERATED_MODELS_DEV_DIRECTORY can also be removed: it has no production or test consumers, belongs to a private/non-exported module, and contributes roughly 205 generated lines plus the unstable ordering input.
If the digest must remain, please first define its contract:
- source provenance: hash and retain/archive the original response bytes; or
- generated-content identity: independently hash each output’s canonical, sorted payload.
Either contract should have a generator test covering input-key reordering and cross-output isolation.
3. [Important] Route the desktop notice output through the release-contract gate
The CI change routes the test script, but one of the generator’s committed outputs remains outside the gate:
planTests(['apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt'])
=> releaseContract: false, cliPackage: false
A manual edit or merge-conflict resolution can therefore corrupt the desktop attribution inventory while automatic PR checks remain green. The normal desktop build does not cover it: check-third-party-notices.mjs validates the renderer asset notice, not this npm notice.
Please:
- add
apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txttoRELEASE_CONTRACT_FILES; - cover both committed notice targets in
ci-test-plan.test.mjs; - move
scripts/generate-third-party-notices.test.mjsfromCLI_PACKAGE_FILEStoRELEASE_CONTRACT_FILES.
The last change preserves check:release while avoiding an unrelated CLI tarball build and installed-package smoke for test-only edits.
Everything else in the reviewed scope looks sound: dropping the commit duplicated by #2907 was correct, the missing-delimiter path now fails closed, and leaving NOTICE unchanged is appropriate. All currently reported exact-head checks are green.
简体中文
我审查的是 exact head e6858c1ec444d87ff61fec79cb1ccdb27a436510。
结论:Request changes。
1. [Blocking] 不要把来源于 Apache-2.0 的 matcher 统一归入 opencode/MIT
packages/runtime/src/edit-replace.ts:21-41 将整个改编 matcher 区域标记为 opencode/MIT;LICENSE:304-316 虽然承认 Gemini 的 Apache-2.0 来源,随后却又声明 MIT 适用于全部 “adapted material”。
固定版本的上游历史表明,这实际上是一个混合许可证区域:
- line-trimmed matcher 经由 Cline 的 Apache-2.0 实现 而来,并由 opencode 的 fc7af31 引入。
- escape matcher 的前八个分支来自 Gemini CLI 的 Apache-2.0 实现,原文件包含
Copyright 2025 Google LLC。opencode 后续增加了第九个分支并修改了 pattern。
因此,当前文字将独立的 Apache-2.0 material 错误地合并进单一 MIT scope,也没有在源码头保留 Google 的 copyright notice。当前说明还间接指向可变的上游 main 路径,其中一个 Cline 路径已经消失,所以“完整记录来源链”的结论不可复验。
请继续把 tool-output.ts 限定为 opencode/MIT,但按 material scope 描述 edit-replace.ts:
- opencode 自有的 MIT 修改和新增;
- 固定 revision 的 Cline,Apache-2.0;
- 固定 revision 的 Gemini CLI,Apache-2.0,并保留 Google copyright notice。
根 LICENSE 已经包含 Apache-2.0,而且这些上游均没有提供 NOTICE,因此无需重复 Apache license 或向 NOTICE 添加无关内容。这也符合 ASF 对第三方作品的处理规则。
2. [Important] 为 snapshot digest 定义唯一、规范的含义,否则直接删除
新增 digest 既不是稳定的生成内容标识,也不是原始滚动数据源的标识:
directory参与 hash 时保留api.json的插入顺序,真正输出时却按 provider ID 排序。仅反转输入 provider key 的顺序,两个生成文件正文逐字节相同,但两个 header digest 会从c27d…变为b644…。- 仅修改 metadata 会造成 pricing 文件只有 header 变化;仅修改 pricing 也会造成 metadata 文件只有 header 变化,因为两个输出共享一个聚合 digest。
- 未映射的上游数据发生变化时,原始数据源可以变化,但 digest 完全不变。
这恰好制造了新注释声称要避免的 header-only diff,而且 “which exact payload produced it” 没有唯一、可验证的解释。
最干净的修复是做减法:保留来源和许可证归属,删除派生 digest。Git 本身已经固定了已提交的生成内容。GENERATED_MODELS_DEV_DIRECTORY 也可以删除:它没有任何生产或测试消费者,所在模块 private 且未 export,却增加了约 205 行生成内容,并引入了上述不稳定顺序。
如果必须保留 digest,请先明确契约:
- 标识上游来源:hash 并保留或归档原始响应字节;或
- 标识生成内容:分别对两个输出各自规范排序后的 payload 计算 hash。
无论选择哪种,都应增加 generator 测试,覆盖输入 key 重排和两个输出之间的隔离。
3. [Important] 让桌面端 notice 输出进入 release-contract gate
当前 CI 变更路由了测试文件,但生成器的一个已提交输出仍然没有进入 release gate:
planTests(['apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt'])
=> releaseContract: false, cliPackage: false
因此,手工编辑或解决合并冲突时可以破坏桌面端 attribution inventory,同时所有自动 PR checks 仍保持绿色。普通 desktop build 无法补上这个缺口:check-third-party-notices.mjs 检查的是 renderer asset notice,不是这里的 npm notice。
请:
- 将
apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt加入RELEASE_CONTRACT_FILES; - 在
ci-test-plan.test.mjs中覆盖两个已提交 notice target; - 将
scripts/generate-third-party-notices.test.mjs从CLI_PACKAGE_FILES移到RELEASE_CONTRACT_FILES。
最后一项仍会执行 check:release,同时避免纯测试修改无关地构建 CLI tarball 并运行 installed-package smoke。
其余审查范围没有发现问题:删除与 #2907 重复的 commit 是正确的;缺失 delimiter 时已经 fail closed;不修改 NOTICE 也是合理的。当前 exact-head 的所有已报告 checks 均为绿色。
The header of `edit-replace.ts` labelled the whole adapted region opencode/MIT. Two of the three matchers there are Apache-2.0 material that opencode passed through, so the label put independently licensed code inside an MIT scope and dropped a required copyright notice. Checked against the pinned upstreams rather than their moving `main`: - `lineTrimmedSpans` is cline's `lineTrimmedFallbackMatch` at 50b43c0 — same line splitting, same trailing-empty-line pop, same trimmed comparison, same `length + 1` index arithmetic. cline is Apache-2.0. - the unescape regular expression in `escapeNormalizedSpans` carries the first eight branches of gemini-cli's `unescapeStringForGeminiBug` at 93909a2, in order; that file is `Copyright 2025 Google LLC` under Apache-2.0. opencode added the ninth branch and changed the pattern. The header now lists each piece with its own source, revision, license, and the modifications Maka made, which is what Apache-2.0 section 4(b) asks for. cline and gemini-cli become their own LICENSE entries; neither ships a NOTICE, and the Apache-2.0 text is already at the top of the file, so nothing is duplicated. `tool-output.ts` is unaffected and stays opencode/MIT. The snapshot digest goes away instead of being repaired. It claimed to identify the payload behind the generated files and could not: `directory` was hashed in `api.json` insertion order but emitted sorted, so reversing the input provider keys produced byte-identical bodies under a different digest; both files shared one aggregate hash, so a metadata-only change rewrote the pricing header. It was a verifier invented for something unverifiable — models.dev keeps no archive to check a hash against — while git already fixes the committed bytes. The files still name their upstream, license, and copyright. `GENERATED_MODELS_DEV_DIRECTORY` goes with it: 205 generated lines with no consumer anywhere in the repository, and the only input the digest read out of order. The committed data is otherwise untouched — it was regenerated from a newer catalog in #3429, and this edit removes lines without moving any value. Generated-by: Claude Code
`planTests` put `apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt` behind no gate at all: not `releaseContract`, not `cliPackage`. Its CLI sibling only reaches one because `packages/cli/**` is covered wholesale, not because anyone routed the notice. A hand edit or a merge-conflict resolution could corrupt the desktop attribution inventory with every PR check green, and the desktop build does not cover it — `check-third-party-notices.mjs` validates the renderer asset notice, not the npm one. Both are committed generator output and `check:release` is what regenerates and diffs them, so both now select the release contract, with a test that fails if either falls out again. `generate-third-party-notices.test.mjs` moves from the CLI package inputs to the release contract. The test only reads the generator; selecting it was making a test-only edit build a CLI tarball and smoke an installed package, roughly fifteen minutes that prove nothing about the change. Generated-by: Claude Code
|
All three accepted and pushed as 1. Confirmed against the pinned revisions you gave. 2. Removed rather than repaired. I reproduced the ordering case — reversing the input provider keys gives byte-identical bodies under a different digest. It was a verifier invented for something unverifiable, since models.dev keeps no archive to check a hash against, and git already fixes the committed bytes. 3. Done, all three parts. Worth noting the gate does reach PR CI: Not done: nothing from your review was left out. |
|
@M4n5ter re-requested review — all three points are addressed in @likun666661 @hqhq1025 the branch has moved twice since you were added: the commit duplicated by #2907 is gone, and this round split the adapted matchers by governing license and removed the snapshot digest. |
M4n5ter
left a comment
There was a problem hiding this comment.
English
Re-reviewed exact head 8383e901439975b3505958ff104eaba5c5237f8c.
All three findings from my previous review are resolved:
- The adapted matchers now preserve the distinct opencode/MIT, Cline/Apache-2.0, and Gemini CLI/Apache-2.0 scopes, including the Google copyright notice and fixed upstream revisions.
- The unverifiable aggregate snapshot digest has been removed, together with the unused
GENERATED_MODELS_DEV_DIRECTORY. - Both committed third-party notice outputs now reach the release-contract gate, while the notice regression test no longer triggers unrelated CLI package validation.
I found no new blocking issue in the updated scope. The exact-head core CI and dependency audit pass; the Windows package check is still running.
One non-blocking cleanup: the PR description still says that the generated headers contain a snapshot digest and lists digest-specific verification. Please update it to reflect the final implementation.
LGTM.
简体中文
已复审 exact head 8383e901439975b3505958ff104eaba5c5237f8c。
我上次 review 提出的三个问题均已解决:
- 改编的 matcher 现在分别保留 opencode/MIT、Cline/Apache-2.0 和 Gemini CLI/Apache-2.0 的许可证范围,并记录了 Google copyright notice 和固定的上游 revision。
- 不可复验的聚合 snapshot digest 已删除,同时移除了无消费者的
GENERATED_MODELS_DEV_DIRECTORY。 - 两个已提交的第三方 notice 输出现在都会进入 release-contract gate;notice 回归测试也不再触发无关的 CLI package 验证。
更新后的范围内没有发现新的阻塞问题。exact-head 的核心 CI 和依赖审计已经通过;Windows package check 仍在运行。
有一项非阻塞清理:PR 描述仍声称生成文件 header 包含 snapshot digest,并列出了 digest 相关验证。建议更新描述,使其与最终实现保持一致。
LGTM。
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed exact head 8383e901439975b3505958ff104eaba5c5237f8c against current main.
Verdict: not ready to merge.
The PR correctly identifies two attribution gaps that npm dependency inventory cannot discover, and the final architecture is mostly simple: source headers identify adapted material, the root LICENSE is the release authority, and both generated notice outputs are protected by the release-contract gate. The previous mixed-license, unstable-digest, and CI-routing findings are resolved.
One licensing blocker remains. The fixed Cline revision recorded by this PR has Copyright 2025 Cline Bot Inc. in its root LICENSE. The new Cline entry and the adapted source header preserve the Apache-2.0 license and modification notice, but omit that applicable copyright notice. Apache-2.0 section 4(c) requires retained source-form copyright notices that pertain to the redistributed work. The neighboring Gemini entry already handles the equivalent obligation correctly.
First principles / Occam: add the missing Cline copyright line to the Cline LICENSE entry and preferably the source attribution block. No new abstraction or generator is needed for this blocker.
Tests: exact-head CI, dependency audit, and Windows packaging are green. Locally, both notice checks, 27 notice/CI-plan tests, formatting, and diff checks passed. These mechanical checks cannot establish that every upstream copyright notice was transcribed.
Required conclusions:
- Problem definition: correct and evidence-backed.
- Optimality: nearly, but the attribution inventory is incomplete.
- Production code to delete: none beyond the removals already made.
- Tests to delete: none identified.
- Deeper refactor: not required for this fix.
- Merge readiness: no, until the Cline notice is retained.
- Residual risks: the PR description still describes the removed digest; generated models.dev attribution synchronization remains a separately acknowledged follow-up.
The cline entry recorded the revision, path, and Apache-2.0 license of the adapted line-trimmed matcher but not the copyright notice cline states in its LICENSE appendix. Apache-2.0 section 4(c) requires retaining source-form copyright notices for redistributed work, so name it in both the LICENSE entry and the edit-replace.ts attribution block, matching how the gemini-cli entry already handles the same obligation. Generated-by: Claude Code
|
@hqhq1025 the Cline copyright blocker is fixed in
|
hqhq1025
left a comment
There was a problem hiding this comment.
Re-reviewed exact head f713441d4a1566d646b1bf70a5b26c3dff2244ad.
The remaining licensing finding is resolved. The fixed Cline revision’s Copyright 2025 Cline Bot Inc. notice is now retained in both the root LICENSE entry and the edit-replace.ts attribution block, matching the neighboring Gemini treatment and preserving the applicable Apache-2.0 source-form notice.
The follow-up delta is exactly two attribution lines. It introduces no new code path or abstraction, and git diff --check passes. Exact-head CI, dependency audit, and Windows packaging are all green; the PR is mergeable against current main.
Required conclusions:
- Problem and mechanism: non-npm adapted/generated material now has durable source and release attribution, and committed notice outputs are guarded by the release gate.
- Problem definition: correct and supported by the pinned upstream revisions.
- First principles / Occam: yes; the solution uses the existing LICENSE and source-header authorities without adding another attribution system.
- Optimality: sufficient for the scoped attribution defects.
- Production code to delete: none beyond the unused generated directory already removed.
- Tests to delete or replace: none identified.
- Deeper refactor: not required for this PR.
- Merge readiness: ready to merge.
- Residual risk: the PR description still contains stale digest language; updating it would improve the historical record but does not block the code or licensing result.
Approved.
What
Two pieces of third-party material ship in every Maka release and appear in no attribution surface: the opencode code adapted into
tool-output.tsandedit-replace.ts, and the models.dev catalog snapshot in the two*.generated.tsfiles. Neither is an npm package, and the notice generator walks npm production dependency trees only, so neither was ever going to be found.Both now have LICENSE entries carrying source, revision, copyright, and the MIT permission notice, and the two adapted files record the same in their headers. They are mixed-origin, so the notice is scoped to the adapted portion — no whole-file SPDX identifier.
This also fixes a defect the entries would otherwise have made worse.
@ai-sdk/provider-utilsships no license file, so the generator substituted the whole root LICENSE as its Apache text — including Maka's own THIRD-PARTY COMPONENTS section. 140 lines about trycua/cua and the bundled fonts were filed under one npm package in both release notices. Truncating at the section header removes that, and without it the new entries would have shipped inside the provider-utils notice too.Third change: the generated headers now stamp the sha256 of everything extracted from
api.json. models.dev serves a rolling document with no version, so this is the only thing that identifies which payload produced the file.Why it looks like this
LICENSE is the authority. It is where ASF release policy puts the notice, it ships with every artifact, and a human reviews it. Nothing here restates it in code.
The upstream chain is recorded in full rather than stopping at opencode: its
edit.tscredits cline diff-apply and the gemini-clieditCorrector, whose escape set shares its first eight branches, in order, with the replacer here. The entry states the chain and leaves any conclusion to a licensing review.An earlier draft of the models.dev entry called the individual records uncopyrightable facts and attributed the selection and arrangement to upstream. Both halves were wrong — the generator copies
model.nameandmodel.descriptionverbatim, and the ordering is its own. The entry now lists the fields actually carried and draws no legal conclusion.Not in scope
Nothing guards the attribution against being dropped. The notice generator is structurally blind to non-npm material, and making it see it needs a per-artifact declaration — the CLI package and the desktop app carry different material, and no single place says which. Filed separately.
Verification
check:third-party-noticesandcheck:cli-third-party-notices— both targets, both cleannode --test scripts/generate-third-party-notices.test.mjs scripts/ci-test-plan.test.mjsnpm run format:checkapi.json: the catalog data is byte-identical, only the digest line movesGENERATED_MODELS_DEV_PROVIDER_FACTSAI disclosure
Claude Code drafted the attribution text, the LICENSE entries, and the generator changes. Reviewed adversarially by Claude and Codex over two rounds; findings from both are in the commit history. The human contributor of record owns the accuracy of the legal statements.