[tommusrhodus]

Avoid reading the local $http_response_header on PHP 8.4+ by using http_get_last_response_headers() when available, falling back to the legacy variable otherwise; make header parsing and http_code extraction null-safe. In Pressable_Site_Shell_Open, add creation/polling for asynchronously provisioned SFTP users, introduce find_site_sftp_user(), and implement retry/backoff logic when connecting to the SSH gateway for newly created users. Improve related messages and verbosity and return clearer failures when provisioning or SSH access does not become available in time.

Fixes #122
Fixes #104

Summary by CodeRabbit

• Bug Fixes

  • Improved SFTP connection reliability with smarter retry/back-off and polling when access is newly created.
  • More robust site connectivity checks by correctly obtaining and parsing HTTP response headers and status codes.

• Chores

  • Updated behavior for compatibility with PHP 8.4+ when retrieving HTTP response headers.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 20f51eb3-0313-4045-b043-5c602ca2cae5

📥 Commits

Reviewing files that changed from the base of the PR and between acea358 and 0edd4d5.

📒 Files selected for processing (2)

• commands/Jetpack_Site_Connection_Triage.php
• includes/functions.php

---

Walkthrough

This PR replaces scoped $http_response_header usage with a version-aware call to http_get_last_response_headers() (falling back when unavailable) in get_remote_content() and Jetpack_Site_Connection_Triage to parse http_code safely. It also adds Pressable SFTP improvements: a find_site_sftp_user() helper, polling for asynchronous SFTP user provisioning when creating a collaborator, and a retry loop for SSH/SFTP connections that retries on gateway exit code 255 with adaptive back-off.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the two main changes: handling PHP 8.4 header deprecation and implementing Pressable SFTP retry logic.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 PHPStan (2.2.1)

/commands/CLI_Commands_Export.php
/commands/DeployHQ_Project_Create.php
/commands/DeployHQ_Project_Private_Key_Rotate.php
/commands/DeployHQ_Project_Repository_Connect.php
/commands/DeployHQ_Project_Server_Create.php
/commands/GitHub_Checklist_Add.php
/commands/GitHub_Pattern_To_Repo_Export.php
/commands/GitHub_Repository_Create.php
/commands/GitHub_Repository_Secret_Update.php
/commands/GitHub_Repository_Self_Add.php
/commands/Jetpack_Module_Search.php
/commands/Jetpack_Plugin_Search.php
/commands/Jetpack_Site_Connection_Triage.php
/commands/Jetpack_Site_Module_Status_Set.php
/commands/Jetpack_Site_Modules_List.php
/commands/Jetpack_Site_Plugins_Export.php
/commands/Jetpack_Site_Plugins_List.php
/commands/Poseidon_PR_Review.php
/commands/Pressable_Add_Bilmur_Tracking.php
/commands/Pressable_Site_Clone.php
/commands/Pressable_Site_Collaborator_Create.php
/commands/Pressable_Site_Collaborator_Delete.php
/commands/Pressable_Site_Create.php
/commands/Pressable_Site_Domain_Add.php
/command

... [truncated 108638 characters] ...

vendor/wp-coding-standards/wpcs/WordPress/Sniffs/WP/EnqueuedResourcesSniff.php
/vendor/wp-coding-standards/wpcs/WordPress/Sniffs/WP/GetMetaSingleSniff.php
/vendor/wp-coding-standards/wpcs/WordPress/Sniffs/WP/GlobalVariablesOverrideSniff.php
/vendor/wp-coding-standards/wpcs/WordPress/Sniffs/WP/I18nSniff.php
/vendor/wp-coding-standards/wpcs/WordPress/Sniffs/WP/PostsPerPageSniff.php
/vendor/wp-coding-standards/wpcs/WordPress/Sniffs/WhiteSpace/CastStructureSpacingSniff.php
/vendor/wp-coding-standards/wpcs/WordPress/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php
/vendor/wp-coding-standards/wpcs/WordPress/Sniffs/WhiteSpace/ObjectOperatorSpacingSniff.php
/vendor/wp-coding-standards/wpcs/WordPress/Sniffs/WhiteSpace/OperatorSpacingSniff.php
/vendor/php-stubs/wordpress-stubs/wordpress-stubs.php

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[tommusrhodus]

No prior Poseidon reviews exist on this PR (reviews API returned []; only a coderabbit issue comment is present), so this is a first review with no Previously-flagged panel. Verification done against the head commit acea358. Here is the review.

---

🔱 Poseidon Review

Summary — Hardens $http_response_header access for PHP 8.4 and adds provisioning/SSH-gateway retry logic to pressable:open-site-shell; low risk, the core changes are correct and the retry bounds are sound. One pre-existing comparison bug is surfaced in the area being edited.

Walkthrough

• 🐛 Fix — Read response headers via http_get_last_response_headers() on PHP 8.4+ and make http_code extraction null-safe across get_remote_content, the Jetpack triage command.
• 🐛 Fix — Pressable_Site_Shell_Open now polls for the asynchronously-provisioned SFTP user after creating a collaborator, and retries the SSH gateway (exit 255) with back-off for newly created users only.
• ♻️ Refactor — Extracts find_site_sftp_user(), matching the email locally against the list endpoint to avoid the noisy per-miss API error from the single-user lookup.

---

🟡 Suggestions

