Skip to content

fix(harness): a settings template adds only SDK setting names (TH-9048) - #298

Open
khushalsonawat wants to merge 4 commits into
feat/TH-8804-source-subfolderfrom
feat/TH-9048-template-names-from-code
Open

khushalsonawat wants to merge 4 commits into
feat/TH-8804-source-subfolderfrom
feat/TH-9048-template-names-from-code

Conversation

@khushalsonawat

@khushalsonawat khushalsonawat commented Nov 11, 2026 •

Copy link
Copy Markdown
Member

Into feat/TH-8804-source-subfolder.

Summary

The agent scan lists, for each agent, the settings names it needs (env_names), and the platform shows them as the connect form's fields. The list is the names the agent's own code reads, then the names in the repository's settings template (.env.example, .env.sample, .env.template) that the code doesn't read.

A template is free text a person wrote, and it often holds pasted values: keys wrapped over lines, open quotes, key blocks. _template_names guesses which lines are values. In each of TH-8950's three change reviews a new shape put a fragment of a secret value into env_names, and each fix closed only the shapes that review found. A fuzzer over the shapes the contract names (1.1 million templates) still got value fragments through by the tens of thousands.

Now a template adds a name only when it is one of 43 names SDKs read for themselves (scan_walk._SDK_SETTINGS): the model providers' keys and base URLs, the tracing names, LiveKit's names and the per-provider keys its plugins read. The filter sits in _env_names, the one function the folder walk, describe_entry and upload mode all call:

env_names = _env_names(code, templates & _SDK_SETTINGS)

What it promises: whatever a template holds, it can add only names from that fixed list. A line inside a value that spells one of them adds that name; no other text reaches env_names. The class is closed without guessing at shapes. The code's names, their order, the early return and the cap of 50 are unchanged, and _template_names and its shape rules stay as they are.

Left out of the list on purpose, each explained beside it: FI_API_KEY, FI_SECRET_KEY and FI_BASE_URL (the platform's own tracing account, which the runner and the connect form refuse; after Dileep's review), GOOGLE_APPLICATION_CREDENTIALS (a key file, which the platform takes through its credential-file control), OTEL_EXPORTER_OTLP_ENDPOINT and _HEADERS (on hosted runs the platform owns trace export), LiteLLM's and LiveKit's OpenAI-plugin provider names (too many to list), and the platform's own ALK_* names.

Ticket: TH-9048. Contract: scan-parts.md P1.11 and P1.12 (1.3l, agreed with Cedar).

Measured

Run over the repositories holding the 25 distinct real templates on this machine, on both routes:

  • Every agent-shaped repository (a single agent at the repository root) loses no name: its template names are either read by the code or SDK names. That covers 12 traceAI TypeScript examples, traceAI/python/e2e_tests, livekit-traceai-voice-agent, core/docs-cookbooks, growth/ava and langgraph-otel-agent.
  • The losses are in Future AGI's own monorepos and simulation-acceptance. There the root template's infrastructure settings (database, cache and storage passwords, a licence key) were listed on every agent row, though no agent's own files read them. On future-agi, 0-17 shown template-only names a row become 0-5. In simulation-acceptance, the names read through a helper (_env(name)) drop from 10 to 5.
  • None of the corpus is a customer agent. The customer can add any name in the connect form.

