Repository navigation
fix(update): tolerate already-disabled claude plugin and reconcile stale refresh intent - #2561
Conversation
…ale refresh intent The claude disabled-state restore shelled 'plugin disable' with allowAlready=false, so an already-disabled plugin (the normal case after 'plugin update') turned the whole integration refresh into FAILED, and the failure kept the refresh-intent file alive so every later update failed the same way. A leftover planned-phase intent also outranked live plugin state: a user who manually re-enabled the plugin had it silently re-disabled by the next update. Planned phase means no destructive command is in flight, so live state is authoritative; later phases keep trusting the file for crash recovery. Reproduced live on v5.260712.1 (stable).
A scheduled cloud session re-added the README metrics block and .genie/agents/metrics-updater state after their retirement, breaking the release-docs guard test and holding dev CI red.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request removes metrics tracking files and the metrics display in the README. It also updates the plugin convergence logic for Claude and Codex to reconcile planned intents with the live state, ensuring that already-disabled plugins do not cause failures. Unit tests have been added to cover these scenarios. The review feedback points out redundant assignments to desiredEnabled in both performCodexPluginConvergence and convergeClaudePlugin that are immediately overwritten later in the functions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| progress.intent = reconcilePlannedIntentWithLiveState(progress.intent, before); | ||
| progress.desiredEnabled = progress.intent?.enabled ?? null; |
There was a problem hiding this comment.
The assignment progress.desiredEnabled = progress.intent?.enabled ?? null; is redundant because progress.desiredEnabled is unconditionally overwritten on line 2307 with progress.desiredEnabled = progress.intent.enabled;. Removing this redundant assignment simplifies the code and avoids dead stores.
| progress.intent = reconcilePlannedIntentWithLiveState(progress.intent, before); | |
| progress.desiredEnabled = progress.intent?.enabled ?? null; | |
| progress.intent = reconcilePlannedIntentWithLiveState(progress.intent, before); |
| intent = reconcilePlannedIntentWithLiveState(intent, before); | ||
| desiredEnabled = intent?.enabled ?? null; |
There was a problem hiding this comment.
The assignment desiredEnabled = intent?.enabled ?? null; is redundant because desiredEnabled is unconditionally overwritten on line 2488 with desiredEnabled = intent.enabled;. Removing this redundant assignment simplifies the code and avoids dead stores.
| intent = reconcilePlannedIntentWithLiveState(intent, before); | |
| desiredEnabled = intent?.enabled ?? null; | |
| intent = reconcilePlannedIntentWithLiveState(intent, before); |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6495bde443
ℹ️ 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".
| ): RefreshIntent | null { | ||
| if (intent === null || intent.phase !== 'planned' || !before.installed) return intent; | ||
| const liveEnabled = before.enabled === true; | ||
| return intent.enabled === liveEnabled ? intent : { ...intent, enabled: liveEnabled }; |
There was a problem hiding this comment.
Preserve pending disabled restore across retries
When a run fails during disabled-state restoration after the refresh has left the plugin enabled, the intent file is left as phase: "planned", enabled: false so a later run can retry the restore. This reconciliation now treats that pending retry as stale and rewrites it from the live enabled state, so the next update skips plugin disable/setCodexPluginEnabled, clears the intent, and silently loses the user's disabled consent. This affects both Claude and Codex whenever the first restore attempt fails transiently while the plugin is live-enabled.
Useful? React with 👍 / 👎.
Problem
genie updatereportsintegration refresh: claude — FAILED: ... Plugin "genie@automagik" is already disabledon every run when the Claude plugin is disabled at update time. Reproduced live on v5.260712.1 (stable).Three coupled defects in
src/lib/runtime-integrations.ts:claude plugin disablewithallowAlready=false.plugin updatenever re-enables a disabled plugin, so the target is already disabled, the CLI exits non-zero, and the refresh reports FAILED. (Codex is unaffected — its restore writes config atomically.)~/.genie/.integration-refresh-claude.json(phaseplanned,enabled:false), so every subsequent update re-reads it and fails identically.desiredEnabledtrusts the leftover file overplugin list; a user who manually re-enables the plugin gets silently re-disabled by the next update, which then reports success.Fix
allowAlready=trueon theplugin disablerestore call — "already disabled" is the desired end state; the existingplugin list+requireExpectedStateverification still confirms it.reconcilePlannedIntentWithLiveState: a leftover planned-phase intent (previous run settled with the plugin installed) adopts the live enabled state. Later phases (command-started,removal-observed,ambiguous-absent) keep trusting the file — crash recovery semantics unchanged (locked by existing tests).convergeClaudePluginandperformCodexPluginConvergence.Tests
Three new regression tests (all fail on unpatched source, verified):
ok:true,preservedDisabled:true, intent cleared) — previously an untested gap; every existing test returned exit 0 forplugin disable.plugin disableissued, plugin stays enabled, intent cleared.Also in this PR
Second commit re-retires the resurrected metrics bot output (README
METRICSblock +.genie/agents/metrics-updaterstate, pushed by a scheduled cloud session at 12:12 UTC today after their retirement). That commit broke therelease-docsguard test and has held dev CI red since — blocking the release pipeline this fix needs to ship through. The still-live scheduled session needs separate attention or it will re-push.Validation
bun run checkexit 0 (typecheck, lint, dead-code, 1399 tests pass / 1 skip).