MokoConsulting/MokoSuiteBackup
1
0
Fork 0
Code Issues 28 Pull Requests Releases 2 Wiki Activity

bug: standalone restore script not uploaded to remote (silent — result ignored) #226

New Issue
Closed
opened 2026-07-06 00:35:17 +00:00 by jmiller · 3 comments
jmiller commented 2026-07-06 00:35:17 +00:00
Owner
Copy Link
Copy Source

Symptom

On suite.dev, the last backup uploaded to the remote (SFTP/"FTP") but the standalone restore script was not present on the remote. The archive uploaded fine; the restore script did not.

Code finding (BackupEngine.php)

In the remote-upload loop, after the archive uploads successfully, the standalone restore script is uploaded — but the result is discarded and nothing is logged:

$result = $uploader->upload($archivePath, $archiveName);
if ($result['success']) {
    $remoteFilename = $result['remote_path'] ?? $archiveName;
    $this->log('  Upload complete: ' . $result['message']);

    if (!empty($restoreScriptPath) && is_file($restoreScriptPath)) {
        $uploader->upload($restoreScriptPath, basename($restoreScriptPath)); // <-- return ignored, no log, no failure tracking
    }
}

(src/Engine/BackupEngine.php ~306–308)

So if this second upload() fails — e.g. the uploader closed its connection after the first file, or a per-file error — the restore script is silently missing: no warning, no $uploadFailed, the backup is still recorded complete.

Two related points:

  1. Only standalone mode uploads a separate script. include_mokorestore = '1' (wrap) embeds the restore script inside the archive (the uploaded archive IS the -mokorestore zip), so there is no separate file. include_mokorestore = 'standalone' writes $restoreScriptPath and uploads it separately. If the suite.dev profile is in wrap mode, the script should be inside the archive — worth confirming which mode the profile uses.
  2. The second upload() reuses the same $uploader instance. Some uploaders open/close a connection per upload() call; a second call may silently no-op or fail depending on implementation (SFTP/S3/GDrive each differ).

Proposed fix

  • Capture and check the restore-script upload result; $this->log() it and set $uploadFailed = true (→ warning status) if it fails, so it's never silent.
  • Verify each uploader supports a second upload() on the same instance (re-open connection if needed).
  • Consider logging which MokoRestore mode is active and, in standalone mode, that the script upload was attempted.

Still to confirm (needs suite.dev)

  • The backup log for that run: is there any "Uploading…" line for the restore script, or did it stop after the archive?
  • The profile's include_mokorestore mode (wrap vs standalone).
  • Whether the archive on the remote already contains the restore script (wrap mode).

Couldn't pull the suite.dev logs/files directly from here (production shell access is gated); happy to walk through them or take a look if access is approved.

## Symptom On suite.dev, the last backup uploaded to the remote (SFTP/"FTP") but the **standalone restore script was not present** on the remote. The archive uploaded fine; the restore script did not. ## Code finding (`BackupEngine.php`) In the remote-upload loop, after the archive uploads successfully, the standalone restore script is uploaded — but the result is **discarded** and nothing is logged: ```php $result = $uploader->upload($archivePath, $archiveName); if ($result['success']) { $remoteFilename = $result['remote_path'] ?? $archiveName; $this->log(' Upload complete: ' . $result['message']); if (!empty($restoreScriptPath) && is_file($restoreScriptPath)) { $uploader->upload($restoreScriptPath, basename($restoreScriptPath)); // <-- return ignored, no log, no failure tracking } } ``` (`src/Engine/BackupEngine.php` ~306–308) So if this second `upload()` fails — e.g. the uploader closed its connection after the first file, or a per-file error — the restore script is **silently missing**: no warning, no `$uploadFailed`, the backup is still recorded `complete`. Two related points: 1. **Only standalone mode uploads a separate script.** `include_mokorestore = '1'` (wrap) embeds the restore script *inside* the archive (the uploaded archive IS the `-mokorestore` zip), so there is no separate file. `include_mokorestore = 'standalone'` writes `$restoreScriptPath` and uploads it separately. If the suite.dev profile is in **wrap** mode, the script should be inside the archive — worth confirming which mode the profile uses. 2. **The second `upload()` reuses the same `$uploader` instance.** Some uploaders open/close a connection per `upload()` call; a second call may silently no-op or fail depending on implementation (SFTP/S3/GDrive each differ). ## Proposed fix - Capture and **check** the restore-script upload result; `$this->log()` it and set `$uploadFailed = true` (→ `warning` status) if it fails, so it's never silent. - Verify each uploader supports a second `upload()` on the same instance (re-open connection if needed). - Consider logging which MokoRestore mode is active and, in standalone mode, that the script upload was attempted. ## Still to confirm (needs suite.dev) - The backup **log** for that run: is there any "Uploading…" line for the restore script, or did it stop after the archive? - The profile's `include_mokorestore` mode (wrap vs standalone). - Whether the archive on the remote already contains the restore script (wrap mode). _Couldn't pull the suite.dev logs/files directly from here (production shell access is gated); happy to walk through them or take a look if access is approved._
jmiller commented 2026-07-06 00:35:22 +00:00
Author
Owner
Copy Link
Copy Source

