diff --git a/inc/Engine/Bundle/AgentBundleArtifactRebase.php b/inc/Engine/Bundle/AgentBundleArtifactRebase.php index d9832c532..ad88c2edf 100644 --- a/inc/Engine/Bundle/AgentBundleArtifactRebase.php +++ b/inc/Engine/Bundle/AgentBundleArtifactRebase.php @@ -303,6 +303,16 @@ public static function policy_burn_in_safe( array $input ): array { $merged_step['handler_configs'] = $merged_hcs; } + self::mirror_preserved_handler_throttle( + $base_hc, + $local_hc, + $base_hcs, + $local_hcs, + $step_id, + $merged_step, + $decisions + ); + $merged_steps[ $step_id ] = $merged_step; } @@ -314,8 +324,8 @@ public static function policy_burn_in_safe( array $input ): array { // it ambiguous so an operator confirms. $top_keys = array_unique( array_merge( - array_keys( is_array( $local ) ? $local : array() ), - array_keys( is_array( $remote ) ? $remote : array() ) + array_keys( $local ), + array_keys( $remote ) ) ); foreach ( $top_keys as $key ) { @@ -366,6 +376,87 @@ public static function policy_burn_in_safe( array $input ): array { ); } + /** + * Keep local burn-in max_items consistent across legacy and canonical shapes. + * + * Flow payloads may carry both `handler_config` and `handler_configs` while + * installs migrate between old and new representations. The two shapes are + * equivalent runtime inputs, so a locally lowered throttle must not be + * preserved in only one of them. + * + * @param array $base_hc Single-handler base config. + * @param array $local_hc Single-handler local config. + * @param array $base_hcs Multi-handler base config. + * @param array $local_hcs Multi-handler local config. + * @param string|int $step_id Flow step ID. + * @param array $merged_step In/out merged step. + * @param array> $decisions In/out decisions map. + */ + private static function mirror_preserved_handler_throttle( + array $base_hc, + array $local_hc, + array $base_hcs, + array $local_hcs, + string|int $step_id, + array &$merged_step, + array &$decisions + ): void { + $single_throttle = self::local_preserved_max_items( $base_hc, $local_hc ); + if ( null !== $single_throttle && is_array( $merged_step['handler_configs'] ?? null ) ) { + foreach ( $merged_step['handler_configs'] as $slug => &$handler_config ) { + if ( ! is_array( $handler_config ) ) { + continue; + } + $handler_config['max_items'] = $single_throttle; + $decisions[ "flow_config.{$step_id}.handler_configs.{$slug}.max_items" ] = array( + 'source' => 'local', + 'reason' => 'burn_in_preserve_throttle_shape_mirror', + ); + } + unset( $handler_config ); + } + + if ( ! is_array( $merged_step['handler_config'] ?? null ) ) { + return; + } + + foreach ( $local_hcs as $slug => $local_config ) { + if ( ! is_array( $local_config ) ) { + continue; + } + $base_config = is_array( $base_hcs[ $slug ] ?? null ) ? $base_hcs[ $slug ] : array(); + $max_items = self::local_preserved_max_items( $base_config, $local_config ); + if ( null === $max_items ) { + continue; + } + $merged_step['handler_config']['max_items'] = $max_items; + $decisions[ "flow_config.{$step_id}.handler_config.max_items" ] = array( + 'source' => 'local', + 'reason' => 'burn_in_preserve_throttle_shape_mirror', + ); + return; + } + } + + /** + * Return locally changed max_items, or null when local did not change it. + * + * @param array $base Base handler config. + * @param array $local Local handler config. + */ + private static function local_preserved_max_items( array $base, array $local ): mixed { + if ( ! array_key_exists( 'max_items', $local ) ) { + return null; + } + + $base_has = array_key_exists( 'max_items', $base ); + if ( $base_has && $local['max_items'] === $base['max_items'] ) { + return null; + } + + return $local['max_items']; + } + /** * 3-way merge a single handler_config map. * @@ -421,7 +512,7 @@ private static function merge_handler_config( ); continue; } - if ( $local_changed && $remote_changed ) { + if ( $local_changed ) { // Both moved this throttle. Default to local (safer) but flag ambiguous. $merged[ $key ] = $local_val; $decisions[ $path ] = array( @@ -464,7 +555,7 @@ private static function merge_handler_config( continue; } - if ( $local_changed && $remote_changed ) { + if ( $local_changed ) { // Both moved a non-throttle key. Default merged value to local // (don't silently clobber) and flag for approval. $merged[ $key ] = $local_val; diff --git a/tests/agent-bundle-artifact-rebase-smoke.php b/tests/agent-bundle-artifact-rebase-smoke.php index 71d7bdec9..2b0adb6d2 100644 --- a/tests/agent-bundle-artifact-rebase-smoke.php +++ b/tests/agent-bundle-artifact-rebase-smoke.php @@ -313,9 +313,62 @@ function rebase_assert_equals( string $label, $expected, $actual ): void { rebase_assert( 'multi: no ambiguous fields', empty( $multi_result['ambiguous'] ) ); // --------------------------------------------------------------------------- -// [6] Hashes are recomputed for the merged payload. +// [6] Equivalent legacy/canonical handler config shapes keep throttle in sync. // --------------------------------------------------------------------------- -echo "\n[6] Rebase output carries reproducible hashes\n"; +echo "\n[6] handler_config max_items mirrors into handler_configs\n"; +$shape_result = AgentBundleArtifactRebase::rebase( + array( + 'artifact_type' => 'flow', + 'artifact_id' => 'shape-transition', + 'base' => array( + 'flow_config' => array( + '1_step_1' => array( + 'handler_config' => array( + 'owner' => 'old', + 'max_items' => 25, + ), + ), + ), + ), + 'local' => array( + 'flow_config' => array( + '1_step_1' => array( + 'handler_config' => array( + 'owner' => 'old', + 'max_items' => 1, + ), + ), + ), + ), + 'remote' => array( + 'flow_config' => array( + '1_step_1' => array( + 'handler_config' => array( + 'owner' => 'Automattic', + 'max_items' => 25, + ), + 'handler_configs' => array( + 'github-a8c' => array( + 'owner' => 'Automattic', + 'max_items' => 25, + ), + ), + ), + ), + ), + ), + AgentBundleArtifactRebase::POLICY_BURN_IN_SAFE +); + +$shape_step = $shape_result['merged']['flow_config']['1_step_1'] ?? array(); +rebase_assert_equals( 'shape: single handler throttle from local', 1, $shape_step['handler_config']['max_items'] ?? null ); +rebase_assert_equals( 'shape: multi handler throttle mirrors local', 1, $shape_step['handler_configs']['github-a8c']['max_items'] ?? null ); +rebase_assert_equals( 'shape: multi handler source-shape remains remote', 'Automattic', $shape_step['handler_configs']['github-a8c']['owner'] ?? null ); + +// --------------------------------------------------------------------------- +// [7] Hashes are recomputed for the merged payload. +// --------------------------------------------------------------------------- +echo "\n[7] Rebase output carries reproducible hashes\n"; $hashed_result = AgentBundleArtifactRebase::rebase( array( 'artifact_type' => 'flow', @@ -332,9 +385,9 @@ function rebase_assert_equals( string $label, $expected, $actual ): void { rebase_assert( 'base_hash is set', is_string( $hashed_result['base_hash'] ) ); // --------------------------------------------------------------------------- -// [7] Default policy is conservative when an unknown policy name is supplied. +// [8] Default policy is conservative when an unknown policy name is supplied. // --------------------------------------------------------------------------- -echo "\n[7] Unknown policy falls back to conservative\n"; +echo "\n[8] Unknown policy falls back to conservative\n"; $unknown_result = AgentBundleArtifactRebase::rebase( array( 'artifact_type' => 'flow',