From f024d3b56091152d0d949ce2edc5e4da26d34912 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 13 May 2026 18:14:29 +0100 Subject: [PATCH] Align action audit verification projection --- cmd/pulse-mcp/main_test.go | 5 +- .../src/api/__tests__/actionAudit.test.ts | 57 ++++++++++-- frontend-modern/src/api/actionAudit.ts | 20 ++++- .../Infrastructure/ResourceActionHistory.tsx | 11 +-- ...ResourceActionHistory.verification.test.ts | 87 +++++++++++++++++-- .../ResourceDetailDrawer.history.test.tsx | 14 ++- frontend-modern/src/types/actionAudit.ts | 9 ++ .../__tests__/actionAuditPresentation.test.ts | 41 +++++++++ .../src/utils/actionAuditPresentation.ts | 14 ++- .../ai/tools/action_audit_execution_test.go | 6 ++ internal/api/activity_audit_handlers.go | 22 +++++ internal/api/agent_events.go | 6 +- internal/api/agent_events_test.go | 34 ++++++++ internal/unifiedresources/actions.go | 80 +++++++++++++++-- internal/unifiedresources/actions_test.go | 57 +++++++++++- internal/unifiedresources/store_test.go | 19 +++- 16 files changed, 446 insertions(+), 36 deletions(-) diff --git a/cmd/pulse-mcp/main_test.go b/cmd/pulse-mcp/main_test.go index 852511581..17cf6a1c3 100644 --- a/cmd/pulse-mcp/main_test.go +++ b/cmd/pulse-mcp/main_test.go @@ -424,7 +424,7 @@ func TestServer_StreamSSEOnceTranslatesEventsToNotifications(t *testing.T) { "event: heartbeat", "", "event: action.completed", - "data: {\"actionId\":\"x1\",\"success\":true}", + "data: {\"actionId\":\"x1\",\"success\":true,\"verification\":{\"ran\":true,\"success\":true,\"commandRedacted\":true}}", "", }, "\n") + "\n" @@ -456,6 +456,9 @@ func TestServer_StreamSSEOnceTranslatesEventsToNotifications(t *testing.T) { if !strings.Contains(body, `"method":"notifications/action.completed"`) { t.Errorf("missing action.completed notification; got %s", body) } + if !strings.Contains(body, `"verification":{"ran":true,"success":true,"commandRedacted":true}`) { + t.Errorf("action.completed verification must round-trip through MCP notification params; got %s", body) + } if strings.Contains(body, "stream.connected") { t.Errorf("stream.connected must be filtered out as transport plumbing; got %s", body) } diff --git a/frontend-modern/src/api/__tests__/actionAudit.test.ts b/frontend-modern/src/api/__tests__/actionAudit.test.ts index b8c3e7976..bc4142def 100644 --- a/frontend-modern/src/api/__tests__/actionAudit.test.ts +++ b/frontend-modern/src/api/__tests__/actionAudit.test.ts @@ -95,13 +95,13 @@ describe('ActionAuditAPI', () => { result: { success: true, output: 'OK', - verification: { - ran: true, - command: "systemctl is-active 'workload'", - output: 'active', - success: true, - ranAt: '2026-04-29T12:00:25Z', - }, + }, + verification: { + ran: true, + command: "systemctl is-active 'workload'", + output: 'active', + success: true, + ranAt: '2026-04-29T12:00:25Z', }, }, ], @@ -110,11 +110,52 @@ describe('ActionAuditAPI', () => { const response = await ActionAuditAPI.listActionAudits({ resourceId: 'vm:42' }); expect(response.audits).toHaveLength(1); - const v = response.audits[0].result?.verification; + const v = response.audits[0].verification; expect(v?.ran).toBe(true); expect(v?.command).toBe("systemctl is-active 'workload'"); expect(v?.output).toBe('active'); expect(v?.success).toBe(true); + expect(response.audits[0].result?.verification).toEqual(v); + }); + + it('normalizes legacy result verification onto the canonical action audit field', async () => { + apiFetchJSONMock.mockResolvedValueOnce({ + audits: [ + { + id: 'action-legacy-verify', + createdAt: '2026-04-29T12:00:00Z', + updatedAt: '2026-04-29T12:00:30Z', + state: 'completed', + request: { + requestId: 'req-legacy-verify', + resourceId: 'vm:42', + capabilityName: 'pulse_control', + reason: 'restart workload after backup', + requestedBy: 'pulse_patrol', + }, + plan: { + actionId: 'action-legacy-verify', + requestId: 'req-legacy-verify', + allowed: true, + requiresApproval: false, + approvalPolicy: 'none', + rollbackAvailable: false, + }, + result: { + success: true, + verification: { + ran: false, + success: false, + }, + }, + }, + ], + count: 1, + } as any); + + const response = await ActionAuditAPI.listActionAudits({ resourceId: 'vm:42' }); + expect(response.audits[0].verification).toEqual({ ran: false, success: false }); + expect(response.audits[0].result?.verification).toEqual({ ran: false, success: false }); }); it('treats gated action audit endpoints as unavailable instead of throwing', async () => { diff --git a/frontend-modern/src/api/actionAudit.ts b/frontend-modern/src/api/actionAudit.ts index bb7ce55de..c728c5bd6 100644 --- a/frontend-modern/src/api/actionAudit.ts +++ b/frontend-modern/src/api/actionAudit.ts @@ -53,7 +53,9 @@ const normalizeActionAuditListResponse = ( raw: RawActionAuditListResponse | null | undefined, fallbackResourceId?: string, ): ActionAuditListResponse => { - const audits = objectArrayFieldOrEmpty(raw, 'audits'); + const audits = objectArrayFieldOrEmpty(raw, 'audits').map( + normalizeActionAuditRecord, + ); const count = Number.isFinite(raw?.count) ? Number(raw?.count) : audits.length; const resourceId = typeof raw?.resourceId === 'string' && raw.resourceId.trim() @@ -68,6 +70,22 @@ const normalizeActionAuditListResponse = ( }; }; +const normalizeActionAuditRecord = (audit: ActionAuditRecord): ActionAuditRecord => { + const verification = audit.verification ?? audit.result?.verification; + if (!verification) return audit; + + return { + ...audit, + verification, + result: audit.result + ? { + ...audit.result, + verification: audit.result.verification ?? verification, + } + : audit.result, + }; +}; + export class ActionAuditAPI { static async listActionAudits( options?: ListActionAuditsOptions, diff --git a/frontend-modern/src/components/Infrastructure/ResourceActionHistory.tsx b/frontend-modern/src/components/Infrastructure/ResourceActionHistory.tsx index 1beb97519..fc5839d84 100644 --- a/frontend-modern/src/components/Infrastructure/ResourceActionHistory.tsx +++ b/frontend-modern/src/components/Infrastructure/ResourceActionHistory.tsx @@ -5,7 +5,9 @@ import { formatRelativeTime } from '@/utils/format'; import { formatActionApprovalPolicyLabel, formatActionCapabilityLabel, + getActionAuditVerification, getActionAuditStatePresentation, + shouldRenderActionAuditVerification, } from '@/utils/actionAuditPresentation'; interface ResourceActionHistoryProps { @@ -20,6 +22,7 @@ const ActionHistoryRow: Component<{ audit: ActionAuditRecord }> = (props) => { const state = () => getActionAuditStatePresentation(props.audit.state); const preflight = () => props.audit.plan?.preflight; const result = () => props.audit.result; + const verification = () => getActionAuditVerification(props.audit); return (
@@ -87,17 +90,15 @@ const ActionHistoryRow: Component<{ audit: ActionAuditRecord }> = (props) => {
- + {(() => { - const v = result()!.verification!; + const v = verification()!; const toneClass = v.success ? 'border-emerald-200 bg-emerald-50 text-emerald-800 dark:border-emerald-800 dark:bg-emerald-950/40 dark:text-emerald-300' : 'border-amber-200 bg-amber-50 text-amber-800 dark:border-amber-800 dark:bg-amber-950/40 dark:text-amber-300'; return (
-
- {v.success ? 'Verified' : 'Verification failed'} -
+
{v.success ? 'Verified' : 'Verification failed'}
{v.command}
 diff --git a/frontend-modern/src/components/Infrastructure/__tests__/ResourceActionHistory.verification.test.ts b/frontend-modern/src/components/Infrastructure/__tests__/ResourceActionHistory.verification.test.ts index e9486da67..12274d9a3 100644 --- a/frontend-modern/src/components/Infrastructure/__tests__/ResourceActionHistory.verification.test.ts +++ b/frontend-modern/src/components/Infrastructure/__tests__/ResourceActionHistory.verification.test.ts @@ -1,11 +1,12 @@ import { readFileSync } from 'node:fs'; import { resolve } from 'node:path'; import { describe, expect, it } from 'vitest'; +import { cleanup, render, screen, within } from '@solidjs/testing-library'; -const sourceText = readFileSync( - resolve(__dirname, '..', 'ResourceActionHistory.tsx'), - 'utf-8', -); +import { ResourceActionHistory } from '../ResourceActionHistory'; +import type { ActionAuditRecord } from '@/types/actionAudit'; + +const sourceText = readFileSync(resolve(__dirname, '..', 'ResourceActionHistory.tsx'), 'utf-8'); describe('ResourceActionHistory verification rendering', () => { it('renders the post-dispatch verification outcome on each audit row when ran=true', () => { @@ -14,7 +15,7 @@ describe('ResourceActionHistory verification rendering', () => { // must surface it so operators can see "Pulse confirmed the workload // service is active" — not just "command exit 0". Pin the wiring so the // surface cannot silently regress to an output-only render. - expect(sourceText).toContain('result()?.verification?.ran'); + expect(sourceText).toContain('shouldRenderActionAuditVerification(props.audit)'); expect(sourceText).toContain('Verified'); expect(sourceText).toContain('Verification failed'); }); @@ -28,4 +29,80 @@ describe('ResourceActionHistory verification rendering', () => { expect(sourceText).toContain('v.output'); expect(sourceText).toContain('v.note'); }); + + it('renders exactly one verification row for ran=true and omits ran=false', () => { + cleanup(); + render(() => + ResourceActionHistory({ + audits: [ + actionAudit({ + id: 'action-verified', + verification: { + ran: true, + success: true, + command: "systemctl is-active 'nginx'", + output: 'active', + note: 'service reported active', + }, + }), + actionAudit({ + id: 'action-unverified', + request: { + requestId: 'req-action-unverified', + resourceId: 'vm:42', + capabilityName: 'restart_service', + reason: 'Ran without a derivable verifier', + requestedBy: 'agent:ops', + }, + verification: { + ran: false, + success: false, + command: 'should not render', + output: 'sensitive output', + note: 'should not render', + }, + }), + ], + count: 2, + loadingLabel: 'Actions loaded', + error: '', + onRetry: () => undefined, + }), + ); + + const actionHistory = within(screen.getByTestId('resource-action-history-section')); + expect(actionHistory.getAllByText('Verified')).toHaveLength(1); + expect(actionHistory.getByText("systemctl is-active 'nginx'")).toBeInTheDocument(); + expect(actionHistory.queryByText('should not render')).toBeNull(); + expect(actionHistory.queryByText('sensitive output')).toBeNull(); + }); +}); + +const actionAudit = (overrides: Partial = {}): ActionAuditRecord => ({ + id: overrides.id ?? 'action-1', + createdAt: '2026-04-29T12:00:00Z', + updatedAt: '2026-04-29T12:01:00Z', + state: 'completed', + request: overrides.request ?? { + requestId: 'req-1', + resourceId: 'vm:42', + capabilityName: 'restart_service', + reason: 'Restart nginx after patching', + requestedBy: 'agent:ops', + }, + plan: overrides.plan ?? { + actionId: overrides.id ?? 'action-1', + requestId: 'req-1', + allowed: true, + requiresApproval: true, + approvalPolicy: 'admin', + rollbackAvailable: true, + }, + result: overrides.result ?? { + success: true, + output: 'completed', + }, + verification: overrides.verification, + approvals: overrides.approvals, + verificationOutcome: overrides.verificationOutcome, }); diff --git a/frontend-modern/src/components/Infrastructure/__tests__/ResourceDetailDrawer.history.test.tsx b/frontend-modern/src/components/Infrastructure/__tests__/ResourceDetailDrawer.history.test.tsx index ab9da9f2a..d77c9af1f 100644 --- a/frontend-modern/src/components/Infrastructure/__tests__/ResourceDetailDrawer.history.test.tsx +++ b/frontend-modern/src/components/Infrastructure/__tests__/ResourceDetailDrawer.history.test.tsx @@ -198,7 +198,9 @@ describe('ResourceDetailDrawer change history section', () => { // verification outcome alongside the dispatch result, not silently // drop it. Pin the wiring so future refactors cannot regress to an // output-only render. - expect(resourceActionHistorySource).toContain('result()?.verification?.ran'); + expect(resourceActionHistorySource).toContain( + 'shouldRenderActionAuditVerification(props.audit)', + ); expect(resourceActionHistorySource).toContain('Verified'); expect(resourceActionHistorySource).toContain('Verification failed'); expect(actionAuditApiSource).toContain('/api/audit/actions'); @@ -781,6 +783,13 @@ describe('ResourceDetailDrawer change history section', () => { success: true, output: 'nginx restarted', }, + verification: { + ran: true, + command: "systemctl is-active 'nginx'", + output: 'active', + success: true, + ranAt: '2026-04-29T12:05:20Z', + }, }, ], }); @@ -822,6 +831,9 @@ describe('ResourceDetailDrawer change history section', () => { expect(actionHistory.getByText('Restart nginx')).toBeInTheDocument(); expect(actionHistory.getByText('Approval scoped to this resource.')).toBeInTheDocument(); expect(actionHistory.getByText('nginx restarted')).toBeInTheDocument(); + expect(actionHistory.getByText('Verified')).toBeInTheDocument(); + expect(actionHistory.getByText("systemctl is-active 'nginx'")).toBeInTheDocument(); + expect(actionHistory.getByText('active')).toBeInTheDocument(); }); it('keeps service details summary-first until the service-local reveal is opened', () => { diff --git a/frontend-modern/src/types/actionAudit.ts b/frontend-modern/src/types/actionAudit.ts index b8fe34771..262267d39 100644 --- a/frontend-modern/src/types/actionAudit.ts +++ b/frontend-modern/src/types/actionAudit.ts @@ -74,6 +74,13 @@ export interface ActionAuditExecutionResult { verification?: ActionVerificationResult; } +export type ActionVerificationStatus = 'unknown' | 'verified' | 'unverified' | 'failed' | string; + +export interface ActionVerificationOutcome { + status: ActionVerificationStatus; + evidenceSummary?: string; +} + export interface ActionAuditRecord { id: string; createdAt: string; @@ -83,6 +90,8 @@ export interface ActionAuditRecord { plan: ActionAuditPlan; approvals?: ActionAuditApprovalRecord[]; result?: ActionAuditExecutionResult; + verification?: ActionVerificationResult; + verificationOutcome?: ActionVerificationOutcome; } export interface ActionAuditListResponse { diff --git a/frontend-modern/src/utils/__tests__/actionAuditPresentation.test.ts b/frontend-modern/src/utils/__tests__/actionAuditPresentation.test.ts index e87341bf8..8e43ce411 100644 --- a/frontend-modern/src/utils/__tests__/actionAuditPresentation.test.ts +++ b/frontend-modern/src/utils/__tests__/actionAuditPresentation.test.ts @@ -3,7 +3,9 @@ import { describe, expect, it } from 'vitest'; import { formatActionApprovalPolicyLabel, formatActionCapabilityLabel, + getActionAuditVerification, getActionAuditStatePresentation, + shouldRenderActionAuditVerification, } from '@/utils/actionAuditPresentation'; describe('actionAuditPresentation', () => { @@ -24,4 +26,43 @@ describe('actionAuditPresentation', () => { expect(formatActionApprovalPolicyLabel('dry_run_only')).toBe('Dry run only'); expect(formatActionApprovalPolicyLabel('mfa')).toBe('MFA approval'); }); + + it('uses the canonical top-level verification field for rendering decisions', () => { + expect( + getActionAuditVerification({ + verification: { + ran: true, + success: true, + command: "systemctl is-active 'nginx'", + }, + }), + ).toMatchObject({ ran: true, command: "systemctl is-active 'nginx'" }); + expect( + getActionAuditVerification({ + result: { + success: true, + verification: { + ran: false, + success: false, + }, + }, + }), + ).toEqual({ ran: false, success: false }); + expect( + shouldRenderActionAuditVerification({ + verification: { + ran: true, + success: true, + }, + }), + ).toBe(true); + expect( + shouldRenderActionAuditVerification({ + verification: { + ran: false, + success: false, + }, + }), + ).toBe(false); + }); }); diff --git a/frontend-modern/src/utils/actionAuditPresentation.ts b/frontend-modern/src/utils/actionAuditPresentation.ts index 3fa7b8ece..c6fefadb9 100644 --- a/frontend-modern/src/utils/actionAuditPresentation.ts +++ b/frontend-modern/src/utils/actionAuditPresentation.ts @@ -1,4 +1,8 @@ -import type { ActionAuditState } from '@/types/actionAudit'; +import type { + ActionAuditRecord, + ActionAuditState, + ActionVerificationResult, +} from '@/types/actionAudit'; export interface ActionAuditStatePresentation { label: string; @@ -76,3 +80,11 @@ export const formatActionApprovalPolicyLabel = (policy: string | undefined): str return formatActionCapabilityLabel(policy || 'Policy'); } }; + +export const getActionAuditVerification = ( + audit: Pick, +): ActionVerificationResult | undefined => audit.verification ?? audit.result?.verification; + +export const shouldRenderActionAuditVerification = ( + audit: Pick, +): boolean => getActionAuditVerification(audit)?.ran === true; diff --git a/internal/ai/tools/action_audit_execution_test.go b/internal/ai/tools/action_audit_execution_test.go index 3174ac1fe..019a46f38 100644 --- a/internal/ai/tools/action_audit_execution_test.go +++ b/internal/ai/tools/action_audit_execution_test.go @@ -1433,10 +1433,16 @@ func TestSetOnActionCompleted_RecordCarriesVerificationResult(t *testing.T) { if received.Result.Verification == nil { t.Fatal("callback record must carry a Verification block for service-restart actions — drift here breaks the certainty loop on action.completed") } + if received.Verification == nil { + t.Fatal("callback record must carry the canonical top-level Verification block") + } v := received.Result.Verification if !v.Ran { t.Error("Verification.Ran must be true after the broker dispatched the probe") } + if received.Verification.Command != v.Command { + t.Errorf("canonical Verification.Command = %q, want %q", received.Verification.Command, v.Command) + } if !v.Success { t.Errorf("Verification.Success: probe returned exit 0, expected Success=true; got %+v", v) } diff --git a/internal/api/activity_audit_handlers.go b/internal/api/activity_audit_handlers.go index 62c656713..c6e0c9da6 100644 --- a/internal/api/activity_audit_handlers.go +++ b/internal/api/activity_audit_handlers.go @@ -520,6 +520,7 @@ func (h *AuditHandlers) HandleListUnifiedActionAudits(w http.ResponseWriter, r * writeErrorResponse(w, http.StatusInternalServerError, "query_failed", "Failed to query action audits", nil) return } + audits = normalizeUnifiedActionAuditResponse(audits) response := map[string]any{ "audits": audits, @@ -533,6 +534,27 @@ func (h *AuditHandlers) HandleListUnifiedActionAudits(w http.ResponseWriter, r * json.NewEncoder(w).Encode(response) } +func normalizeUnifiedActionAuditResponse(audits []unifiedresources.ActionAuditRecord) []unifiedresources.ActionAuditRecord { + if len(audits) == 0 { + return audits + } + normalized := make([]unifiedresources.ActionAuditRecord, 0, len(audits)) + for _, audit := range audits { + canonical, err := unifiedresources.NormalizeActionAuditRecord(audit) + if err != nil { + canonical = audit + canonical.Verification = unifiedresources.CanonicalActionVerification(audit) + if canonical.Result != nil && canonical.Verification != nil && canonical.Result.Verification == nil { + result := *canonical.Result + result.Verification = canonical.Verification + canonical.Result = &result + } + } + normalized = append(normalized, canonical) + } + return normalized +} + // HandleListUnifiedActionLifecycleEvents handles GET /api/audit/actions/{id}/events. func (h *AuditHandlers) HandleListUnifiedActionLifecycleEvents(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { diff --git a/internal/api/agent_events.go b/internal/api/agent_events.go index 7b24387ed..1066004ab 100644 --- a/internal/api/agent_events.go +++ b/internal/api/agent_events.go @@ -372,9 +372,9 @@ func (b *AgentEventBroadcaster) PublishActionCompletedRecord(record unifiedresou if record.Result != nil { payload.Success = record.Result.Success payload.ErrorMessage = record.Result.ErrorMessage - if v := projectAgentResourceVerification(record.Result.Verification); v != nil { - payload.Verification = v - } + } + if v := projectAgentResourceVerification(unifiedresources.CanonicalActionVerification(record)); v != nil { + payload.Verification = v } b.PublishActionCompleted(payload) } diff --git a/internal/api/agent_events_test.go b/internal/api/agent_events_test.go index ac255bb5a..2a923ba50 100644 --- a/internal/api/agent_events_test.go +++ b/internal/api/agent_events_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/config" + unifiedresources "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" ) func TestAgentEventBroadcaster_SubscribeReceivesPublishedEvents(t *testing.T) { @@ -610,6 +611,39 @@ func TestAgentEventBroadcaster_PublishActionCompletedRoundTripsVerification(t *t case <-time.After(time.Second): t.Fatal("timed out waiting for action.completed event") } + + recordBroadcaster := NewAgentEventBroadcaster() + recordEvents, recordUnsub := recordBroadcaster.Subscribe() + defer recordUnsub() + recordBroadcaster.PublishActionCompletedRecord(unifiedresources.ActionAuditRecord{ + ID: "action-verify-record", + UpdatedAt: ranAt, + State: unifiedresources.ActionStateCompleted, + Request: unifiedresources.ActionRequest{ + ResourceID: "container:web-1", + CapabilityName: "restart_service", + RequestedBy: "agent:ops", + }, + Result: &unifiedresources.ExecutionResult{Success: true}, + Verification: &unifiedresources.ActionVerificationResult{ + Ran: true, + Success: true, + Command: "systemctl is-active 'nginx'", + RanAt: ranAt, + }, + }) + select { + case event := <-recordEvents: + payload, ok := event.Payload.(AgentEventActionCompletedPayload) + if !ok { + t.Fatalf("record payload type: got %T", event.Payload) + } + if payload.Verification == nil || payload.Verification.Command != "systemctl is-active 'nginx'" { + t.Fatalf("record canonical verification did not project onto action.completed: %+v", payload.Verification) + } + case <-time.After(time.Second): + t.Fatal("timed out waiting for record-backed action.completed event") + } } func TestAgentEventBroadcaster_PublishActionCompletedAbsentVerificationOmitsField(t *testing.T) { diff --git a/internal/unifiedresources/actions.go b/internal/unifiedresources/actions.go index 9c83f8289..ce7d1cd71 100644 --- a/internal/unifiedresources/actions.go +++ b/internal/unifiedresources/actions.go @@ -171,17 +171,64 @@ func NormalizeVerificationOutcome(outcome VerificationOutcome) VerificationOutco return outcome } +// NormalizeActionVerificationResult applies the canonical verification field +// hygiene used by stored audit records and every action-audit projection. +func NormalizeActionVerificationResult(result *ActionVerificationResult) *ActionVerificationResult { + if result == nil { + return nil + } + normalized := *result + normalized.Command = strings.TrimSpace(normalized.Command) + normalized.Output = strings.TrimSpace(normalized.Output) + normalized.Note = strings.TrimSpace(normalized.Note) + if normalized.RanAt.IsZero() { + normalized.RanAt = time.Time{} + } else { + normalized.RanAt = normalized.RanAt.UTC() + } + if !normalized.Ran { + normalized.Command = "" + normalized.Output = "" + normalized.Success = false + normalized.RanAt = time.Time{} + normalized.Note = "" + } + return &normalized +} + +func cloneActionVerificationResult(result *ActionVerificationResult) *ActionVerificationResult { + if result == nil { + return nil + } + clone := *result + return &clone +} + +// CanonicalActionVerification returns the top-level verification projection for +// an audit record, falling back to legacy result.verification records while +// older persisted rows are still being read. +func CanonicalActionVerification(record ActionAuditRecord) *ActionVerificationResult { + if result := NormalizeActionVerificationResult(record.Verification); result != nil { + return result + } + if record.Result == nil { + return nil + } + return NormalizeActionVerificationResult(record.Result.Verification) +} + // ActionAuditRecord tracks the full end-to-end lifecycle of a tool invocation. type ActionAuditRecord struct { - ID string `json:"id"` - CreatedAt time.Time `json:"createdAt"` - UpdatedAt time.Time `json:"updatedAt"` - State ActionState `json:"state"` - Request ActionRequest `json:"request"` - Plan ActionPlan `json:"plan"` - Approvals []ActionApprovalRecord `json:"approvals,omitempty"` - Result *ExecutionResult `json:"result,omitempty"` - VerificationOutcome VerificationOutcome `json:"verificationOutcome"` + ID string `json:"id"` + CreatedAt time.Time `json:"createdAt"` + UpdatedAt time.Time `json:"updatedAt"` + State ActionState `json:"state"` + Request ActionRequest `json:"request"` + Plan ActionPlan `json:"plan"` + Approvals []ActionApprovalRecord `json:"approvals,omitempty"` + Result *ExecutionResult `json:"result,omitempty"` + Verification *ActionVerificationResult `json:"verification,omitempty"` + VerificationOutcome VerificationOutcome `json:"verificationOutcome"` } // ActionLifecycleEvent represents an append-only state transition in an action's life. @@ -555,6 +602,21 @@ func NormalizeActionAuditRecord(record ActionAuditRecord) (ActionAuditRecord, er } } record.Plan.Preflight = NormalizeActionPreflight(record.Plan.Preflight, record.Request, record.Plan) + + if record.Result != nil { + result := *record.Result + result.Output = strings.TrimSpace(result.Output) + result.ErrorMessage = strings.TrimSpace(result.ErrorMessage) + result.Verification = NormalizeActionVerificationResult(result.Verification) + record.Result = &result + } + record.Verification = NormalizeActionVerificationResult(record.Verification) + if record.Verification == nil && record.Result != nil { + record.Verification = cloneActionVerificationResult(record.Result.Verification) + } + if record.Verification != nil && record.Result != nil { + record.Result.Verification = cloneActionVerificationResult(record.Verification) + } record.VerificationOutcome = NormalizeVerificationOutcome(record.VerificationOutcome) for i := range record.Approvals { diff --git a/internal/unifiedresources/actions_test.go b/internal/unifiedresources/actions_test.go index 98154807b..f5cbbd7e9 100644 --- a/internal/unifiedresources/actions_test.go +++ b/internal/unifiedresources/actions_test.go @@ -51,6 +51,48 @@ func TestNormalizeActionAuditRecordPopulatesGovernedPlanPreflight(t *testing.T) if len(record.Plan.Preflight.SafetyChecks) == 0 || len(record.Plan.Preflight.VerificationSteps) == 0 { t.Fatalf("preflight should carry safety and verification checks: %#v", record.Plan.Preflight) } + + withLegacyVerification := record + withLegacyVerification.Result = &ExecutionResult{ + Success: true, + Verification: &ActionVerificationResult{ + Ran: true, + Command: " systemctl is-active 'nginx' ", + Output: " active\n", + Success: true, + RanAt: now.Add(time.Minute), + }, + } + withLegacyVerification, err = NormalizeActionAuditRecord(withLegacyVerification) + if err != nil { + t.Fatalf("NormalizeActionAuditRecord(legacy verification) error = %v", err) + } + if withLegacyVerification.Verification == nil || withLegacyVerification.Verification.Command != "systemctl is-active 'nginx'" || withLegacyVerification.Verification.Output != "active" { + t.Fatalf("canonical verification not populated from result verification: %#v", withLegacyVerification.Verification) + } + if withLegacyVerification.Result.Verification == nil || withLegacyVerification.Result.Verification.Command != withLegacyVerification.Verification.Command { + t.Fatalf("legacy verification not kept aligned: result=%#v canonical=%#v", withLegacyVerification.Result.Verification, withLegacyVerification.Verification) + } + + withUnrunVerification := record + withUnrunVerification.Verification = &ActionVerificationResult{ + Ran: false, + Command: "should not persist", + Output: "sensitive", + Success: true, + RanAt: now.Add(time.Minute), + Note: "details", + } + withUnrunVerification, err = NormalizeActionAuditRecord(withUnrunVerification) + if err != nil { + t.Fatalf("NormalizeActionAuditRecord(unrun verification) error = %v", err) + } + if withUnrunVerification.Verification == nil || withUnrunVerification.Verification.Ran { + t.Fatalf("expected canonical ran=false verification, got %#v", withUnrunVerification.Verification) + } + if withUnrunVerification.Verification.Command != "" || withUnrunVerification.Verification.Output != "" || withUnrunVerification.Verification.Note != "" || !withUnrunVerification.Verification.RanAt.IsZero() { + t.Fatalf("ran=false verification must not retain details: %#v", withUnrunVerification.Verification) + } } func TestNormalizeActionAuditRecordRejectsUngovernedRecords(t *testing.T) { @@ -365,13 +407,26 @@ func TestCompleteActionExecutionRecordsResult(t *testing.T) { }, } - updated, event, err := CompleteActionExecution(record, &ExecutionResult{Success: true, Output: "done"}, "operator@example.com", now) + updated, event, err := CompleteActionExecution(record, &ExecutionResult{ + Success: true, + Output: "done", + Verification: &ActionVerificationResult{ + Ran: true, + Command: "systemctl is-active 'nginx'", + Output: "active", + Success: true, + RanAt: now, + }, + }, "operator@example.com", now) if err != nil { t.Fatalf("CompleteActionExecution: %v", err) } if updated.State != ActionStateCompleted || updated.Result == nil || updated.Result.Output != "done" { t.Fatalf("completed action = %#v", updated) } + if updated.Verification == nil || !updated.Verification.Ran || updated.Verification.Command != "systemctl is-active 'nginx'" { + t.Fatalf("completed action verification = %#v", updated.Verification) + } if event.State != ActionStateCompleted || event.Message != "Action execution completed." { t.Fatalf("completed event = %#v", event) } diff --git a/internal/unifiedresources/store_test.go b/internal/unifiedresources/store_test.go index d8c01a728..50daf2ad1 100644 --- a/internal/unifiedresources/store_test.go +++ b/internal/unifiedresources/store_test.go @@ -1202,7 +1202,17 @@ func TestActionAuditRecord_RoundTrip(t *testing.T) { now := time.Date(2026, 3, 18, 13, 0, 0, 0, time.UTC) expires := now.Add(15 * time.Minute) approvedAt := now.Add(2 * time.Minute) - result := &ExecutionResult{Success: true, Output: "completed"} + result := &ExecutionResult{ + Success: true, + Output: "completed", + Verification: &ActionVerificationResult{ + Ran: true, + Command: "systemctl is-active 'nginx'", + Output: "active", + Success: true, + RanAt: now.Add(4 * time.Minute), + }, + } record := ActionAuditRecord{ ID: "action-1", @@ -1274,6 +1284,13 @@ func TestActionAuditRecord_RoundTrip(t *testing.T) { if got.Result == nil || !got.Result.Success || got.Result.Output != result.Output { t.Fatalf("result round-trip failed: %+v", got.Result) } + verification := CanonicalActionVerification(got) + if verification == nil || !verification.Ran || verification.Command != result.Verification.Command { + t.Fatalf("canonical verification round-trip failed: %+v", verification) + } + if got.Result.Verification == nil || got.Result.Verification.Command != verification.Command { + t.Fatalf("result verification did not stay aligned with canonical verification: result=%+v canonical=%+v", got.Result.Verification, verification) + } } func TestMemoryStore_RecordActionAudit_UpsertsByID(t *testing.T) {