Branch created: feature/226-bug-standalone-restore-script-not-upload

git fetch origin
git checkout feature/226-bug-standalone-restore-script-not-upload
Branch created: [`feature/226-bug-standalone-restore-script-not-upload`](https://git.mokoconsulting.tech/MokoConsulting/MokoSuiteBackup/src/branch/feature/226-bug-standalone-restore-script-not-upload) ```bash git fetch origin git checkout feature/226-bug-standalone-restore-script-not-upload ```
jmiller commented 2026-07-06 01:17:12 +00:00
Author
Owner
Copy Link
Copy Source

Root cause found and fixed in #228 (merged to dev).

The restore script was never uploaded because it was never generated — MokoRestore::generateStandaloneScript() had a str_replace() call at MokoRestore.php:181 (the "Backup Archive" pre-check rewrite) that was missing its third $php subject argument. On PHP 8 that is a fatal ArgumentCountError at runtime, so every standalone-mode backup died at the "Generating standalone restore.php…" step:

str_replace() expects at least 3 arguments, 2 given
  .../Engine/MokoRestore.php:181

The archive itself still finalized and uploaded, which is why the backup looked successful while restore.php silently went missing on the remote.

Two-part fix now on dev:

  1. #228 — added the missing subject arg so standalone generation actually produces restore.php.
  2. #227 — the restore-script upload result is now checked/logged, and a failed script upload marks the backup warning instead of passing silently.

Will confirm on suite.dev after the next dev build, then close.

Root cause found and fixed in #228 (merged to `dev`). The restore script was never uploaded because it was **never generated** — `MokoRestore::generateStandaloneScript()` had a `str_replace()` call at `MokoRestore.php:181` (the "Backup Archive" pre-check rewrite) that was **missing its third `$php` subject argument**. On PHP 8 that is a fatal `ArgumentCountError` at runtime, so every standalone-mode backup died at the *"Generating standalone restore.php…"* step: ``` str_replace() expects at least 3 arguments, 2 given .../Engine/MokoRestore.php:181 ``` The archive itself still finalized and uploaded, which is why the backup looked successful while `restore.php` silently went missing on the remote. Two-part fix now on `dev`: 1. **#228** — added the missing subject arg so standalone generation actually produces `restore.php`. 2. **#227** — the restore-script upload result is now checked/logged, and a failed script upload marks the backup `warning` instead of passing silently. Will confirm on suite.dev after the next `dev` build, then close.
jmiller commented 2026-07-06 02:45:51 +00:00
Author
Owner
Copy Link
Copy Source

✅ Verified fixed on suite.dev (deployed 02.58.09-dev, profile 1 = standalone MokoRestore + SFTP remote).

  • Before: the SFTP remote held the archive but no restore.php — confirming the generateStandalone() FATAL meant the script was never produced.
  • After a fresh backup on the fixed build: [OK] Backup complete … (123.74 MB) with no str_replace error, and the remote now lists restore.php alongside the new archive.

Root cause (#228) + non-silent upload (#227) both confirmed working end to end. Closing.

✅ **Verified fixed on suite.dev** (deployed `02.58.09-dev`, profile 1 = standalone MokoRestore + SFTP remote). - **Before:** the SFTP remote held the archive but **no `restore.php`** — confirming the `generateStandalone()` FATAL meant the script was never produced. - **After a fresh backup on the fixed build:** `[OK] Backup complete … (123.74 MB)` with **no `str_replace` error**, and the remote now lists **`restore.php`** alongside the new archive. Root cause (#228) + non-silent upload (#227) both confirmed working end to end. Closing.
Sign in to join this conversation.
Labels
Clear labels
component: admin
Admin UI and views
 component: api
REST API
 component: engine
Backup/restore engine
 component: remote
Remote storage (FTP/GDrive)
 component: scheduler
Scheduled tasks
bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 wontfix
This won't be fixed
No labels
Priority Medium
Type Feature
Milestone
No items
No Milestone
Assignees
Clear assignees
jmiller (Jonathan Miller)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: MokoConsulting/MokoSuiteBackup#226
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?