Repository navigation
refactor(sandbox)!: forward any attribution key as sandbox tags instead of a fixed trio - #49
polakamtejas wants to merge 1 commit into
Conversation
…ad of a fixed trio The Modal and E2B providers kept only product, customer and team from a sandbox's attribution, so every other key was dropped. That fixed set was kept when attribution became an open dict, so emitted billing labels stayed byte-identical for one deployment; core no longer names any dimension. Modal App tags are now the deployment-wide [sandbox.attribution] defaults only: one App serves every run and its tags are set on first lookup, so a run's values there were the first run's. A run's attribution, unset keys filled from those defaults, goes on each sandbox's tags with keys and values made Modal-valid; an empty value is left out, since Modal has no empty tag. E2B sandbox metadata carries the same resolved attribution, and an attribution key cannot stand in for agent-env's exposed-ports record. The sandbox-started log line nests the full tag set under modal_sandbox_tags, so an arbitrary key cannot clash with a LogRecord attribute; pipeline_step and run_id stay top-level. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| key: _modal_tag_value(attribution[key]) | ||
| for key in (PIPELINE_STEP_KEY, RUN_ID_KEY) | ||
| if attribution.get(key) | ||
| _modal_tag_value(key): _modal_tag_value(str(value)) |
There was a problem hiding this comment.
Modal drops an attribution label
If a run has both billing team and billing/team, _modal_tag_value turns both keys into billing-team. The later value silently replaces the earlier one, so the run loses a label. Reject keys that collide or give them distinct tag names.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agent_env/providers/sandbox_providers/modal_sandbox.py
Line: 420
Comment:
**Modal drops an attribution label**
If a run has both `billing team` and `billing/team`, `_modal_tag_value` turns both keys into `billing-team`. The later value silently replaces the earlier one, so the run loses a label. Reject keys that collide or give them distinct tag names.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| _modal_tag_value(key): _modal_tag_value(str(value)) | ||
| for key, value in attribution.items() | ||
| if value is not None and str(value) != "" |
There was a problem hiding this comment.
Empty key stops a sandbox
If a step supplies {"": "value"}, _modal_tags keeps the empty key because it filters only empty values. The key then reaches Modal even though the tag format requires at least one character, so Modal can reject sandbox creation. Reject empty keys before sending tags.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agent_env/providers/sandbox_providers/modal_sandbox.py
Line: 420-422
Comment:
**Empty key stops a sandbox**
If a step supplies `{"": "value"}`, `_modal_tags` keeps the empty key because it filters only empty values. The key then reaches Modal even though the tag format requires at least one character, so Modal can reject sandbox creation. Reject empty keys before sending tags.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| } | ||
| Modal's billing report breaks cost down by App tags, and one App serves every run with its | ||
| tags set on first lookup, so per-run attribution goes on sandbox tags instead.""" | ||
| return _modal_tags(apply_default_attribution({})) |
There was a problem hiding this comment.
Old App tags remain
When [sandbox.attribution] has no defaults, _build_app_tags returns {}. Both Modal providers skip set_tags for an empty dictionary, so an existing App keeps tags from earlier runs or an earlier configuration. After an upgrade or removal of defaults, Modal can keep grouping billing under those old labels. Clear the App's tags when the configured set is empty.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agent_env/providers/sandbox_providers/modal_sandbox.py
Line: 407
Comment:
**Old App tags remain**
When `[sandbox.attribution]` has no defaults, `_build_app_tags` returns `{}`. Both Modal providers skip `set_tags` for an empty dictionary, so an existing App keeps tags from earlier runs or an earlier configuration. After an upgrade or removal of defaults, Modal can keep grouping billing under those old labels. Clear the App's tags when the configured set is empty.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Stacked on #45; retarget to main once #45 merges.
This finishes the move to generic attribution: core no longer names any attribution dimension. The Modal and E2B providers kept only
product,customerandteamfrom a sandbox's attribution (_build_cost_attribution_tags), and every other key was silently dropped. That fixed set was kept on purpose when attribution became an opendict[str, str], so emitted billing labels stayed byte-identical for one deployment. In an open-source core it's just three names that nothing else defines.Modal: App tags vs sandbox tags
_build_app_tags) are the deployment-wide config.toml[sandbox.attribution]defaults only, under any keys. Modal's billing report breaks cost down by App tags. Since refactor(attribution)!: remove project_id and the LiteLLM cost attribution built on it #45 one App serves every run, and its tags are set on first lookup, so a run's own values there were really the first run's._build_sandbox_tags) are the run's full attribution, with unset keys filled from[sandbox.attribution]. That includespipeline_stepandrun_idfrom the deploy step; a task-authored value still wins, as before. Before, sandbox tags carried onlypipeline_stepandrun_id._modal_tag_value:[a-zA-Z0-9._-], at most 63 characters (Modal'scheck_tag_dict). An unset or empty value is left out, since Modal has no empty tag. An explicit empty value still isn't replaced by the default.modal_vm.E2B
_attribution_metadata). E2B no longer imports a Modal-private helper.agent_env_exposed_portsstays agent-env's own: an attribution key of that name is never written, with or without exposed ports, so reconnect can't misread it.Log line
_log_sandbox_startedused to spread the sandbox tags into the log record'sextra. With arbitrary keys, one namednameormessagewould raise fromLogRecord. The full set is now undermodal_sandbox_tags;pipeline_stepandrun_idstay top-level, so the existing container-id join is unchanged.Docs: one sentence in AGENTS.md (providers row), and a commented
[sandbox.attribution]example in.agentenv/config.example.toml.Breaking for downstream
product/customer/teamfrom a caller's attribution. They carry exactly what[sandbox.attribution]configures, so a deployment that billed on those keys sets them there.modal_sandbox.py, don't appear in Modal's billing report.modal_sandbox_tags.check_plugin_api.pyreports no plugin-surface break; the!is for the emitted-tag change.Testing
attribution_wire_test.pyis rewritten for the generic wire, with neutral sample keys (group,owner,batch):modal_vmmodal_sandbox_tags_test.py: sandbox tags carry every key. A provider-level test checks thatcreate_containergives_get_appthe configured defaults only, while the sandbox gets the run's attribution over them.e2b_sandbox_provider_test.py: arbitrary keys pass through, configured defaults fill unset keys, and an attributionagent_env_exposed_portsnever lands, with or without ports.modal_sandbox_started_log_test.py: a tag namednamelogs without clashing. It fails against the old spreading code.teamonly as a sample key now usegroup.test_the_quartet_is_no_longer_a_keyword_on_any_backendkeepsteam=on purpose: it checks the old keyword is refused.pytest tst/unit packages/agentenv-protocol/tests -n auto: 5783 passed. The only failures are the 2test_mcp_cli_buildertests, which fail the same way on main (the generated CLI's interpreter has noclick).git grep -nE '"(product|customer|team)"' -- srcis empty.🤖 Generated with Claude Code
This PR is not ready to merge because Modal can lose attribution labels, reject an empty tag key, or keep old App billing labels.
Fix with agent prompt
Summary
Modal and E2B now carry attribution under any key, filling values from
[sandbox.attribution]when a run leaves them unset. Modal keeps deployment defaults on App tags and run values on sandbox tags; E2B writes them to sandbox metadata.pipeline_stepandrun_idat the top level.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR C["[sandbox.attribution] defaults"] --> A["Modal App tags"] C --> M["Merge with run attribution"] R["Run attribution"] --> M M --> S["Modal sandbox tags"] M --> E["E2B metadata"]Reviews (1) · Last reviewed commit: "refactor(sandbox)!: forward any attribut..."