Repository navigation
fix(plugins): remove duplicate Host SDK permission state - #1720
Conversation
Why: A trusted installed plugin uses the public SDK through its active instance token and canonical caller context. Per-plugin capability policies, inventory grants, whole-feature eligibility and native MCP method lists duplicate that boundary and can leave registered tools without credentials. Preserve current upstream Task controller, lifecycle, owner, package integrity and CAS behavior while porting the reviewed fork cleanup. Old packages retain their verified raw manifest and receive legacy hello metadata only at the existing output boundary. This is a draft port of source 7c96f90c739b5f25c3c0a32cd73b81c795630f6a onto upstream f005686; related-base resolutions need fresh review. Same-schema persistent rollback remains unresolved and blocks merge or deployment.
Why: removing retired controller grant coordinates must not rewrite immutable receipts or break completed operation IDs. Preserve historical receipt input only for digest replay, keep current bindings and new receipts grant-free, and fix the Draft CI cleanup regressions.
Why: the reviewed archive replacement API no longer carries duplicate grant fields. Regenerate its canonical MCP surface snapshot so the existing governance check describes the actual schema; no registry, gate rule, runtime code or permission mechanism changes.
Why: the upstream port retained obsolete assumptions about exposed provider tokens, Kimi config rendering and moved materializer imports. Exercise the real secret descriptor and Kimi lease, preserve artifact conditions, and leave production behavior unchanged.
Why: the new Contract no longer exports duplicate grant validation, and current inventory and Manager surfaces have no API grant rows. Keep Alpha isolation, nested-copy, desktop lifecycle and read-only legacy-origin behavior tests; remove only their retired permission plumbing and the obsolete mixed-grant refusal case.
Why: the installation-race fixture was already disabled, so assigning disabled again did not change the configuration fence. Advance its lifecycle revision to verify stale preflight rejection and remove the fixture's retired grant inputs. [砚砚/gpt-6.1-sol🐾]
…61026 chore(fork): rebuild develop_base with latest upstream and full zts212653#1720
) Why: keep the complete reviewed PR1720 and CI corrections as the second and final fork delta. The resulting tree is byte-for-byte identical to built merge 51cca01; only ancestry is simplified under the operator's explicit force-push authorization. Preserve old-plugin forward compatibility, immutable receipts, actual handler dispatch and instance/caller/lifecycle fences. [砚砚/gpt-6.1-sol🐾]
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Landy，#1720 已完成本轮 Host 重复 API/action 权限模型删除，现已 Ready for review。当前 HEAD 为 旧 alpha.26 插件无需更换即可在更新后的 Host 运行，实际只读调用已通过；本 PR 不依赖插件仓 #118 完成。此前仅涉及旧 Host 降级读取的阻断已按 operator 决定撤回，历史 receipt 的前向幂等问题已修复。 请审查此 PR；批准后请按仓库要求同步最新 main、确认最终 HEAD 的 checks 后推进合入。当前 GitHub 状态为无冲突、BEHIND、REVIEW_REQUIRED，我们未代为 merge。 [砚砚/gpt-6.1-sol🐾] |
zts212653
left a comment
zts212653
left a comment
There was a problem hiding this comment.
Independent formal review: APPROVE 40f83e1, paired with clowder-ai-plugins#118 at d3b30172c799871f7d661705902050ac19f28906. No blocking implementation finding on this Host HEAD.
Reviewed authorization consumers, legacy raw manifest/hello compatibility, native routing/currentness, controller receipt parsing/digests/replay, inventory/Broker restart and lifecycle fences, owner surfaces and dependency pins. Instance/token/contribution/caller/thread/invocation, resource ownership, lifecycle and Task CAS remain real authorities. Actual own handlers determine method availability.
Independent verification on isolated checkouts: API/shared/MCP build; 169/169 Host tests, covering native callback policy/currentness, inventory/Broker restart, old-client hello, Task controller/governance and legacy archive replacement; 4/4 tests with the isolated Redis runner, covering unchanged historical bind/revoke receipt JSON/receiptRef/payloadDigest, operationId replay/conflicts, concurrent CAS and expired carrier fences. No skipped tests in these selections. Paired plugin checks: wire/SDK 518/518, Collective business 399/399, control/Work 59/59, installed SDK lifecycle 1/1.
Combination: the vendored Contract/SDK SHA256 values match d98817b1476ef33bb00cf59e27c3e90f4097bbf4f9ad74232944cb0aa2f14ce9 / 5a0f7d0823048b3da588308fa7f7f11abe7a4408d7d7814ce04a4b41a311647c. All 196 Contract and 116 SDK dist members match the independently built plugin source; the installed SDK resolves the same physical Contract package as Host. A rebuilt paired package passed actual production archive admission, persisted/reloaded file inventory, production module carrier, failed-start/retry/bootstrap/disable/re-enable and retained-state checks. Its SHA256 is 6e19d4abbacd5de2798055d24060bf7d37eb906237f6925fab1dd47a3a73a550. This uses isolated owner/neutral ports and an in-memory Host TaskStore. It is a rebuilt artifact, not the catalog candidate or real-account/full-C1/Settings/browser acceptance.
Maintainer assessment: the paired cleanup directly addresses #117 and keeps compatibility at existing entry points. It is suitable for subsequent invariant-preserving home intake, which remains separate. The operator's forward-upgrade scope is respected; direct old-Host downgrade and a new migration/backup framework are not requirements.
This approval covers the stated HEAD. The branch is BEHIND: integrate the required latest base, disclose the real delta/continuity, and obtain final checks before merge. The plugin PR currently has a Chrome alpha.8 candidate-identity/merge conflict, for which I requested changes. The pair is not jointly ready. Existing alpha.26 can use this Host without replacing its package; new 0.3 packages require the 0.3 Host. Once both final heads are ready, deploy the Host before installing/activating the new packages. No merge, npm publication, live catalog activation, home intake, installation or runtime restart is authorized by this review.
Reviewer: codex61-sol / gpt-6.1-sol.
Why: bring the reviewed Host permission-model removal onto current main without restoring duplicate authorization. Preserve new static verification diagnostics and workspace publication isolation. Resolve the sole Chrome fixture conflict by retaining alpha.7 provenance coverage and omitting retired grant state. Validation: workspace build; API targeted 138 pass / 2 isolated-Redis skips / 0 fail; Web activation projection 6/6. Parent review applies to 40f83e1; merge continuity and final CI are reported separately.
|
Landy，已按正式批准中的要求正常合入最新 main 仅一个文本冲突：Chrome artifact fixture 保留 main 新 alpha.7 provenance 覆盖、去掉退役 effectiveGrants。13 个重叠文件保留两侧实质变化；456 个 upstream-only 文件逐 blob 与 main 相同。没有新增权限表、协商或运行数据操作。 最终树完整 workspace build PASS；22 个 API 文件 138 pass / 2 隔离 Redis skip / 0 fail；Web 6/6；MCP governance、Biome/index/brand/diff 检查通过。正文已区分旧 40f83e1 批准和最终合流的 continuity/验证，未把旧 CI 当成新 HEAD 绿。 请按这个最终 HEAD 的 CI/review 作后续判断；我们不代为 merge。旧云审 failed 且没有实际 review thread，不冒称通过。 [砚砚/gpt-6.1-sol🐾] @codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
zts212653
left a comment
zts212653
left a comment
There was a problem hiding this comment.
Independent formal re-review: APPROVE f7b6120. This supersedes my dismissed approval of 40f83e1 for the updated Host tree.
The new HEAD ordinarily merges upstream main a8a8ae9. I independently compared the two bases and both source heads: the PR still changes the same 276 paths, 263 non-overlapping paths are byte-identical to the previously reviewed head, and 13 overlap upstream work. Git remerge-diff shows one manually resolved fixture conflict: main's Chrome alpha.7 provenance case is retained while the retired effectiveGrants input is removed. I inspected the overlapping static verification diagnostics, binary/config-root publication isolation, startup wiring, tests and MCP baseline. The authored baseline delta still removes only the two replacement-grant fields; upstream changes remain present. No new blocking finding.
Fresh verification on an isolated checkout of this exact HEAD: frozen install and API/shared/MCP build; 212 passing API checks with one artifact-dependent skip, followed by that exact skipped Chrome case passing 1/1 using the npmjs alpha.7 archive after checking SHA256 30686b56939340b8877c52ba363c78a1c98b52885f0265f88bc90f4fa529f808 and the Host's complete SRI pin; real isolated Redis history/replay/CAS 4/4; Web activation projection 6/6. These exercise native identity/currentness, inventory/Broker restart, controller history and conflicts, static package/lease revocation, publication isolation and the changed fixture.
The preserved paired Collective artifact (SHA256 6e19d4abbacd5de2798055d24060bf7d37eb906237f6925fab1dd47a3a73a550) also passed this new Host's production archive admission, persisted/reloaded inventory, module carrier, failed-start/retry/bootstrap/disable/re-enable and retained-state journey. Contract/SDK pins and the associated Host logic are unchanged. That artifact is our previously rebuilt source-consumption probe, not the catalog candidate; owner/neutral ports are isolated fixtures and TaskStore is the Host's memory implementation. No Settings/browser/real-account/full-C1 or production deployment claim is made.
Remote CI on f7b6120 was still running when this review was prepared; my earlier-HEAD CI is not counted as final-HEAD success. Complete the current required checks before any merge decision. The paired plugin PR #118 is still at d3b30172 and remains CHANGES_REQUESTED for the Chrome alpha.8 candidate identity conflict; its advertised local alpha.9 fix has not been pushed or reviewed yet. The pair is not jointly ready. No merge, publication, live activation, home intake, installation or runtime operation is authorized by this review.
Reviewer: codex61-sol / gpt-6.1-sol.
|
Landy，两项最终 source/review/CI 已齐：
main active Ruleset13860210包含restrict updates和required linear history；当前作者凭据WRITE且current_user_can_bypass=never。Host完整reviewThreads查询为空。请有权限的维护者在最终SHA不变且checks仍满足时，按线性历史规则使用squash等允许方式合入。我们不改规则、不使用管理绕过、不代为merge。 Host cloud重试unknown error无具体finding，不冒充云审通过；最终maintainer re-review与实物验证已分别记录。旧alpha.26无需等新包即可继续运行，新0.3包须在0.3 Host之后安装。合入代码与npm发布/安装/实际加载分开，本次没有发布、安装或运行操作。 [砚砚/gpt-6.1-sol🐾] |
Problem and resulting behavior
Installed plugins could remain visible as MCP tools while duplicate Host API grant tables and static native method allowlists prevented calls before reaching the active plugin. This change removes that repeated authorization model. The active-instance token, canonical caller/thread/invocation, package integrity, lifecycle, resource ownership and Task CAS remain. The current instance's own handlers determine whether a method exists.
Scope
Current source and main integration
HEAD:
f7b6120866f7ff435220659ab987009d61d22499. Normal merge of upstream maina8a8ae9e465f66815a41e8d9e0bcd39673d98cd4into reviewed source40f83e1193c86445832e320071dd0e79fb25fe73; no force push.Of 469 upstream-changed paths, 13 overlap this PR. All 456 upstream-only paths remain byte-identical to main. The only text conflict was
personal-chrome-setup-artifact.test.js: retain main's alpha.6/alpha.7 provenance coverage and omit retiredeffectiveGrantsfixture data. Automatic production merges preserve main's static verification cost/metadata diagnostics, private package seals, workspace publication isolation and shared-MCP launch changes while retaining this PR's permission cleanup. No unrelated upstream feature is removed.Landy's formal approval covers 40f83e1, with explicit instructions to integrate main and confirm final checks. It is not represented as a new exact-HEAD approval. Earlier independent receipt review covers
06b0f47536d23a0228012d2b8a89361ff90f2360; subsequent pre-merge changes were test/snapshot-only. This merge's continuity and validation are reported separately for final maintainer assessment.Vendored SHA256 values are unchanged:
d98817b1476ef33bb00cf59e27c3e90f4097bbf4f9ad74232944cb0aa2f14ce95a0f7d0823048b3da588308fa7f7f11abe7a4408d7d7814ce04a4b41a311647cValidation
On the final merged tree:
cat_cafe_scan_paw_feel_inboxaddition; this PR adds no tool. Biome precommit checked 433 files; brand/index and diff checks passed.Earlier evidence remains separately attributed: exact 40f83e1 CI/public tests, Windows, macOS passed; these do not stand in for final merge-HEAD CI. Immutable receipt repair was independently checked with isolated Redis and real Task codec/store; historical JSON and operationId results remain unchanged. Landy's formal review also records its independent Host 169/169 and isolated Redis 4/4, without skips.
The earlier cloud run for 40f83e1 failed to complete. Its summary flags findings but the complete reviewThreads connection is empty; it is not claimed as a successful review. Final-HEAD CI and review activity are tracked normally.
Operator-controlled fork runtime acceptance and compatibility
The operator restarted the fork after reviewed intake. Read-only verification at fork
a67d587377ce5fca53d5a47292e66e443bfc85c0confirmed Contract 0.3.0, healthy durable callback storage, and the original installed GitHub alpha.26 enabled/running with unchanged package bytes. Startup recovery removed retired inventory grant state while retaining the original package declaration for the old SDK. Actual declared read-onlygithub_list_trackingexecution returned retained records; no business registration was repeated. This was the direct-tool lane, distinct from earlier isolated real stdio/native recovery evidence.That runtime evidence predates this main merge; it is not claimed as runtime loading of
f7b6120866f7ff435220659ab987009d61d22499. This iteration does not install, activate, restart, or modify runtime configuration/inventory.The operator explicitly selected forward upgrades for this client application: direct old-Host downgrade is not a requirement. The old-reader limitation remains factual, but it is not a delivery blocker and does not justify adding grant-state writers or a migration/backup framework. Forward compatibility, immutable history, owner/instance/lifecycle and CAS correctness remain required and tested.
Ready for final upstream review/checks. No merge or deployment has been performed by this PR.