Repository navigation
perf(plugin): check every plugin at once and skip sources that did not move - #812
Conversation
…t move The Plugins page's update check read every Agent CLI, cloned and hashed every source, and asked npm about every package one after another, so it grew with each plugin added. A single Agent CLI that stopped answering held the whole page at "Checking for updates" with no way to tell which plugin was stuck. The check now asks every Agent and checks every plugin at the same time. Plugins share one read of each Agent and each source, so an Agent with many plugins is still asked once, and a plugin waits only on its own Agents, including when proving its own leftover installs. Before downloading a remote source with no pinned ref, the check asks the remote which commit its default branch points at with `git ls-remote` and reports no update when that is the installed commit. A check of three unchanged GitHub plugins on two Agents went from 4.3s with three clones to 0.6s with none. The dashboard reads the check from a new stream, GET /api/plugins/check/stream, that sends each plugin as soon as it is checked. The page marks rows as they arrive and counts progress in the header. The stream stops waiting after 60 seconds: a plugin bound to an Agent that has not answered by then shows "Check failed" with the reason, and plugins on other Agents are unaffected. There is no limit on how many plugins run together, because plugins waiting on a hung Agent would otherwise hold every slot and starve the rest. A cancelled command did not return when the Agent CLI was a shim that starts the real client, such as an npm, mise or volta wrapper: killing the shim left the child holding its output, so the command waited until the child exited and no timeout took effect. Commands now stop waiting for output two seconds after they are cancelled; a command that exits on its own while a helper keeps its output open still counts as answered.
Codex Review Summary: ✅ CompletedThis 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81237f1ed0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| answered.add(name); | ||
| if (plan) plans[name] = plan; | ||
| if (why) failed[name] = why; | ||
| setUpdates((u) => withChecked(u, name, plan)); |
There was a problem hiding this comment.
Keep the update dialog's plan synchronized with retained updates
If the initial check emits an available update and then the stream disconnects before done, this publishes the update while checked remains null because setChecked only runs after successful completion. The header consequently offers Update N, but PluginUpdateDialog derives selectable targets from checked.changes, so the dialog has no selectable targets and its update button stays disabled. The same mismatch occurs when a completed recheck retains a previously found update for a blocked Agent but replaces its checked plan with the blocked result. Keep the dialog's plan synchronized with incremental and retained updates, or derive its selectable targets from the retained update data.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f6fd3a4. The dialog read its selectable targets from the last completed check plan (checked), while the header counted updates, so the two could disagree. Both cases you describe were instances of that: a first check that broke off after finding an update never set the plan, and a recheck that could not reach an Agent kept its earlier update while the plan said blocked.
checked is now derived by checkedPlan(checkedPlans, updates) in pluginsFlow.ts: each plugin's last answer in name order, with every kept update standing in for a blocked or missing answer for that Agent. The dialog and the header now read the same data. A broken-off check also merges the answers it did receive.
Covered by two new tests in pluginsFlow.test.ts: "offers in the update dialog an update a broken-off check found" and "offers in the update dialog an update kept for an Agent the check could not reach". Both failed before the change. make check passes.
The header counted updates from the per-plugin marks, but the update dialog offered only the targets in the last completed check's plan. Two cases left the header saying Update N with nothing to tick in the dialog: a first check that broke off after finding an update never set the plan, and a recheck that could not reach an Agent kept that Agent's earlier update while the plan said blocked. The dialog's plan is now derived from each plugin's last answer together with the kept updates, so a kept update stands in for the blocked or missing answer and both always agree. A broken-off check also keeps the answers it did receive.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ 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". |
Type
proposals/only — see CONTRIBUTING.md) — not a proposalapprovedlabel) — no issue; requested directly by the maintainerLinked Issue
None. The maintainer reported that the Plugins page's update check takes longer with every plugin added, and was still spinning minutes later.
What changes
Faster check
git ls-remote <url> HEADbefore downloading a source. If that is the installed commit, the plugin reports no update without a clone.Per-plugin results on the dashboard
GET /api/plugins/check/stream(SSE) sends each plugin as soon as it is checked. The page marks rows as they arrive and showsn/totalin the header.Hung-CLI fix
runCommandsetsWaitDelay = 2s. Before, killing a CLI shim (npm, mise or volta wrappers) left the real client holding stdout, so a cancelled command never returned and no timeout took effect.Docs:
sharing-plugins.mdandreference/commands/plugin.md, plus their ja, ko, zh-Hans and zh-Hant copies. UI: theplugins.checkFailedkey in all 11 locales.Verification
make checkin the devcontainer: passes.runCommandreturns when cancelled with a child holding its output;runCommandkeeps the answer when a helper outlives the CLI;plugin checkagainst theorigin/mainbuild;Not covered
source_refis still downloaded on every check.ls-remoteis used only for the default branch.skillshare plugin checkhas no 60-second overall deadline. Each command keeps its 90-second timeout.Checklist
make check) — for code changes