fix(bots): give the engineer-bot e2e repro a token the env scrub can't strip#872
Merged
Merged
Conversation
…t strip The bug-fix flow's REQUIRED tests/e2e repro authenticates through conftest.py, which reads the token from DATABRICKS_TOKEN. But the engineer-bot runs pytest in an agent-driven subprocess whose environment is scrubbed of every credential- shaped variable (the engine's shared/env_scrub.py strips *TOKEN*/*SECRET*/... ), so DATABRICKS_TOKEN is gone before pytest starts. The connection is then built with access_token=None and hangs — observed on issue #791: every e2e test (incl. existing, unmodified ones) stalled ~10 min per attempt until the 45-min job timeout. The same warehouse/env is used by code-coverage.yml's e2e job, which passes because it runs pytest directly (no agent, no scrub). Fix (per the engine's centralize-bot-workflows design doc): pass the connection details through a file instead of a credential-named env var. - engineer-bot.yml: a new step writes host/http_path/token/user/catalog to $RUNNER_TEMP/e2e-connection.json (built with jq so the secret is never shell- interpolated; chmod 600) and exports its PATH to the author step as DATABRICKS_TEST_CONFIG_FILE — a name env_scrub deliberately preserves. - conftest.py: each connection fixture now falls back to that file when its env var is absent (env var still wins). Normal CI and local dev leave the path var unset, so the file dict is empty and behavior is byte-for-byte unchanged. Author phase only: the follow-up phase deliberately runs mocked tests/unit and forbids tests/e2e, so it needs no live token (left untouched). Signed-off-by: eric-wang-1990 <[email protected]> Co-authored-by: Isaac
eric-wang-1990
temporarily deployed
to
azure-prod
July 17, 2026 20:31 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
eric-wang-1990
temporarily deployed
to
azure-prod
July 17, 2026 20:31 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
![peco-review-bot[bot]](https://avatars.githubusercontent.com/in/3801177?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — a clean, well-scoped fix that passes the e2e token through a file (DATABRICKS_TEST_CONFIG_FILE) the env scrub preserves, keeping the env var as the winning source so normal CI/local dev is unchanged. One low note about the or-based fallback changing empty-string-vs-unset semantics for the fixtures. Nit: _test_config_from_file only guards OSError/ValueError; a valid-but-non-object JSON (e.g. a bare string/list) would pass json.load and later raise AttributeError on .get(...) in the fixtures — not reachable given the jq -n '{...}' writer, but a isinstance(..., dict) check would harden it."
eric-wang-1990
added
the
engineer-bot
eric-wang-1990
temporarily deployed
to
azure-prod
July 17, 2026 22:26 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
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
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.
Problem
The engineer-bot's bug-fix flow requires a live
tests/e2erepro. The e2e suite authenticates throughconftest.py, which reads the token fromDATABRICKS_TOKEN. But the bot runs pytest inside an agent-driven subprocess whose env is scrubbed of every credential-shaped variable (the engine'sshared/env_scrub.pystrips*TOKEN*/*SECRET*/*PASSWORD*/…). SoDATABRICKS_TOKENis removed before pytest starts, the connection is built withaccess_token=None, and it hangs.Observed on #791: every e2e test — including existing, unmodified ones — stalled ~10 min per attempt, burning the 45-min job timeout. The same warehouse and
azure-prodsecrets are used bycode-coverage.yml's e2e job, which passes only because it runs pytest directly (no agent, no scrub). That's the whole difference; it is not warehouse cold-start.Fix
Pass the connection details through a file instead of a credential-named env var — the pattern the engine's
centralize-bot-workflowsdesign doc prescribes verbatim.engineer-bot.yml: a new step writeshost/http_path/token/user/catalogto$RUNNER_TEMP/e2e-connection.json(built withjqso the secret is never shell-interpolated;chmod 600) and exports its path to the author step asDATABRICKS_TEST_CONFIG_FILE— a nameenv_scrubdeliberately preserves.conftest.py: each connection fixture now falls back to that file when its env var is absent. The env var still wins, so normal CI and local dev are byte-for-byte unchanged (they leave the path var unset → empty file dict).Author phase only. The follow-up phase deliberately runs mocked
tests/unitand forbidstests/e2e, so it needs no live token — left untouched.Verification
access_tokenresolves from the file; (b) token present + no config file → env token, unchanged.This pull request and its description were written by Isaac.