From 68b11282a50261597d2f33ffc97eb2eca9a8ecfe Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Sun, 20 Sep 2026 15:50:43 -0400 Subject: [PATCH] fix(relay): let the rehome evidence parser read a line the director grew (#21823) The enable workflow reads the director's `[orca-relay] regional rehome inventory` line out of Cloud Logging and pins the whole line with one regex. Adding `hostNotArrivedLast24Hours` in #21813 made every healthy line stop matching, so "Read fresh aggregate completion and abort evidence" threw "no aggregate regional rehome inventory evidence" and the fail-closed step disabled the durable switch at control generation 26. The parser now requires the six original fields and tolerates further ones in any order. Extra fields stay fenced by value shape rather than by pinning the whole line: a field must be a bare name and a non-negative integer or `none`, so `hostId=someone` is still not a counter and cannot ride along. An absent count reads as null, not zero, because an older director not reporting leaks is not the same as reporting none. `hostNotArrivedLast24Hours` and `oldestActiveAgeMs` now reach the evidence JSON and the operator step summary. Two guards close the chain, each verified to fail on the regression it exists for: a census in the relay package feeds the real formatter's output to the real parser, and a script-side test pins the parser's output to the fields the workflow summary renders. Claude-Session: https://claude.ai/session/ced32ebb-7155-4413-adad-1eccd14c2010 --- ...ud-operate-relay-production-rehome-job.yml | 2 +- ...ional-rehome-inventory-line-census.test.ts | 163 ++++++++++++++++++ .../relay-rehome-aggregate-evidence.mjs | 59 ++++++- .../relay-rehome-aggregate-evidence.test.mjs | 97 +++++++++++ 4 files changed, 311 insertions(+), 10 deletions(-) create mode 100644 cloud/apps/relay/src/regional-rehome-inventory-line-census.test.ts diff --git a/.github/workflows/cloud-operate-relay-production-rehome-job.yml b/.github/workflows/cloud-operate-relay-production-rehome-job.yml index a34552b898f..b16132d9ba5 100644 --- a/.github/workflows/cloud-operate-relay-production-rehome-job.yml +++ b/.github/workflows/cloud-operate-relay-production-rehome-job.yml @@ -320,7 +320,7 @@ jobs: echo '### Regional rehome control' jq -r '"- mode: `\(.mode)`\n- generation: `\(.control.generation)`\n- enabled: `\(.control.enabled)`"' \ "${RUNNER_TEMP}/relay-rehome-control.json" - jq -r '"- active: `\(.active)`\n- awaiting receipt: `\(.awaitingReceipt)`\n- target registered: `\(.targetRegistered)`\n- completed (24h): `\(.completedLast24Hours)`\n- aborted (24h): `\(.abortedLast24Hours)`"' \ + jq -r '"- active: `\(.active)`\n- awaiting receipt: `\(.awaitingReceipt)`\n- target registered: `\(.targetRegistered)`\n- completed (24h): `\(.completedLast24Hours)`\n- aborted (24h): `\(.abortedLast24Hours)`\n- host not arrived (24h): `\(.hostNotArrivedLast24Hours // "not reported")`\n- oldest active age (ms): `\(.oldestActiveAgeMs // "none")`"' \ "${RUNNER_TEMP}/relay-rehome-inventory.json" } >> "${GITHUB_STEP_SUMMARY}" diff --git a/cloud/apps/relay/src/regional-rehome-inventory-line-census.test.ts b/cloud/apps/relay/src/regional-rehome-inventory-line-census.test.ts new file mode 100644 index 00000000000..6e0b00a263c --- /dev/null +++ b/cloud/apps/relay/src/regional-rehome-inventory-line-census.test.ts @@ -0,0 +1,163 @@ +import { describe, expect, it } from 'vitest' +import { + formatAssignmentInventorySnapshot, + type AssignmentInventorySnapshot +} from './assignment-inventory-snapshot.js' + +// The enable workflow reads the director's rehome inventory line out of Cloud +// Logging with a parser that lives in another language, in another package, and +// is never exercised against the formatter that writes the line. When +// `hostNotArrivedLast24Hours` shipped, the parser read a healthy line as no +// evidence at all and the run failed closed, disabling the durable switch. This +// test is the missing edge: the real formatter's output, through the real +// parser, so a future field fails here instead of in an operator's run. + +type InventoryEvidence = { + active: number + awaitingReceipt: number + targetRegistered: number + completedLast24Hours: number + abortedLast24Hours: number + hostNotArrivedLast24Hours: number | null + oldestActiveAgeMs: number | null +} + +// Imported through a computed URL on purpose: the script is plain ESM outside +// this package's compile scope, so a static import would not resolve. +async function loadParser(): Promise<(entries: unknown[], options: unknown) => InventoryEvidence> { + const source = new URL( + '../../../dev/scripts/relay-rehome-aggregate-evidence.mjs', + import.meta.url + ).href + const loaded: unknown = await import(/* @vite-ignore */ source) + if (!(loaded !== null && typeof loaded === 'object' && 'parseRegionalRehomeInventory' in loaded)) { + throw new Error('relay-rehome-aggregate-evidence.mjs no longer exports its parser') + } + const parse = loaded.parseRegionalRehomeInventory + if (typeof parse !== 'function') throw new Error('parseRegionalRehomeInventory is not callable') + return (entries, options) => readEvidence(parse(entries, options)) +} + +function readEvidence(value: unknown): InventoryEvidence { + if (value === null || typeof value !== 'object') throw new Error('parser returned no evidence') + const counts = ['active', 'awaitingReceipt', 'targetRegistered', 'completedLast24Hours', 'abortedLast24Hours'] as const + const evidence: Record = {} + for (const key of [...counts, 'hostNotArrivedLast24Hours', 'oldestActiveAgeMs'] as const) { + if (!(key in value)) throw new Error(`parser dropped ${key}`) + const read: unknown = Reflect.get(value, key) + if (read !== null && typeof read !== 'number') throw new Error(`${key} is not a count`) + evidence[key] = read + } + for (const key of counts) { + if (evidence[key] === null) throw new Error(`${key} must be a number`) + } + return { + active: Number(evidence['active']), + awaitingReceipt: Number(evidence['awaitingReceipt']), + targetRegistered: Number(evidence['targetRegistered']), + completedLast24Hours: Number(evidence['completedLast24Hours']), + abortedLast24Hours: Number(evidence['abortedLast24Hours']), + hostNotArrivedLast24Hours: evidence['hostNotArrivedLast24Hours'] ?? null, + oldestActiveAgeMs: evidence['oldestActiveAgeMs'] ?? null + } +} + +function snapshot( + regionalRehomes: AssignmentInventorySnapshot['regionalRehomes'] +): AssignmentInventorySnapshot { + return { + cells: [], + activityLeases: { total: 0, expired: 0, requestUnits: 0 }, + connectionReservations: { outstanding: 0, lateArrivalDebt: 0 }, + regionalRehomes + } +} + +function inventoryLine(snapshotValue: AssignmentInventorySnapshot): string { + const line = formatAssignmentInventorySnapshot(snapshotValue).find((candidate) => + candidate.startsWith('[orca-relay] regional rehome inventory ') + ) + if (!line) throw new Error('the formatter no longer emits a rehome inventory line') + return line +} + +describe('regional rehome inventory line census', () => { + it('parses what the director actually prints, field for field', async () => { + const parse = await loadParser() + const regionalRehomes = { + active: 3, + awaitingReceipt: 1, + targetRegistered: 2, + completedLast24Hours: 41, + abortedLast24Hours: 12, + hostNotArrivedLast24Hours: 5, + oldestActiveAgeMs: 77_731_209 + } + const now = Date.parse('2026-09-20T12:00:00Z') + + const evidence = parse( + [{ timestamp: '2026-09-20T11:59:00Z', textPayload: inventoryLine(snapshot(regionalRehomes)) }], + { now, maxAgeMs: 5 * 60_000 } + ) + + expect(evidence).toEqual({ ...regionalRehomes }) + }) + + it('parses the line an idle fleet prints, with no oldest active age', async () => { + const parse = await loadParser() + const now = Date.parse('2026-09-20T12:00:00Z') + + const evidence = parse( + [ + { + timestamp: '2026-09-20T11:59:00Z', + textPayload: inventoryLine( + snapshot({ + active: 0, + awaitingReceipt: 0, + targetRegistered: 0, + completedLast24Hours: 0, + abortedLast24Hours: 0, + hostNotArrivedLast24Hours: 0, + oldestActiveAgeMs: null + }) + ) + } + ], + { now, maxAgeMs: 5 * 60_000 } + ) + + expect(evidence.oldestActiveAgeMs).toBeNull() + expect(evidence.hostNotArrivedLast24Hours).toBe(0) + }) + + it('covers every counter the formatter puts on the line', async () => { + const parse = await loadParser() + // A field the parser ignores is a field the operator never sees, so the + // census fails when the formatter gains one and this test is not updated. + const line = inventoryLine( + snapshot({ + active: 1, + awaitingReceipt: 1, + targetRegistered: 1, + completedLast24Hours: 1, + abortedLast24Hours: 1, + hostNotArrivedLast24Hours: 1, + oldestActiveAgeMs: 1 + }) + ) + const printed = line + .slice('[orca-relay] regional rehome inventory '.length) + .split(' ') + .map((field) => field.split('=')[0]) + + const surfaced = Object.keys( + parse([{ timestamp: '2026-09-20T11:59:00Z', textPayload: line }], { + now: Date.parse('2026-09-20T12:00:00Z'), + maxAgeMs: 5 * 60_000 + }) + ) + + expect([...printed].sort()).toEqual([...surfaced].sort()) + }) +}) diff --git a/cloud/dev/scripts/relay-rehome-aggregate-evidence.mjs b/cloud/dev/scripts/relay-rehome-aggregate-evidence.mjs index 82bc2fb522a..a0563ffa94a 100644 --- a/cloud/dev/scripts/relay-rehome-aggregate-evidence.mjs +++ b/cloud/dev/scripts/relay-rehome-aggregate-evidence.mjs @@ -1,6 +1,24 @@ import { pathToFileURL } from 'node:url' -const INVENTORY = /^\[orca-relay\] regional rehome inventory active=(\d+) awaitingReceipt=(\d+) targetRegistered=(\d+) completedLast24Hours=(\d+) abortedLast24Hours=(\d+) oldestActiveAgeMs=(none|\d+)$/ +const INVENTORY_PREFIX = '[orca-relay] regional rehome inventory ' +// A counters line, so every field is a bare name and a non-negative integer or +// `none`. Pinning the whole line instead is what broke the enable workflow when +// `hostNotArrivedLast24Hours` shipped: the director grew a field and the parser +// read a healthy line as no evidence at all. Tolerating extra fields is safe +// only because the value shape stays fenced — `hostId=someone` is still not a +// counter, so an identity-bearing lookalike cannot slip through as an extra. +const FIELD = /^([A-Za-z][A-Za-z0-9]*)=(none|\d{1,15})$/ +// `oldestActiveAgeMs` is the one required field the director can report as +// `none`; a count that reads `none` is a line this parser does not recognise, +// not evidence worth failing the run over. +const REQUIRED_COUNTS = [ + 'active', + 'awaitingReceipt', + 'targetRegistered', + 'completedLast24Hours', + 'abortedLast24Hours' +] +const REQUIRED_FIELDS = [...REQUIRED_COUNTS, 'oldestActiveAgeMs'] function count(value, name) { const parsed = Number(value) @@ -8,20 +26,43 @@ function count(value, name) { return parsed } +// Returns the field map, or null for anything that is not this line. +export function readRegionalRehomeInventoryFields(textPayload) { + if (typeof textPayload !== 'string' || !textPayload.startsWith(INVENTORY_PREFIX)) return null + const fields = new Map() + for (const token of textPayload.slice(INVENTORY_PREFIX.length).split(' ')) { + const field = FIELD.exec(token) + if (!field || fields.has(field[1])) return null + fields.set(field[1], field[2]) + } + if (!REQUIRED_FIELDS.every((name) => fields.has(name))) return null + if (REQUIRED_COUNTS.some((name) => fields.get(name) === 'none')) return null + return fields +} + +// Absent is not zero: a director on an older image emits no such field, and +// reporting 0 would read as "no leaks" rather than "not measured". +function optionalCount(fields, name) { + const value = fields.get(name) + if (value === undefined || value === 'none') return null + return count(value, name) +} + export function parseRegionalRehomeInventory(entries, options = {}) { if (!Array.isArray(entries)) throw new Error('logging response must be an array') const parsed = entries.flatMap((entry) => { - const match = INVENTORY.exec(entry?.textPayload ?? '') + const fields = readRegionalRehomeInventoryFields(entry?.textPayload ?? '') const timestamp = Date.parse(entry?.timestamp ?? '') - if (!match || !Number.isFinite(timestamp)) return [] + if (!fields || !Number.isFinite(timestamp)) return [] return [{ timestamp, - active: count(match[1], 'active'), - awaitingReceipt: count(match[2], 'awaiting receipt'), - targetRegistered: count(match[3], 'target registered'), - completedLast24Hours: count(match[4], 'completed'), - abortedLast24Hours: count(match[5], 'aborted'), - oldestActiveAgeMs: match[6] === 'none' ? null : count(match[6], 'oldest active age') + active: count(fields.get('active'), 'active'), + awaitingReceipt: count(fields.get('awaitingReceipt'), 'awaiting receipt'), + targetRegistered: count(fields.get('targetRegistered'), 'target registered'), + completedLast24Hours: count(fields.get('completedLast24Hours'), 'completed'), + abortedLast24Hours: count(fields.get('abortedLast24Hours'), 'aborted'), + hostNotArrivedLast24Hours: optionalCount(fields, 'hostNotArrivedLast24Hours'), + oldestActiveAgeMs: optionalCount(fields, 'oldestActiveAgeMs') }] }).sort((left, right) => right.timestamp - left.timestamp) if (parsed.length === 0) throw new Error('no aggregate regional rehome inventory evidence') diff --git a/cloud/dev/scripts/relay-rehome-aggregate-evidence.test.mjs b/cloud/dev/scripts/relay-rehome-aggregate-evidence.test.mjs index 2ce2627fb93..e85c4d5606d 100644 --- a/cloud/dev/scripts/relay-rehome-aggregate-evidence.test.mjs +++ b/cloud/dev/scripts/relay-rehome-aggregate-evidence.test.mjs @@ -1,6 +1,7 @@ import assert from 'node:assert/strict' import { test } from 'node:test' import { parseRegionalRehomeInventory } from './relay-rehome-aggregate-evidence.mjs' +import { readRelayWorkflow } from './relay-repository.mjs' const now = Date.parse('2026-08-14T12:00:00Z') @@ -22,10 +23,38 @@ test('selects the newest fresh aggregate-only regional rehome inventory', () => targetRegistered: 1, completedLast24Hours: 9, abortedLast24Hours: 0, + hostNotArrivedLast24Hours: null, oldestActiveAgeMs: 30_000 }) }) +test('reads a line the director grew a field on, wherever the field sits', () => { + const result = parseRegionalRehomeInventory([{ + timestamp: '2026-08-14T11:58:00Z', + textPayload: + '[orca-relay] regional rehome inventory hostNotArrivedLast24Hours=4 active=2' + + ' awaitingReceipt=1 targetRegistered=1 completedLast24Hours=9 abortedLast24Hours=7' + + ' oldestActiveAgeMs=30000 someFieldFromALaterRelease=11' + }], { now, maxAgeMs: 5 * 60_000 }) + assert.equal(result.hostNotArrivedLast24Hours, 4) + assert.equal(result.abortedLast24Hours, 7) + assert.equal(result.oldestActiveAgeMs, 30_000) +}) + +test('reports an unmeasured host-not-arrived count as absent, not as zero', () => { + const [withField, withoutField] = ['4', null].map((value) => + parseRegionalRehomeInventory([{ + timestamp: '2026-08-14T11:58:00Z', + textPayload: + '[orca-relay] regional rehome inventory active=0 awaitingReceipt=0 targetRegistered=0' + + ' completedLast24Hours=0 abortedLast24Hours=0 oldestActiveAgeMs=none' + + (value === null ? '' : ` hostNotArrivedLast24Hours=${value}`) + }], { now, maxAgeMs: 5 * 60_000 }) + ) + assert.equal(withField.hostNotArrivedLast24Hours, 4) + assert.equal(withoutField.hostNotArrivedLast24Hours, null) +}) + test('rejects stale, malformed, and identity-bearing lookalikes', () => { assert.throws(() => parseRegionalRehomeInventory([{ timestamp: '2026-08-14T11:00:00Z', @@ -36,3 +65,71 @@ test('rejects stale, malformed, and identity-bearing lookalikes', () => { textPayload: '[orca-relay] regional rehome inventory active=0 hostId=secret' }], { now }), /no aggregate/) }) + +test('keeps out an identity-bearing field riding along on a complete line', () => { + const complete = + '[orca-relay] regional rehome inventory active=0 awaitingReceipt=0 targetRegistered=0' + + ' completedLast24Hours=0 abortedLast24Hours=0 oldestActiveAgeMs=none' + for (const extra of [' hostId=secret', ' userId=someone@example.test', ' note=a b']) { + assert.throws( + () => parseRegionalRehomeInventory( + [{ timestamp: '2026-08-14T11:59:00Z', textPayload: complete + extra }], + { now } + ), + /no aggregate/, + extra + ) + } +}) + +test('refuses a line missing a required field, or repeating one', () => { + const missing = + '[orca-relay] regional rehome inventory active=0 awaitingReceipt=0 targetRegistered=0' + + ' completedLast24Hours=0 oldestActiveAgeMs=none' + assert.throws( + () => parseRegionalRehomeInventory( + [{ timestamp: '2026-08-14T11:59:00Z', textPayload: missing }], + { now } + ), + /no aggregate/ + ) + assert.throws( + () => parseRegionalRehomeInventory( + [{ timestamp: '2026-08-14T11:59:00Z', textPayload: `${missing} abortedLast24Hours=0 abortedLast24Hours=1` }], + { now } + ), + /no aggregate/ + ) + assert.throws( + () => parseRegionalRehomeInventory( + [{ + timestamp: '2026-08-14T11:59:00Z', + textPayload: missing.replace('active=0', 'active=none') + ' abortedLast24Hours=0' + }], + { now } + ), + /no aggregate/ + ) +}) + +// The third edge of the chain the enable workflow depends on. The formatter is +// pinned against this parser in the relay package's inventory-line census; this +// pins the parser against the summary an operator reads, so a field that +// reaches the evidence JSON and stops there fails here. +test('publishes every parsed counter in the operator step summary', () => { + const job = readRelayWorkflow('operate-relay-production-rehome-job.yml') + // The jq program and the file it reads sit on separate continuation lines, so + // match the whole render rather than one line of it. + const summary = /jq -r '([^']*)' \\\n\s*"\$\{RUNNER_TEMP\}\/relay-rehome-inventory\.json"/.exec(job)?.[1] + assert.ok(summary, 'the rehome job no longer renders the inventory evidence') + const evidence = parseRegionalRehomeInventory([{ + timestamp: '2026-08-14T11:58:00Z', + textPayload: + '[orca-relay] regional rehome inventory active=0 awaitingReceipt=0 targetRegistered=0' + + ' completedLast24Hours=0 abortedLast24Hours=0 hostNotArrivedLast24Hours=0 oldestActiveAgeMs=none' + }], { now, maxAgeMs: 5 * 60_000 }) + for (const key of Object.keys(evidence)) { + if (key === 'timestamp') continue + assert.ok(summary.includes(`.${key}`), `${key} is missing from the step summary`) + } +})