Skip to content

Commit 630e4fc

Browse files
authored
Fix silent agent install rollback and misleading upgrade rejection (#1801) (#1806)
The bundler's import path used to claim an agent_id, mutate pipelines/flows/agent_config across multiple unwrapped wpdb writes, and only check sub-step results for the agent insert. Under SQLite contention on Studio that produced "success: true" responses with a populated agent_id summary while the underlying rows were silently rolled back, leaving operators with a `Agent "..." not found` on the very next CLI call. This change wraps the post-claim work in a transaction with try/catch + manual rollback, throws on any sub-step write failure, re-fetches the agent row after the final update_agent and verifies the artifact registry actually persisted, and surfaces typed errors (`install_post_claim_failure`, `install_invalid_bundle`, `install_slug_collision`, `install_bundle_slug_mismatch`) instead of bare strings. The manual_rollback() path is the safety net for engines that silently no-op ROLLBACK; it deletes only rows this call inserted, never pre-existing ones. `agent upgrade` now passes `is_upgrade => true` so an existing agent row is treated as the upgrade target instead of returning "Agent slug already exists. Use --slug=<new-slug> to rename on import." when a bundle's artifacts lack `portable_slug` and the live pipelines/flows have been edited (`local_modified`). Conflicts come back through `result.conflicts` and the CLI hands them to the existing PendingActions staging path, matching what `agent diff` already reports. Defensive cleanup: `InstalledBundleArtifacts::register()` listens on `datamachine_agent_deleted` and wipes any tracked rows for the deleted agent_id. The importer does not write to that table today, but extensions can — and a stale row would mis-classify a fresh install as an upgrade against a non-existent agent. Also adds two test seam actions, `datamachine_bundle_import_post_claim_started` and `datamachine_bundle_import_pre_commit`, so tests can throw inside the critical section without needing a live SQLite race to reproduce the bug. Tests: tests/Unit/Core/Agents/AgentBundlerImportTest.php covers all three regression shapes: silent partial success → rollback + typed error, upgrade-against-local-modified → success with conflicts, agent delete → bundle artifact registry cleared.
1 parent a8bc102 commit 630e4fc

5 files changed

Lines changed: 520 additions & 74 deletions

File tree

data-machine.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ function () {
9090
// Load and instantiate all handlers - they self-register via constructors
9191
datamachine_load_handlers();
9292
\DataMachine\Engine\Bundle\AuthRefHandlerConfig::register();
93+
\DataMachine\Core\Database\BundleArtifacts\InstalledBundleArtifacts::register();
9394

9495
// Initialize FetchHandler to register skip_item tool for all fetch-type handlers
9596
\DataMachine\Core\Steps\Fetch\Handlers\FetchHandler::init();

inc/Cli/Commands/AgentBundleCommand.php

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,20 @@ public function upgrade( array $args, array $assoc_args ): void {
224224
}
225225

226226
$owner_id = isset( $assoc_args['owner'] ) ? $this->resolve_user_id( $assoc_args['owner'] ) : 0;
227-
$result = $this->bundler()->import( $bundle, '' !== $slug ? $slug : null, $owner_id, false, array( 'reconcile_runtime' => $reconcile_runtime ) );
227+
$result = $this->bundler()->import(
228+
$bundle,
229+
'' !== $slug ? $slug : null,
230+
$owner_id,
231+
false,
232+
array(
233+
'reconcile_runtime' => $reconcile_runtime,
234+
// Mark this as an upgrade so the importer treats an existing agent row as the upgrade
235+
// target instead of returning "Agent slug already exists" when local pipelines/flows
236+
// have been edited (#1801). Local-modified artifacts come back in `result.conflicts`
237+
// and get staged as PendingActions below.
238+
'is_upgrade' => true,
239+
)
240+
);
228241
if ( empty( $result['success'] ) ) {
229242
WP_CLI::error( (string) ( $result['error'] ?? 'Bundle upgrade failed.' ) );
230243
return;

0 commit comments

Comments
 (0)