ci(deploy): only POST plugin activation when not already active#61
Merged
JohnRDOrazio merged 2 commits intomainfrom May 1, 2026
Merged
ci(deploy): only POST plugin activation when not already active#61JohnRDOrazio merged 2 commits intomainfrom
JohnRDOrazio merged 2 commits intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe deploy workflow's plugin activation step now GETs each plugin to read its current ChangesPlugin Activation Idempotency
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
Up to standards ✅🟢 Issues
|
Each POST /wp/v2/plugins/<slug> with {"status":"active"} triggers
WordPress's flush_rewrite_rules(), which rewrites .htaccess. Plesk's
nginx mirrors .htaccess into its own internal rewrite-rule list with
a small lag during regeneration; during that lag, /wp-json/* requests
fall through nginx's try_files ladder and 404 from the filesystem
layer.
The cdcf-queue-worker.service systemd unit on the same VPS fires 5
parallel POSTs every 15 seconds against /wp-json/cdcf/v1/process-queue,
so it consistently hits the lag window during deploys, producing the
~30-50% 404 wave reported in #41.
Skipping the activation POST when the plugin is already active
eliminates the redundant flush_rewrite_rules() calls for the common
case where deploys don't change activation state. Tarballs still get
extracted and plugin code updates land; only the no-op rewrite-rule
flush is removed.
Pausing the queue worker during deploys would be a complementary fix
but isn't viable from the existing deploy automation surface.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JohnRDOrazio
force-pushed
the
fix/idempotent-plugin-activation
branch
from
0ba6c61 to
deb673d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/deploy.yml:
- Around line 343-345: The curl calls that set CURRENT (curl -s -w
'%{http_code}' -X GET "$WP_URL/$SLUG" -H "Authorization: Basic $AUTH") and the
activation POST calls should include explicit timeouts to avoid hanging; update
those curl invocations (the one assigning CURRENT and the activation POST block
around lines 370-374) to add --connect-timeout and --max-time (or -m) options
with sensible values so the workflow will fail fast on network issues while
preserving error handling.
- Around line 334-336: The explanatory comment referencing
flush_rewrite_rules(), WordPress, Plesk's nginx mirror and the /wp-json/* 404s
(issue `#41`) is written as a definitive root cause; make it hypothesis-neutral by
rephrasing to indicate a possible or likely cause—for example replace "the root
cause" with "a possible cause" or "may contribute to" and remove absolute
language so the comment (the explanatory block mentioning flush_rewrite_rules(),
Plesk nginx mirror and the 404 wave) reads as a hypothesis rather than a
confirmed fact.
- Around line 357-363: The multiline inline Python inside the command
substitution breaks YAML parsing; replace it with a single-line, YAML-safe
invocation so CURRENT_STATUS is assigned reliably from BODY. Change the block to
a one-liner such as: CURRENT_STATUS=$(echo "$BODY" | python3 -c 'import
json,sys; print(json.load(sys.stdin).get("status","unknown"))' 2>/dev/null ||
echo unknown) so the Python code is on a single line and the quoting is safe for
the workflow parser; keep references to CURRENT_STATUS and BODY to locate the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ee30fcc2-bfc1-4b71-9b01-37ecd783dfad
📒 Files selected for processing (1)
.github/workflows/deploy.yml
- Add --connect-timeout/--max-time to curl GET and POST so the workflow fails fast on network hangs; switch -s to -sS so curl errors still surface on stderr. - Soften the explanatory comment from "the root cause" to "the suspected cause" since #41 is hypothesis-driven. - Collapse the inline Python status parser to a single line; the try/except was redundant with the outer `|| echo unknown` fallback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This was referenced May 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #41.
This PR is still useful as code hygiene — eliminating redundant
flush_rewrite_rules()invocations is correct regardless — but it's no longer expected to materially move the deploy-time error rate. The dominant deploy-time symptom is HTTP 504 from PHP-FPM pool exhaustion, not 404 from a routing race. That needs a different fix (worker throttling, FPM pool sizing, opcache settings).Why the change is still worth merging
do_action( 'activate_<file>' ), and may invoke anyregister_activation_hookcallbacks the plugins define. Skipping the call when status is already"active"removes that work cleanly.Skip <plugin>: already activevs.Activate <plugin> (was inactive): HTTP 200shows what state actually changed.Change
Read each plugin's current
statusfirst viaGET /wp/v2/plugins/<slug>and onlyPOSTwhen status isn't already"active". Tarballs still get extracted and plugin code updates land; only the no-op activation request is skipped.The fallback parser uses
"unknown"if JSON parsing fails, which forces the POST path — safe default, preserves current behavior on transient response oddities.Test plan
workflow_dispatchdeploy. Logs should showSkip <plugin>: already activefor both plugins (since they were both active prior to this PR).Activate <plugin> (was inactive): HTTP 200.🤖 Generated with Claude Code
Summary by CodeRabbit