diff --git a/agent/src/observability.py b/agent/src/observability.py index 460b060d5..81e86af5e 100644 --- a/agent/src/observability.py +++ b/agent/src/observability.py @@ -15,6 +15,8 @@ from opentelemetry import baggage, context, trace from opentelemetry.trace import StatusCode +from shell import log + if TYPE_CHECKING: from collections.abc import Generator @@ -69,6 +71,12 @@ def current_otel_trace_id() -> str | None: propagating. Callers read this inside DDB-write try-blocks (progress_writer), where a raised trace error would otherwise be misclassified as a DDB failure and trip the shared progress circuit breaker. + + The degrade is *logged*, not silent (#756): a tracer fault and "no recording + span" both yield ``None``, and a caller cannot tell them apart, so the WARN + line is the only place that difference survives. It is deliberately emitted + per occurrence rather than once — this runs per progress event, so repetition + is the signal that the tracer is broken for the whole task, not one event. """ try: span = trace.get_current_span() @@ -80,8 +88,13 @@ def current_otel_trace_id() -> str | None: # the X-Ray console renders trace ids as ``1-{8hex}-{24hex}``; to look this # up there, transform to that form (the timestamp is the first 8 hex chars). return trace.format_trace_id(ctx.trace_id) - except Exception: - # nosemgrep: py-silent-success-masking -- tracer fault must not fail the write path + except Exception as exc: + log( + "WARN", + f"current_otel_trace_id: tracer fault ({type(exc).__name__}: {exc}); " + "persisting the record without a correlation id", + ) + # nosemgrep: py-silent-success-masking -- graceful-missing correlation field, logged above; raising would surface inside the caller's DDB-write try-block and trip the progress circuit breaker (see docstring) # noqa: E501 return None diff --git a/agent/src/server.py b/agent/src/server.py index 50942d8f8..9357d13c6 100644 --- a/agent/src/server.py +++ b/agent/src/server.py @@ -1444,6 +1444,7 @@ def _parse_terminate_microvm_id(raw: bytes) -> str: # Not silent: an unreadable body is worth a breadcrumb even though it # cannot change the outcome. _emit_stdout_line(f"[server/warn] /terminate hook body is not JSON ({exc}); ignoring it") + # nosemgrep: py-silent-success-masking -- /terminate must answer 200 for ANY body (see docstring); "" is the EXPECTED production id, raising would report a hook failure for a teardown that succeeded, and the unreadable body is logged above # noqa: E501 return "" if not isinstance(parsed, dict): _emit_stdout_line( diff --git a/agent/tests/test_observability.py b/agent/tests/test_observability.py index d07b8d498..aa7bbcd52 100644 --- a/agent/tests/test_observability.py +++ b/agent/tests/test_observability.py @@ -54,6 +54,33 @@ def test_degrades_to_none_when_tracer_raises(self): ): assert observability.current_otel_trace_id() is None + def test_logs_the_tracer_fault_rather_than_degrading_silently(self, capfd): + # The degrade is justified but must not be invisible (#756): a tracer + # fault and "no recording span" both return None, so the WARN line is the + # only surviving evidence of which one happened. capfd (not capsys) + # because ``shell.log`` writes at the fd level — see its docstring. + with patch.object( + observability.trace, "get_current_span", side_effect=RuntimeError("tracer boom") + ): + assert observability.current_otel_trace_id() is None + out = capfd.readouterr().out + assert "current_otel_trace_id: tracer fault" in out + assert "RuntimeError" in out + assert "tracer boom" in out + + def test_does_not_log_when_there_is_simply_no_recording_span(self, capfd): + # The common, uninteresting case: tracing disabled locally. An invalid + # span context is not a fault, so it must stay quiet — otherwise every + # progress event in a non-traced run emits a WARN and the real fault + # signal above becomes noise. + span = MagicMock() + ctx = MagicMock() + ctx.is_valid = False + span.get_span_context.return_value = ctx + with patch.object(observability.trace, "get_current_span", return_value=span): + assert observability.current_otel_trace_id() is None + assert "tracer fault" not in capfd.readouterr().out + class TestPropagateCorrelationContext: """``propagate_correlation_context`` propagates the correlation envelope diff --git a/cdk/src/handlers/registry-publish.ts b/cdk/src/handlers/registry-publish.ts index 07b3d1f21..786964e19 100644 --- a/cdk/src/handlers/registry-publish.ts +++ b/cdk/src/handlers/registry-publish.ts @@ -31,6 +31,7 @@ import { REGISTRY_KINDS, RESERVED_KINDS, parseConstraint } from './shared/regist import { RegistryPublishIncompleteError, type PublishInput, type RuntimePayload } from './shared/registry/types'; import { ErrorCode, errorResponse, successResponse } from './shared/response'; import type { RegistryPublishRequest, RegistryRecordResponse } from './shared/types'; +import { parseBody } from './shared/validation'; const NAMESPACE_RE = /^[a-z][a-z0-9-]*$/; const NAME_RE = /^[a-z0-9][a-z0-9._-]*$/; @@ -52,7 +53,11 @@ export async function handler(event: APIGatewayProxyEvent): Promise(event.body); if (!body) { return errorResponse(400, ErrorCode.VALIDATION_ERROR, 'Request body must be valid JSON.', requestId); } @@ -108,16 +113,6 @@ export async function handler(event: APIGatewayProxyEvent): Promise