Decouple the Codex captured-env path from the generic exporter - #70
Merged
Conversation
#69 only consulted the persisted meta snapshot when config.capture_env_patterns was non-empty. But that value comes from THIRDEYE_CAPTURE_ENV in os.environ -- the same detached notify environment that is missing WB_*. When Codex spawns notify with a bare env (neither var present), the guard was false and the fallback never ran, so codex turns still exported without wb.* attributes or tags. Gate on platform == "codex" instead: it is the only platform whose turn export runs detached from the agent env. Cursor and Claude export from a real child hook, so their os.environ stays authoritative and a stale snapshot can never leak in. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The earlier approach threaded session_dir_ and a `platform == "codex"`
check into otel_export._capture_attributes -- platform branching in a
module whose docstring says platform branching does not belong there.
Instead:
- otel_export exposes shape_captured_env(raw) -> attributes+tags, and
export_turn / export_spans / export_subagent_turn take an optional
keyword `captured_env`. When omitted the raw dict is read from
os.environ (claude, cursor, pr_writer -- unchanged). When supplied it
is used verbatim.
- platforms/codex/captured_env.resolve_captured_env() does the Codex-only
resolution -- live capture, else the session-start snapshot in meta --
and codex/hooks.py + interrupt_marker.py pass the result in.
Also make the attribute-key transform uniform: every captured name
becomes `name.lower().replace("_", ".")`, so WB_PLAN -> wb.plan and
BUILD_LABEL -> build.label. No pattern is privileged; the workbench
namespace falls out of the general rule rather than a hardcoded prefix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
duncankmckinnon
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #69 (v0.2.1). Two problems with how the codex captured-env fallback landed:
session_dir_+ aplatform == "codex"string check intootel_export._capture_attributes— platform branching in a module whose own docstring says that doesn't belong there.WB_(WB_PLAN→wb.plan, everything else → flatsnake_case). Matching was already generic; only the key shape was privileged.Changes
Generic exporter stays platform-agnostic
otel_export.shape_captured_env(raw)→{attributes, logfire.tags}, with a uniform transform:name.lower().replace("_", ".").WB_PLAN→wb.plan(unchanged),BUILD_LABEL→build.label. The workbench namespace falls out of the general rule.export_turn/export_spans/export_subagent_turntake an optional keywordcaptured_env. Omitted → read fromos.environvia the configured patterns (claude, cursor, pr_writer: unchanged). Supplied → used verbatim.Codex owns its own quirk
platforms/codex/captured_env.resolve_captured_env(config, session_dir_)— livecapture_envfirst, else the session-start snapshot persisted inmeta.extra["captured_env"](that persist, from Fall back to a persisted env snapshot for Codex turn export #69, is unchanged).codex/hooks.py::notify()andcodex/interrupt_marker.pycall it and passcaptured_env=.Composes with #71:
Config.load()now fillscapture_env_patternsfromconfig.yaml, so the codex notify process has patterns even withoutTHIRDEYE_CAPTURE_ENV; this PR covers the case where the values aren't in that process's env.Tests
test_otel_export.py:shape_captured_envuniform transform + tag dedup;export_turn(..., captured_env=...)forwarding and precedence.test_codex_captured_env.py:resolve_captured_env— live wins, meta fallback, both-empty, missing/'malformed meta.test_codex_tracing.py: mocks widened to accept the new kwarg.🤖 Generated with Claude Code