• commands/Jetpack_Site_Connection_Triage.php:210-212 — $wpcom_status resolves to an integer (parse_http_headers() casts http_code via (int) at includes/functions.php:96), but line 212 compares it with the string '400' using ===, so '400' === 400 is always false and the 'Site is not connected to WordPress.com…' note can never fire. This is pre-existing, but the PR rewrites the exact line that produces $wpcom_status, so it is worth fixing in the same pass: change the comparison to 400 === $wpcom_status (matching the integer style already used for $status_code at lines 189/193/199/201). (Inline target: line 210.)

---

💡 Nitpicks (1)

• includes/functions.php:67-69 — The PHP < 8.4 fallback branch is : $http_response_header with no ?? array(), whereas the identical pattern in Jetpack_Site_Connection_Triage.php:185 and :209 uses ( $http_response_header ?? array() ). On the success path $http_response_header is always defined (the loop only breaks when $result !== false), so this cannot currently throw — but parse_http_headers() is typed array $http_response_header, so wrapping the fallback in ?? array() keeps the three call sites consistent and defensive. (Summary-only; not posted inline.)

---

🤖 Prompt for AI agents — fix all findings

Verify each finding against the current code and only fix it if
needed.

Suggestions:
In `commands/Jetpack_Site_Connection_Triage.php`:
- Line 212: The branch `if ( '400' === $wpcom_status )` compares a
  string literal against `$wpcom_status`, which is assigned on line
  210 from `parse_http_headers( $wpcom_headers )['http_code']`. That
  value is cast to an integer inside parse_http_headers()
  (`$headers['http_code'] = (int) $out[1];` in includes/functions.php),
  so the strict comparison `'400' === 400` is always false and the
  "Site is not connected to WordPress.com" note is dead code. Replace
  the string `'400'` with the integer `400` so the comparison reads
  `400 === $wpcom_status`, matching the integer comparisons used for
  `$status_code` elsewhere in this loop (lines 189, 193, 199, 201).
  Preserve the surrounding `if/else` that falls through to the
  json_decode( $wpcom_result ) handling.

Nitpicks:
In `includes/functions.php`:
- Line 69: The ternary fallback `: $http_response_header` passes the
  raw magic variable straight into `parse_http_headers()`, whose
  signature is `array $http_response_header`. The two equivalent
  fallbacks in Jetpack_Site_Connection_Triage.php (lines 185 and 209)
  wrap it as `( $http_response_header ?? array() )`. Update the
  assignment to `$response_headers` so the PHP < 8.4 branch also reads
  `( $http_response_header ?? array() )` for consistency and to stay
  null-safe if the variable is ever unset; keep the
  function_exists( 'http_get_last_response_headers' ) check and the
  `?? array()` on the PHP 8.4+ branch unchanged.

---

ℹ️ Review info

• Commit: acea358
• Files reviewed: commands/Jetpack_Site_Connection_Triage.php, commands/Pressable_Site_Shell_Open.php, includes/functions.php
• Files skipped: none
• Model: claude-opus-4-8

Notes on what was checked and cleared (no findings): the do…while retry in Pressable_Site_Shell_Open.php:134-150 correctly gives existing users a single attempt ($retry_delays is array(), so $delay is null on the first miss) and bounds created-user retries to ~120s; the 255 !== $result_code → SUCCESS early return is a reasonable heuristic; find_site_sftp_user() is null-safe on both the list (?? array()) and email (?? ''); and the PHP 8.4 http_get_last_response_headers() swap returns the same header-line format parse_http_headers() expects.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

includes/functions.php (1)

67-69: 💤 Low value

Align legacy $http_response_header fallback with the null-safe pattern

parse_http_headers() accepts an array, but the legacy branch passes $http_response_header directly. After a successful file_get_contents() it should usually be an array, yet for non-HTTP/HTTPS URLs (or wrapper edge cases) it can be unset/null, which risks a TypeError. Aligning with the existing null-safe intent keeps behavior consistent:

♻️ Optional: align fallback with the null-safe pattern

 	$response_headers = function_exists( 'http_get_last_response_headers' )
 		? ( http_get_last_response_headers() ?? array() )
-		: $http_response_header;
+		: ( $http_response_header ?? array() );

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@includes/functions.php` around lines 67 - 69, $response_headers may be
null/undefined when falling back to the legacy $http_response_header and
parse_http_headers() expects an array; update the fallback in the
$response_headers assignment to mirror the null-safe pattern used for
http_get_last_response_headers() so it supplies an empty array when
$http_response_header is null (i.e., use $http_response_header ?? array()),
ensuring parse_http_headers() always receives an array.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@commands/Jetpack_Site_Connection_Triage.php`:
- Around line 207-212: The check against the parsed HTTP status is using a
string literal ('400') but parse_http_headers() sets ['http_code'] as an int, so
change the strict comparison to use an integer: replace the condition "if (
'400' === $wpcom_status )" with "if ( 400 === $wpcom_status )" (or cast
$wpcom_status to int first) so the $wpcom_status check correctly detects a 400
response coming from parse_http_headers().

---

Nitpick comments:
In `@includes/functions.php`:
- Around line 67-69: $response_headers may be null/undefined when falling back
to the legacy $http_response_header and parse_http_headers() expects an array;
update the fallback in the $response_headers assignment to mirror the null-safe
pattern used for http_get_last_response_headers() so it supplies an empty array
when $http_response_header is null (i.e., use $http_response_header ?? array()),
ensuring parse_http_headers() always receives an array.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7c696b14-96fe-45bf-94c6-334a0dae74a2

📥 Commits

Reviewing files that changed from the base of the PR and between a5b6002 and acea358.

📒 Files selected for processing (3)

• commands/Jetpack_Site_Connection_Triage.php
• commands/Pressable_Site_Shell_Open.php
• includes/functions.php