Known limits

  • A name read only by code outside the row's own files (a shared module, scripts/, a deeper agent's folder) is no longer vouched for by the template. An upload counts the root, so it keeps them.
  • Names read through helpers, settings dicts, import.meta.env, a pydantic-settings field, or built at run time aren't collected, unless they're SDK names.
  • A name only an SDK not on the list reads for itself (LiteLLM's provider names, for one).
  • The shape rules' existing losses, now only for SDK names; relaxing or deleting the rules is a separate change.
  • An SDK name after a template's first 200 names (_NAMES_PER_FILE, applied before the filter), as today.
  • Older, not from this PR (in the older-findings register, Medium): credentials.py lists ELEVENLABS_API_KEY for LiveKit's ElevenLabs plugin, but the installed plugin reads ELEVEN_API_KEY, so an agent using elevenlabs.TTS() is told the wrong required name. _SDK_SETTINGS imports credentials.py's LiveKit names (since a740fbb2) and lists ELEVEN_API_KEY itself too, so a template that names either is listed.

Tests

  • New:
    • test_a_template_adds_only_sdk_names (the root agent, three routes);
    • test_a_template_value_never_becomes_a_name (each value shape in four encodings, 42 cases, each also checking the template was read);
    • test_a_template_value_never_becomes_a_name_in_random_values (200 seeded values, with a guard that the shapes still let value lines past the reader alone);
    • test_a_value_line_that_spells_an_sdk_name_adds_it;
    • test_a_template_name_matches_exactly;
    • test_a_shared_module_does_not_vouch_for_a_template_name;
    • test_the_sdk_names, whose 43 names are typed out rather than taken from the constant;
    • test_no_sdk_name_is_one_the_runner_refuses, test_the_livekit_names_are_credentials_own and test_a_tracing_template_lists_no_platform_account_name (after Dileep's review).
  • Ten existing tests expected a template's non-SDK names; each is rewritten to the rule and still tests what its name says. With only the source change, exactly those ten failed.

Review

  • Design: two reviewers, two rounds. Round 1: not clear (the base). Round 2: clear. The rule is the Coordinator's call.
  • Plan: two reviewers, clear.
  • Change: two reviewers, clear.
    • One Medium: two comments pointed at the ticket's design page; removed.
    • Two Lows: each value-shape case now checks the template was read, and §9's test list gains the random-values test.
  • Planted bugs: every behaviour's bug was planted in the reviewers' own copies (P1-P10, the filter on one route only, upload unfiltered, a renamed name, and more), and each failed a named test.

Merges with

#293 (Cedar's TH-9449) also adds tests at the end of test_scan_walk.py. The reviewer merged the two: scan_walk.py merges clean, the test file's one conflict is resolved by keeping both, and the merged tree passes.

Verification

  • Where the tests run: CI's SDK smoke runs test_scan_walk.py, test_agent_scan.py, test_scan_document.py, test_scan_ai.py and test_authoring_entrypoint_find_agents.py on every push.
  • Fix-only check of a740fbb2 (Dileep's P2 and P3): clear, no High or Medium (one reviewer, 572 passed on 3.12.3). Its two Lows are fixed in b9da87ac: a test that the LiveKit names are imported from credentials.py, not copied, and one that no listed name starts with ALK_ (the runner refuses that prefix). Fix-only check of b9da87ac: clear, no findings; test_scan_walk.py 212 passed.
  • Fix-only check (one reviewer) of the changes made after the change review cleared (the Medium's two comments removed, the two Lows' tests) and the hand-merged test file in 9617660b: clear, no findings. The merge keeps every test from both sides with none duplicated; 212 passed on 3.12.3.
  • On a740fbb2, Python 3.12.3: the five files above, 377 passed; ruff check clean.
  • Python 3.12.3 (the hosted image), on the merged head 9617660b: test_scan_walk.py 209, test_agent_scan.py 84, test_scan_document.py 17, test_scan_ai.py 59, test_authoring_entrypoint_find_agents.py 5 passed.
  • ruff check clean on both files; changed lines wrapped at 99; compileall on src/.
  • Names only in any output or log, never values.

A template is free text, and each review of the shape rules found a new shape that put a
fragment of a pasted value into env_names. _env_names now adds a template name only when
it is one of 46 names SDKs read for themselves (_SDK_SETTINGS); the code's names, their
order and the cap are unchanged. Whatever a template holds, it can add only names from
that list. The left-out names are explained beside it.

Ten tests that expected a template's other names are rewritten to the rule; new tests
cover each route, every value shape in four encodings, 200 seeded values, an exact match,
shared code, and the list itself, typed out rather than taken from the constant.

TH-9048

Signed-off-by: khushalsonawat <khushalsonawat@gmail.com>
…ate-names-from-code

test_scan_walk.py: both sides appended tests at the end (this branch's template-name tests,

Signed-off-by: khushalsonawat <khushalsonawat@gmail.com>
#293's folder-charge tests); both kept.

@cdileep23 cdileep23 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the comments.

Comment thread src/fi/alk/harness/scan_walk.py Resolved
Comment thread src/fi/alk/harness/scan_walk.py Resolved
…it's names from credentials (TH-9048)

FI_API_KEY, FI_SECRET_KEY and FI_BASE_URL are the platform's own tracing
account, which the runner and the connect form refuse, so a template no
longer adds them: the list is 43 names. A test checks it shares no name
with the runner's reserved set. The LiveKit names and its plugins' keys
now come from credentials.py, so the two lists can't drift.

Signed-off-by: khushalsonawat <khushalsonawat@gmail.com>
…LK_ name is listed (TH-9048)

From the fix-only check of a740fbb: a literal copy of credentials.py's
lists passed every test, and the reserved-name test didn't cover the
runner's ALK_ prefix.

Signed-off-by: khushalsonawat <khushalsonawat@gmail.com>

@cdileep23 cdileep23 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the comments.

"AWS_REGION",
"AWS_DEFAULT_REGION",
}
| frozenset(_CONNECTOR_REQUIREMENTS["livekit"])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3. The code is good now. Two lines in the PR body are stale: the Known limits bullet still says _SDK_SETTINGS copies credentials.py's list, but it imports it since a740fbb (here); and the Verification box "Fix-only check pending ... the hand-merged test file in 9617660" is still unticked. Update both.

@cdileep23 cdileep23 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the comments.

"GEMINI_API_KEY",
"GOOGLE_GENAI_USE_VERTEXAI",
"GOOGLE_CLOUD_PROJECT",
"GOOGLE_CLOUD_LOCATION",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3. Two names an SDK reads for itself are not on the list, so a template that names them now adds nothing.

  1. GOOGLE_GENAI_USE_ENTERPRISE. The locked google-genai 2.25.0 reads it beside GOOGLE_GENAI_USE_VERTEXAI (_api_client.py and live.py), and google-adk warns that GOOGLE_GENAI_USE_VERTEXAI is deprecated in its favour. Repro: agent.py with genai.Client() and a .env.example of GOOGLE_GENAI_USE_ENTERPRISE=true, GOOGLE_CLOUD_PROJECT=, GOOGLE_CLOUD_LOCATION= gives only the last two. Without the flag the client runs in API-key mode and fails.
  2. TAVILY_API_KEY. TavilySearch() in a LangGraph agent reads it for itself, so the code never names it. Repro: create_react_agent(ChatAnthropic(...), [TavilySearch()]) with ANTHROPIC_API_KEY= and TAVILY_API_KEY= gave both before this PR and only ANTHROPIC_API_KEY now. The run then fails at the first search, mid-call, not at start.

Fix: add both names (and GOOGLE_GEMINI_BASE_URL / GOOGLE_VERTEX_BASE_URL if base URLs stay listed), and add them to SDK_NAMES in the test.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants