[object Object]

← back to La Socrata Ingester

fix(la-data): scope allowFailure to the audited failure, not the source name

f6042cdfb05d4f05ecae973f9ab432246c444eb3 · 2026-09-10 08:01:31 -0700 · Steve

Contrarian review (TK-10955) found that the identity guard shipped in 88e40a8 was
silently neutered for the one source that carries an exemption. classifyRunResults()
tolerated ANY failure on a source whose allowFailure.scope matched, keyed purely on the
source NAME and never on the cause. So a future NavigateLA re-index that drifted zoning's
layer 72 would throw "ArcGIS layer identity drift" -> land in the tolerated bucket ->
exit 0, and the cron exit code (the one alarm that actually caught the 2026-09-05
incident) would never fire for the exact failure class the guard exists to make loud.

- allowFailure gains an optional `match` array; a failure is tolerated only if its error
  text contains one of the audited signatures. Omitting `match` keeps prior behaviour.
- gis_zoning pins match to its audited gateway degradation in BOTH error shapes:
  the HTTP-level form ("HTTP 502 after 5 tries") and the ArcGIS body-level form
  ("ArcGIS error: {"code":502,...}").
- test/run-policy.test.mjs: the audited 502 still exits 0; identity drift exits 1;
  asserted against the live SOURCES config, not just a mock.

