feat: tighten decision record storage contract

2026-05-08 13:30:17 +08:00
parent 30cb206438
commit a440187962
2 changed files with 216 additions and 42 deletions
--- a/plugins/reporting-governance/src/storage/decision-artifact.mjs
+++ b/plugins/reporting-governance/src/storage/decision-artifact.mjs
@@ -2,6 +2,16 @@ import crypto from 'node:crypto';

 const EXPECTED_KIND = 'DecisionRecordArtifact';
 const EXPECTED_API_VERSION = 'reporting-governance/v1alpha1';
+const CANONICAL_TIMESTAMP_PATTERN = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/;
+const SOURCE_EVENT_ID_PATTERN = /^(?:evt_[A-Za-z0-9._:-]+|\/?[A-Za-z0-9._-]+(?:\/[A-Za-z0-9._-]+)+|[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12})$/i;
+const RECEIPT_DELIVERY_STATES = new Set(['prepared', 'queued', 'dispatched', 'pending_external_send', 'acked', 'blocked', 'failed', 'partial']);
+const RECEIPT_STATUS_RULES = {
+  planned: ['prepared', 'queued', 'dispatched', 'pending_external_send', 'partial'],
+  acked: ['acked'],
+  blocked: ['blocked'],
+  degraded: ['partial', 'failed', 'blocked'],
+  failed: ['failed']
+};

 function assertNonEmptyString(value, label) {
  if (typeof value !== 'string' || value.trim() === '') {
@@ -28,14 +38,71 @@ function sanitizeFileSegment(value, fallback) {
  return normalized || fallback;
 }

-function assertIsoTimestamp(value, label) {
+function assertCanonicalIsoTimestamp(value, label) {
  const normalized = assertNonEmptyString(value, label);
-  if (Number.isNaN(Date.parse(normalized))) {
-    throw new Error(`${label} must be a valid ISO-8601 timestamp`);
+  if (!CANONICAL_TIMESTAMP_PATTERN.test(normalized) || Number.isNaN(Date.parse(normalized))) {
+    throw new Error(`${label} must be a canonical ISO-8601 UTC timestamp with millisecond precision`);
  }
  return normalized;
 }

+function assertSourceEventId(value, label) {
+  const normalized = assertNonEmptyString(value, label);
+  if (!SOURCE_EVENT_ID_PATTERN.test(normalized)) {
+    throw new Error(`${label} must be a stable event reference (evt_* | path-like artifact ref | UUID)`);
+  }
+  return normalized;
+}
+
+function validateReceiptContract(receipt) {
+  const deliveryState = assertNonEmptyString(receipt.delivery_state, 'decision record artifact spec.receipt.delivery_state');
+  if (!RECEIPT_DELIVERY_STATES.has(deliveryState)) {
+    throw new Error(`decision record artifact spec.receipt.delivery_state must be one of: ${Array.from(RECEIPT_DELIVERY_STATES).join(', ')}`);
+  }
+
+  const status = assertNonEmptyString(receipt.status, 'decision record artifact spec.receipt.status');
+  const allowedStates = RECEIPT_STATUS_RULES[status];
+  if (!allowedStates) {
+    throw new Error(`decision record artifact spec.receipt.status must be one of: ${Object.keys(RECEIPT_STATUS_RULES).join(', ')}`);
+  }
+  if (!allowedStates.includes(deliveryState)) {
+    throw new Error(`decision record artifact spec.receipt.status=${status} is incompatible with delivery_state=${deliveryState}`);
+  }
+
+  if (receipt.operator_notice_required != null && typeof receipt.operator_notice_required !== 'boolean') {
+    throw new Error('decision record artifact spec.receipt.operator_notice_required must be a boolean when present');
+  }
+  if (!Array.isArray(receipt.enforcement_intent)) {
+    throw new Error('decision record artifact spec.receipt.enforcement_intent must be an array');
+  }
+  if (!Array.isArray(receipt.blocked_actions)) {
+    throw new Error('decision record artifact spec.receipt.blocked_actions must be an array');
+  }
+  if (!Array.isArray(receipt.notes)) {
+    throw new Error('decision record artifact spec.receipt.notes must be an array');
+  }
+
+  if (status === 'failed') {
+    const failureReason = receipt.failure_reason == null
+      ? null
+      : assertNonEmptyString(receipt.failure_reason, 'decision record artifact spec.receipt.failure_reason');
+    if (!failureReason) {
+      throw new Error('decision record artifact spec.receipt.failure_reason is required when status=failed');
+    }
+  }
+
+  if (deliveryState === 'partial') {
+    if (receipt.blocked_actions.length === 0) {
+      throw new Error('decision record artifact spec.receipt.blocked_actions must be non-empty when delivery_state=partial');
+    }
+    if (receipt.enforcement_intent.length === 0) {
+      throw new Error('decision record artifact spec.receipt.enforcement_intent must be non-empty when delivery_state=partial');
+    }
+  }
+
+  return { deliveryState, status };
+}
+
 export function validateDecisionRecordArtifact(artifact) {
  if (!artifact || typeof artifact !== 'object' || Array.isArray(artifact)) {
    throw new Error('decision record artifact must be an object');
@@ -53,12 +120,12 @@ export function validateDecisionRecordArtifact(artifact) {
  const receipt = assertObjectRecord(spec.receipt, 'decision record artifact spec.receipt');
  const source = spec.source == null ? null : assertObjectRecord(spec.source, 'decision record artifact spec.source');

-  const recordedAt = assertIsoTimestamp(metadata.recorded_at, 'decision record artifact metadata.recorded_at');
+  const recordedAt = assertCanonicalIsoTimestamp(metadata.recorded_at, 'decision record artifact metadata.recorded_at');
  const metadataPolicyId = assertNonEmptyString(metadata.policy_id, 'decision record artifact metadata.policy_id');
  const metadataDecision = assertNonEmptyString(metadata.decision, 'decision record artifact metadata.decision');
  const decisionPolicyId = assertNonEmptyString(decision.policy_id, 'decision record artifact spec.decision.policy_id');
  const decisionName = assertNonEmptyString(decision.decision, 'decision record artifact spec.decision.decision');
-  assertNonEmptyString(receipt.delivery_state, 'decision record artifact spec.receipt.delivery_state');
+  const { deliveryState, status } = validateReceiptContract(receipt);

  if (metadataPolicyId !== decisionPolicyId) {
    throw new Error('decision record artifact policy_id mismatch between metadata and spec.decision');
@@ -66,8 +133,17 @@ export function validateDecisionRecordArtifact(artifact) {
  if (metadataDecision !== decisionName) {
    throw new Error('decision record artifact decision mismatch between metadata and spec.decision');
  }
-  if (source?.event_id != null) {
-    assertNonEmptyString(source.event_id, 'decision record artifact spec.source.event_id');
+
+  const metadataEventId = metadata.event_id == null ? null : assertSourceEventId(metadata.event_id, 'decision record artifact metadata.event_id');
+  const sourceEventId = source?.event_id == null ? null : assertSourceEventId(source.event_id, 'decision record artifact spec.source.event_id');
+  if ((metadataEventId == null) !== (sourceEventId == null)) {
+    throw new Error('decision record artifact event_id must be present in both metadata and spec.source when either side declares it');
+  }
+  if (metadataEventId != null && sourceEventId != null && metadataEventId !== sourceEventId) {
+    throw new Error('decision record artifact event_id mismatch between metadata and spec.source');
+  }
+  if (source != null && source.task_id == null && source.correlation_id == null && sourceEventId == null) {
+    throw new Error('decision record artifact spec.source must declare at least one linkage field');
  }
  if (source?.task_id != null) {
    const sourceTaskId = assertNonEmptyString(source.task_id, 'decision record artifact spec.source.task_id');
@@ -85,8 +161,14 @@ export function validateDecisionRecordArtifact(artifact) {
  metadata.recorded_at = recordedAt;
  metadata.policy_id = metadataPolicyId;
  metadata.decision = metadataDecision;
+  metadata.event_id = metadataEventId;
  decision.policy_id = decisionPolicyId;
  decision.decision = decisionName;
+  receipt.delivery_state = deliveryState;
+  receipt.status = status;
+  if (source != null) {
+    source.event_id = sourceEventId;
+  }

  return artifact;
 }
@@ -102,7 +184,7 @@ export function createDecisionRecordArtifact({ decision, receipt, recordedAt = n
    apiVersion: EXPECTED_API_VERSION,
    metadata: {
      record_id: `dec_${crypto.randomUUID()}`,
-      recorded_at: assertNonEmptyString(recordedAt, 'recordedAt'),
+      recorded_at: assertCanonicalIsoTimestamp(recordedAt, 'recordedAt'),
      policy_id: policyId,
      decision: decisionName,
      correlation_id: source.correlation_id ?? null,
@@ -135,4 +217,8 @@ export const __testables = {
  EXPECTED_KIND,
  EXPECTED_API_VERSION,
  sanitizeFileSegment,
+  CANONICAL_TIMESTAMP_PATTERN,
+  SOURCE_EVENT_ID_PATTERN,
+  RECEIPT_DELIVERY_STATES,
+  RECEIPT_STATUS_RULES,
 };
--- a/plugins/reporting-governance/test/decision-store.test.mjs
+++ b/plugins/reporting-governance/test/decision-store.test.mjs
@@ -59,18 +59,22 @@ function createPlannedDecision() {
  });
 }

-test('decision artifact validates minimal package-owned contract', () => {
+function createArtifact(overrides = {}) {
  const planned = createPlannedDecision();
-  const artifact = createDecisionRecordArtifact({
-    decision: planned.decision,
-    receipt: planned.receipt,
-    recordedAt: '2026-05-08T04:00:00.000Z',
-    source: {
+  return createDecisionRecordArtifact({
+    decision: overrides.decision ?? planned.decision,
+    receipt: overrides.receipt ?? planned.receipt,
+    recordedAt: overrides.recordedAt ?? '2026-05-08T04:00:00.000Z',
+    source: overrides.source ?? {
      event_id: 'evt_watchdog_001',
      task_id: 'task-reporting-governance',
      correlation_id: 'corr-001',
    },
  });
+}
+
+test('decision artifact validates minimal package-owned contract', () => {
+  const artifact = createArtifact();

  assert.equal(artifact.kind, 'DecisionRecordArtifact');
  assert.equal(artifact.apiVersion, 'reporting-governance/v1alpha1');
@@ -82,12 +86,7 @@ test('decision artifact validates minimal package-owned contract', () => {
 test('decision artifact validator rejects malformed top-level or nested fields', () => {
  assert.throws(() => validateDecisionRecordArtifact(null), /decision record artifact must be an object/);

-  const planned = createPlannedDecision();
-  const artifact = createDecisionRecordArtifact({
-    decision: planned.decision,
-    receipt: planned.receipt,
-    recordedAt: '2026-05-08T04:00:00.000Z',
-  });
+  const artifact = createArtifact({ source: { task_id: 'task-reporting-governance' } });

  const wrongKind = structuredClone(artifact);
  wrongKind.kind = 'NotDecisionRecordArtifact';
@@ -103,12 +102,9 @@ test('decision artifact validator rejects malformed top-level or nested fields',
 });

 test('decision artifact validator enforces metadata and spec consistency', () => {
-  const planned = createPlannedDecision();
-  const artifact = createDecisionRecordArtifact({
-    decision: planned.decision,
-    receipt: planned.receipt,
-    recordedAt: '2026-05-08T04:00:00.000Z',
+  const artifact = createArtifact({
    source: {
+      event_id: 'evt_watchdog_001',
      task_id: 'task-reporting-governance',
      correlation_id: 'corr-001',
    },
@@ -131,25 +127,100 @@ test('decision artifact validator enforces metadata and spec consistency', () =>
  assert.throws(() => validateDecisionRecordArtifact(mismatchedCorrelation), /correlation_id mismatch between metadata and spec\.source/);
 });

-test('decision artifact validator rejects malformed recorded_at timestamp', () => {
-  const planned = createPlannedDecision();
-  const artifact = createDecisionRecordArtifact({
-    decision: planned.decision,
-    receipt: planned.receipt,
-    recordedAt: '2026-05-08T04:00:00.000Z',
+test('decision artifact validator rejects non-canonical recorded_at timestamp', () => {
+  const artifact = createArtifact({ source: { task_id: 'task-reporting-governance' } });
+
+  artifact.metadata.recorded_at = '2026-05-08T12:00:00+08:00';
+  assert.throws(() => validateDecisionRecordArtifact(artifact), /recorded_at must be a canonical ISO-8601 UTC timestamp with millisecond precision/);
 });

-  artifact.metadata.recorded_at = 'not-a-timestamp';
-  assert.throws(() => validateDecisionRecordArtifact(artifact), /recorded_at must be a valid ISO-8601 timestamp/);
+test('decision artifact validator rejects invalid or mismatched event source linkage', () => {
+  const artifact = createArtifact();
+
+  const missingSourceEvent = structuredClone(artifact);
+  missingSourceEvent.spec.source.event_id = null;
+  assert.throws(() => validateDecisionRecordArtifact(missingSourceEvent), /event_id must be present in both metadata and spec\.source/);
+
+  const mismatchedSourceEvent = structuredClone(artifact);
+  mismatchedSourceEvent.spec.source.event_id = 'evt_watchdog_002';
+  assert.throws(() => validateDecisionRecordArtifact(mismatchedSourceEvent), /event_id mismatch between metadata and spec\.source/);
+
+  const malformedSourceEvent = structuredClone(artifact);
+  malformedSourceEvent.spec.source.event_id = '   ';
+  assert.throws(() => validateDecisionRecordArtifact(malformedSourceEvent), /spec\.source\.event_id must be a non-empty string/);
+});
+
+test('decision artifact validator accepts stable path-like event refs used by runtime artifacts', () => {
+  const artifact = createArtifact({
+    source: {
+      event_id: 'state/long-task-watchdog-events/2026-05-07T142633Z-reporting-governance-plugin-watchdog-watchdog-fired.json',
+      task_id: 'task-reporting-governance',
+      correlation_id: 'corr-001',
+    },
+  });
+
+  assert.equal(validateDecisionRecordArtifact(artifact), artifact);
+  assert.equal(artifact.metadata.event_id, 'state/long-task-watchdog-events/2026-05-07T142633Z-reporting-governance-plugin-watchdog-watchdog-fired.json');
+});
+
+test('decision artifact validator enforces receipt status enum and required fields for pending failed and partial states', () => {
+  const pendingArtifact = createArtifact();
+  pendingArtifact.spec.receipt.status = 'planned';
+  pendingArtifact.spec.receipt.delivery_state = 'pending_external_send';
+  assert.equal(validateDecisionRecordArtifact(pendingArtifact), pendingArtifact);
+
+  const failedArtifact = createArtifact({
+    receipt: {
+      policy_id: 'no-silence.missed-checkpoint',
+      decision: 'force_checkpoint',
+      status: 'failed',
+      delivery_state: 'failed',
+      enforcement_intent: [],
+      blocked_actions: [],
+      operator_notice_required: true,
+      notes: ['sender failed'],
+      failure_reason: 'sender_exit_nonzero'
+    }
+  });
+  assert.equal(validateDecisionRecordArtifact(failedArtifact), failedArtifact);
+
+  const failedMissingReason = structuredClone(failedArtifact);
+  delete failedMissingReason.spec.receipt.failure_reason;
+  assert.throws(() => validateDecisionRecordArtifact(failedMissingReason), /failure_reason is required when status=failed/);
+
+  const partialArtifact = createArtifact({
+    receipt: {
+      policy_id: 'no-silence.missed-checkpoint',
+      decision: 'force_checkpoint',
+      status: 'degraded',
+      delivery_state: 'partial',
+      enforcement_intent: [{ action: 'emit_event', mandatory: true }],
+      blocked_actions: [{ action: 'notify_operator', mandatory: true, reason: 'binding unavailable' }],
+      operator_notice_required: true,
+      notes: ['event emitted but notice not sent']
+    }
+  });
+  assert.equal(validateDecisionRecordArtifact(partialArtifact), partialArtifact);
+
+  const partialMissingBlocked = structuredClone(partialArtifact);
+  partialMissingBlocked.spec.receipt.blocked_actions = [];
+  assert.throws(() => validateDecisionRecordArtifact(partialMissingBlocked), /blocked_actions must be non-empty when delivery_state=partial/);
+});
+
+test('decision artifact validator rejects unsupported receipt state combinations', () => {
+  const artifact = createArtifact();
+
+  artifact.spec.receipt.status = 'acked';
+  artifact.spec.receipt.delivery_state = 'pending_external_send';
+  assert.throws(() => validateDecisionRecordArtifact(artifact), /status=acked is incompatible with delivery_state=pending_external_send/);
+
+  const badStatus = createArtifact();
+  badStatus.spec.receipt.status = 'mystery';
+  assert.throws(() => validateDecisionRecordArtifact(badStatus), /spec\.receipt\.status must be one of/);
 });

 test('decision artifact filename is stable and readable', () => {
-  const planned = createPlannedDecision();
-  const artifact = createDecisionRecordArtifact({
-    decision: planned.decision,
-    receipt: planned.receipt,
-    recordedAt: '2026-05-08T04:00:00.000Z',
-  });
+  const artifact = createArtifact({ source: { task_id: 'task-reporting-governance' } });

  const fileName = createDecisionRecordFileName(artifact);
  assert.match(fileName, /^2026-05-08T04-00-00-000Z-no-silence\.missed-checkpoint-force_checkpoint-dec_[a-f0-9-]+\.json$/);
@@ -166,7 +237,7 @@ test('decision artifact filename sanitizes unsafe policy and decision segments',
      decision: 'force checkpoint!',
      correlation_id: null,
      task_id: null,
-      event_id: null,
+      event_id: 'evt_1234',
    },
    spec: {
      decision: {
@@ -174,10 +245,17 @@ test('decision artifact filename sanitizes unsafe policy and decision segments',
        decision: 'force checkpoint!',
      },
      receipt: {
+        policy_id: '../policy with spaces?',
+        decision: 'force checkpoint!',
+        status: 'planned',
        delivery_state: 'pending_external_send',
+        enforcement_intent: [],
+        blocked_actions: [],
+        operator_notice_required: false,
+        notes: [],
      },
      source: {
-        event_id: null,
+        event_id: 'evt_1234',
        task_id: null,
        correlation_id: null,
      },
@@ -251,6 +329,7 @@ test('file decision store load rejects corrupted or malformed artifacts', async
      recorded_at: '',
      policy_id: 'no-silence.missed-checkpoint',
      decision: 'force_checkpoint',
+      event_id: 'evt_watchdog_001',
    },
    spec: {
      decision: {
@@ -258,10 +337,17 @@ test('file decision store load rejects corrupted or malformed artifacts', async
        decision: 'force_checkpoint',
      },
      receipt: {
+        policy_id: 'no-silence.missed-checkpoint',
+        decision: 'force_checkpoint',
+        status: 'planned',
        delivery_state: 'pending_external_send',
+        enforcement_intent: [],
+        blocked_actions: [],
+        operator_notice_required: false,
+        notes: [],
      },
      source: {
-        event_id: null,
+        event_id: 'evt_watchdog_001',
        task_id: null,
        correlation_id: null,
      },
@@ -287,11 +373,13 @@ test('file decision store produces distinct filenames for same decision written
    decision: planned.decision,
    receipt: planned.receipt,
    recordedAt: '2026-05-08T04:00:00.000Z',
+    source: { task_id: 'task-reporting-governance' }
  });
  const second = store.write({
    decision: planned.decision,
    receipt: planned.receipt,
    recordedAt: '2026-05-08T04:00:00.000Z',
+    source: { task_id: 'task-reporting-governance' }
  });

  assert.notEqual(first.artifact.metadata.record_id, second.artifact.metadata.record_id);