From b95bcd7f6ea2e9ee22972e14942f670628f8ce04 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Thu, 7 May 2026 09:48:22 +0100 Subject: [PATCH] Carry expired approval fix context into Assistant --- .../v6/internal/subsystems/api-contracts.md | 6 +- .../subsystems/frontend-primitives.md | 5 +- .../subsystems/patrol-intelligence.md | 6 ++ .../src/components/patrol/ApprovalSection.tsx | 34 ++++++-- .../patrol/__tests__/ApprovalSection.test.tsx | 70 ++++++++++++++++ .../patrolInvestigationContextModel.test.ts | 40 +++++++++ .../patrol/patrolInvestigationContextModel.ts | 83 +++++++++++++++++-- ...patrol-assistant-operator-briefing.spec.ts | 58 ++++++++++++- 8 files changed, 282 insertions(+), 20 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 0204328d6..d38bb66a0 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -337,7 +337,11 @@ the canonical monitored-system blocked payload. carry that Patrol-owned operator briefing, current `fix_queued` posture, request-local approval-required mode, and safe suggested prompts; they must not degrade into generic Assistant investigation chat or imply that - execution can proceed from missing command payloads. Direct + execution can proceed from missing command payloads. Expired-approval + recovery handoffs may use a still-available structured proposed-fix payload + only as safe metadata: description, target, risk, rationale, destructive + posture, and command count may enter the briefing, while raw command text + remains owned by governed remediation or approval surfaces. Direct alert-investigation API handoffs through `internal/api/ai_handlers.go` must enforce that same request-scoped boundary by setting `ai.ExecuteRequest.AutonomousMode` to diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 70d93974c..0a8cb9ebd 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -827,7 +827,10 @@ frontend primitive boundary. raw approval, command, or rollback command payload text. Missing-detail queued-fix recovery actions must still provide the feature-owned Patrol briefing and request-local approval-required posture rather than opening the - shared drawer as context-free generic Assistant chat. + shared drawer as context-free generic Assistant chat. If a feature-owned + expired-approval recovery action still has structured proposed-fix metadata, + the shared drawer may receive only safe summary fields and command counts; + raw command text remains outside shared Assistant primitives. When those feature-owned helpers attach backend model-only context, the drawer store may carry only bounded handoff text and structured resource references for the shared chat transport; approval, lifecycle, and command diff --git a/docs/release-control/v6/internal/subsystems/patrol-intelligence.md b/docs/release-control/v6/internal/subsystems/patrol-intelligence.md index b22655d41..554edce1f 100644 --- a/docs/release-control/v6/internal/subsystems/patrol-intelligence.md +++ b/docs/release-control/v6/internal/subsystems/patrol-intelligence.md @@ -181,6 +181,12 @@ Patrol-specific presentation helpers. with the same Patrol-owned visible briefing and approval-required posture from current finding facts; it must not fall back to generic investigation chat or invite execution from missing command details. + If the live approval is gone but the structured proposed-fix payload is still + available, the recovery Assistant briefing must carry only safe proposed-fix + metadata such as description, target, risk, rationale, destructive posture, + and command count. Raw command text remains in the governed remediation or + approval panel, while Assistant gets enough context to explain approval + recovery and risk without becoming an execution surface. If the referenced finding is no longer current, Assistant must drop the stored handoff instead of continuing from stale Patrol context. Assistant handoff context must also carry the unified diff --git a/frontend-modern/src/components/patrol/ApprovalSection.tsx b/frontend-modern/src/components/patrol/ApprovalSection.tsx index 57a8bdf82..dac87a58e 100644 --- a/frontend-modern/src/components/patrol/ApprovalSection.tsx +++ b/frontend-modern/src/components/patrol/ApprovalSection.tsx @@ -24,6 +24,15 @@ interface ApprovalSectionProps { resourceId?: string; } +interface AssistantBriefingFixSource { + description?: string | null; + commands?: string[] | null; + target_host?: string | null; + risk_level?: string | null; + rationale?: string | null; + destructive?: boolean | null; +} + export const ApprovalSection: Component = (props) => { const [actionLoading, setActionLoading] = createSignal(null); const [executionResult, setExecutionResult] = createSignal(null); @@ -39,7 +48,10 @@ export const ApprovalSection: Component = (props) => { const canAutoFix = createMemo(() => hasFeature('ai_autofix')); - const approvalBriefing = (approval: ApprovalRequest | null) => + const approvalBriefing = ( + approval: ApprovalRequest | null, + fix?: AssistantBriefingFixSource | null, + ) => buildPatrolAssistantFindingBriefing({ title: props.findingTitle || 'Patrol finding', subject: props.resourceName || 'affected resource', @@ -56,17 +68,21 @@ export const ApprovalSection: Component = (props) => { targetName: approval.targetName, } : null, + proposedFix: fix + ? { + description: fix.description, + riskLevel: fix.risk_level, + targetHost: fix.target_host, + rationale: fix.rationale, + commandCount: fix.commands?.length ?? 0, + destructive: fix.destructive, + } + : null, }); const handleFixWithAssistant = ( approval: ApprovalRequest | null, - fix: { - description?: string; - commands?: string[]; - target_host?: string; - risk_level?: string; - rationale?: string; - } | null, + fix: AssistantBriefingFixSource | null, e: Event, ) => { e.stopPropagation(); @@ -88,7 +104,7 @@ export const ApprovalSection: Component = (props) => { targetType: props.resourceType, targetId: props.resourceId, findingId: props.findingId, - briefing: approvalBriefing(approval), + briefing: approvalBriefing(approval, fix), autonomousMode: false, }); }; diff --git a/frontend-modern/src/components/patrol/__tests__/ApprovalSection.test.tsx b/frontend-modern/src/components/patrol/__tests__/ApprovalSection.test.tsx index 33374dd7f..5a1bd0b2b 100644 --- a/frontend-modern/src/components/patrol/__tests__/ApprovalSection.test.tsx +++ b/frontend-modern/src/components/patrol/__tests__/ApprovalSection.test.tsx @@ -207,6 +207,76 @@ describe('ApprovalSection', () => { expect(JSON.stringify(context.briefing)).not.toContain('systemctl restart nginx'); }); + it('opens Assistant from an expired approval with safe proposed-fix briefing metadata', async () => { + getInvestigationMock.mockResolvedValue({ + id: 'session-1', + finding_id: 'finding-1', + session_id: 'session-1', + status: 'completed', + started_at: '2026-05-06T12:00:00Z', + turn_count: 1, + outcome: 'fix_queued', + proposed_fix: { + id: 'fix-1', + description: 'Restart the workload service', + commands: ['systemctl restart nginx'], + risk_level: 'high', + destructive: true, + target_host: 'node-1', + rationale: 'Service is wedged after IO pressure.', + }, + }); + + render(() => ( + + )); + + expect(await screen.findByText('approval expired')).toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: /fix with assistant/i })); + + expect(openWithPromptMock).toHaveBeenCalledTimes(1); + const [prompt, context] = openWithPromptMock.mock.calls[0]; + expect(prompt).toContain('queued a governed fix for review'); + expect(prompt).toContain('**Proposed fix:** Restart the workload service'); + expect(prompt).toContain('**Target:** node-1'); + expect(prompt).toContain('**Risk level:** high'); + expect(prompt).not.toContain('systemctl restart nginx'); + expect(context).toEqual({ + targetType: 'agent', + targetId: 'agent-1', + findingId: 'finding-1', + briefing: expect.objectContaining({ + sourceLabel: 'Pulse Patrol', + title: 'Operator briefing attached', + subject: 'CPU saturation on node-1', + statusLabel: 'Fix Queued', + detailLines: expect.arrayContaining([ + expect.stringContaining('fix queued for governed review'), + expect.stringContaining('Proposed fix: Restart the workload service'), + expect.stringContaining('Recover or regenerate the governed approval before execution'), + ]), + actionLabel: 'Restart the workload service', + commandSummary: '1 command recorded for approval context', + safetyNote: + 'Command details stay in approval context; destructive actions require governed approval.', + suggestedPrompts: [ + 'Review approval risk and next step', + 'Explain current finding status', + 'Summarize remediation without command text', + ], + }), + autonomousMode: false, + }); + expect(JSON.stringify(context.briefing)).not.toContain('systemctl restart nginx'); + }); + it('recreates and executes a queued fix when autofix is available', async () => { state.hasAutoFix = true; getInvestigationMock.mockResolvedValue(null); diff --git a/frontend-modern/src/features/patrol/__tests__/patrolInvestigationContextModel.test.ts b/frontend-modern/src/features/patrol/__tests__/patrolInvestigationContextModel.test.ts index 35d16c914..bb5434db7 100644 --- a/frontend-modern/src/features/patrol/__tests__/patrolInvestigationContextModel.test.ts +++ b/frontend-modern/src/features/patrol/__tests__/patrolInvestigationContextModel.test.ts @@ -614,4 +614,44 @@ describe('patrolInvestigationContextModel', () => { ], }); }); + + it('builds queued-fix recovery briefing from safe proposed-fix metadata', () => { + expect( + buildPatrolAssistantFindingBriefing({ + title: 'CPU saturation', + subject: 'node-1', + findingStatus: 'active', + investigationOutcome: 'fix_queued', + loopState: 'fix_queued', + proposedFix: { + description: 'Restart workload service', + riskLevel: 'high', + targetHost: 'node-1', + rationale: 'service is wedged', + commandCount: 1, + destructive: true, + }, + }), + ).toEqual({ + sourceLabel: 'Pulse Patrol', + title: 'Operator briefing attached', + subject: 'CPU saturation on node-1', + statusLabel: 'Fix Queued', + detailLines: [ + 'Attention: active finding; loop fix queued; fix queued for governed review', + 'Proposed fix: Restart workload service; target node-1; high risk; 1 command recorded for approval context; destructive proposed fix; rationale service is wedged', + 'Decision: Recover or regenerate the governed approval before execution; do not execute from chat context.', + ], + evidence: [], + actionLabel: 'Restart workload service', + commandSummary: '1 command recorded for approval context', + safetyNote: + 'Command details stay in approval context; destructive actions require governed approval.', + suggestedPrompts: [ + 'Review approval risk and next step', + 'Explain current finding status', + 'Summarize remediation without command text', + ], + }); + }); }); diff --git a/frontend-modern/src/features/patrol/patrolInvestigationContextModel.ts b/frontend-modern/src/features/patrol/patrolInvestigationContextModel.ts index 895d00652..6be8dcae1 100644 --- a/frontend-modern/src/features/patrol/patrolInvestigationContextModel.ts +++ b/frontend-modern/src/features/patrol/patrolInvestigationContextModel.ts @@ -68,6 +68,15 @@ export interface PatrolAssistantApprovalBriefingInput { targetName?: string | null; } +export interface PatrolAssistantProposedFixBriefingInput { + description?: string | null; + riskLevel?: string | null; + targetHost?: string | null; + rationale?: string | null; + commandCount?: number | null; + destructive?: boolean | null; +} + export interface PatrolAssistantFindingBriefingInput { title: string; subject: string; @@ -80,6 +89,7 @@ export interface PatrolAssistantFindingBriefingInput { lastRegressionAt?: string | null; remediationId?: string | null; pendingApproval?: PatrolAssistantApprovalBriefingInput | null; + proposedFix?: PatrolAssistantProposedFixBriefingInput | null; investigationRecord?: InvestigationRecord | null; } @@ -1021,6 +1031,7 @@ export function buildPatrolAssistantFindingBriefing( const title = normalizeText(input.title) || 'Patrol finding'; const subject = normalizeText(input.subject) || 'affected resource'; const pendingApproval = normalizeApprovalBriefing(input.pendingApproval); + const proposedFix = record.proposedFix || normalizeProposedFixBriefing(input.proposedFix); const approvalStatusParts = !record.hasRecord ? [ pendingApproval.status ? `${formatIdentifierLabel(pendingApproval.status)} approval` : '', @@ -1039,11 +1050,15 @@ export function buildPatrolAssistantFindingBriefing( if (!record.hasRecord && !attentionReason && !operatorDecision) { return undefined; } + const proposedFixDetail = record.proposedFix + ? undefined + : formatPatrolAssistantProposedFixDetail(proposedFix); const detailLines = [ attentionReason ? `Attention: ${attentionReason}` : undefined, record.conclusion, record.recommendedAction, + proposedFixDetail, operatorDecision ? `Decision: ${operatorDecision}` : undefined, ] .filter(isNonEmptyString) @@ -1058,11 +1073,16 @@ export function buildPatrolAssistantFindingBriefing( detailLines, evidence: [...record.evidenceSummaries, ...verificationLines].slice(0, 4), actionLabel: - record.proposedFix?.description || + proposedFix?.description || (pendingApproval.id ? `Approval ${pendingApproval.id}` : undefined), - commandSummary: record.proposedFix?.commandSummary, - safetyNote: buildPatrolAssistantSafetyNote(record, pendingApproval), - suggestedPrompts: buildPatrolFindingSuggestedPrompts(input, record, pendingApproval), + commandSummary: proposedFix?.commandSummary, + safetyNote: buildPatrolAssistantSafetyNote(proposedFix, pendingApproval), + suggestedPrompts: buildPatrolFindingSuggestedPrompts( + input, + record, + pendingApproval, + proposedFix, + ), }; } @@ -1096,6 +1116,7 @@ function buildPatrolFindingSuggestedPrompts( input: PatrolAssistantFindingBriefingInput, record: PatrolInvestigationRecordPresentation, pendingApproval: Required, + proposedFix?: PatrolInvestigationRecordPresentation['proposedFix'], ): string[] { const prompts: string[] = []; const requiresApproval = patrolAssistantFindingHandoffRequiresApprovalMode({ @@ -1127,7 +1148,7 @@ function buildPatrolFindingSuggestedPrompts( prompts.push('List evidence to gather before action'); } - if (record.proposedFix?.commandSummary) { + if (proposedFix?.commandSummary) { prompts.push('Summarize remediation without command text'); } else if (requiresApproval) { prompts.push('List approval prerequisites before action'); @@ -1360,11 +1381,11 @@ function buildPatrolAssistantOperatorDecision( } function buildPatrolAssistantSafetyNote( - record: PatrolInvestigationRecordPresentation, + proposedFix?: PatrolInvestigationRecordPresentation['proposedFix'], pendingApproval?: Required, ): string | undefined { - const hasCommands = Boolean(record.proposedFix?.commandSummary); - const isDestructive = Boolean(record.proposedFix?.destructive); + const hasCommands = Boolean(proposedFix?.commandSummary); + const isDestructive = Boolean(proposedFix?.destructive); if (hasCommands && isDestructive) { return 'Command details stay in approval context; destructive actions require governed approval.'; } @@ -1380,6 +1401,52 @@ function buildPatrolAssistantSafetyNote( return undefined; } +function normalizeProposedFixBriefing( + proposedFix?: PatrolAssistantProposedFixBriefingInput | null, +): PatrolInvestigationRecordPresentation['proposedFix'] | undefined { + const commandSummary = formatCommandSummary(normalizeNonNegativeCount(proposedFix?.commandCount)); + const normalized = { + description: normalizeText(proposedFix?.description), + riskLabel: formatIdentifierLabel(proposedFix?.riskLevel), + targetHost: normalizeText(proposedFix?.targetHost), + rationale: normalizeText(proposedFix?.rationale), + commandSummary, + destructive: Boolean(proposedFix?.destructive), + }; + + if ( + !normalized.description && + !normalized.riskLabel && + !normalized.targetHost && + !normalized.rationale && + !normalized.commandSummary && + !normalized.destructive + ) { + return undefined; + } + + return normalized; +} + +function formatPatrolAssistantProposedFixDetail( + proposedFix?: PatrolInvestigationRecordPresentation['proposedFix'], +): string | undefined { + if (!proposedFix) return undefined; + const detail = formatBriefingStringList( + [ + proposedFix.description, + proposedFix.targetHost ? `target ${proposedFix.targetHost}` : undefined, + proposedFix.riskLabel ? `${proposedFix.riskLabel.toLowerCase()} risk` : undefined, + proposedFix.commandSummary, + proposedFix.destructive ? 'destructive proposed fix' : undefined, + proposedFix.rationale ? `rationale ${proposedFix.rationale}` : undefined, + ], + 6, + 'proposed-fix facts', + ); + return detail ? `Proposed fix: ${detail}` : undefined; +} + function normalizeApprovalBriefing( approval?: PatrolAssistantApprovalBriefingInput | null, ): Required { diff --git a/tests/integration/tests/73-patrol-assistant-operator-briefing.spec.ts b/tests/integration/tests/73-patrol-assistant-operator-briefing.spec.ts index cccae09bd..1a89efeac 100644 --- a/tests/integration/tests/73-patrol-assistant-operator-briefing.spec.ts +++ b/tests/integration/tests/73-patrol-assistant-operator-briefing.spec.ts @@ -48,6 +48,7 @@ test.describe("Patrol Assistant operator briefing", () => { const approvalRequestedAt = new Date(Date.now() - 60_000).toISOString(); const approvalExpiresAt = new Date(Date.now() + 10 * 60_000).toISOString(); let includePendingApproval = true; + let includeInvestigationProposedFix = false; await page.route("**/api/security/status", async (route) => { await route.fulfill({ @@ -392,7 +393,29 @@ test.describe("Patrol Assistant operator briefing", () => { await route.fulfill({ status: 200, contentType: "application/json", - body: JSON.stringify(null), + body: JSON.stringify( + includeInvestigationProposedFix + ? { + id: "session-operator-briefing", + finding_id: "finding-operator-briefing", + session_id: "session-operator-briefing", + status: "completed", + started_at: "2026-05-06T12:00:00Z", + turn_count: 1, + outcome: "fix_queued", + proposed_fix: { + id: "fix-expired-1", + description: "Restart the workload service", + commands: ["systemctl restart workload.service"], + risk_level: "high", + destructive: true, + target_host: "web-server", + rationale: + "Workload service stayed wedged after backup pressure.", + }, + } + : null, + ), }); }); @@ -486,5 +509,38 @@ test.describe("Patrol Assistant operator briefing", () => { await expect( queuedAssistantContext.getByText("systemctl restart workload.service"), ).toHaveCount(0); + + includeInvestigationProposedFix = true; + await page.reload({ waitUntil: "domcontentloaded" }); + await expect(page.getByRole("button", { name: "Findings" })).toBeVisible(); + + await page.getByText("High CPU usage").click(); + const expiredFinding = page.locator("#finding-finding-operator-briefing"); + await expect(expiredFinding.getByText("approval expired")).toBeVisible(); + await expiredFinding + .getByRole("button", { name: "Fix with Assistant" }) + .last() + .click(); + + const expiredAssistantContext = page.getByLabel("Assistant context"); + await expect(expiredAssistantContext).toBeVisible(); + await expect(expiredAssistantContext).toContainText( + "Operator briefing attached", + ); + await expect(expiredAssistantContext).toContainText("Fix Queued"); + await expect(expiredAssistantContext).toContainText( + "Proposed fix: Restart the workload service; target web-server; high risk; 1 command recorded for approval context; destructive proposed fix; rationale Workload service stayed wedged after backup pressure.", + ); + await expect(expiredAssistantContext).toContainText( + "Command details stay in approval context; destructive actions require governed approval.", + ); + await expect( + expiredAssistantContext.getByRole("button", { + name: "Summarize remediation without command text", + }), + ).toBeVisible(); + await expect( + expiredAssistantContext.getByText("systemctl restart workload.service"), + ).toHaveCount(0); }); });