Repository navigation
fix(extensions)!: coordinate removal, activation and shutdown - #361
Conversation
Package removal ignored runtime usage and retained activation intent. World removal now shares the lifecycle queue and preserves configuration on failure. Tests cover reinstall, rollback, and activation/removal races. Environment rendering failures preserve the runtime status of every World. BREAKING CHANGE: uninstall rejects in-use extensions with HTTP 409 and writes worlds.<id>.enabled=false on successful World removal. Deactivate before uninstall and explicitly activate after reinstall. Uninstall performs the configuration migration. The worlds endpoint adds envPromptError; clients should preserve the returned status when showing that error.
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. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Vague actions. The PR description says "The lifecycle queue protects the current process."
AGENTS.md §4 AI slop checklist, Vague actions: "State what an operation changes and when it takes effect. Replace phrases such as "touches the disk" or "finishes up" with the actual behavior."
The verb does not say what the queue writes, rejects, or serializes. The checklist's instruction is to replace that phrase with the actual behavior; prefer deletion.
— Cortico PR Gate via Cursor Cloud Agent
Sent by Cursor Automation: Cortico PR Gate
Config options still read startup definitions after inactive World removal. Read the current assembly and return HTTP 409 when activation wins the removal queue. API and lifecycle regressions cover both cases. BREAKING CHANGE: removed Worlds no longer contribute config options. Clients refresh options after removal; no on-disk migration is required.
Removing a package left its module registered. Withdraw it while the package operation is pending and restore registration on failure. Endpoint validation and captured panel contributions reject stale requests; tests cover both save/removal orderings and rollback.
Import failures lost the ID needed to clear activation, and process shutdown could interrupt package writes. Persist accepted ownership, exclude conflicting IDs, and drain complete transactions before stopping services. Supervisors receive shutdown phases so their deadlines start after the drain. Integration tests cover failed imports, replacement owners, rollback and concurrent HTTP requests. BREAKING CHANGE: loadExtensions automatically creates extensions/world-owners.json from accepted World exports. Existing packages that already fail import without an ownership record cannot have historical IDs inferred. Shutdown refuses new package mutations with HTTP 503 and waits for active transactions; hosts must honor cortico:shutdown phases before applying forced-exit deadlines.
kwk1001
changed the title
There was a problem hiding this comment.
Hype and reassurance. The PR description says "so later import failures retain reliable ownership."
AGENTS.md §4 AI slop checklist, Hype and reassurance: "Remove praise for the implementation, subjective modifiers such as "quietly" or "reliably", and unsupported assurances such as "rest assured"."
The checklist's instruction is to remove the subjective modifier; prefer deletion.
The earlier Vague actions finding on "The lifecycle queue protects the current process." no longer applies; that sentence is gone.
— Cortico PR Gate via Cursor Cloud Agent
Sent by Cursor Automation: Cortico PR Gate
- 生产路径总会传这两项;可选时，手搭的扩展集合与运行状态会退回另一套「归属」「在用」的算法。 - 测试改为显式传入；内建供应商的 enabled 按传入的引用判断来断言。
- 根因：加载时直接 readJsonObject，文件截断、手改或值不是字符串都会抛错，启动失败，报错还写成 config.json 解析失败。 - 现在读不出按空表处理并重写，非字符串值丢掉;写入改为整份写临时文件再 rename，按 retryBusy 退避，失败只记警告。 - 测试覆盖：坏 JSON、非字符串值、读写都失败三种情况下 loadExtensions 照常返回。
- 原来 11 种语言都写「停用后才能卸载」:供应商没有停用这一步，要删除引用它的供应实例;这份部署在用的 bot 包不能卸载。 - zh-Hant、ko 的 shuttingDown 改用文件里其余条目的用词(解除安裝、擴充功能、程序;확장)。
- removeModule 借用的 inUse 写的是「供应实例正被这些部署使用」，列出的其实是引用该供应商的实例名。新增 kindInUse,11 种语言。
- 根因：启动时的记录仍把它记作已加载的供应商，第二次卸载又去撤下已经撤下的模块，providerModule() 抛出未翻译的 Unknown provider module,回 400,包留到重启。 - 本进程卸载过的包跳过撤下，直接删包。测试覆盖卸载、重装、再卸载。
- 根因：回滚用 persist(id, { enabled }),assignDeep 跳过 undefined,原来没写 enabled 的 World 会留下 enabled: false;内存里是合并后的默认值时，又会把默认值写进 config.json。
- 现在分别记下内存与 config.json 里的原值;回滚本身写失败时仍报删包的错。
- 根因：等装卸放在 runShutdown 之前且没有时限，启动器的 35 秒上限也删了。pnpm 卡住，或卸载排在一个 start() 不返回的 World 激活后面时，进程一直不退;等待期间事件照常投递。 - 等待改为 pause 之后的 extensions 步骤，超时记为失败，后面的步骤照常执行;bot.stop() 同样限时。waiting-extensions 在关机开始时发,stopping 在这一步之后发。 - 启动器恢复整体时限：各步上限之和加 2 秒，现为 95 秒。关机阶段的消息名导出为 SHUTDOWN_MESSAGE。 - 测试覆盖：装卸挂住时先暂停投递,60 秒后 extensions 步失败，其余步骤照常完成。
- docs/extensions.md 的「卸载」按现行规则重写，删掉与 providers.md、deployment.md、webui.md 重复的段落，去掉「进程内锁」「此协调」这类没定义的说法。 - worlds.md 把 removeDefinition 挪到写回 enabled 的那段旁边;providers README 补上 removeProviderModule 与 removeModule;owner 头里的 bot.ts、world.ts 标出具体导出。
|
先合了一次 main，然后在这个 PR 上补了 8 个提交，把审查里发现的问题直接修掉了：
新增的测试在改动前都会失败。squash 时提交说明我会整理一下，写清供应商 |
There was a problem hiding this comment.
The earlier Jargon finding on 「进程内锁」 in docs/extensions.md, the Accuracy finding on a single “deactivate” recovery string, and the Vague actions finding on “package mutations have settled” in src/boot.ts no longer apply; those sentences are gone or split by kind.
— Cortico PR Gate via Cursor Cloud Agent
Sent by Cursor Automation: Cortico PR Gate
| */ | ||
| const SHUTDOWN_BUDGET_MS = { | ||
| pause: 2_000, | ||
| extensions: 60_000, |
There was a problem hiding this comment.
A 60-second shutdown wait arrives without a derivation for that value.
CONTRIBUTING.md, "Limits and guards": "The value carries its derivation from the path it protects, naming what produces the load and what drains it."
SHUTDOWN_BUDGET_MS.extensions is 60_000. runShutdown and Bot.stop wait on extensions.close() with this budget. A pnpm that never returns (the new shutdown test) or close() blocked on removeWorld behind a start() that does not return will sit on that wait. The PR description does not mention 60 seconds. Commit 0a2b368 states the hang and the number; it does not say what produces 60 seconds of load or what drains it.
Add the evidence and the derivation to the PR description, or remove the hunk.
— Cortico PR Gate via Cursor Cloud Agent
- Pal-AI-Lab/Cortico#361:/api/extensions 中供应商的 enabled 改为有模型连接在用，原来是已加载;运行中的扩展卸载回 409,提示按类型说明;关机时先暂停投递，等正在进行的装卸至多 60 秒;/api/worlds 带 envPromptError。 - tests/console-api.test.ts 锁上 envPromptError:环境提示词渲染失败的 World 仍是 active,envPrompt 为空。
- 已卸载的条目只当包状态显示，不再沿用旧的 World 运行信息：没有运行控件、实时状态灯和配置入口，标红留在「待生效」直到重启;新安装的条目显示「已安装，待重启」;停用 World 时侧栏入口收起。 - 卸载前检查运行状态，确认后重新读取清单再查一次;运行中的 World 与被模型连接占用的供应商不能删除，提示按类型说明。切回「扩展市场」时重读清单，删掉最后一个连接后删除按钮随之可用。 - World 清单读取失败时保留扩展清单里已知的运行状态;Core 报告单个 World 的 envPromptError 时，原因从状态标签旁打开。 - vendor/cortico 升到 Cortico main 3ad3a3f(Pal-AI-Lab/Cortico#361):供应商的 enabled 改为有模型连接在用，卸载与关机协调，envPromptError。tests/console-api.test.ts 锁上 envPromptError。 Co-authored-by: kwk1001 <kwk1111234@gmail.com> Co-authored-by: Fiber <leaf@airo.cc>
Previously enabled Worlds could reactivate after uninstall/reinstall, including when their package failed to import. Provider packages also remained available for creating endpoints after uninstall, and shutdown could interrupt package mutations.
This change:
extensions/world-owners.json, so later import failures retain reliable ownership. Reserved IDs and IDs accepted from another package are excluded from the old owner's cleanup.cortico:shutdownphases; the forced-exit deadline must begin afterwaiting-extensionschanges tostopping.Validation:
pnpm test --maxWorkers=4(5,192 passed, 3 skipped),pnpm run typecheck. Tests cover failed-import reinstall, ownership conflicts, failed removal rollback, stale provider editors, both save/removal orderings, and concurrent HTTP installation/shutdown.Migration: the loader creates the ownership file automatically from accepted exports. A package already broken before this record exists has no recoverable historical ID; cleanup does not infer one from the package name. Uninstall clears only the current deployment's activation intent. In-process coordination does not protect another process sharing the extension directory, nor forced OS termination.
Downstream: Coopanion has local companion changes to honor the shutdown phases and remove its independent 30-second child exit deadline. Its submodule remains unchanged until this PR reaches Cortico main.