This is closer to the ratified 2026-08-30 DTD intent ("exit 1 only for unexpected
failures") than the previous blanket-tolerate: a genuine 400 on gis_zoning was also
being swallowed and now correctly exits 1.

TK-10955

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019LHiEXMR2FCqRqUmoK4kQY

Files touched

Diff

commit f6042cdfb05d4f05ecae973f9ab432246c444eb3
Author: Steve <steve@designerwallcoverings.com>
Date:   Thu Sep 10 08:01:31 2026 -0700

    fix(la-data): scope allowFailure to the audited failure, not the source name
    
    Contrarian review (TK-10955) found that the identity guard shipped in 88e40a8 was
    silently neutered for the one source that carries an exemption. classifyRunResults()
    tolerated ANY failure on a source whose allowFailure.scope matched, keyed purely on the
    source NAME and never on the cause. So a future NavigateLA re-index that drifted zoning's
    layer 72 would throw "ArcGIS layer identity drift" -> land in the tolerated bucket ->
    exit 0, and the cron exit code (the one alarm that actually caught the 2026-09-05
    incident) would never fire for the exact failure class the guard exists to make loud.
    
    - allowFailure gains an optional `match` array; a failure is tolerated only if its error
      text contains one of the audited signatures. Omitting `match` keeps prior behaviour.
    - gis_zoning pins match to its audited gateway degradation in BOTH error shapes:
      the HTTP-level form ("HTTP 502 after 5 tries") and the ArcGIS body-level form
      ("ArcGIS error: {"code":502,...}").
    - test/run-policy.test.mjs: the audited 502 still exits 0; identity drift exits 1;
      asserted against the live SOURCES config, not just a mock.
    
    This is closer to the ratified 2026-08-30 DTD intent ("exit 1 only for unexpected
    failures") than the previous blanket-tolerate: a genuine 400 on gis_zoning was also
    being swallowed and now correctly exits 1.
    
    TK-10955
    
    Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
    Claude-Session: https://claude.ai/code/session_019LHiEXMR2FCqRqUmoK4kQY
---
 src/run-policy.js        | 18 +++++++++++++++---
 src/sources.js           |  6 ++++++
 test/run-policy.test.mjs | 31 +++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/src/run-policy.js b/src/run-policy.js
index 7e9e09d..3269b97 100644
--- a/src/run-policy.js
+++ b/src/run-policy.js
@@ -4,9 +4,21 @@
 export function classifyRunResults(results, sources, targetToken) {
   const failures = results.filter((result) => result.error);
   const aggregateAll = targetToken === 'all';
-  const tolerated = aggregateAll
-    ? failures.filter((result) => sources[result.name]?.allowFailure?.scope === 'aggregate-all-only')
-    : [];
+  // An allowFailure exemption must be scoped to the SPECIFIC audited failure, not to the
+  // source name. Otherwise it silences EVERY future error on that source — including an
+  // ArcGIS layer-identity drift, which is exactly the class the identity guard exists to
+  // make loud (TK-10955: upstream re-indexed and 7 sources silently ingested the wrong
+  // layer for 6 days). `match` is an array of substrings; a failure is tolerated only if
+  // its error text contains one of them. Omitting `match` keeps the older, broader
+  // behaviour for back-compat.
+  const isAuditedFailure = (result) => {
+    const allow = sources[result.name]?.allowFailure;
+    if (allow?.scope !== 'aggregate-all-only') return false;
+    if (!Array.isArray(allow.match) || allow.match.length === 0) return true;
+    const text = String(result.error ?? '');
+    return allow.match.some((needle) => text.includes(needle));
+  };
+  const tolerated = aggregateAll ? failures.filter(isAuditedFailure) : [];
   const toleratedNames = new Set(tolerated.map((result) => result.name));
   const unexpected = failures.filter((result) => !toleratedNames.has(result.name));
 
diff --git a/src/sources.js b/src/sources.js
index 22bd085..5c59794 100644
--- a/src/sources.js
+++ b/src/sources.js
@@ -222,6 +222,12 @@ Object.assign(SOURCES, {
       since: '2026-08-11',
       scope: 'aggregate-all-only',
       reason: 'NavigateLA gateway consistently HTTP 502s on geometry pagination near offset 42000',
+      // Scoped to the AUDITED failure only. Without this, the exemption would also swallow
+      // an 'ArcGIS layer identity drift' throw — silencing, for the one source that carries
+      // an exemption, precisely the alarm the identity guard exists to raise (TK-10955).
+      // Both error shapes: the HTTP-level form from fetchJson ("HTTP 502 after 5 tries")
+      // and the ArcGIS body-level form ("ArcGIS error: {\"code\":502,...}").
+      match: ['HTTP 502', 'HTTP 503', 'HTTP 504', '"code":502', '"code":503', '"code":504'],
     },
   }),
   // Land use: hosted LADCP FeatureServer (reliable) instead of the flaky NavigateLA
diff --git a/test/run-policy.test.mjs b/test/run-policy.test.mjs
index 2ab9106..867414d 100644
--- a/test/run-policy.test.mjs
+++ b/test/run-policy.test.mjs
@@ -59,3 +59,34 @@ assert.equal(mockedProcessStatus(knownFailure, 'all'), 0, 'known aggregate failu
 assert.equal(mockedProcessStatus(unexpectedFailure, 'all'), 1, 'unexpected aggregate failure OS exit must be 1');
 assert.equal(mockedProcessStatus(knownFailure, 'gis_zoning'), 1, 'direct known-source failure OS exit must be 1');
 console.log('PASS process boundary: mocked no-DB child exits 0/1/1 as required');
+
+// ── TK-10955 (contrarian hole #2): an allowFailure exemption must be scoped to the
+// AUDITED failure, not to the source name. Otherwise the one source carrying an
+// exemption is the one source whose layer-identity drift is silently swallowed.
+const zoningLike = {
+  gis_zoning: {
+    allowFailure: { scope: 'aggregate-all-only', match: ['HTTP 502', 'HTTP 503', 'HTTP 504'] },
+  },
+};
+const audited502 = classifyRunResults(
+  [{ name: 'gis_zoning', error: 'HTTP 502 after 5 tries: https://maps.lacity.org/...' }],
+  zoningLike, 'all'
+);
+assert.equal(audited502.exitCode, 0, 'the audited 502 must stay tolerated');
+
+const drift = classifyRunResults(
+  [{ name: 'gis_zoning', error: 'ArcGIS layer identity drift: .../MapServer/72 is now "Foo" but this source expects "Generalized Zoning".' }],
+  zoningLike, 'all'
+);
+assert.equal(drift.exitCode, 1, 'layer-identity drift must NOT be swallowed by the 502 exemption');
+assert.deepEqual(drift.unexpected.map((r) => r.name), ['gis_zoning']);
+assert.equal(drift.tolerated.length, 0);
+
+// Real config must actually carry the scoping (not just the mock).
+assert.ok(Array.isArray(SOURCES.gis_zoning.allowFailure.match), 'gis_zoning must scope its exemption');
+assert.equal(
+  classifyRunResults([{ name: 'gis_zoning', error: 'ArcGIS layer identity drift: ...' }], SOURCES, 'all').exitCode,
+  1, 'live SOURCES config must also refuse to tolerate identity drift'
+);
+
+console.log('PASS allowFailure is scoped to the audited failure; identity drift still exits 1');

← 88e40a8 fix(la-data): repoint 9 NavigateLA layers after upstream +1  ·  back to La Socrata Ingester  ·  fix(la-data): repoint 9 NavigateLA layers after a SECOND ups 314f1dc →