diff --git a/src/cli/diagnose.ts b/src/cli/diagnose.ts index 58d230d0..aec73d63 100644 --- a/src/cli/diagnose.ts +++ b/src/cli/diagnose.ts @@ -537,6 +537,18 @@ export function renderDeployedDiagnosis(diagnosis: DeployedFactoryDiagnosis): st if (readiness.skipReasons) { lines.push(` skip reasons : ${formatSkipReasons(readiness.skipReasons)}`) } + // Rendered whenever the producer reports it, zero included: `skipReasons` + // drops a zero bucket, so without this line "no dispatch failed" and "this + // daemon cannot tell you" look identical to a reader (#355). + if (readiness.dispatchFailures !== undefined) { + lines.push( + ` dispatch failures : ${readiness.dispatchFailures}${ + readiness.dispatchFailureReasons + ? ` (${formatSkipReasons(readiness.dispatchFailureReasons)})` + : '' + }`, + ) + } } const capacity = health.dispatchCapacity if (capacity) { diff --git a/src/index.ts b/src/index.ts index 3e5b7861..bd9db8c9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -498,6 +498,13 @@ export { factorySweepSkipReasonCounts, } from './orchestrator/sweep-skip-reason' export type { FactorySweepSkipReasonCode } from './orchestrator/sweep-skip-reason' +export { + FACTORY_DISPATCH_FAILURE_REASON_CODES, + factoryDispatchFailureReasonCode, + factoryDispatchFailureReasonCodeForErrorClass, + factoryDispatchFailureReasonCounts, +} from './orchestrator/dispatch-failure-reason' +export type { FactoryDispatchFailureReasonCode } from './orchestrator/dispatch-failure-reason' export type { FleetControlPlaneState, FleetControlPlaneStatus } from './fleet/control-plane-circuit' export { LOAD_EVIDENCE_CONTRACT, diff --git a/src/orchestrator/dispatch-failure-reason.ts b/src/orchestrator/dispatch-failure-reason.ts new file mode 100644 index 00000000..309c72a8 --- /dev/null +++ b/src/orchestrator/dispatch-failure-reason.ts @@ -0,0 +1,136 @@ +/** + * Why a dispatch *attempt* failed, as a closed set (#355). + * + * `skipReasons` (#358) answers "why did the sweep decline this work unit", and + * on the live container it answers it with `dispatch-failed: 5` — every sweep, + * with the control-plane breaker closed, the fleet agent online and + * `readinessReconcile` healthy. That bucket is a *count*. It says the sweep got + * all the way to dispatching and dispatch threw; it does not say what threw, + * and the message that would say so goes to the daemon's stdout, which does not + * reach the deployed container's operator. + * + * So this is the second level of the same breakdown: `dispatchFailures` is the + * total (the same number `skipReasons['dispatch-failed']` carries), and + * `dispatchFailureReasons` splits it by cause. Same discipline as #358 — + * counts only, keys from this vocabulary rather than from the record, unknown + * codes folded into `other` rather than dropped so the parts still sum. + * + * As in #358 the code is recorded at the skip site alongside the free text, + * never derived from it: `perItemDispatchFailureCode` classifies a thrown value + * by type, and the two named-by-class-name entries below are the explicit + * exception, documented there. + * + * The three `unclassified-*` codes are not a failure of this vocabulary — they + * are its most useful answer when nothing named matches, because the *phase* + * that threw already names the owner. A pass that reaches dispatch and fails is + * a fleet problem; one that fails in triage never got near the fleet; one that + * fails in the gates ahead of triage failed reading durable state. Those are + * three different people's bugs, and separating them costs one variable. + */ +export const FACTORY_DISPATCH_FAILURE_REASON_CODES = [ + /** Relayfile shed an operation this dispatch needed (#297). Clears on its own. */ + 'relayfile-overloaded', + /** Live provider state moved under the dispatch; the unit returns to the queue. */ + 'live-state-changed', + /** The never-placed deadline released the dispatch mid-spawn (#303). Self-healing. */ + 'late-placement-released', + /** The durable lifecycle record is terminal: this unit will not be retried. */ + 'lifecycle-terminal', + /** Another publisher holds the durable lifecycle lease. Clears on release. */ + 'lifecycle-owned-elsewhere', + /** The fleet control-plane circuit refused admission. Dispatch is globally paused. */ + 'control-plane-open', + /** Something in the attempt hit its own deadline — see the note on class names. */ + 'timed-out', + /** The fleet broker never acknowledged the spawn within its ack timeout. */ + 'spawn-ack-timeout', + /** The fleet client holds a read-only identity and cannot spawn at all. */ + 'fleet-identity-read-only', + /** Registering the spawned agent with the broker failed. */ + 'agent-registration-failed', + /** Threw in the gates ahead of triage: scope, orphan recovery, durable reads. */ + 'unclassified-gate', + /** Threw inside triage, before the fleet was ever asked for anything. */ + 'unclassified-triage', + /** Threw inside dispatch itself, and no code above matched it. */ + 'unclassified-dispatch', + /** Recorded by a producer this vocabulary does not know. */ + 'other', +] as const + +export type FactoryDispatchFailureReasonCode = typeof FACTORY_DISPATCH_FAILURE_REASON_CODES[number] + +/** + * Codes recognised by allowlisted error *class name* rather than by `instanceof`. + * + * The rule everywhere else is to classify by type, because a message is one + * rename away from collapsing a bucket. These five are the exception, for one + * reason: their classes live in `src/fleet/relay-fleet-client.ts`, and + * `factory.ts` does not import that module. Adding the edge to get five + * `instanceof` checks would pull the relay SDK into the orchestrator's module + * graph — a real cost — to buy precision over a *class name*, which is + * code-controlled and already the identifier `telemetryErrorClass` publishes + * everywhere else in this codebase. A rename here degrades one bucket to + * `unclassified-dispatch`; it cannot leak and it cannot break the sum. + * + * `TimeoutError` is deliberately generic: `FleetControlPlaneTimeoutError` sets + * that name on purpose, and so do `AbortSignal.timeout` and several SDK paths. + * Mapping it to the control plane specifically would be a guess; mapping it to + * "something in this attempt hit a deadline" is exactly true, and with 34-minute + * sweeps against 60-second attempts it is the answer worth being able to see. + */ +const DISPATCH_FAILURE_ERROR_CLASS_CODES: Readonly> = { + RelaySpawnAckTimeoutError: 'spawn-ack-timeout', + ReadOnlyFleetIdentityError: 'fleet-identity-read-only', + FactoryAgentRegistrationError: 'agent-registration-failed', + TimeoutError: 'timed-out', + AbortError: 'timed-out', +} + +/** The code an allowlisted error class name names, if this vocabulary knows one. */ +export const factoryDispatchFailureReasonCodeForErrorClass = ( + errorClass: string, +): FactoryDispatchFailureReasonCode | undefined => + Object.hasOwn(DISPATCH_FAILURE_ERROR_CLASS_CODES, errorClass) + ? DISPATCH_FAILURE_ERROR_CLASS_CODES[errorClass] + : undefined + +/** + * Coerce an arbitrary value onto the vocabulary. + * + * Unrecognised codes collapse to `other` rather than being dropped, for the + * same reason #358 gives: a failure that vanished from the breakdown would make + * the parts stop summing to `dispatchFailures`, and a reader comparing the two + * would conclude the counter was broken rather than that the producer was newer. + */ +export const factoryDispatchFailureReasonCode = (value: unknown): FactoryDispatchFailureReasonCode => + typeof value === 'string' && + (FACTORY_DISPATCH_FAILURE_REASON_CODES as readonly string[]).includes(value) + ? value as FactoryDispatchFailureReasonCode + : 'other' + +/** + * The per-cause breakdown of one sweep's failed dispatch attempts, counts only. + * + * Counts exactly the entries the sweep recorded as `dispatch-failed`, so the + * parts sum to `dispatchFailures` and to `skipReasons['dispatch-failed']` by + * construction. An entry whose producer recorded no `failureCode` folds into + * `other` for the same reason an unknown one does. + * + * Zero-count codes are omitted — the vocabulary is fixed and published, so an + * absent key reads as zero unambiguously. The total is carried separately by + * `dispatchFailures`, which is written whenever a sweep completes, so "ran and + * every dispatch succeeded" stays distinguishable from "never ran". + */ +export function factoryDispatchFailureReasonCounts( + skipped: Iterable<{ code?: unknown; failureCode?: unknown }> | undefined, +): Partial> { + const counts: Partial> = {} + if (!skipped) return counts + for (const entry of skipped) { + if (entry === null || typeof entry !== 'object' || entry.code !== 'dispatch-failed') continue + const code = factoryDispatchFailureReasonCode(entry.failureCode) + counts[code] = (counts[code] ?? 0) + 1 + } + return counts +} diff --git a/src/orchestrator/dispatch-failure-reasons.test.ts b/src/orchestrator/dispatch-failure-reasons.test.ts new file mode 100644 index 00000000..94dbce11 --- /dev/null +++ b/src/orchestrator/dispatch-failure-reasons.test.ts @@ -0,0 +1,404 @@ +import { describe, expect, it } from 'vitest' +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { + FactoryConfigSchema, + createFactory, + publicHealthFromHeartbeat, + type FactoryConfig, + type LinearIssue, + type TriageDecision, + type TriageEngine, +} from '../index' +import { FakeFleetClient, FakeMountClient } from '../testing' +import { RelaySpawnAckTimeoutError } from '../fleet/relay-fleet-client' +import { normalizePublicHealth } from './public-health' +import type { FactoryPublicReadinessReconcileHealth, FactoryReadinessReconcileStatus } from '../types' +import type { SpawnInput, SpawnResult } from '../ports/fleet' + +/** + * Why a dispatch attempt failed, end to end (#355). + * + * The live container publishes `skipReasons: { 'dispatch-failed': 5 }` on every + * sweep, with the control-plane breaker closed, the fleet agent online and + * `readinessReconcile` healthy. Five eligible issues are selected, dispatch is + * attempted, and it fails — and the bucket is a count, so it does not say what + * failed. The daemon knows; it writes the reason to stdout, which does not + * reach the deployed container's operator. + * + * Every assertion below drives the real writer: a live daemon running a real + * sweep whose real dispatch really throws. Hand-setting `failureCode` on a + * report fixture would prove the projection copies a field it was handed, and + * prove nothing about the sweep that has to produce it. + */ + +const ready = '11111111-1111-4111-8111-111111111111' +const implementing = '22222222-2222-4222-8222-222222222222' +const done = '33333333-3333-4333-8333-333333333333' +const planning = '44444444-4444-4444-8444-444444444444' + +const config = (overrides: Record = {}): FactoryConfig => FactoryConfigSchema.parse({ + workspaceId: 'factory-dispatch-failure-reasons', + repos: { + byLabel: { pear: 'AgentWorkforce/pear' }, + clonePaths: { 'AgentWorkforce/pear': '/work/pear' }, + default: 'AgentWorkforce/pear', + }, + triage: { maxImplementers: 4 }, + batchSize: 4, + stateIds: { readyForAgent: ready, agentImplementing: implementing, done, inPlanning: planning }, + verification: { enabled: false }, + ...overrides, +}) + +const issuePath = (n: number) => `/linear/issues/AR-${n}__uuid-${n}.json` + +const issueFile = (n: number) => ({ + provider: 'linear', + objectType: 'issue', + objectId: `uuid-${n}`, + payload: { + id: `uuid-${n}`, + identifier: `AR-${n}`, + title: `[factory-e2e] Fix factory issue ${n}`, + description: 'Implement the requested fix in src/orchestrator/factory.ts and verify it with tests.', + stateId: ready, + url: `https://linear.app/agent-relay/issue/AR-${n}/factory-issue-${n}`, + labels: [{ name: 'pear' }], + labelIds: ['label-id-not-used-by-parser'], + team: { key: 'AR', name: 'Agent Relay' }, + project: { name: 'Factory' }, + state: { id: ready, name: 'Ready for Agent' }, + }, +}) + +class StaticTriage implements TriageEngine { + async triage(issue: LinearIssue): Promise { + const number = issue.key.match(/\d+/)?.[0] ?? '0' + return { + issue: { uuid: issue.uuid, key: issue.key, path: issue.path }, + routes: [{ repo: 'AgentWorkforce/pear', clonePath: '/work/pear', rationale: 'test route' }], + scope: 'single', + implementers: [{ + name: `ar-${number}-impl`, + role: 'implementer', + capability: 'spawn:codex', + model: 'codex', + task: `Implement ${issue.key}`, + repo: 'AgentWorkforce/pear', + clonePath: '/work/pear', + node: 'self', + }], + reviewer: { + name: `ar-${number}-review`, + role: 'reviewer', + capability: 'spawn:claude', + model: 'claude', + task: `Review ${issue.key}`, + repo: 'AgentWorkforce/pear', + clonePath: '/work/pear', + node: 'self', + }, + thin: false, + confidence: 'high', + rationale: 'static test decision', + } + } +} + +/** + * Triage that throws, so the failure happens before the fleet is ever asked. + * + * Also the one dispatch-attempt path that does NOT wrap what it throws, which + * makes it the only way to drive an unwrapped error all the way to the skip + * site — see the shed test below. + */ +class ThrowingTriage implements TriageEngine { + constructor(private readonly error?: Error) {} + + async triage(issue: LinearIssue): Promise { + throw this.error ?? + new Error(`triage backend refused ${issue.key} at /linear/issues/${issue.key}.json`) + } +} + +/** + * A fleet whose spawn throws for issues this test names. + * + * The failure is injected at the spawn, not at the skip site, precisely because + * the classification has to survive the trip: `#dispatchUnlocked` and + * `contextualError` both rethrow wrapped, so a classifier reading only the + * outermost error would see a plain `Error` and lose every named cause. + */ +class SpawnFailingFleetClient extends FakeFleetClient { + constructor(private readonly failure: (input: SpawnInput) => Error | undefined) { + super() + } + + override async spawn(input: SpawnInput): Promise { + const error = this.failure(input) + if (error) throw error + return super.spawn(input) + } +} + +async function sweepReadiness( + files: Record, + deps: { fleet?: FakeFleetClient; triage?: TriageEngine } = {}, +): Promise { + const root = await mkdtemp(join(tmpdir(), 'factory-dispatch-failure-reasons-')) + const factory = createFactory( + config({ loop: { registryPath: join(root, 'registry.json'), heartbeatPath: join(root, 'heartbeat.json') } }), + { + mount: new FakeMountClient(files), + fleet: deps.fleet ?? new FakeFleetClient(), + triage: deps.triage ?? new StaticTriage(), + logger: {}, + }, + ) + try { + await factory.start({ + mode: 'live', + // The startup backfill is the sweep under test and settles before + // `start()` resolves; a periodic pass landing mid-read would make the + // numbers describe a sweep this test never set up. + liveSubscription: { transport: 'subscribe', reconcileIntervalMs: 600_000 }, + }) + const status = factory.status().readinessReconcile + if (!status) throw new Error('live daemon published no readinessReconcile status') + return status + } finally { + await factory.stop() + await rm(root, { recursive: true, force: true }) + } +} + +/** The public record a container would serve for this readiness status. */ +const published = (status: FactoryReadinessReconcileStatus): FactoryPublicReadinessReconcileHealth => { + const health = publicHealthFromHeartbeat({ + pid: 1, + status: 'running', + iteration: 1, + maxIterations: 1, + updatedAt: new Date(1_700_000_000_000).toISOString(), + updatedAtMs: 1_700_000_000_000, + readinessReconcile: status, + }, { nowMs: 1_700_000_000_000 }) + if (!health.readinessReconcile) throw new Error('public health carried no readinessReconcile block') + return health.readinessReconcile +} + +/** MUST-FIRE: dispatch was attempted, it failed, and the surface names why. */ +const expectNamedDispatchFailures = (readiness: FactoryPublicReadinessReconcileHealth): void => { + expect(readiness.dispatchFailures).toBeGreaterThan(0) + expect(readiness.dispatchFailureReasons).toBeDefined() + expect(Object.values(readiness.dispatchFailureReasons ?? {}).reduce((sum, n) => sum + n, 0)) + .toBe(readiness.dispatchFailures) +} + +/** MUST-NOT-FIRE: dispatch was attempted and nothing failed — a real zero. */ +const expectNoDispatchFailures = (readiness: FactoryPublicReadinessReconcileHealth): void => { + expect(readiness.dispatchFailures).toBe(0) + expect(Object.hasOwn(readiness, 'dispatchFailures')).toBe(true) + expect(Object.hasOwn(readiness, 'dispatchFailureReasons')).toBe(false) +} + +describe('dispatch failure reasons (#355)', () => { + it('names the cause of a failed dispatch, and the parts sum to the bucket that counted it', async () => { + const fleet = new SpawnFailingFleetClient((input) => + input.name.startsWith('ar-951') || input.name.startsWith('ar-952') + ? new RelaySpawnAckTimeoutError('spawn', 60_000) + : undefined) + + const status = await sweepReadiness({ + [issuePath(951)]: issueFile(951), + [issuePath(952)]: issueFile(952), + [issuePath(953)]: issueFile(953), + }, { fleet }) + + expect(status).toMatchObject({ + candidates: 3, + dispatched: 1, + skipped: 2, + skipReasons: { 'dispatch-failed': 2 }, + dispatchFailures: 2, + dispatchFailureReasons: { 'spawn-ack-timeout': 2 }, + }) + + const readiness = published(status) + expectNamedDispatchFailures(readiness) + expect(readiness.dispatchFailureReasons).toEqual({ 'spawn-ack-timeout': 2 }) + // The breakdown refines exactly the bucket it sits under. A reader holding + // both numbers can check that, and it is the only integrity check this + // surface offers. + expect(readiness.dispatchFailures).toBe(readiness.skipReasons?.['dispatch-failed']) + }) + + it('CONTROL: a zero is published as a zero, so "none failed" never reads as "cannot say"', async () => { + // The trap this test exists for. `skipReasons` omits zero counts, so on + // that field alone a sweep in which every dispatch succeeded is the same + // absence as a producer that has never heard of dispatch failures — and + // 0.1.72 is in production being exactly the second thing. If + // `dispatchFailures` were built with a helper that coerced absent to zero, + // or that dropped a zero as uninteresting, this pair would collapse. + const clean = published(await sweepReadiness({ + [issuePath(961)]: issueFile(961), + [issuePath(962)]: issueFile(962), + })) + expect(clean).toMatchObject({ candidates: 2, dispatched: 2, skipped: 0 }) + expectNoDispatchFailures(clean) + // `skipReasons` cannot answer the question, which is why the total exists. + expect(clean.skipReasons).toBeUndefined() + + const failing = published(await sweepReadiness( + { [issuePath(963)]: issueFile(963) }, + { + fleet: new SpawnFailingFleetClient(() => new RelaySpawnAckTimeoutError('spawn', 60_000)), + }, + )) + + // Neither expectation can stand in for the other: swap them and the suite + // fails, so a pass here is evidence about the sweep rather than about a + // constant. + expect(() => expectNoDispatchFailures(failing)).toThrow() + expect(() => expectNamedDispatchFailures(clean)).toThrow() + expect(() => expectNamedDispatchFailures(failing)).not.toThrow() + expect(() => expectNoDispatchFailures(clean)).not.toThrow() + + // And the zero survives the wire, which is where a coercion would show up. + const roundTripped = normalizePublicHealth(JSON.parse(JSON.stringify({ + schemaVersion: 1, + ok: true, + status: 'ok', + stale: false, + degradedSubsystems: [], + readinessReconcile: clean, + }))) + expect(roundTripped?.readinessReconcile?.dispatchFailures).toBe(0) + expect(Object.hasOwn(roundTripped?.readinessReconcile ?? {}, 'dispatchFailures')).toBe(true) + }) + + it('leaves the field absent until a sweep completes, so "never attempted" stays its own reading', async () => { + const root = await mkdtemp(join(tmpdir(), 'factory-dispatch-failure-reasons-')) + const factory = createFactory( + config({ loop: { registryPath: join(root, 'registry.json'), heartbeatPath: join(root, 'heartbeat.json') } }), + { mount: new FakeMountClient(), fleet: new FakeFleetClient(), triage: new StaticTriage(), logger: {} }, + ) + try { + const status = factory.status().readinessReconcile + expect(status?.state).toBe('not-running') + expect(status && Object.hasOwn(status, 'dispatchFailures')).toBe(false) + // Three readings, three shapes: absent here, `0` for a clean sweep, a + // positive integer with a breakdown for a failing one. + expect(status && Object.hasOwn(published(status), 'dispatchFailures')).toBe(false) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('separates a failure in triage from one in dispatch, because they are different owners', async () => { + // Nothing named matches either of these, and that is the case the phase + // codes are for: a unit that never reached the fleet is not a fleet bug, + // and on a surface carrying no messages the phase is the only thing left + // that still says who should look. + const triageFailed = await sweepReadiness( + { [issuePath(971)]: issueFile(971) }, + { triage: new ThrowingTriage() }, + ) + expect(triageFailed).toMatchObject({ + candidates: 1, + dispatched: 0, + dispatchFailures: 1, + dispatchFailureReasons: { 'unclassified-triage': 1 }, + }) + + const dispatchFailed = await sweepReadiness( + { [issuePath(972)]: issueFile(972) }, + { fleet: new SpawnFailingFleetClient(() => new Error('broker refused the spawn')) }, + ) + expect(dispatchFailed).toMatchObject({ + candidates: 1, + dispatched: 0, + dispatchFailures: 1, + dispatchFailureReasons: { 'unclassified-dispatch': 1 }, + }) + }) + + it('classifies through the wrapper, so a rethrown cause does not lose its name', async () => { + // `#dispatchUnlocked` and `contextualError` both rethrow wrapped. A + // classifier that read only the outermost error would see a plain `Error` + // here and report `unclassified-dispatch` for a failure that names itself. + const wrapped = new Error('dispatch failed for AR-981') + ;(wrapped as Error & { cause?: unknown }).cause = new RelaySpawnAckTimeoutError('spawn', 60_000) + + const status = await sweepReadiness( + { [issuePath(981)]: issueFile(981) }, + { fleet: new SpawnFailingFleetClient(() => wrapped) }, + ) + expect(status?.dispatchFailureReasons).toEqual({ 'spawn-ack-timeout': 1 }) + }) + + // #361 review (P2, codex): overload classification must follow the same + // bounded cause chain as every named error. These two pin the direct and + // wrapped paths to the loop's single shedding predicate. + it('reports an unwrapped relayfile shed as a shed', async () => { + const shed = Object.assign(new Error('workspace durable object is busy'), { + status: 429, + reason: 'inflight_limit', + }) + const status = await sweepReadiness( + { [issuePath(1001)]: issueFile(1001) }, + { triage: new ThrowingTriage(shed) }, + ) + expect(status).toMatchObject({ + dispatchFailures: 1, + dispatchFailureReasons: { 'relayfile-overloaded': 1 }, + }) + }) + + it('treats a relayfile shed wrapped by spawn context as the same shed', async () => { + // `#spawnAgent` wraps this provider failure before the skip site. The + // source overload predicate must still drive the loop's counter/fuse path + // and the published reason; relabelling only the surface would lie. + const wrapped = Object.assign(new Error('workspace durable object is busy'), { + status: 429, + reason: 'inflight_limit', + }) + const status = await sweepReadiness( + { [issuePath(1002)]: issueFile(1002) }, + { fleet: new SpawnFailingFleetClient(() => wrapped) }, + ) + expect(status?.dispatchFailureReasons).toEqual({ 'relayfile-overloaded': 1 }) + expect(status?.dispatchFailures).toBe(1) + }) + + it('publishes counts only: no issue key, path, title or error message crosses', async () => { + const status = await sweepReadiness( + { + [issuePath(991)]: issueFile(991), + [issuePath(992)]: issueFile(992), + }, + { + fleet: new SpawnFailingFleetClient((input) => + input.name.startsWith('ar-991') + ? new Error('spawn failed for AR-991 at /linear/issues/AR-991__uuid-991.json (relay token expired)') + : undefined), + }, + ) + + const serialized = JSON.stringify(published(status)) + expect(serialized).not.toContain('AR-991') + expect(serialized).not.toContain('/linear/issues') + expect(serialized).not.toContain('relay token') + expect(serialized).not.toContain('factory-e2e') + expect(serialized).not.toContain('AgentWorkforce/pear') + + const record = JSON.parse(serialized) as Record + expect(record.dispatchFailures).toBe(1) + for (const count of Object.values(record.dispatchFailureReasons as Record)) { + expect(typeof count).toBe('number') + } + }) +}) diff --git a/src/orchestrator/factory.test.ts b/src/orchestrator/factory.test.ts index 481bbc9e..c8548e25 100644 --- a/src/orchestrator/factory.test.ts +++ b/src/orchestrator/factory.test.ts @@ -5485,6 +5485,9 @@ describe('FactoryLoop', () => { issue: { uuid: 'AgentWorkforce/pear#59', key: '59', path: racedPath }, reason: 'live state changed during dispatch', code: 'dispatch-failed', + // The publishable half of the same classification (#355). The bucket + // count says five dispatches failed; this says they raced the provider. + failureCode: 'live-state-changed', }) expect(report.dispatched.map((result) => result.issue.key)).toEqual(['60']) expect(fleet.spawns.map((spawn) => spawn.name)).toEqual([ @@ -5578,6 +5581,7 @@ describe('FactoryLoop', () => { issue: { uuid: 'AgentWorkforce/pear#59', key: '59', path: blockedPath }, reason: 'dispatch lifecycle already terminal', code: 'dispatch-failed', + failureCode: 'lifecycle-terminal', }) expect(report.dispatched.map((result) => result.issue.key)).toEqual(['60']) expect(fleet.spawns.map((spawn) => spawn.name)).toEqual([ @@ -5612,6 +5616,10 @@ describe('FactoryLoop', () => { // carries a classification rather than raw provider text. reason: 'dispatch failed (TypeError)', code: 'dispatch-failed', + // Nothing names a `TypeError: fetch failed`, so the phase does: it + // threw in triage, which is a different owner from a fleet fault even + // though both land in the same `dispatch-failed` bucket (#355). + failureCode: 'unclassified-triage', }) expect(report.dispatched.map((result) => result.issue.key)).toEqual(['60']) expect(factory.status().counters.dispatchItemFailuresSkipped).toBe(1) @@ -5735,6 +5743,10 @@ describe('FactoryLoop', () => { issue: { uuid: 'AgentWorkforce/pear#59', key: '59', path: blockedPath }, reason: 'dispatch failed (Error)', code: 'dispatch-failed', + // The injected fault is the `unrelated per-item fault` above, thrown + // from triage — not the open circuit, which this dry run is exempt + // from. The phase code says so rather than leaving them conflated. + failureCode: 'unclassified-triage', }) expect(report.dispatched.map((result) => result.issue.key)).toEqual(['60']) expect(fleet.spawns).toEqual([]) diff --git a/src/orchestrator/factory.ts b/src/orchestrator/factory.ts index 40658bee..642e2b41 100644 --- a/src/orchestrator/factory.ts +++ b/src/orchestrator/factory.ts @@ -152,6 +152,11 @@ import { } from './public-health' import { factorySweepSkipReasonCounts } from './sweep-skip-reason' import type { FactorySweepSkipReasonCode } from './sweep-skip-reason' +import { + factoryDispatchFailureReasonCodeForErrorClass, + factoryDispatchFailureReasonCounts, +} from './dispatch-failure-reason' +import type { FactoryDispatchFailureReasonCode } from './dispatch-failure-reason' import { boundedRunCostTotal, CostLedger, type RunCostTotal, type UnpricedModelCostRecord } from '../cost/ledger' import { createTicketDispatchDelivery, type TicketDispatchDelivery } from '../delivery/ticket-dispatch' import { @@ -864,6 +869,16 @@ export class FactoryLoop implements Factory { dispatched: number skipped: number skipReasons: Partial> + /** + * The `dispatch-failed` bucket's own total, held even when it is zero. + * + * `skipReasons` omits zero counts, so a sweep in which every dispatch + * succeeded and a daemon that does not report the field are the same + * absence there. Here they are not, and telling them apart is the whole + * point of the breakdown beside it (#355). + */ + dispatchFailures: number + dispatchFailureReasons: Partial> /** * When the pass these counts describe finished enumerating (#359 review). * @@ -1918,6 +1933,8 @@ export class FactoryLoop implements Factory { // `skipped: 0` next to a non-empty breakdown — and this log is what a // local operator reads. skipReasons: factorySweepSkipReasonCounts(report.skipped), + dispatchFailures: report.skipped.filter((entry) => entry.code === 'dispatch-failed').length, + dispatchFailureReasons: factoryDispatchFailureReasonCounts(report.skipped), discoveryDeferred: report.discoveryDeferred, }) } catch (error) { @@ -3126,6 +3143,10 @@ export class FactoryLoop implements Factory { continue } const recoveredIdentity = recoveredOrphan ? githubIssueRefIdentity(issueRef(issue)) : undefined + // Reset per work unit, and advanced by assignment immediately before + // each stage rather than inferred in the catch: the whole value of the + // `unclassified-*` codes is that they name the stage honestly (#355). + let attemptPhase: DispatchAttemptPhase = 'gate' try { if (recoveredOrphan) { const dispatchBlock = await this.#dispatchBlockReason(issue) @@ -3149,8 +3170,10 @@ export class FactoryLoop implements Factory { continue } + attemptPhase = 'triage' const decision = await this.triageIssue(issue) triaged.push(decision) + attemptPhase = 'dispatch' const result = await this.dispatch(decision, { dryRun }) // A completed dispatch — even one that parks or escalates the issue — // proves the pipeline still works, so the fuse below starts over. @@ -3250,6 +3273,11 @@ export class FactoryLoop implements Factory { issue: issueRef(issue), reason: perItemDispatchSkipReason(error), code: 'dispatch-failed', + // The publishable half of the same classification. `reason` is the + // operator's sentence and stays off the health surface; this token + // is what tells a reader watching `dispatch-failed: 5` which of + // five very different bugs they are looking at (#355). + failureCode: perItemDispatchFailureCode(error, attemptPhase), }) continue } finally { @@ -5091,6 +5119,11 @@ export class FactoryLoop implements Factory { dispatched: report.dispatched.length, skipped: report.skipped.length, skipReasons: factorySweepSkipReasonCounts(report.skipped), + // Counted from the same entries `skipReasons` counts, so the parts sum to + // `skipReasons['dispatch-failed']` by construction rather than by a + // second traversal agreeing with the first. + dispatchFailures: report.skipped.filter((entry) => entry.code === 'dispatch-failed').length, + dispatchFailureReasons: factoryDispatchFailureReasonCounts(report.skipped), // The caller's completion stamp, not a fresh clock read: on a pass that // enumerated, `lastEnumeratedAtMs` and `lastCompletedAtMs` describe the // same instant and must not drift apart by a tick. @@ -5185,6 +5218,13 @@ export class FactoryLoop implements Factory { ...(Object.keys(this.#readinessReconcileLastSweep.skipReasons).length > 0 ? { skipReasons: { ...this.#readinessReconcileLastSweep.skipReasons } } : {}), + // Unconditional, unlike the breakdown below it: a zero here is the + // fact "this sweep attempted dispatches and none of them failed", + // which no other field on this surface can express. + dispatchFailures: this.#readinessReconcileLastSweep.dispatchFailures, + ...(Object.keys(this.#readinessReconcileLastSweep.dispatchFailureReasons).length > 0 + ? { dispatchFailureReasons: { ...this.#readinessReconcileLastSweep.dispatchFailureReasons } } + : {}), lastEnumeratedAtMs: this.#readinessReconcileLastSweep.enumeratedAtMs, } : {}), @@ -20254,20 +20294,27 @@ type RelayfileOverload = { retryAfterSeconds?: number } -const relayfileOverload = (error: unknown): RelayfileOverload | undefined => { - const flat = asRecord(error) ?? {} +/** How far to follow wrapped provider failures without trusting an unbounded chain. */ +const RELAYFILE_OVERLOAD_CAUSE_DEPTH = 4 + +const relayfileOverload = (error: unknown, depth = 0): RelayfileOverload | undefined => { + if (depth > RELAYFILE_OVERLOAD_CAUSE_DEPTH) return undefined + const flat = asRecord(error) + if (!flat) return undefined const response = asRecord(flat.response) ?? {} const data = asRecord(flat.data) ?? asRecord(response.data) ?? {} const details = asRecord(flat.details) ?? asRecord(data.details) ?? {} const statusValue = flat.status ?? flat.statusCode ?? response.status ?? response.statusCode const status = typeof statusValue === 'number' ? statusValue : Number(statusValue) - if (status !== 429) return undefined - const retryValue = flat.retryAfterSeconds ?? details.retryAfterSeconds ?? data.retryAfterSeconds - const parsedRetry = typeof retryValue === 'number' ? retryValue : Number(retryValue) - const retryAfterSeconds = Number.isFinite(parsedRetry) && parsedRetry >= 0 ? parsedRetry : undefined - const reason = stringValue(flat.reason) ?? stringValue(details.reason) ?? stringValue(data.reason) ?? - stringValue(flat.code) ?? stringValue(data.code) ?? 'rate_limited' - return { status, reason, ...(retryAfterSeconds === undefined ? {} : { retryAfterSeconds }) } + if (status === 429) { + const retryValue = flat.retryAfterSeconds ?? details.retryAfterSeconds ?? data.retryAfterSeconds + const parsedRetry = typeof retryValue === 'number' ? retryValue : Number(retryValue) + const retryAfterSeconds = Number.isFinite(parsedRetry) && parsedRetry >= 0 ? parsedRetry : undefined + const reason = stringValue(flat.reason) ?? stringValue(details.reason) ?? stringValue(data.reason) ?? + stringValue(flat.code) ?? stringValue(data.code) ?? 'rate_limited' + return { status, reason, ...(retryAfterSeconds === undefined ? {} : { retryAfterSeconds }) } + } + return relayfileOverload(flat.cause, depth + 1) } /** @@ -20885,16 +20932,21 @@ const PASS_FATAL_CAUSE_DEPTH = 4 * classification has to follow the cause chain rather than trust the outermost * type. */ -const wrapsErrorOfType = ( +const findWrappedErrorOfType = ( error: unknown, - type: abstract new (...args: never[]) => Error, + type: abstract new (...args: never[]) => T, depth = 0, -): boolean => { - if (depth > PASS_FATAL_CAUSE_DEPTH || !(error instanceof Error)) return false - if (error instanceof type) return true - return wrapsErrorOfType((error as { cause?: unknown }).cause, type, depth + 1) +): T | undefined => { + if (depth > PASS_FATAL_CAUSE_DEPTH || !(error instanceof Error)) return undefined + if (error instanceof type) return error + return findWrappedErrorOfType((error as { cause?: unknown }).cause, type, depth + 1) } +const wrapsErrorOfType = ( + error: unknown, + type: abstract new (...args: never[]) => Error, +): boolean => findWrappedErrorOfType(error, type) !== undefined + /** * How many *unclassified* per-item failures without an intervening successful * dispatch end the pass. Named per-item conditions (a lifecycle claim refusal, @@ -20962,6 +21014,64 @@ const perItemDispatchSkipReason = (error: unknown): string => { return `dispatch failed (${telemetryErrorClass(error)})` } +/** + * How far a work unit got before it threw, which is half of its classification. + * + * The per-issue `try` in `#performRunOnce` spans three different jobs: the + * durable/scope gates, triage, then dispatch. They fail for unrelated reasons + * and belong to unrelated owners, and once a thrown value carries no name this + * vocabulary knows, the phase is the only thing left that still says who should + * look. Tracked by assignment rather than inferred, so it cannot drift. + */ +type DispatchAttemptPhase = 'gate' | 'triage' | 'dispatch' + +const UNCLASSIFIED_PHASE_CODES: Readonly> = { + gate: 'unclassified-gate', + triage: 'unclassified-triage', + dispatch: 'unclassified-dispatch', +} + +/** + * Why a dispatch attempt failed, as a code from the published vocabulary (#355). + * + * The sibling of `perItemDispatchSkipReason`: that returns the operator's + * sentence, this returns the one token that may cross onto the unauthenticated + * health surface, and both are recorded at the skip site from the same thrown + * value. The token is never parsed back out of the sentence — a reworded + * message would silently empty a bucket, and this vocabulary is what an + * operator reads when the daemon's stdout does not reach them. + * + * Ordered most specific first. Every branch follows the bounded cause chain, + * because `contextualError` and the control-plane guard both rethrow wrapped. + * `relayfileOverload` is also the loop's shedding predicate, so widening it at + * the source keeps the health code, skip counter, fuse, and durable overload + * ratchet on one verdict instead of merely relabelling the published bucket. + */ +const perItemDispatchFailureCode = ( + error: unknown, + phase: DispatchAttemptPhase, +): FactoryDispatchFailureReasonCode => { + if (relayfileOverload(error) !== undefined) return 'relayfile-overloaded' + if (wrapsErrorOfType(error, LiveDispatchStateChangedError)) return 'live-state-changed' + if (wrapsErrorOfType(error, LatePlacementReleasedError)) return 'late-placement-released' + const refused = findWrappedErrorOfType(error, DispatchLifecycleClaimRefusedError) + if (refused) return refused.refusal === 'terminal' ? 'lifecycle-terminal' : 'lifecycle-owned-elsewhere' + if (wrapsErrorOfType(error, FleetControlPlaneCircuitOpenError)) return 'control-plane-open' + // The class-name tail of the vocabulary; see its own doc comment for why + // these five are not `instanceof`. Walked down the cause chain like the + // branches above, because `contextualError` wraps in a plain `Error` and + // reading only the outermost name would miss every wrapped spawn failure. + // `telemetryErrorClass` is the same allowlist that guards every other + // identifier leaving this process, so a hostile `name` cannot invent a key + // here either — it collapses to `Error`, which the map does not hold. + for (let cursor: unknown = error, depth = 0; cursor instanceof Error && depth <= PASS_FATAL_CAUSE_DEPTH; depth += 1) { + const named = factoryDispatchFailureReasonCodeForErrorClass(telemetryErrorClass(cursor)) + if (named) return named + cursor = (cursor as { cause?: unknown }).cause + } + return UNCLASSIFIED_PHASE_CODES[phase] +} + const triageEscalationQuestion = (decision: TriageDecision, issue?: { title?: string }): string => { const routedRepos = decision.routes.map((route) => route.repo).filter(Boolean) const subject = issue?.title?.trim() || decision.issue.key diff --git a/src/orchestrator/public-health.test.ts b/src/orchestrator/public-health.test.ts index d1f8eda8..40657546 100644 --- a/src/orchestrator/public-health.test.ts +++ b/src/orchestrator/public-health.test.ts @@ -988,4 +988,148 @@ describe('sweep counters on the public surface (#355)', () => { expect(JSON.stringify(reread)).not.toContain('workspace-key') expect(reread?.readinessReconcile?.skipReasons).toEqual({ other: 7 }) }) + + // The measurement this whole block exists for: the live container publishes + // `skipReasons: { 'dispatch-failed': 5 }` on every sweep, with the breaker + // closed and readiness healthy, and that bucket is a count with no cause + // attached. The daemon knows the cause; it writes it to stdout, which does + // not reach the deployed operator. + it('breaks the dispatch-failed bucket down by cause, and the parts sum to its total', () => { + const readiness = swept({ + candidates: 27, + dispatched: 0, + skipped: 27, + skipReasons: { 'not-ready': 21, 'parked-dependency': 1, 'dispatch-failed': 5 }, + dispatchFailures: 5, + dispatchFailureReasons: { 'spawn-ack-timeout': 3, 'unclassified-dispatch': 2 }, + }) + + expect(readiness?.dispatchFailures).toBe(5) + expect(readiness?.dispatchFailureReasons) + .toEqual({ 'spawn-ack-timeout': 3, 'unclassified-dispatch': 2 }) + expect(Object.values(readiness?.dispatchFailureReasons ?? {}).reduce((sum, n) => sum + n, 0)) + .toBe(readiness?.dispatchFailures) + // ...and to the bucket it refines, which is the integrity check a reader + // holding both numbers can actually run. + expect(readiness?.dispatchFailures).toBe(readiness?.skipReasons?.['dispatch-failed']) + }) + + // THE CONTROL. `skipReasons` omits zero counts, so on that field alone + // "every dispatch succeeded" is the same absence as "this producer has never + // heard of dispatch failures" — and 0.1.72 is in production right now being + // exactly the second thing. `dispatchFailures` is the field that separates + // them, and it only does that if nothing coerces its absence to a zero or + // its zero to an absence. + it('keeps a zero, an absence and a producer without the field three different readings', () => { + const noneFailed = swept({ + candidates: 4, + dispatched: 4, + skipped: 0, + dispatchFailures: 0, + }) + expect(noneFailed?.dispatchFailures).toBe(0) + expect(Object.hasOwn(noneFailed ?? {}, 'dispatchFailures')).toBe(true) + // No breakdown, because there is nothing to break down — and the total + // still says so out loud. + expect(Object.hasOwn(noneFailed ?? {}, 'dispatchFailureReasons')).toBe(false) + + // A 0.1.72 daemon: the trio it does publish must survive intact, or this + // change would delete the counters that are currently the only view of the + // outage. + const olderProducer = swept({ candidates: 4, dispatched: 4, skipped: 0 }) + expect(olderProducer).toMatchObject({ candidates: 4, dispatched: 4, skipped: 0 }) + expect(Object.hasOwn(olderProducer ?? {}, 'dispatchFailures')).toBe(false) + + // No sweep has completed at all. + const neverRan = swept() + expect(Object.hasOwn(neverRan ?? {}, 'dispatchFailures')).toBe(false) + }) + + it('re-reads its own published zero without turning it back into an absence', () => { + const published = swept({ candidates: 4, dispatched: 4, skipped: 0, dispatchFailures: 0 }) + const reread = normalizePublicHealth({ + schemaVersion: FACTORY_PUBLIC_HEALTH_SCHEMA_VERSION, + ok: true, + status: 'ok', + stale: false, + degradedSubsystems: [], + readinessReconcile: published, + }) + expect(reread?.readinessReconcile?.dispatchFailures).toBe(0) + expect(Object.hasOwn(reread?.readinessReconcile ?? {}, 'dispatchFailures')).toBe(true) + }) + + // MUST-NOT-FIRE, the same leak vector #358 closed one level up: the + // breakdown's *keys* arrive from a remote record, and an object key is as + // publishable as a value. + it('rebuilds the dispatch-failure breakdown from its own vocabulary', () => { + const readiness = swept({ + candidates: 9, + dispatched: 0, + skipped: 9, + dispatchFailures: 9, + dispatchFailureReasons: { + 'spawn-ack-timeout': 4, + // Not in the vocabulary, and carrying exactly what must never publish. + ['AR-350 /linear/issues/AR-350__uuid.json']: 3, + ['Error: connect ECONNREFUSED 10.0.0.4:443']: 2, + } as Record, + }) + + expect(JSON.stringify(readiness)).not.toContain('AR-350') + expect(JSON.stringify(readiness)).not.toContain('/linear/issues') + expect(JSON.stringify(readiness)).not.toContain('ECONNREFUSED') + // Folded into `other`, not dropped, so the parts still sum to the total and + // a reader comparing them detects a newer producer rather than a broken + // counter. + expect(readiness?.dispatchFailureReasons).toEqual({ 'spawn-ack-timeout': 4, other: 5 }) + expect(Object.values(readiness?.dispatchFailureReasons ?? {}).reduce((sum, n) => sum + n, 0)) + .toBe(9) + }) + + it('drops counts a reader cannot use, and a breakdown that has lost its total', () => { + expect(swept({ + candidates: 1, + dispatched: 0, + skipped: 1, + dispatchFailures: 1, + dispatchFailureReasons: { + 'spawn-ack-timeout': Number.NaN, + 'timed-out': -3, + 'live-state-changed': 0, + } as Record, + })?.dispatchFailureReasons).toBeUndefined() + + // A breakdown with no total is an orphan: nothing to check the parts + // against, which is the one integrity check this surface offers. + const orphaned = swept({ + candidates: 1, + dispatched: 0, + skipped: 1, + dispatchFailureReasons: { 'timed-out': 1 } as Record, + }) + expect(Object.hasOwn(orphaned ?? {}, 'dispatchFailureReasons')).toBe(false) + }) + + it('applies the same key rebuild to a record that arrived over the wire', () => { + const reread = normalizePublicHealth({ + schemaVersion: FACTORY_PUBLIC_HEALTH_SCHEMA_VERSION, + ok: true, + status: 'ok', + stale: false, + degradedSubsystems: [], + readinessReconcile: { + state: 'healthy', + consecutiveFailures: 0, + failureThreshold: 3, + candidates: 7, + dispatched: 0, + skipped: 7, + dispatchFailures: 7, + dispatchFailureReasons: { '/srv/agent-workforce/.relay/workspace-key': 7 }, + }, + }) + expect(JSON.stringify(reread)).not.toContain('workspace-key') + expect(reread?.readinessReconcile?.dispatchFailureReasons).toEqual({ other: 7 }) + }) }) diff --git a/src/orchestrator/public-health.ts b/src/orchestrator/public-health.ts index ed11a2b0..b761dd15 100644 --- a/src/orchestrator/public-health.ts +++ b/src/orchestrator/public-health.ts @@ -8,6 +8,11 @@ import { factorySweepSkipReasonCode, } from './sweep-skip-reason' import type { FactorySweepSkipReasonCode } from './sweep-skip-reason' +import { + FACTORY_DISPATCH_FAILURE_REASON_CODES, + factoryDispatchFailureReasonCode, +} from './dispatch-failure-reason' +import type { FactoryDispatchFailureReasonCode } from './dispatch-failure-reason' import type { FactoryDispatchCapacityStatus, FactoryEventListenerStatus, @@ -212,6 +217,35 @@ const skipReasonCounts = ( return Object.fromEntries(ordered.map((code) => [code, counts[code] as number])) } +/** + * The last sweep's dispatch-failure breakdown, rebuilt key by key (#355). + * + * Identical discipline to `skipReasonCounts` and for the identical reason: the + * keys come from this module's own copy of the vocabulary, never from the + * record, so a producer on another version cannot put an arbitrary string onto + * an unauthenticated surface by using it as an object key. Unknown keys fold + * into `other` rather than being dropped, so the parts still sum to + * `dispatchFailures`. + */ +const dispatchFailureReasonCounts = ( + value: unknown, +): Partial> | undefined => { + const record = plainRecord(value) + if (!record) return undefined + const counts: Partial> = {} + for (const [key, raw] of Object.entries(record)) { + const parsed = finiteNumber(raw) + if (parsed === undefined || parsed < 0) continue + const floored = Math.floor(parsed) + if (floored === 0) continue + const code = factoryDispatchFailureReasonCode(key) + counts[code] = (counts[code] ?? 0) + floored + } + const ordered = FACTORY_DISPATCH_FAILURE_REASON_CODES.filter((code) => counts[code] !== undefined) + if (ordered.length === 0) return undefined + return Object.fromEntries(ordered.map((code) => [code, counts[code] as number])) +} + /** * The last enumerating sweep's arithmetic, published (#355). * @@ -232,14 +266,23 @@ const sweepOutcome = ( dispatched?: unknown skipped?: unknown skipReasons?: unknown + dispatchFailures?: unknown + dispatchFailureReasons?: unknown discoveryDeferred?: unknown lastEnumeratedAtMs?: unknown enumerationCountsInvalid?: unknown }, ): Partial> => { // Independent of the trio (#358 review, CodeRabbit): the counts describe the // last sweep that ENUMERATED, and this describes the most recent pass. A @@ -268,11 +311,27 @@ const sweepOutcome = ( } } const skipReasons = skipReasonCounts(status.skipReasons) + // Deliberately NOT joined to the all-or-nothing trio above. A daemon on + // 0.1.72 publishes the trio and knows nothing about this field, and requiring + // it would drop that producer's whole sweep block — deleting the counters + // that are currently the only view of the outage. Independently optional, and + // `optionalCount` keeps a zero a zero: absent means "no completed sweep, or a + // producer without the field", `0` means "a sweep completed and no dispatch + // it attempted failed". Those are the two facts a bucket count cannot tell + // apart, which is why this number exists next to the breakdown. + const dispatchFailures = optionalCount('dispatchFailures', status.dispatchFailures) + const dispatchFailureReasons = dispatchFailureReasonCounts(status.dispatchFailureReasons) return { ...candidates, ...dispatched, ...skipped, ...(skipReasons ? { skipReasons } : {}), + ...dispatchFailures, + // A breakdown with no total is an orphan: a reader cannot check that the + // parts sum, which is the one integrity check this surface offers. + ...(dispatchFailures.dispatchFailures !== undefined && dispatchFailureReasons + ? { dispatchFailureReasons } + : {}), // Part of the same atomic snapshot as the counts: it is what dates them, // and without it retained counts have no freshness a reader can recover // (#359 review). diff --git a/src/types.ts b/src/types.ts index 17c52af7..4df9ed71 100644 --- a/src/types.ts +++ b/src/types.ts @@ -9,6 +9,7 @@ import type { CloseProbePrInput, CloseProbePrResult } from './github/probe-close import type { GhRunner, GithubMergeGate } from './github/merge-gate' import type { AgentProcessFinder, ProcessIdentity } from './orchestrator/process-identity' import type { FactorySweepSkipReasonCode } from './orchestrator/sweep-skip-reason' +import type { FactoryDispatchFailureReasonCode } from './orchestrator/dispatch-failure-reason' import type { DispatchRelayflowOptions, RelayflowPolicyRegistry } from './dispatch/relayflow-registry' import type { VerificationGate } from './environments/verification-pipeline' import type { CostLedger } from './cost/ledger' @@ -266,6 +267,24 @@ export interface FactoryReadinessReconcileStatus { * themselves are a fixed published vocabulary, so an absent key is a zero. */ skipReasons?: Partial> + /** + * Dispatch attempts the last enumerating sweep made that failed (#355). + * + * The same number `skipReasons['dispatch-failed']` carries, published in its + * own right so it can be a zero. `skipReasons` omits zero-count codes, so + * "every dispatch succeeded" and "this daemon does not report the field" are + * the same absence there; here they are not. + * + * Optional and never defaulted, exactly like `candidates`: absent means no + * sweep has completed *or* the producer predates the field, `0` means a sweep + * completed and nothing it dispatched failed. + */ + dispatchFailures?: number + /** + * `dispatchFailures` split by cause. Zero-count codes are omitted; the codes + * are a fixed published vocabulary, so an absent key is a zero. + */ + dispatchFailureReasons?: Partial> /** * When the pass the counts above describe finished enumerating (#359 review). * @@ -327,6 +346,17 @@ export interface FactoryPublicReadinessReconcileHealth { skipped?: number /** `skipped` split by a closed vocabulary of causes; zero counts omitted. */ skipReasons?: Partial> + /** + * Failed dispatch attempts in the last enumerating sweep, and why (#355). + * + * `dispatchFailures` is absent until a sweep completes and is a zero + * thereafter, so it separates "never attempted" from "attempted, none + * failed" — which `skipReasons` alone cannot, since it drops zero counts. + * The breakdown is counts only, keyed by a fixed vocabulary, and its parts + * sum to `dispatchFailures`. + */ + dispatchFailures?: number + dispatchFailureReasons?: Partial> /** * When the pass the counts describe finished enumerating. Dates them — * `lastCompletedAtMs` does not, since it advances on deferred passes too. @@ -653,7 +683,17 @@ export interface IterationReport { * `reason` is free text for an operator; `code` is the closed vocabulary * that may cross onto the unauthenticated health surface (#355). */ - skipped: Array<{ issue: IssueRef; reason: string; code?: FactorySweepSkipReasonCode }> + skipped: Array<{ + issue: IssueRef + reason: string + code?: FactorySweepSkipReasonCode + /** + * Why the dispatch *attempt* failed, for `code: 'dispatch-failed'` only + * (#355). Recorded at the skip site by type, never parsed back out of + * `reason`, and absent on every other skip code. + */ + failureCode?: FactoryDispatchFailureReasonCode + }> dryRun: boolean slackDegraded?: boolean /**