merge: agent session work
This commit is contained in:
commit
7af716747e
@ -24,7 +24,7 @@ ## Completed Workstreams (no new action needed)
|
|||||||
- **US1 Inventory (Phase 3)**: Filament policy listing with type/category/platform filters; tenant-scoped.
|
- **US1 Inventory (Phase 3)**: Filament policy listing with type/category/platform filters; tenant-scoped.
|
||||||
- **US2 Backups (Phase 4)**: Backup sets/items in JSONB, immutable snapshots, audit logging, relation manager UX for attaching policies, soft-delete rules with restore-run guard.
|
- **US2 Backups (Phase 4)**: Backup sets/items in JSONB, immutable snapshots, audit logging, relation manager UX for attaching policies, soft-delete rules with restore-run guard.
|
||||||
- **US3 Versions/Diffs (Phase 5)**: Version capture, timelines, human+JSON diffs, soft-deletes with audit.
|
- **US3 Versions/Diffs (Phase 5)**: Version capture, timelines, human+JSON diffs, soft-deletes with audit.
|
||||||
- **US4 Restore (Phase 6)**: Preview, selective execution, conflict warnings, per-type restore level (enabled vs preview-only), PowerShell decode/encode respected, audit of outcomes; settings catalog fallback creates a new policy when the settings endpoint is unsupported, recording the new policy id and a manual cleanup warning.
|
- **US4 Restore (Phase 6)**: Preview, selective execution, conflict warnings, per-type restore level (enabled vs preview-only), PowerShell decode/encode respected, audit of outcomes; settings catalog fallback creates a new policy when the settings endpoint is unsupported, retrying metadata-only creation if settings are not accepted, recording the new policy id and manual warnings.
|
||||||
- **US6 Tenant Setup & Highlander (Phases 8 & 12)**: Tenant CRUD/verify, INTUNE_TENANT_ID override, `is_current` unique enforcement, “Make current” action, block deactivated tenants.
|
- **US6 Tenant Setup & Highlander (Phases 8 & 12)**: Tenant CRUD/verify, INTUNE_TENANT_ID override, `is_current` unique enforcement, “Make current” action, block deactivated tenants.
|
||||||
- **US6 Permissions/Health (Phase 9)**: Required permissions list, compare/check service, Verify action updates status and audit, permissions panel in Tenant detail.
|
- **US6 Permissions/Health (Phase 9)**: Required permissions list, compare/check service, Verify action updates status and audit, permissions panel in Tenant detail.
|
||||||
- **US1b Settings Display (Phase 13)**: PolicyNormalizer + SnapshotValidator, warnings for malformed snapshots, normalized settings and pretty JSON on policy/version detail, list badges, README section.
|
- **US1b Settings Display (Phase 13)**: PolicyNormalizer + SnapshotValidator, warnings for malformed snapshots, normalized settings and pretty JSON on policy/version detail, list badges, README section.
|
||||||
|
|||||||
@ -362,7 +362,7 @@ ### Functional Requirements
|
|||||||
- **FR-020**: For PowerShell script objects (`deviceManagementScript` in `scope.supported_types`), the `scriptContent` MUST be base64-decoded when stored in backups/versions for readability/diffing and encoded again when sent back to Graph during restore.
|
- **FR-020**: For PowerShell script objects (`deviceManagementScript` in `scope.supported_types`), the `scriptContent` MUST be base64-decoded when stored in backups/versions for readability/diffing and encoded again when sent back to Graph during restore.
|
||||||
- **FR-021**: Restore behavior MUST follow the per-type configuration in `scope.restore_matrix`: `backup` determines full vs metadata-only snapshots; `restore` determines whether automated restore is enabled or preview-only; `risk` informs warning/confirmation UX.
|
- **FR-021**: Restore behavior MUST follow the per-type configuration in `scope.restore_matrix`: `backup` determines full vs metadata-only snapshots; `restore` determines whether automated restore is enabled or preview-only; `risk` informs warning/confirmation UX.
|
||||||
- **FR-022**: For high-risk types with `restore: preview-only` in `scope.restore_matrix` (e.g., `conditionalAccessPolicy`, `enrollmentRestriction`), TenantPilot MUST provide full backups, version history, and diffs plus detailed restore previews, but MUST NOT expose direct Graph apply actions; restore is manual, guided by the preview.
|
- **FR-022**: For high-risk types with `restore: preview-only` in `scope.restore_matrix` (e.g., `conditionalAccessPolicy`, `enrollmentRestriction`), TenantPilot MUST provide full backups, version history, and diffs plus detailed restore previews, but MUST NOT expose direct Graph apply actions; restore is manual, guided by the preview.
|
||||||
- **FR-036**: When `settingsCatalogPolicy` settings apply fails because the Graph settings endpoint is unsupported (route missing / method not allowed), the system MUST attempt a safe fallback by creating a new policy from the snapshot (including settings), record the new policy id, and mark the restore item as partial with a manual cleanup warning.
|
- **FR-036**: When `settingsCatalogPolicy` settings apply fails because the Graph settings endpoint is unsupported (route missing / method not allowed), the system MUST attempt a safe fallback by creating a new policy from the snapshot and record the new policy id. If creating with settings is not supported, the system MUST retry with a metadata-only payload, mark the restore item as partial, and surface a manual settings-apply warning.
|
||||||
|
|
||||||
### Key Entities *(include if feature involves data)*
|
### Key Entities *(include if feature involves data)*
|
||||||
|
|
||||||
|
|||||||
@ -710,12 +710,12 @@ ### Implementation
|
|||||||
2. **Sanitizer:** In `GraphContractRegistry` allow and preserve `@odata.type` inside `settingInstance` and nested children (recursively); continue to strip read-only/meta fields and `id`.
|
2. **Sanitizer:** In `GraphContractRegistry` allow and preserve `@odata.type` inside `settingInstance` and nested children (recursively); continue to strip read-only/meta fields and `id`.
|
||||||
3. **RestoreService:** Build `settingsPayload = sanitizeSettingsApplyPayload(snapshot['settings'])` and `POST` to the contract path; on failure mark item `manual_required` and persist Graph meta (`request_id`, `client_request_id`, error message).
|
3. **RestoreService:** Build `settingsPayload = sanitizeSettingsApplyPayload(snapshot['settings'])` and `POST` to the contract path; on failure mark item `manual_required` and persist Graph meta (`request_id`, `client_request_id`, error message).
|
||||||
4. **UI:** RestoreRun Results view shows clear admin message when `manual_required` due to settings_apply, including request ids.
|
4. **UI:** RestoreRun Results view shows clear admin message when `manual_required` due to settings_apply, including request ids.
|
||||||
5. **Fallback create:** If the settings apply call fails with route missing / method not allowed, create a new Settings Catalog policy via `POST deviceManagement/configurationPolicies` using a sanitized payload that includes the settings. Record the new policy id in restore results and mark the item as partial with a manual cleanup warning.
|
5. **Fallback create:** If the settings apply call fails with route missing / method not allowed, create a new Settings Catalog policy via `POST deviceManagement/configurationPolicies` using a sanitized payload that includes the settings. If Graph returns `NotSupported`, retry with a metadata-only payload (no settings) and mark the item as partial with a manual settings apply warning. Record the new policy id in restore results.
|
||||||
|
|
||||||
### Tests (Pest)
|
### Tests (Pest)
|
||||||
- Unit: `tests/Unit/GraphContractRegistrySettingsApplySanitizerTest.php` (preserve `@odata.type`, strip ids)
|
- Unit: `tests/Unit/GraphContractRegistrySettingsApplySanitizerTest.php` (preserve `@odata.type`, strip ids)
|
||||||
- Feature: `tests/Feature/Filament/SettingsCatalogRestoreApplySettingsTest.php` (mock Graph, assert POST body includes `@odata.type` and success/failure flows)
|
- Feature: `tests/Feature/Filament/SettingsCatalogRestoreApplySettingsTest.php` (mock Graph, assert POST body includes `@odata.type` and success/failure flows)
|
||||||
- Feature: add a restore test that simulates a settings apply route-missing error and verifies fallback policy creation + new policy id recorded.
|
- Feature: add a restore test that simulates a settings apply route-missing error and verifies fallback policy creation + new policy id recorded, including metadata-only retry when create returns `NotSupported`.
|
||||||
|
|
||||||
### Verification
|
### Verification
|
||||||
- `./vendor/bin/pest tests/Unit/GraphContractRegistrySettingsApplySanitizerTest.php`
|
- `./vendor/bin/pest tests/Unit/GraphContractRegistrySettingsApplySanitizerTest.php`
|
||||||
|
|||||||
@ -150,6 +150,7 @@ public function execute(
|
|||||||
$settings = [];
|
$settings = [];
|
||||||
$resultReason = null;
|
$resultReason = null;
|
||||||
$createdPolicyId = null;
|
$createdPolicyId = null;
|
||||||
|
$createdPolicyMode = null;
|
||||||
|
|
||||||
if ($item->policy_type === 'settingsCatalogPolicy') {
|
if ($item->policy_type === 'settingsCatalogPolicy') {
|
||||||
$settings = $this->extractSettingsCatalogSettings($originalPayload);
|
$settings = $this->extractSettingsCatalogSettings($originalPayload);
|
||||||
@ -182,11 +183,17 @@ public function execute(
|
|||||||
|
|
||||||
if ($createOutcome['success']) {
|
if ($createOutcome['success']) {
|
||||||
$createdPolicyId = $createOutcome['policy_id'];
|
$createdPolicyId = $createOutcome['policy_id'];
|
||||||
|
$createdPolicyMode = $createOutcome['mode'] ?? null;
|
||||||
$itemStatus = 'partial';
|
$itemStatus = 'partial';
|
||||||
$resultReason = 'Settings endpoint unsupported; created new policy. Manual cleanup required.';
|
$mode = $createOutcome['mode'] ?? 'settings';
|
||||||
|
|
||||||
|
$resultReason = $mode === 'metadata_only'
|
||||||
|
? 'Settings endpoint unsupported; created metadata-only policy. Manual settings apply required.'
|
||||||
|
: 'Settings endpoint unsupported; created new policy. Manual cleanup required.';
|
||||||
|
|
||||||
if ($settingsApply !== null && $createdPolicyId) {
|
if ($settingsApply !== null && $createdPolicyId) {
|
||||||
$settingsApply['created_policy_id'] = $createdPolicyId;
|
$settingsApply['created_policy_id'] = $createdPolicyId;
|
||||||
|
$settingsApply['created_policy_mode'] = $mode;
|
||||||
}
|
}
|
||||||
} elseif ($settingsApply !== null && $createOutcome['response']) {
|
} elseif ($settingsApply !== null && $createOutcome['response']) {
|
||||||
$settingsApply['issues'][] = [
|
$settingsApply['issues'][] = [
|
||||||
@ -259,6 +266,10 @@ public function execute(
|
|||||||
$result['created_policy_id'] = $createdPolicyId;
|
$result['created_policy_id'] = $createdPolicyId;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if ($createdPolicyMode) {
|
||||||
|
$result['created_policy_mode'] = $createdPolicyMode;
|
||||||
|
}
|
||||||
|
|
||||||
if ($resultReason !== null) {
|
if ($resultReason !== null) {
|
||||||
$result['reason'] = $resultReason;
|
$result['reason'] = $resultReason;
|
||||||
} elseif ($itemStatus !== 'applied') {
|
} elseif ($itemStatus !== 'applied') {
|
||||||
@ -612,7 +623,7 @@ private function shouldAttemptSettingsCatalogCreate(array $settingsApply): bool
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @return array{success:bool,policy_id:?string,response:?object}
|
* @return array{success:bool,policy_id:?string,response:?object,mode:string}
|
||||||
*/
|
*/
|
||||||
private function createSettingsCatalogPolicy(
|
private function createSettingsCatalogPolicy(
|
||||||
array $originalPayload,
|
array $originalPayload,
|
||||||
@ -629,10 +640,11 @@ private function createSettingsCatalogPolicy(
|
|||||||
'success' => false,
|
'success' => false,
|
||||||
'policy_id' => null,
|
'policy_id' => null,
|
||||||
'response' => null,
|
'response' => null,
|
||||||
|
'mode' => 'failed',
|
||||||
];
|
];
|
||||||
}
|
}
|
||||||
|
|
||||||
$payload = $this->buildSettingsCatalogCreatePayload($originalPayload, $sanitizedSettings, $fallbackName);
|
$payload = $this->buildSettingsCatalogCreatePayload($originalPayload, $sanitizedSettings, $fallbackName, true);
|
||||||
|
|
||||||
$this->graphLogger->logRequest('create_settings_catalog_policy', $context + [
|
$this->graphLogger->logRequest('create_settings_catalog_policy', $context + [
|
||||||
'endpoint' => $resource,
|
'endpoint' => $resource,
|
||||||
@ -648,24 +660,67 @@ private function createSettingsCatalogPolicy(
|
|||||||
'settings_count' => count($sanitizedSettings),
|
'settings_count' => count($sanitizedSettings),
|
||||||
]);
|
]);
|
||||||
|
|
||||||
$policyId = null;
|
$policyId = $this->extractCreatedPolicyId($response);
|
||||||
if ($response->successful() && isset($response->data['id']) && is_string($response->data['id'])) {
|
$mode = 'settings';
|
||||||
$policyId = $response->data['id'];
|
|
||||||
|
if ($response->failed() && $this->shouldRetrySettingsCatalogCreateWithoutSettings($response)) {
|
||||||
|
$fallbackPayload = $this->buildSettingsCatalogCreatePayload($originalPayload, $sanitizedSettings, $fallbackName, false);
|
||||||
|
|
||||||
|
$this->graphLogger->logRequest('create_settings_catalog_policy_fallback', $context + [
|
||||||
|
'endpoint' => $resource,
|
||||||
|
'method' => 'POST',
|
||||||
|
]);
|
||||||
|
|
||||||
|
$response = $this->graphClient->request('POST', $resource, ['json' => $fallbackPayload] + Arr::except($graphOptions, ['platform']));
|
||||||
|
|
||||||
|
$this->graphLogger->logResponse('create_settings_catalog_policy_fallback', $response, $context + [
|
||||||
|
'endpoint' => $resource,
|
||||||
|
'method' => 'POST',
|
||||||
|
]);
|
||||||
|
|
||||||
|
$policyId = $this->extractCreatedPolicyId($response);
|
||||||
|
$mode = 'metadata_only';
|
||||||
}
|
}
|
||||||
|
|
||||||
return [
|
return [
|
||||||
'success' => $response->successful(),
|
'success' => $response->successful(),
|
||||||
'policy_id' => $policyId,
|
'policy_id' => $policyId,
|
||||||
'response' => $response,
|
'response' => $response,
|
||||||
|
'mode' => $mode,
|
||||||
];
|
];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function shouldRetrySettingsCatalogCreateWithoutSettings(object $response): bool
|
||||||
|
{
|
||||||
|
$code = strtolower((string) ($response->meta['error_code'] ?? ''));
|
||||||
|
$message = strtolower((string) ($response->meta['error_message'] ?? ''));
|
||||||
|
|
||||||
|
if ($code === 'notsupported' || str_contains($code, 'notsupported')) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
return str_contains($message, 'not supported');
|
||||||
|
}
|
||||||
|
|
||||||
|
private function extractCreatedPolicyId(object $response): ?string
|
||||||
|
{
|
||||||
|
if ($response->successful() && isset($response->data['id']) && is_string($response->data['id'])) {
|
||||||
|
return $response->data['id'];
|
||||||
|
}
|
||||||
|
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @param array<int, mixed> $settings
|
* @param array<int, mixed> $settings
|
||||||
* @return array<string, mixed>
|
* @return array<string, mixed>
|
||||||
*/
|
*/
|
||||||
private function buildSettingsCatalogCreatePayload(array $originalPayload, array $settings, string $fallbackName): array
|
private function buildSettingsCatalogCreatePayload(
|
||||||
{
|
array $originalPayload,
|
||||||
|
array $settings,
|
||||||
|
string $fallbackName,
|
||||||
|
bool $includeSettings,
|
||||||
|
): array {
|
||||||
$payload = [];
|
$payload = [];
|
||||||
$name = $this->resolvePayloadString($originalPayload, ['name', 'displayName']);
|
$name = $this->resolvePayloadString($originalPayload, ['name', 'displayName']);
|
||||||
|
|
||||||
@ -696,7 +751,7 @@ private function buildSettingsCatalogCreatePayload(array $originalPayload, array
|
|||||||
$payload['templateReference'] = $this->stripOdataAndReadOnly($templateReference);
|
$payload['templateReference'] = $this->stripOdataAndReadOnly($templateReference);
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($settings !== []) {
|
if ($includeSettings && $settings !== []) {
|
||||||
$payload['settings'] = $settings;
|
$payload['settings'] = $settings;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -49,8 +49,14 @@
|
|||||||
@endif
|
@endif
|
||||||
|
|
||||||
@if (! empty($item['created_policy_id']))
|
@if (! empty($item['created_policy_id']))
|
||||||
|
@php
|
||||||
|
$createdMode = $item['created_policy_mode'] ?? null;
|
||||||
|
$createdMessage = $createdMode === 'metadata_only'
|
||||||
|
? 'New policy created (metadata only). Apply settings manually.'
|
||||||
|
: 'New policy created (manual cleanup required).';
|
||||||
|
@endphp
|
||||||
<div class="mt-2 text-xs text-amber-800">
|
<div class="mt-2 text-xs text-amber-800">
|
||||||
New policy created: {{ $item['created_policy_id'] }} (manual cleanup required).
|
{{ $createdMessage }} ID: {{ $item['created_policy_id'] }}
|
||||||
</div>
|
</div>
|
||||||
@endif
|
@endif
|
||||||
|
|
||||||
|
|||||||
@ -434,7 +434,21 @@ public function request(string $method, string $path, array $options = []): Grap
|
|||||||
],
|
],
|
||||||
);
|
);
|
||||||
|
|
||||||
$createResponse = new GraphResponse(
|
$createFailResponse = new GraphResponse(
|
||||||
|
success: false,
|
||||||
|
data: ['error' => ['code' => 'NotSupported', 'message' => 'NotSupported']],
|
||||||
|
status: 400,
|
||||||
|
errors: [['code' => 'NotSupported', 'message' => 'NotSupported']],
|
||||||
|
warnings: [],
|
||||||
|
meta: [
|
||||||
|
'error_code' => 'NotSupported',
|
||||||
|
'error_message' => 'NotSupported',
|
||||||
|
'request_id' => 'req-create-fail',
|
||||||
|
'client_request_id' => 'client-create-fail',
|
||||||
|
],
|
||||||
|
);
|
||||||
|
|
||||||
|
$createSuccessResponse = new GraphResponse(
|
||||||
success: true,
|
success: true,
|
||||||
data: ['id' => 'new-policy-123'],
|
data: ['id' => 'new-policy-123'],
|
||||||
status: 201,
|
status: 201,
|
||||||
@ -443,7 +457,7 @@ public function request(string $method, string $path, array $options = []): Grap
|
|||||||
meta: ['request_id' => 'req-create', 'client_request_id' => 'client-create'],
|
meta: ['request_id' => 'req-create', 'client_request_id' => 'client-create'],
|
||||||
);
|
);
|
||||||
|
|
||||||
$client = new SettingsCatalogRestoreGraphClient($policyResponse, [$settingsResponse, $createResponse]);
|
$client = new SettingsCatalogRestoreGraphClient($policyResponse, [$settingsResponse, $createFailResponse, $createSuccessResponse]);
|
||||||
|
|
||||||
app()->instance(GraphClientInterface::class, $client);
|
app()->instance(GraphClientInterface::class, $client);
|
||||||
|
|
||||||
@ -513,11 +527,16 @@ public function request(string $method, string $path, array $options = []): Grap
|
|||||||
expect($run->status)->toBe('partial');
|
expect($run->status)->toBe('partial');
|
||||||
expect($run->results[0]['status'])->toBe('partial');
|
expect($run->results[0]['status'])->toBe('partial');
|
||||||
expect($run->results[0]['created_policy_id'])->toBe('new-policy-123');
|
expect($run->results[0]['created_policy_id'])->toBe('new-policy-123');
|
||||||
|
expect($run->results[0]['created_policy_mode'])->toBe('metadata_only');
|
||||||
expect($run->results[0]['settings_apply']['created_policy_id'])->toBe('new-policy-123');
|
expect($run->results[0]['settings_apply']['created_policy_id'])->toBe('new-policy-123');
|
||||||
|
expect($run->results[0]['settings_apply']['created_policy_mode'])->toBe('metadata_only');
|
||||||
|
|
||||||
expect($client->requestCalls)->toHaveCount(2);
|
expect($client->requestCalls)->toHaveCount(3);
|
||||||
expect($client->requestCalls[0]['path'])->toBe('deviceManagement/configurationPolicies/scp-5/settings');
|
expect($client->requestCalls[0]['path'])->toBe('deviceManagement/configurationPolicies/scp-5/settings');
|
||||||
expect($client->requestCalls[1]['path'])->toBe('deviceManagement/configurationPolicies');
|
expect($client->requestCalls[1]['path'])->toBe('deviceManagement/configurationPolicies');
|
||||||
expect($client->requestCalls[1]['payload'])->toHaveKey('settings');
|
expect($client->requestCalls[1]['payload'])->toHaveKey('settings');
|
||||||
expect($client->requestCalls[1]['payload'])->toHaveKey('name');
|
expect($client->requestCalls[1]['payload'])->toHaveKey('name');
|
||||||
|
expect($client->requestCalls[2]['path'])->toBe('deviceManagement/configurationPolicies');
|
||||||
|
expect($client->requestCalls[2]['payload'])->not->toHaveKey('settings');
|
||||||
|
expect($client->requestCalls[2]['payload'])->toHaveKey('name');
|
||||||
});
|
});
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user